I would like to propose the following order of precedence: 1. If wait_for_completion=False, it should ignore what the default_deferrable is set to and should not wait for completion whether synchronously or asynchronously(deferral) 2. if wait_for_completion=True, deferrable=True (maybe set explicitly or via default deferral), trigger deferral and poll async using the triggerer 3. if wait_for_completion=True, deferrable=False(maybe set explicitly or via default deferral or default value), poll synchronously.
In my opinion, generally, "wait_*" params should take precedence over default_deferral. Regards, Pankaj Koti *Senior Software Engineer, *OSS Engineering Team. Location: Pune, India Timezone: Indian Standard Time (IST) Email: pankaj.k...@astronomer.io Mobile: +91 9730079985 On Fri, Jul 21, 2023 at 4:25 PM Wei Lee <weilee...@gmail.com> wrote: > Hi, > > Avoiding unexpected behavior is one of the reasons we set the default > default_deferrable to False. We want to minimize the impact on the users. > For those who didn't use this feature, everything should work as it used > to. Those who opt to enable it should know it might change the behavior of > existing DAGs. But yes, we can add these checks to mitigate the potential > surprise. I'm now working on a list of all the operators that might be > affected. As the changes might take time, we might want to add some > descriptions in the "default_deferrable" section that this might happen? > > Best, > Wei > > > On Jul 21, 2023, at 3:07 AM, Jarek Potiuk <ja...@potiuk.com> wrote: > > > > Do we know how many of those affected operators have "wait=False" by > > default? I think that should help us to determine the action. The > > "default_deferrable" is an option that has not been advertised (and it is > > not yet in config published in any released airflow version, so it is not > > yet "officially released" - even if some providers with > default_deferrable > > have been released already. So we still have chance to make decision that > > is going to be announced in 2.7.0. We can change slightly name of the > > configuration parameter before 2.7.0, and release new providers with the > > support for new parameter, so that someone will not trigger it > accidentally > > for those providers with different behaviour from earlier provider > > versions. That would be enough of "compatibility" protection I think, > > > > And yes - changing the behaviour from no wait to wait is undesirable. > > > > J. > > > > > > On Tue, Jul 18, 2023 at 12:50 AM Vandon, Raphael > <vand...@amazon.com.invalid> > > wrote: > > > >> Hello, > >> This is a repost from slack as Jarek suggested this ML would be a better > >> place to discuss it. > >> > >> I have a concern with the introduction of a config to control the > default > >> value for deferrable (https://github.com/apache/airflow/pull/31712) > >> A good portion of operators had a parameter to control whether they wait > >> or not (wait_until_finished, wait_for_completion, wait_for_termination, > or > >> just wait). > >> I think most implementations of deferrable treat it as an override for > >> this setting (i.e. if deferrable = True then the value of wait does not > >> matter anymore) > >> My concern is that if users have operators that do not wait (either > >> because the default value for wait was False, or they set it manually to > >> False), and they set default_deferrable=True, then all their operators > will > >> start waiting, which might not be what they expect. > >> I think what’d be nice is if setting the default only > >> changed how operators work (deferring instead of waiting in the worker), > >> not whether they wait or not. > >> It might be too late to change this because there has been a release > since > >> then, but we could say that: > >> • if deferrable is manually set to True, we always wait & defer > >> • else if wait_for_completion is False (default value, or set > manually), > >> we don’t wait > >> • in other cases (wait for completion True and no set value for > >> deferrable), we rely on the config. > >> That way, users setting this conf wouldn’t see their dags suddenly > taking > >> more time because they start waiting where they weren’t before. > >> > >> This would impact any operator that has a legacy mechanism to wait > >> synchronously and a deferrable mode. > >> Especially those where the default value to wait was False, like > >> RedshiftCreateClusterOperator for instance (but there are others). > >> > >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org > For additional commands, e-mail: dev-h...@airflow.apache.org > >
