apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
35.21k stars 13.76k forks source link

[DRAFT] Add deferrable lambdainvocation #40425

Closed gopidesupavan closed 2 days ago

gopidesupavan commented 3 days ago

Closes: #40307


^ Add meaningful description above Read the Pull Request Guidelines for more information. In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed. In case of a new dependency, check compliance with the ASF 3rd Party License Policy. In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

gopidesupavan commented 3 days ago

@eladkal @vincbeck Please have a look is that something usable? 😄 I agree the points you mentioned we dont have anything to get status. does this approach helpful to execute invocation in deferrable?

gopidesupavan commented 3 days ago

Still there are some pending things on this, system test, docs update. If you feel this is useful to add this support to operator, i can update rest of the missing checks and trigger some runs to see how it behaves. 😄

vincbeck commented 3 days ago

Could you please run some tests to check this is working? The pattern is so different than the different triggers that I am having hard time figuring out if that's something that might work

vincbeck commented 3 days ago

I like the creating try though!

vincbeck commented 3 days ago

What I am pretty positive is, that will not work with invocation_type=Event

gopidesupavan commented 3 days ago

What I am pretty positive is, that will not work with invocation_type=Event

Yes agree your absolutely correct. it won't work with event type and feel no point using deferrable when invocation type is event. It is just fire and forgot.

gopidesupavan commented 3 days ago

Could you please run some tests to check this is working? The pattern is so different than the different triggers that I am having hard time figuring out if that's something that might work

Sure @vincbeck I will do couple of runs with some wait time in lambda. Will update the outcome thank you 😀

gopidesupavan commented 3 days ago

Hi @vincbeck , i had triggered couple of runs , could see that trigger is hitting multiple times say for every minute, this is causing multiple invocations to lambda function from the trigger async. this is defiantly issue in this instance. agree with your comment, its not possible to make this deferrable until unless in future if there are any apis exposed to check the status of the invocation.

image
gopidesupavan commented 2 days ago

@eladkal @vincbeck WDYT should we close this and wait until if any API exposed in future. Or do you suggest any alternate approach.

vincbeck commented 2 days ago

Hi @vincbeck , i had triggered couple of runs , could see that trigger is hitting multiple times say for every minute, this is causing multiple invocations to lambda function from the trigger async. this is defiantly issue in this instance. agree with your comment, its not possible to make this deferrable until unless in future if there are any apis exposed to check the status of the invocation.

image

Now you say that, that makes total sense! Triggers are executed frequently

vincbeck commented 2 days ago

I'd vote to close it. I dont know any other solution (that not involve using other services such as steps functions, eventbrige, ...). If someone has a better idea, feel free to comment and we'll re-open.

gopidesupavan commented 2 days ago

I'd vote to close it. I dont know any other solution (that not involve using other services such as steps functions, eventbrige, ...). If someone has a better idea, feel free to comment and we'll re-open.

Thanks for closing this. Yes agree possible way by introducing other services as you mentioned above.

gopidesupavan commented 2 days ago

Hi @vincbeck , i had triggered couple of runs , could see that trigger is hitting multiple times say for every minute, this is causing multiple invocations to lambda function from the trigger async. this is defiantly issue in this instance. agree with your comment, its not possible to make this deferrable until unless in future if there are any apis exposed to check the status of the invocation.

image

Now you say that, that makes total sense! Triggers are executed frequently

Yes. Appreciate your help. Able to test and see the behaviour.