Cool!

I see that you are using async code for it in the draft PR, and I remember you saying somewhere that Python 3.6 (which is still supported) doesn't have great support for this.

Should we have an explicit check for Py 3.6 and refuse to allow triggers to be run there? (and not let `trigger` process run either.

-ash

On Mon, Apr 26 2021 at 14:37:23 +0100, Kaxil Naik <kaxiln...@gmail.com> wrote:
Thanks Andrew, that answers my questions.

If we don't have any other questions by the end of the week we should start a VOTE.

Regards,
Kaxil

On Tue, Apr 20, 2021 at 2:24 AM Andrew Godwin <andrew.god...@astronomer.io.invalid> wrote:
Thanks Kaxil - notes on those two things:

- A timeout is probably a reasonable thing to have in most situations, as it gives you that little bit of self-healing ability (I put a created timestamp into the Trigger schema partially out of this kind of caution). I'll update the AIP to mention that triggers will come with an optional timeout.

- Correct, users who don't want the new process shouldn't be affected at all. The main bad failure case here is that if you don't have the process and then try to use a deferred operator, it would silently hang forever; my plan was to take a cue from the code in the webserver that warns you the scheduler isn't running, and do something similar for triggers, somewhere (it can't be an immediate error when you try to run the task, as the triggerer process may just be down for a few seconds for a redeploy, or similar).

Andrew

On Mon, Apr 19, 2021 at 6:10 PM Kaxil Naik <kaxiln...@gmail.com <mailto:kaxiln...@gmail.com>> wrote:
+1

This is awesome Andrew, this is going to be a huge cost saver.

The AIP is quite detailed and the Draft PR certainly helps.

Just minor comments:

Should we have a timeout on publishing a Trigger / Task to the triggerer similar to <https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#operation-timeout>In How are users affected by the change? <https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=177050929#AIP40:Deferrable(%22Async%22)Operators-Howareusersaffectedbythechange?(e.g.DBupgraderequired?)> section, do the users who don't want to use the new process affected by it. Just confirming that if a user does not opt-in they are unaffected. <https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=177050929#AIP40:Deferrable(%22Async%22)Operators-Howareusersaffectedbythechange?(e.g.DBupgraderequired?)>
Regards,
Kaxil

On Thu, Apr 15, 2021 at 9:34 PM Andrew Godwin <andrew.god...@astronomer.io.invalid> wrote:
Hi all,

After a period of designing and prototyping, I've completed a first draft of AIP-40 that I'd love to get some feedback on from the community: <https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=177050929>

This AIP proposes a way of adding what I'm calling "deferrable" Operators into Airflow - essentially, taking the goal of Smart Sensors and making it much more generalisable, where any Sensor or Operator can choose to "defer" its execution based on an asynchronous trigger, and where all the triggers run in one (or more) processes for efficiency.

It also means that any Operator can be made deferrable in a backwards-compatible way should we wish to in future, though I'm not proposing that for a first release.

It comes with a working prototype, too, should you wish to see what kind of code footprint this would have: <https://github.com/apache/airflow/pull/15389>

I personally think this would be a huge improvement for Airflow's efficiency - I suspect we might be able to reduce the amount of resources some Airflow installs use by over 50% if all their idling operators were ported to be deferrable - but I would welcome further opinions.

Thanks,
Andrew

Reply via email to