> Then something would go wrong or am I overthinking this? Same applies to providers.
No. Not overthinking. It's a valid concern. I tried to see if we could change ValueError and others __bases__ to derive also from (say) __InternalAirflowException (that could be dynamically aliased from AirflowException) - but it is not possible. Those are c-level built-ins that cannot be modified. I think the best we could come up with is to have something like `AirflowValueError(ValueError,__InternalAirflowException)` and make sure that task sdk is using those instead of ValueError and such - not ideal, but nice for backwards compatibility. The __InternalAirflowException could be then aliased with deprecation (dynamically, no API, no public interface, no IDE completion) from AirflowException. Then we could even have two levels of deprecation warnings: * when you declare `catch AirflowException` - during parsing warning would say "don't, switch to regular exceptions" * when you actually catch such AirflowValueError via AirflowException instance, we could (not sure if that is possible- but likely - from stack trace/frames) - write the exact exception to be caught (catch ValueError instead). And then all the "airflow-core" could use "regular" exceptions anyway once we separate task-sdk - compatibility is **only** a question of TaskSdk and it can be contained within task-sdk exclusively. J On Wed, Aug 27, 2025 at 11:02 AM Ephraim Anierobi <ephraimanier...@gmail.com> wrote: > > - Should we remove the existing ones? (We previously had concerns about > backward compatibility.) Or should we only apply this pattern to new pull > requests? > > The major problem in removing it is backward compatibility as you > mentioned, especially between previous airflow core and recent task-sdk for > example or vice versa. I almost removed the AirflowException in task-sdk > but the thought of it being a breaking change made me revert the change in > this PR: https://github.com/apache/airflow/pull/54505. > > Let me paint a scenario. Let's say someone is on airflow-core 3.0.5 where > there are usages of `except AirflowException: ...` and on task-sdk 1.1.0 > where we have raised RuntimeError for example in place of AirflowException > (dag.get_task raises AirflowException). Then something would go wrong or am > I overthinking this? Same applies to providers. > > On Wed, 27 Aug 2025 at 09:38, Jarek Potiuk <ja...@potiuk.com> wrote: > > > +1 - it's always been a little "cargo cult" on using Airflow Exception. > > There was no benefits of using it - I do recall (very, very early days of > > mine in Airflow where I had completely no idea what I was doing) - in > 1.10 > > there was a slight different handling in case there was "Airflow > Exception" > > thrown and "other exceptions" - but I even can't remember what it was and > > it is a long gone, yet "cargo cult" remained in Providers. > > > > 1. In most cases, we should prioritize using Python’s standard exceptions > > (e.g., `ValueError`, `TypeError`, `OSError`) instead of wrapping > everything > > in `AirflowException`. > > > > Yes > > > > 2. In `airflow-core`, we should define and use more specific subclasses > > under `airflow.exceptions`. > > Yes > > > > 3. For the providers, we should also define exceptions within > > `airflow.providers.<provider>.exceptions`. > > > > Yes if none of the 1. is appropriate (which should be a rarity). This > > should only be needed if we want to use some "custom" behaviour, property > > or otherwise "common" thing between the exceptions (for example aws > > exception could hold region, url and other things that could help to > > identify the failure better). > > > > - Should we remove the existing ones? (We previously had concerns about > > backward compatibility.) Or should we only apply this pattern to new pull > > requests? > > > > Yes. I would remove it completely with deprecation_tools as we do for > other > > removed airflow modules and classes. It will make it invisible to IDE > tools > > and autocompletion but old usages will still work. There is a bit of > > difficulty with removal, because currently others could catch > > `AirflowException` - so we would have to figure out how to make our > > "specific" exceptions to still derive from AirflowException that is > > missing. But I think we can work it out - by aliasing AirflowException to > > be something completely different (__InternalAirflowException) placed > in a > > different module and suppressing the warning when they are used by our > > internal exception - or we could do some meta-classes magic to handle it. > > > > - Should we enforce it through development tools? > > > > If we remove the AirflowException - no enforcement will be needed. > > > > - Is there an acceptable exception for using `AirflowException`? > > > > Not that I am aware of in our code. But users could have used it and we > > should keep compatibility. > > > > > > > > On Wed, Aug 27, 2025 at 10:16 AM Wei Lee <weilee...@gmail.com> wrote: > > > > > Hi all, > > > > > > I would like to propose avoiding, or at least reducing, the direct use > of > > > `AirflowException`. > > > > > > `AirflowException` is used extensively across our codebase, with the > > > following occurrences: > > > > > > - 54 in `airflow-core` > > > - 1,515 in `providers` > > > - 20 in `task-sdk` > > > > > > `AirflowException` itself does not provide much more information than a > > > standard `Exception`, aside from indicating that it originates from > > > Airflow. > > > > > > I would like to propose the following practices, combining ideas from > our > > > previous discussion on Slack: > > > > > > 1. In most cases, we should prioritize using Python’s standard > exceptions > > > (e.g., `ValueError`, `TypeError`, `OSError`) instead of wrapping > > everything > > > in `AirflowException`. > > > 2. In `airflow-core`, we should define and use more specific subclasses > > > under `airflow.exceptions`. > > > 3. For the providers, we should also define exceptions within > > > `airflow.providers.<provider>.exceptions`. > > > > > > If this approach makes sense, there are some following questions. > > > > > > - Should we remove the existing ones? (We previously had concerns about > > > backward compatibility.) Or should we only apply this pattern to new > pull > > > requests? > > > - Should we enforce it through development tools? > > > - Is there an acceptable exception for using `AirflowException`? > > > > > > Best regards, > > > Wei Lee > > > > > >
