Skip to content

Add deferrable mode to BeamRunPythonPipelineOperator - #31471

Merged
pankajastro merged 4 commits into
apache:mainfrom
lwyszomi:beam-python-pipeline-def
Jul 13, 2023
Merged

Add deferrable mode to BeamRunPythonPipelineOperator#31471
pankajastro merged 4 commits into
apache:mainfrom
lwyszomi:beam-python-pipeline-def

Conversation

@VladaZakharova

Copy link
Copy Markdown
Contributor

This PR adds the ability to start BeamRunPythonOperator asynchronously using deferrable mode. The documentation was updated with the description of how to work with deferrable parameter.

Additionally, the existing implementation was fixed:

  • for the BeamRunPythonOperator unused call of dataflow_hook.wait_for_done was removed, since the waiting process for the pipeline to be successfully finished was covered by the beam_hook.start_python_pipeline itself: this method creates the process to execute command in Apache Beam and waits for its end of execution in parallel with outputting logs. The same approach was used while implementing deferrable mode for the Operator, but using asynchronous calls.
  • version of Apache-beam package in system tests was updated

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

@VladaZakharova

Copy link
Copy Markdown
ContributorAuthor

Hi @potiuk !
Could you please take a look on this PR or add someone here to review changes?
Thanks!

@Lee-WLee-W mentioned this pull request Jun 6, 2023
@potiuk
potiukforce-pushed the beam-python-pipeline-def branch from f57cc11 to 3094bceCompareJune 8, 2023 19:19
@potiuk

Copy link
Copy Markdown
Member

Some docstrings need fixing as we already added D400 check for docstrings.

@VladaZakharova
VladaZakharovaforce-pushed the beam-python-pipeline-def branch from 3094bce to 606047dCompareJune 12, 2023 08:30
@VladaZakharova

Copy link
Copy Markdown
ContributorAuthor

@potiuk
Hi there :)
I have rebased and fixed static checks, could you please verify? Thanks!

@eladkaleladkal changed the title Add deferrable mode to BeamRunPythonPipelineOperatorAdd deferrable mode to BeamRunPythonPipelineOperatorJul 9, 2023
@potiuk

Copy link
Copy Markdown
Member

There is one more "default deferrable" to fix.

@VladaZakharova
VladaZakharovaforce-pushed the beam-python-pipeline-def branch from 7b217c9 to d2f32d8CompareJuly 10, 2023 11:45
@potiuk
potiukforce-pushed the beam-python-pipeline-def branch from d2f32d8 to 6c49c4eCompareJuly 10, 2023 16:42
@VladaZakharova
VladaZakharovaforce-pushed the beam-python-pipeline-def branch from 6c49c4e to c6c1852CompareJuly 11, 2023 13:13
@VladaZakharova

Copy link
Copy Markdown
ContributorAuthor

@potiuk
Hi there! Could you please recheck this PR after fixing the "deferrable" parameter? Thanks a lot!

Comment threadairflow/providers/apache/beam/hooks/beam.py
Comment threadairflow/providers/apache/beam/hooks/beam.py
@VladaZakharova

Copy link
Copy Markdown
ContributorAuthor

@potiuk@pankajastro
Hi! Do we have anything else to fix here?

@pankajastro

Copy link
Copy Markdown
Member

@potiuk@pankajastro
Hi! Do we have anything else to fix here?

Hi just posted one small question otherwise looks good,

Comment threadtests/system/providers/apache/beam/example_python_async.py Outdated
@VladaZakharova
VladaZakharovaforce-pushed the beam-python-pipeline-def branch from 9358e0e to 5f2ad03CompareJuly 13, 2023 15:46
@VladaZakharova

Copy link
Copy Markdown
ContributorAuthor

@pankajastro
I have fixed this small mistake, could you please check? :)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@VladaZakharova@potiuk@pankajastro