Skip to content

[BEAM-11360] Updates Dataflow Python multi-language pipelines to use portable job submission by default#13536

Merged
chamikaramj merged 3 commits intoapache:masterfrom
chamikaramj:x-lang_portable_job_submission
Dec 15, 2020
Merged

[BEAM-11360] Updates Dataflow Python multi-language pipelines to use portable job submission by default#13536
chamikaramj merged 3 commits intoapache:masterfrom
chamikaramj:x-lang_portable_job_submission

Conversation

@chamikaramj
Copy link
Contributor

@chamikaramj chamikaramj commented Dec 12, 2020

Also cleans up multi-language pipelines related hooks in Python SDK that are not needed when we use portable job submission (hence directly generate Dataflow steps from the Runner API proto).
These hooks were mostly were added in PRs #8270 and #11185.


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang SDK Dataflow Flink Samza Spark Twister2
Go Build Status --- Build Status --- Build Status ---
Java Build Status Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status Build Status
Build Status
Build Status
Build Status
Python Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
--- Build Status ---
XLang Build Status Build Status Build Status --- Build Status ---

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website Whitespace Typescript
Non-portable Build Status Build Status
Build Status
Build Status
Build Status
Build Status Build Status Build Status Build Status
Portable --- Build Status --- --- --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

See CI.md for more information about GitHub Actions CI.

@chamikaramj
Copy link
Contributor Author

R: @robertwb @ihji

@chamikaramj chamikaramj requested a review from robertwb December 14, 2020 19:49
Copy link
Contributor

@robertwb robertwb left a comment

Choose a reason for hiding this comment

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

So much nicer!

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have to do something with external_transform_finder.contains_external_transforms? (Or is this fully redundant with the statement above?)

(IMHO, this visitor pattern is a bit error-prone, perhaps it'd be worth a static method on ExternalTransformFinder that does the visiting and returns a boolean.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was redundant. Updated.

Also, updated the visitor class by adding a static method like you mentioned.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to check, this implies runner v2, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, updated to also include a Runner v2 check.

…submission by default

Also cleans up hooks in Python SDK that are not needed when we directly generate Dataflow steps from the Runner API proto.
@chamikaramj chamikaramj force-pushed the x-lang_portable_job_submission branch from 0b1ab4b to eb404ad Compare December 15, 2020 04:33
Copy link
Contributor Author

@chamikaramj chamikaramj left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, updated to also include a Runner v2 check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was redundant. Updated.

Also, updated the visitor class by adding a static method like you mentioned.

@chamikaramj chamikaramj merged commit 9f97585 into apache:master Dec 15, 2020
dxichen pushed a commit to linkedin/beam that referenced this pull request Aug 9, 2021
…portable job submission by default (apache#13536)

* Updates Dataflow Python multi-language pipelines to use portable job submission by default

Also cleans up hooks in Python SDK that are not needed when we directly generate Dataflow steps from the Runner API proto.

* Fixes yapf.

* Addresses reviewer comments.
Sign up for free to 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.

2 participants