DhanushAnegondi opened a new pull request, #71591:
URL: https://github.com/apache/airflow/pull/71591
`match_glob` is in `GCSToS3Operator.template_fields`, so the constructor
only ever sees the un-rendered Jinja expression, never the value the DAG author
meant. The guard that rejects `match_glob` on an unsupported google provider
tested it for truthiness:
```python
if not self.__is_match_glob_supported and match_glob:
```
which is what `validate-operators-init` flags, and why the class is on the
exemption list.
**The check stays in `__init__`.** It is a provision check, not a value
check: it asks whether `match_glob` was supplied, combined with an environment
capability the constructor can already answer for itself. It never inspects the
value. Per the false-positives section of #70296 those belong in the
constructor and get rewritten in place rather than moved — with
`render_template_as_native_obj=True` a provided field can render to `None`, so
the same check in `execute()` would report a supplied argument as missing; and
raising at construction surfaces a static authoring mistake as a Dag import
error rather than once per task instance and per retry.
Since #70505 narrowed the hook to sanction `is None` / `is not None` reads,
the in-place rewrite passes and the exemption entry still goes.
### What this changes
* rewrites the guard to `match_glob is not None`
* removes `GCSToS3Operator` from
`scripts/ci/prek/validate_operators_init_exemptions.txt` — the hook fails on
stale exemptions, so this has to land in the same commit as the fix
* adds three test cases where nothing previously covered this error at all
### Behavioural impact
One cell changes, on google providers older than 10.3.0:
| `match_glob` | before | after |
| --- | --- | --- |
| omitted (`None`) | no raise | no raise |
| `"**/*.csv"` | raises | raises |
| `""` | **no raise** | **raises** |
`match_glob=""` was supplied by the user and truthiness read it as absent.
An empty glob is not a valid pattern, and on a provider below 10.3.0 it was
never going to be honoured anyway — it would have been passed to a
`GCSHook.list()` call that cannot accept it. This is the stricter direction
that `05_pull_requests.rst` sanctions for provision checks.
### Relationship to #70723
Flagging this up front: #70723 is open against the same entry and reaches
the same one-line fix. I worked this independently and only found that PR
afterwards, so this is not a deliberate competing implementation — I'm raising
it because the two diffs are close enough that a reviewer deserves to know
rather than discover it.
Where they differ:
* #70723 additionally narrows `AirflowException` to `ValueError` and drops
the now-stale `gcs_to_s3.py::1` entry from
`generated/known_airflow_exceptions.txt`, matching the #70359 precedent. This
PR does not — I had scoped that out as a separate concern before I was aware of
the precedent, and I have deliberately not back-filled it here, since that
would just be copying their work.
* This PR covers the omitted-argument case, which #70723 does not. Without
it the suite only pins the raising direction, so a regression that made the
guard unconditional would still pass.
If maintainers prefer #70723 as the base — which is reasonable, it is older
and more complete on the exception-type question — I am happy for this to be
closed and will offer the missing test case there instead. I would rather that
than have two near-identical PRs consuming review time.
### Deliberately not changed
* **`raise AirflowException`** — kept as-is, for the reason above.
* **The unreachable `except ImportError`.** `gcs_to_s3.py` already does a
module-level `from airflow.providers.google.cloud.hooks.gcs import GCSHook`, so
the `except ImportError` around the function-level `from
airflow.providers.google import __version__` in `__init__` cannot fire — the
module import would have failed first. Real, but a separate concern.
* **The `flatten_structure` / `keep_directory_structure` warning.** Reads
two fields that are not template fields, so the rule does not apply and the
hook does not flag it.
### Verification
Run in a WSL checkout, from `providers/amazon`:
```bash
# baseline, before any change
uv run pytest tests/unit/amazon/aws/transfers/test_gcs_to_s3.py -q
# 30 passed
# new tests against the UNFIXED source, to prove they discriminate
# 1 failed, 3 passed — the failure is [empty_string_is_still_supplied],
# which is exactly the one behavioural cell this PR changes
# with the fix
uv run pytest tests/unit/amazon/aws/transfers/test_gcs_to_s3.py -q
# 33 passed
```
```bash
python scripts/ci/prek/validate_operators_init.py \
providers/amazon/src/airflow/providers/amazon/aws/transfers/gcs_to_s3.py
# exit 0 — clears on both counts: no finding, and no longer a stale exemption
prek run --from-ref upstream/main --to-ref HEAD
# exit 0 — all checks passed
```
No newsfragment: `providers/amazon` has no `newsfragments/` directory, and
`providers/AGENTS.md` says never to use them for providers.
related: #70296
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Opus 5) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
