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