Uh oh!
There was an error while loading. Please reload this page.
Enable ruff B008 (function-call-in-default-argument) and fix violations - #66979
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables Ruff B008 across the Airflow repo and updates several function signatures to avoid function calls or mutable objects in default arguments.
Changes:
- Enables B008 and configures immutable-call exceptions for FastAPI DI and SHA256.
- Moves several config-derived or mutable defaults into function bodies.
- Updates a test helper to avoid a mutable
SimpleNamespacedefault.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml | Enables B008 and adds bugbear immutable-call exceptions. |
task-sdk/src/airflow/sdk/definitions/operator_resources.py | Defers resource config defaults to Resources construction. |
task-sdk/src/airflow/sdk/bases/sensor.py | Defers sensor timeout config default to construction. |
shared/observability/src/airflow_shared/observability/metrics/statsd_logger.py | Replaces shared validator defaults with per-instance defaults. |
shared/observability/src/airflow_shared/observability/metrics/otel_logger.py | Replaces shared validator default with per-instance default. |
shared/observability/src/airflow_shared/observability/metrics/datadog_logger.py | Replaces shared validator defaults with per-instance defaults. |
providers/openlineage/tests/system/openlineage/operator.py | Creates the Jinja environment per operator instance. |
providers/google/src/airflow/providers/google/cloud/transfers/s3_to_gcs.py | Defers deferrable config default to operator construction. |
providers/google/src/airflow/providers/google/cloud/operators/vertex_ai/ray.py | Creates default Ray head node resources per operator instance. |
providers/google/src/airflow/providers/google/cloud/hooks/vertex_ai/ray.py | Creates default Ray head node resources inside the hook call. |
providers/fab/tests/unit/fab/auth_manager/api_fastapi/conftest.py | Creates the default test user per context-manager call. |
airflow-core/src/airflow/api_fastapi/common/parameters.py | Replaces a FastAPI default config call with a module constant and adds a B008 suppression. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Addresses remaining Copilot review comments on apache#66979: - Hook test: verify calling create_ray_cluster with head_node_type=None passes a fresh Resources() instance to vertex_ray.create_ray_cluster - New operator test file for vertex_ai/ray.py: verify omitting head_node_type uses a fresh Resources() per instance (not shared), and that execute() forwards a Resources() instance to the hook
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Addresses remaining Copilot review comments on apache#66979: - Hook test: verify calling create_ray_cluster with head_node_type=None passes a fresh Resources() instance to vertex_ray.create_ray_cluster - New operator test file for vertex_ai/ray.py: verify omitting head_node_type uses a fresh Resources() per instance (not shared), and that execute() forwards a Resources() instance to the hook
9078e2a to
35adb98CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Adds [tool.ruff.lint.flake8-bugbear] extend-immutable-calls to exempt FastAPI DI callables (Depends, Query, Path, Body, Security) and the stateless cryptography SHA256 descriptor from B008, then fixes all remaining violations where conf.get*() or mutable objects were evaluated once at import time rather than at call time: - task-sdk: sensor timeout and Resources cpus/ram/disk/gpus now read from config at instantiation - providers/google: s3_to_gcs deferrable default, Vertex AI Ray head_node_type mutable default - shared/observability: SafeStatsdLogger, SafeDogStatsdLogger, and SafeOtelLogger metrics_validator/metric_tags_validator mutable defaults (PatternAllowListValidator shared across instances) - openlineage system test: setup_jinja() mutable Jinja Environment default - api_fastapi/common/parameters: LimitFilter.depends conf.getint moved to module-level constant (_FALLBACK_PAGE_LIMIT) to preserve OpenAPI schema default while making the intent explicit - providers/fab tests: types.SimpleNamespace mutable default in conftest
- Make RayHook.create_ray_cluster use `is not None` for head_node_type fallback so an explicit falsy value is forwarded instead of being replaced; matches the operator's behavior. - Add regression tests for the conf-default-at-instantiation behavior: * Sensor `timeout` reads `sensors.default_timeout` at construction * `Resources()` reads `operators.default_*` at construction and preserves explicit 0 instead of falling back to config * `S3ToGCSOperator.deferrable` reads `operators.default_deferrable` at construction; explicit `False` overrides a truthy config value
Addresses remaining Copilot review comments on apache#66979: - Hook test: verify calling create_ray_cluster with head_node_type=None passes a fresh Resources() instance to vertex_ray.create_ray_cluster - New operator test file for vertex_ai/ray.py: verify omitting head_node_type uses a fresh Resources() per instance (not shared), and that execute() forwards a Resources() instance to the hook
- parameters.py: compute Query() before inner() instead of suppressing B008 with noqa - cleaner and avoids the linter exception - s3_to_gcs.py: revert deferrable=None pattern; restore the canonical conf.getboolean() default enforced by check_deferrable_default checker; add type annotation so B008 doesn't flag the unannotated call
…ex_ai/ray.py Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
…datadog_logger.py Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
…vertex_ai/ray.py Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
Address Jens's review comment on PR apache#66979 — the ListValidator instances assigned to `metrics_validator` / `metric_tags_validator` are never falsy except when `None`, so the shorter `or` form is equivalent and easier to read.
f2669d8 to
bec9beaCompareUh oh!
There was an error while loading. Please reload this page.
Summary
Enables ruff rule B008 (
function-call-in-default-argument) which catches function calls used as default argument values — a common source of bugs where mutable objects or config reads are shared across all calls instead of being fresh per call.[tool.ruff.lint.flake8-bugbear] extend-immutable-callsto exempt FastAPI DI callables (Depends,Query,Path,Body,Security) and the statelesscryptographySHA256 descriptor, which are intentionally used in argument defaultsBaseSensorMixin:conf.getfloat("sensors", "default_timeout")was read once at import time; now read at instantiationResources:conf.getint(...)forcpus/ram/disk/gpusread at import time; now read at instantiationS3ToGCSOperator:conf.getboolean("operators", "default_deferrable")read at import timeresources.Resources()mutable default shared across all callsSafeStatsdLogger,SafeDogStatsdLogger,SafeOtelLogger):PatternAllowListValidator()mutable instances shared across all logger instancessetup_jinja()mutable JinjaEnvironmentshared across operator instancesLimitFilter.depends:conf.getint("api", "fallback_page_limit")in FastAPI dependency method moved to module-level constant_FALLBACK_PAGE_LIMIT, preserving the OpenAPI schema default value while making the intent explicittypes.SimpleNamespace(...)mutable default in context manager helperWas generative AI tooling used to co-author this PR?
Generated-by: Claude Sonnet 4.6 (claude-sonnet-4-6) following the guidelines