Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ruff for linting #771

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,8 @@ jobs:

# Runs the sdk features repo tests with this repo's current SDK code
features-tests:
uses: temporalio/features/.github/workflows/python.yaml@uv
uses: temporalio/features/.github/workflows/python.yaml@main
with:
python-repo-path: ${{github.event.pull_request.head.repo.full_name}}
version: ${{github.event.pull_request.head.ref}}
version-is-repo-ref: true
features-repo-ref: uv
50 changes: 27 additions & 23 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,40 +54,30 @@ dev = [
[tool.poe.tasks]
build-develop = "uv run maturin develop --uv"
build-develop-with-release = { cmd = "uv run maturin develop --release --uv" }
format = [{cmd = "uv run ruff check --select I --fix"}, {cmd = "uv run ruff format"}, ]
format = [{cmd = "uv run ruff check --fix"}, {cmd = "uv run ruff format"}, ]
gen-docs = "uv run python scripts/gen_docs.py"
gen-protos = "uv run python scripts/gen_protos.py"
lint = [
{cmd = "uv run ruff check --select I"},
{cmd = "uv run ruff check"},
{cmd = "uv run ruff format --check"},
{ref = "lint-types"},
{cmd = "uv run pyright"},
{ref = "lint-types-mypy"},
{ref = "lint-types-pyright"},
{ref = "lint-docs"},
]
bridge-lint = { cmd = "cargo clippy -- -D warnings", cwd = "temporalio/bridge" }
# TODO(cretz): Why does pydocstyle complain about @overload missing docs after
# https://github.com/PyCQA/pydocstyle/pull/511?
lint-docs = "uv run pydocstyle --ignore-decorators=overload"
lint-types = "uv run mypy --namespace-packages --check-untyped-defs ."
lint-types-mypy = "uv run mypy --namespace-packages --check-untyped-defs ."
lint-types-pyright = "uv run pyright"
run-bench = "uv run python scripts/run_bench.py"
test = "uv run pytest"

[tool.ruff]
target-version = "py39"
exclude = ["*_pb2.py", "*_pb2_grpc.py"]
lint.ignore = ["E741"] # we occasionally use e.g. O as a type var and l as a loop variable

[tool.pytest.ini_options]
asyncio_mode = "auto"
# Do not use log_cli since this shows logging for all tests, not just the ones
# that failed. Instead, show all logs for failed tests at the end.
log_level = "DEBUG"
log_format = "%(asctime)s [%(levelname)8s] %(message)s (%(filename)s:%(lineno)s)"
testpaths = ["tests"]
timeout = 600
timeout_func_only = true
filterwarnings = [
"error::temporalio.workflow.UnfinishedUpdateHandlersWarning",
"error::temporalio.workflow.UnfinishedSignalHandlersWarning",
"ignore::pytest.PytestDeprecationWarning",
"ignore::DeprecationWarning",
]

[tool.cibuildwheel]
before-all = "pip install protoc-wheel-0"
Expand Down Expand Up @@ -183,9 +173,6 @@ exclude = [
"temporalio/bridge/testing.py",
]

[tool.ruff]
target-version = "py39"

[build-system]
requires = ["maturin>=1.0,<2.0"]
build-backend = "maturin"
Expand All @@ -202,3 +189,20 @@ exclude = [
[tool.uv]
# Prevent uv commands from building the package by default
package = false

[tool.pytest.ini_options]
asyncio_mode = "auto"
# Do not use log_cli since this shows logging for all tests, not just the ones
# that failed. Instead, show all logs for failed tests at the end.
log_level = "DEBUG"
log_format = "%(asctime)s [%(levelname)8s] %(message)s (%(filename)s:%(lineno)s)"
testpaths = ["tests"]
timeout = 600
timeout_func_only = true
filterwarnings = [
"error::temporalio.workflow.UnfinishedUpdateHandlersWarning",
"error::temporalio.workflow.UnfinishedSignalHandlersWarning",
"ignore::pytest.PytestDeprecationWarning",
"ignore::DeprecationWarning",
]

10 changes: 5 additions & 5 deletions scripts/gen_protos.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import tempfile
from functools import partial
from pathlib import Path
from typing import List, Mapping, Optional
from typing import List, Mapping

base_dir = Path(__file__).parent.parent
proto_dir = (
Expand All @@ -25,8 +25,8 @@
v
for v in proto_dir.glob("**/*.proto")
if not str(v).startswith(str(testsrv_proto_dir / "dependencies"))
and not "health" in str(v)
and not "google" in str(v)
and "health" not in str(v)
and "google" not in str(v)
]
proto_paths.extend(test_proto_dir.glob("**/*.proto"))
proto_paths.extend(additional_proto_dir.glob("**/*.proto"))
Expand Down Expand Up @@ -95,7 +95,7 @@ def fix_generated_output(base_path: Path):
message_names = sorted(message_names)
if message_names:
f.write(
f'\n__all__ = [\n "' + '",\n "'.join(message_names) + '",\n]\n'
'\n__all__ = [\n "' + '",\n "'.join(message_names) + '",\n]\n'
)
# gRPC imports
if "service_pb2_grpc" in imports:
Expand All @@ -115,7 +115,7 @@ def fix_generated_output(base_path: Path):
message_names.append(message)
# __all__
message_names = sorted(message_names)
f.write(f' __all__.extend(["' + '", "'.join(message_names) + '"])\n')
f.write(' __all__.extend(["' + '", "'.join(message_names) + '"])\n')
f.write("except ImportError:\n pass")


Expand Down
2 changes: 1 addition & 1 deletion temporalio/activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ def in_activity() -> bool:
Returns:
True if in an activity, False otherwise.
"""
return not _current_context.get(None) is None
return _current_context.get(None) is not None


def info() -> Info:
Expand Down
4 changes: 2 additions & 2 deletions temporalio/api/cloud/cloudservice/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,9 @@

# gRPC is optional
try:
import grpc
import grpc # noqa: F401

from .service_pb2_grpc import (
from .service_pb2_grpc import ( # noqa: F401
CloudServiceServicer,
CloudServiceStub,
add_CloudServiceServicer_to_server,
Expand Down
4 changes: 2 additions & 2 deletions temporalio/api/operatorservice/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@

# gRPC is optional
try:
import grpc
import grpc # noqa: F401

from .service_pb2_grpc import (
from .service_pb2_grpc import ( # noqa: F401
OperatorServiceServicer,
OperatorServiceStub,
add_OperatorServiceServicer_to_server,
Expand Down
4 changes: 2 additions & 2 deletions temporalio/api/testservice/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@

# gRPC is optional
try:
import grpc
import grpc # noqa: F401

from .service_pb2_grpc import (
from .service_pb2_grpc import ( # noqa: F401
TestServiceServicer,
TestServiceStub,
add_TestServiceServicer_to_server,
Expand Down
4 changes: 2 additions & 2 deletions temporalio/api/workflowservice/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,9 @@

# gRPC is optional
try:
import grpc
import grpc # noqa: F401

from .service_pb2_grpc import (
from .service_pb2_grpc import ( # noqa: F401
WorkflowServiceServicer,
WorkflowServiceStub,
add_WorkflowServiceServicer_to_server,
Expand Down
2 changes: 1 addition & 1 deletion temporalio/bridge/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import temporalio.bridge.runtime
import temporalio.bridge.temporal_sdk_bridge
from temporalio.bridge.temporal_sdk_bridge import RPCError
from temporalio.bridge.temporal_sdk_bridge import RPCError # noqa: F401


@dataclass
Expand Down
2 changes: 1 addition & 1 deletion temporalio/bridge/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from temporalio.bridge.temporal_sdk_bridge import (
CustomSlotSupplier as BridgeCustomSlotSupplier,
)
from temporalio.bridge.temporal_sdk_bridge import PollShutdownError
from temporalio.bridge.temporal_sdk_bridge import PollShutdownError # noqa: F401


@dataclass
Expand Down
2 changes: 1 addition & 1 deletion temporalio/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
)

import google.protobuf.internal.containers
from typing_extensions import ClassVar, NamedTuple, Self, TypeAlias, get_origin
from typing_extensions import NamedTuple, Self, TypeAlias, get_origin

import temporalio.api.common.v1
import temporalio.api.enums.v1
Expand Down
2 changes: 1 addition & 1 deletion temporalio/converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@ def from_failure(
stack_trace = encoded_attributes.get("stack_trace")
if isinstance(stack_trace, str):
failure.stack_trace = stack_trace
except:
except Exception:
pass

err: temporalio.exceptions.FailureError
Expand Down
2 changes: 1 addition & 1 deletion temporalio/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ def _on_logs(
# just in case)
try:
message += f" {log.fields}"
except:
except Exception:
pass
record = self.logger.makeRecord(
name,
Expand Down
12 changes: 6 additions & 6 deletions temporalio/testing/_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,10 @@ async def start_local(
),
server,
)
except:
except Exception:
try:
await server.shutdown()
except:
except Exception:
logger.warn(
"Failed stopping local server on client connection failure",
exc_info=True,
Expand Down Expand Up @@ -326,10 +326,10 @@ async def start_time_skipping(
),
server,
)
except:
except Exception:
try:
await server.shutdown()
except:
except Exception:
logger.warn(
"Failed stopping test server on client connection failure",
exc_info=True,
Expand Down Expand Up @@ -474,13 +474,13 @@ async def time_skipping_unlocked(self) -> AsyncIterator[None]:
await self.client.test_service.lock_time_skipping(
temporalio.api.testservice.v1.LockTimeSkippingRequest()
)
except:
except Exception:
# Lock it back, swallowing error
try:
await self.client.test_service.lock_time_skipping(
temporalio.api.testservice.v1.LockTimeSkippingRequest()
)
except:
except Exception:
logger.exception("Failed locking time skipping after error")
raise

Expand Down
4 changes: 2 additions & 2 deletions temporalio/worker/_activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ async def _run_activity(
if running_activity.last_heartbeat_task:
try:
await running_activity.last_heartbeat_task
except:
except Exception:
# Should never happen because it's trapped in-task
temporalio.activity.logger.exception(
"Final heartbeat task didn't trap error"
Expand Down Expand Up @@ -750,7 +750,7 @@ def _execute_sync_activity(
if isinstance(heartbeat, SharedHeartbeatSender):
# To make mypy happy
heartbeat_sender = heartbeat
heartbeat_fn = lambda *details: heartbeat_sender.send_heartbeat(
heartbeat_fn = lambda *details: heartbeat_sender.send_heartbeat( # noqa: E731
info.task_token, *details
)
else:
Expand Down
2 changes: 1 addition & 1 deletion temporalio/worker/_replayer.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ async def workflow_replay_iterator(
replayed.
"""
try:
last_replay_failure: Optional[Exception]
last_replay_failure: Optional[Exception] = None
last_replay_complete = asyncio.Event()

# Create eviction hook
Expand Down
6 changes: 3 additions & 3 deletions temporalio/worker/_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

from ._activity import SharedStateManager, _ActivityWorker
from ._interceptor import Interceptor
from ._tuning import WorkerTuner, _to_bridge_slot_supplier
from ._tuning import WorkerTuner
from ._workflow import _WorkflowWorker
from ._workflow_instance import UnsandboxedWorkflowRunner, WorkflowRunner
from .workflow_sandbox import SandboxedWorkflowRunner
Expand Down Expand Up @@ -504,7 +504,7 @@ async def raise_on_shutdown():
if self._config["on_fatal_error"]:
try:
await self._config["on_fatal_error"](exception)
except:
except Exception:
logger.warning("Fatal error handler failed")

except asyncio.CancelledError as user_cancel_err:
Expand Down Expand Up @@ -560,7 +560,7 @@ async def raise_on_shutdown():
# Do final shutdown
try:
await self._bridge_worker.finalize_shutdown()
except:
except Exception:
# Ignore errors here that can arise in some tests where the bridge
# worker still has a reference
pass
Expand Down
3 changes: 2 additions & 1 deletion temporalio/worker/workflow_sandbox/_restrictions.py
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,8 @@ def __init__(

def bind_f(instance: _RestrictedProxy, obj: Any) -> Callable:
def i_op(self: Any, other: Any) -> _RestrictedProxy:
f(self, other) # type: ignore
# TODO: wat
f(self, other) # type: ignore # noqa: F821
Comment on lines +936 to +937
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not know what I was thinking here, but it's likely a bug. Would definitely welcome a test that covers this and fixes it to do what it is supposed to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 #772

return instance

return i_op.__get__(obj, type(obj)) # type: ignore
Expand Down
6 changes: 3 additions & 3 deletions temporalio/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -1660,8 +1660,8 @@ def _assert_dynamic_handler_args(
if (
not arg_types
or len(arg_types) != 2
or arg_types[0] != str
or arg_types[1] != Sequence[temporalio.common.RawValue]
or arg_types[0] is not str
or arg_types[1] is not Sequence[temporalio.common.RawValue]
):
raise RuntimeError(
"Dynamic handler must have 3 arguments: self, str, and Sequence[temporalio.common.RawValue]"
Expand Down Expand Up @@ -4273,7 +4273,7 @@ class ContinueAsNewError(BaseException):

def __init__(self, *args: object) -> None:
"""Direct instantiation is disabled. Use :py:func:`continue_as_new`."""
if type(self) == ContinueAsNewError:
if type(self) is ContinueAsNewError:
raise RuntimeError("Cannot instantiate ContinueAsNewError directly")
super().__init__(*args)

Expand Down
10 changes: 4 additions & 6 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import asyncio
import logging
import multiprocessing
import os
import sys
from typing import AsyncGenerator
Expand All @@ -25,6 +23,10 @@
# Unless specifically overridden, we expect tests to run under protobuf 4.x/5.x lib
import google.protobuf

from temporalio.client import Client
from temporalio.testing import WorkflowEnvironment
from tests.helpers.worker import ExternalPythonWorker, ExternalWorker

protobuf_version = google.protobuf.__version__
if os.getenv("TEMPORAL_TEST_PROTO3"):
assert protobuf_version.startswith(
Expand All @@ -35,10 +37,6 @@
"5."
), f"Expected protobuf 4.x/5.x, got {protobuf_version}"

from temporalio.client import Client
from temporalio.testing import WorkflowEnvironment
from tests.helpers.worker import ExternalPythonWorker, ExternalWorker


def pytest_addoption(parser):
parser.addoption(
Expand Down
2 changes: 1 addition & 1 deletion tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ async def test_query(client: Client, worker: ExternalWorker):
await handle.result()
assert "some query arg" == await handle.query("some query", "some query arg")
# Try a query not on the workflow
with pytest.raises(WorkflowQueryFailedError) as err:
with pytest.raises(WorkflowQueryFailedError):
await handle.query("does not exist")


Expand Down
Loading
Loading