>Actually, modifying DAGs code is the easiest part of the migration I strongly disagree. Modifying source code of DAGs might be easy for individual devs, but it is what blocks customers who have significant size deployments from perfoming upgrades and migrations. Even if the code change is seemingly simple and automated, it becomes a massive operational overhead.
To be honest, I don't think we're net positive on this cleanup - sure, the code get cleaner, there are no potentially overlapping parameters, we separate email which is not core Airflow feature, etc. All great stuff. But the cost is forcing all users of this popular feature to modify source code of their DAGs, putting another hurdle for Airflow 3 adoption. pon., 27 sty 2025, 23:21 użytkownik Elad Kalif <elad...@apache.org> napisał: > I vote for option 1. complete removal. > > Email is just another integration. We should not favor it. > > I agree with Michal that it's massively used but I do not agree on the > risk. > > From my point of view, the SmtpNotifier (Which has templates! thanks to > https://github.com/apache/airflow/pull/36226) has existed for a long time. > It provides almost all the needed functionality (except the > default_email_on_failure) and the migration to it is very simple. We can > catch all the cases with the upgrade tool we plan and alert users where > they need to make changes. > > Given that: > a. We can prevent broken dags by catching all cases in migration tool > b. Migration is easy. No thinking required. It's just a copy - paste > replacement. > c. There are no edge cases > > I think it's considered safe to continue with complete removal. > Now, the question left is: Is it worth it? What value do we get from doing > that? > I don't know about other people's experience but I am tired of explaining > to users about the difference between on_failure_call_back and > email_on_failure. What happens when someone use both, etc... > It confuses so many people. We have a chance to get it simple. Let's do > that. > > However, My answer did not cover the default_email_on_failure. Personally, > I don't know anyone who actually uses it. > In most Airflow envs I saw there is more than 1 person / 1 team so it makes > little sense to have a global notification address. > For those kind of deployments it's no issue because they are not using > it, for other, smaller deployments that do use it - I can argue that the > change is very low impact as it's very easy to fix, but we can also modify > the SMTP connection to hold default_to_email if we want to mitigate that > risk. > > On Tue, Jan 28, 2025 at 12:19 AM Hussein Awala <huss...@awala.fr> wrote: > > > > avoiding forcing users to modify their DAGs code > > > > Actually, modifying DAGs code is the easiest part of the migration > > for users, because we will provide migration rules via ruff. For more > > details, please see https://github.com/apache/airflow/issues/41641. > > > > On Mon, Jan 27, 2025 at 11:01 PM Michał Modras > > <michalmod...@google.com.invalid> wrote: > > > > > Email notifications are a *massively* used Airflow feature. I suggest > > that > > > any change does not break the current syntax of BaseOperator, avoiding > > > forcing users to modify their DAGs code. Changes on the config side and > > > provider separation should be fine though (seems separating provider > > while > > > keeping the parameters of BaseOperator would require some safe > defaults). > > > > > > On Mon, Jan 27, 2025 at 10:51 PM Hussein Awala <huss...@awala.fr> > wrote: > > > > > > > Hello everyone! > > > > > > > > We had an implicit consensus about removing the email/SMTP > integration > > > from > > > > Airflow Core and replacing it with the SMTP provider and > SmtpNotifier. > > > This > > > > change will simplify Airflow's core, improve maintainability, and > > > provide a > > > > more consistent experience with other providers. Now is the best > moment > > > to > > > > do this before Airflow 3.0. > > > > > > > > All the Airflow configurations in the *smtp* section are supported in > > the > > > > SMTP connection, so removing the whole section should have minimal > > impact > > > > on users. The migration effort is expected to be very low. > > > > > > > > This thread focuses on how best to handle the configurations > currently > > in > > > > the email section, mainly: > > > > > > > > - > > > > > > > > *default_email_on_failure* > > > > - > > > > > > > > *default_email_on_retry* > > > > - > > > > > > > > *email_backend* > > > > - > > > > > > > > *email_conn_id* > > > > > > > > The other configurations could be replaced by connection extras. > > > > > > > > Options for handling email configuration: > > > > > > > > - > > > > > > > > Option 1: Complete Removal > > > > > > > > - > > > > > > > > Remove the entire email section from the Airflow configuration. > > > > - > > > > > > > > Remove the *email_on_failure* and *email_on_retry* arguments > from > > > > BaseOperator. > > > > - > > > > > > > > Implications: > > > > - > > > > > > > > Users will need to explicitly define notifiers (using > > > > on_*_callbacks) for any task that requires email > > notifications. > > > > - > > > > > > > > There will be no built-in mechanism for default email > > > > notifications. > > > > - > > > > > > > > This offers the most significant simplification of Airflow's > > > core > > > > but may increase the effort required to configure email > > alerts. > > > > - > > > > > > > > Option 2: Replacement Configurations > > > > > > > > - > > > > > > > > Introduce new configuration options: > > > > - > > > > > > > > *default_notification_on_failure*: (boolean) Enables default > > > > notifications on task failure. > > > > - > > > > > > > > *default_notification_on_retry*: (boolean) Enables default > > > > notifications on task retry. > > > > - > > > > > > > > *default_notifier*: (string) Specifies a method that > > initializes > > > > and returns the default notifier. > > > > - > > > > > > > > Replace *email_on_failure* and *email_on_retry* in BaseOperator > > > with > > > > *notify_on_failure* and *notify_on_retry*, which would default > to > > > the > > > > values set in the new configuration options. > > > > - > > > > > > > > Replace the old email handling behavior by simply adding the > > > notifier > > > > to the corresponding callback. > > > > - > > > > > > > > Implications: > > > > - > > > > > > > > Preserve the concept of default notifications while moving > > away > > > > from the mail-specific configuration. > > > > - > > > > > > > > Support other notification types and send notifications by > > > > default. > > > > - > > > > > > > > Requires introducing new configuration options and modifying > > > > BaseOperator. > > > > - > > > > > > > > Option 3: Dedicated Notification Connection > > > > > > > > - > > > > > > > > Create a new connection (of any type) with a specific and > > reserved > > > > name for notifications (e.g., *notifications_connection*). > > > > - > > > > > > > > This connection would have extras to configure the notifier and > > > > specify whether to send notifications by default. > > > > - > > > > > > > > Implications: > > > > - > > > > > > > > Provides a centralized and extensible way to manage all > > > > notification settings without Airflow configuration. > > > > - > > > > > > > > Allows users to easily switch between different notification > > > > methods and send notifications by default. > > > > - > > > > > > > > May require more initial setup for users but offers greater > > > > flexibility in the long term. > > > > - > > > > > > > > Require more dev on our side. > > > > > > > > Regarding migration, a fully automated migration might not be > possible > > in > > > > all cases because of the need to migrate Airflow configurations, we > > will > > > > provide comprehensive documentation and examples to assist users in > > > > transitioning their email configurations. > > > > > > > > I'm interested in hearing your thoughts and any other suggestions you > > may > > > > have. > > > > > > > > P.S: I have created an almost ready-to-merge PR for option 1 ( > > > > https://github.com/apache/airflow/pull/46041) > > > > > > > > > >
