FrankYang0529 commented on PR #70351: URL: https://github.com/apache/airflow/pull/70351#issuecomment-5635924459
Thanks for the review. This PR started as part of #70296, which moves template field validation out of `__init__`. #70333 fixed that part for this operator first, so I turned this PR into another change I found along the way: replacing `raise AirflowException(...)` with `raise ValueError(...)`. The change follows the [contributing guide](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#dont-raise-airflowexception-directly), and [AGENTS.md](https://github.com/apache/airflow/blob/1cf0d2c884c96e5f54e02630b93cd86e07ccb8af/AGENTS.md?plain=1#L142) says "When you touch code that already raises `AirflowException`, prefer narrowing it to a more specific exception rather than leaving or duplicating it." Several merged PRs have already replaced `AirflowException` with a built-in exception in code that runs during a task. None of them added a class that extends `AirflowException`. - #70831 - #70539 - #70540 - #69786 - #70218 - #66279 - #70537 - #70535 - #60688 - #54812 - #55462 - #56911 A user who wraps any of these in `try/except AirflowException` runs into the same change as with this PR. Should these also get a new class that extends `AirflowException`? If so, it seems better to decide that once for all providers rather than in this PR alone. I'm happy to add the class here if we go that way. -- 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]
