shahar1 commented on code in PR #66979:
URL: https://github.com/apache/airflow/pull/66979#discussion_r3246231727
##########
pyproject.toml:
##########
@@ -653,6 +653,7 @@ extend-select = [
"B004", # Checks for use of hasattr(x, "__call__") and replaces it with
callable(x)
"B006", # Checks for uses of mutable objects as function argument defaults.
"B007", # Checks for unused variables in the loop
+ "B008", # Do not perform function call in argument defaults (use
extend-immutable-calls for FastAPI DI)
Review Comment:
Verified locally with `ruff 0.15.12`: B008 does **not** flag function-call
defaults when the parameter has a type annotation. `def f(x =
conf.getboolean(...))` is caught; `def f(x: bool = conf.getboolean(...))` is
**not**. The remaining `deferrable: bool = conf.getboolean(...)` defaults in
providers and `authorization: str = Header(...)` defaults in
`providers/edge3/.../auth.py` are all annotated, so they slip past the rule and
CI passes (current run:
https://github.com/apache/airflow/actions/runs/25902711928).
The annotated-call gap is a real footgun in ruff's implementation that's
worth a follow-up — but it doesn't block enabling the rule for the un-annotated
violations this PR already fixes.
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @shahar1 before posting
##########
providers/google/src/airflow/providers/google/cloud/hooks/vertex_ai/ray.py:
##########
@@ -115,7 +115,7 @@ def create_ray_cluster(
"""
aiplatform.init(project=project_id, location=location,
credentials=self.get_credentials())
cluster_path = vertex_ray.create_ray_cluster(
- head_node_type=head_node_type,
+ head_node_type=head_node_type or resources.Resources(),
Review Comment:
Fixed in 7f41000182 — switched to `head_node_type if head_node_type is not
None else resources.Resources()` to match the operator's behavior and avoid
silently replacing explicit falsy values.
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @shahar1 before posting
##########
task-sdk/src/airflow/sdk/bases/sensor.py:
##########
@@ -128,6 +128,8 @@ def __init__(
super().__init__(**kwargs)
self.poke_interval =
self._coerce_poke_interval(poke_interval).total_seconds()
self.soft_fail = soft_fail
+ if timeout is None:
+ timeout = conf.getfloat("sensors", "default_timeout")
Review Comment:
Added in 7f41000182 —
`TestBaseSensor.test_sensor_timeout_default_read_from_conf_at_instantiation`
constructs `DummySensor` with no `timeout` under `conf_vars({("sensors",
"default_timeout"): "12345"})` and asserts the value is picked up at
instantiation. Two config values are tested to confirm the read happens
per-call, not at import time.
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @shahar1 before posting
##########
task-sdk/src/airflow/sdk/definitions/operator_resources.py:
##########
@@ -125,15 +125,15 @@ class Resources:
def __init__(
self,
- cpus=conf.getint("operators", "default_cpus"),
- ram=conf.getint("operators", "default_ram"),
- disk=conf.getint("operators", "default_disk"),
- gpus=conf.getint("operators", "default_gpus"),
+ cpus=None,
+ ram=None,
+ disk=None,
+ gpus=None,
):
- self.cpus = CpuResource(cpus)
- self.ram = RamResource(ram)
- self.disk = DiskResource(disk)
- self.gpus = GpuResource(gpus)
+ self.cpus = CpuResource(cpus if cpus is not None else
conf.getint("operators", "default_cpus"))
+ self.ram = RamResource(ram if ram is not None else
conf.getint("operators", "default_ram"))
+ self.disk = DiskResource(disk if disk is not None else
conf.getint("operators", "default_disk"))
+ self.gpus = GpuResource(gpus if gpus is not None else
conf.getint("operators", "default_gpus"))
Review Comment:
Added in 7f41000182:
- `TestResources.test_defaults_read_from_conf_at_instantiation` — confirms
`Resources()` with no args reads
`operators.default_cpus|default_ram|default_disk|default_gpus` at construction
time.
- `TestResources.test_falsy_zero_values_are_preserved` — confirms explicit
`0` for any field is not replaced by the config default (only `None` falls back
to config).
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @shahar1 before posting
##########
providers/google/src/airflow/providers/google/cloud/transfers/s3_to_gcs.py:
##########
@@ -178,7 +178,11 @@ def __init__(
self.verify = verify
self.gzip = gzip
self.google_impersonation_chain = google_impersonation_chain
- self.deferrable = deferrable
+ self.deferrable = (
+ deferrable
+ if deferrable is not None
+ else conf.getboolean("operators", "default_deferrable",
fallback=False)
+ )
Review Comment:
Added in 7f41000182:
- `test_deferrable_default_read_from_conf_at_instantiation` —
`S3ToGCSOperator` without an explicit `deferrable` reads
`operators.default_deferrable` at construction time (both `True` and `False`
config values verified).
- `test_deferrable_explicit_false_overrides_truthy_conf` — passing
`deferrable=False` while the config says `True` keeps the explicit value,
confirming only `None` falls back to config.
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @shahar1 before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
