shahar1 commented on code in PR #70359:
URL: https://github.com/apache/airflow/pull/70359#discussion_r3655925678
##########
providers/amazon/src/airflow/providers/amazon/aws/operators/s3.py:
##########
@@ -697,11 +697,6 @@ def __init__(
self._keys: str | list[str] = ""
- if not exactly_one(keys is None, all(var is None for var in [prefix,
from_datetime, to_datetime])):
- raise AirflowException(
- "Either keys or at least one of prefix, from_datetime,
to_datetime should be set."
- )
-
Review Comment:
#70505 narrows the rule: field is None checks read provision, not the
un-rendered value, so they belong in __init__. This one qualifies.
Please keep both copies, though. `keys = self.keys or
self.hook.list_keys(prefix=..., ...)` lists the whole bucket when every filter
is `None` — reachable past a correct constructor check, since a templated keys
can render to `None` under `render_template_as_native_obj=True`. The
`execute()` guard catches that; the `__init__` guard keeps the plain authoring
mistake a parse-time error.
The exemption entry can still go — the hook just doesn't see comparisons
inside a comprehension:
```python
by_scan = prefix is not None or from_datetime is not None or to_datetime is
not None
if not exactly_one(keys is not None, by_scan):
raise ValueError("Either keys or at least one of prefix, from_datetime,
to_datetime should be set.")
```
Identical on every input including `keys=[]`. Context in #70296's "False
positives" section.
Drafted-by: Claude Code (Opus 5); 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]
