potiuk commented on code in PR #70723:
URL: https://github.com/apache/airflow/pull/70723#discussion_r3681895048
##########
providers/amazon/src/airflow/providers/amazon/aws/transfers/gcs_to_s3.py:
##########
@@ -184,6 +180,10 @@ def execute(self, context: Context) -> list[str]:
}
if self.__is_match_glob_supported:
list_kwargs["match_glob"] = self.match_glob
+ elif self.match_glob:
+ raise AirflowException(
Review Comment:
Worth narrowing this to `ValueError` while you're moving it.
Airflow is actively reducing direct `AirflowException` raises (there's a
`check-no-new-airflow-exceptions` prek hook enforcing it). Relocating an
existing one verbatim during a refactor is explicitly allowed, so this isn't a
violation — but this is a plain "caller passed an argument this environment
can't support", which is exactly what `ValueError` is for.
The clincher is that **#70574 makes the identical change for
`GCSToAzureBlobStorageOperator` and uses `ValueError`**. Same bug, same fix,
two providers, two different exception types would be an odd thing to land in
the same week. Worth syncing with that PR on which one wins — I'd suggest
`ValueError` for both.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk 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]
