Skip to content

Add deferrable param in BatchOperator - #30865

Merged
jedcunningham merged 3 commits into
apache:mainfrom
astronomer:batch_async_op
May 25, 2023
Merged

Add deferrable param in BatchOperator#30865
jedcunningham merged 3 commits into
apache:mainfrom
astronomer:batch_async_op

Conversation

@pankajastro

@pankajastropankajastro commented Apr 25, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Add the deferrable param in BatchOperator.
This will allow running BatchOperator in an async way
that means we only submit a job from the worker to run a batch job
then defer to the trigger for polling and wait for a job the job status
and the worker slot won't be occupied for the whole period of
task execution.

original PR: #30500

^ 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.

@boring-cyborgboring-cyborgBot added area:providers provider:amazon AWS/Amazon - related issues labels Apr 25, 2023
@pankajastro
pankajastro marked this pull request as ready for review April 25, 2023 15:31
@pankajastropankajastro mentioned this pull request Apr 25, 2023
Closed
syedahsn
syedahsn suggested changes Apr 26, 2023
View reviewed changes

@syedahsnsyedahsn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Just need to make a few minor changes.

Comment threadairflow/providers/amazon/aws/operators/batch.py Outdated
Comment threadairflow/providers/amazon/aws/triggers/batch.py Outdated
Comment threadairflow/providers/amazon/aws/triggers/batch.py Outdated
@pankajastro
pankajastroforce-pushed the batch_async_op branch 2 times, most recently from a3f47f3 to dd1286eCompareApril 26, 2023 16:52
syedahsn
syedahsn reviewed Apr 26, 2023
View reviewed changes
Comment threadairflow/providers/amazon/aws/operators/batch.py
syedahsn
syedahsn approved these changes Apr 26, 2023
View reviewed changes

@syedahsnsyedahsn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Nice work!

@pankajkoti

pankajkoti commented Apr 28, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Can we apply the full tests needed label on this, please?

cc: @potiuk

@potiukpotiuk added the full tests needed We need to run full set of tests for this PR to merge label Apr 28, 2023
@potiukpotiuk closed this Apr 28, 2023
@potiukpotiuk reopened this Apr 28, 2023
@potiuk

potiuk commented Apr 28, 2023

Copy link
Copy Markdown
Member

Applied. Closed/reopened to rebuild it.

pankajkoti
pankajkoti reviewed May 3, 2023
View reviewed changes
Comment threadairflow/providers/amazon/aws/waiters/batch.json
@pankajastro
pankajastro marked this pull request as draft May 25, 2023 14:17
@pankajastro
pankajastroforce-pushed the batch_async_op branch 2 times, most recently from 7073f19 to 3f177d0CompareMay 25, 2023 14:18
@pankajastro
pankajastro marked this pull request as ready for review May 25, 2023 15:29
@pankajastro@phanikumv
Add deferrable param in BatchOperator
d60913b
Add the deferrable param in BatchOperator.
This will allow running BatchOperator in an async way
that means we only submit a job from the worker to run a batch job
then defer to the trigger for polling and wait for a job the job status
and the worker slot won't be occupied for the whole period of
task execution.
@pankajastro@phanikumv
Add logs in trigger
b6d9a78
jedcunningham
jedcunningham reviewed May 25, 2023
View reviewed changes
Comment threadairflow/providers/amazon/aws/operators/batch.py Outdated
Comment threadairflow/providers/amazon/aws/waiters/batch.json
@pankajastro@jedcunningham
Apply review suggestion
cc7da71
Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
jedcunningham
jedcunningham approved these changes May 25, 2023
View reviewed changes
Comment threadairflow/providers/amazon/aws/waiters/batch.json
syedahsn
syedahsn approved these changes May 25, 2023
View reviewed changes

@syedahsnsyedahsn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Nice work!

@jedcunningham
jedcunningham merged commit 5b0f668 into apache:mainMay 25, 2023
@jedcunningham
jedcunningham deleted the batch_async_op branch May 25, 2023 23:24
@pankajastropankajastro mentioned this pull request May 29, 2023
Closed
2 tasks
@hussein-awalahussein-awala mentioned this pull request May 31, 2023
Closed
2 tasks
@phanikumvphanikumv mentioned this pull request Jun 5, 2023
Closed
@eladkaleladkal mentioned this pull request Jun 20, 2023
Closed
86 tasks
@vandonr-amzvandonr-amz mentioned this pull request Aug 2, 2023
Closed
2 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pankajkotipankajkotipankajkoti left review comments
@jedcunninghamjedcunninghamjedcunningham approved these changes
@vincbeckvincbeckvincbeck approved these changes
@eladkaleladkalAwaiting requested review from eladkal
@o-nikolaso-nikolasAwaiting requested review from o-nikolas
+1 more reviewer
@syedahsnsyedahsnsyedahsn approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:providersfull tests neededWe need to run full set of tests for this PR to mergeprovider:amazonAWS/Amazon - related issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@pankajastro@pankajkoti@potiuk@jedcunningham@vincbeck@syedahsn