Skip to content

Comments

[BEAM-11021] Add unsupported BundleFinalizationHandler to portable batch Flink.#13054

Merged
boyuanzz merged 1 commit intoapache:masterfrom
boyuanzz:flink
Oct 12, 2020
Merged

[BEAM-11021] Add unsupported BundleFinalizationHandler to portable batch Flink.#13054
boyuanzz merged 1 commit intoapache:masterfrom
boyuanzz:flink

Conversation

@boyuanzz
Copy link
Contributor

@boyuanzz boyuanzz commented Oct 8, 2020

This PR added an unsupported BundleFinalizationHandler to portable Flink in batch mode. This will give users a more detailed error message like:

ERROR:root:java.lang.UnsupportedOperationException: Portable Flink runner doesn't support bundle finalization in batch mode. For more details, please refer to https://issues.apache.org/jira/browse/BEAM-11021.

when users try to use bundle finalization in batch.

r: @mxm
cc: @lukecwik @ibzib


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

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.

@codecov
Copy link

codecov bot commented Oct 9, 2020

Codecov Report

Merging #13054 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13054      +/-   ##
==========================================
- Coverage   82.49%   82.48%   -0.01%     
==========================================
  Files         455      455              
  Lines       54876    54876              
==========================================
- Hits        45270    45266       -4     
- Misses       9606     9610       +4     
Impacted Files Coverage Δ
.../python/apache_beam/transforms/periodicsequence.py 96.49% <0.00%> (-1.76%) ⬇️
sdks/python/apache_beam/runners/common.py 88.75% <0.00%> (-0.45%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 571338b...6b00b0b. Read the comment docs.

@boyuanzz
Copy link
Contributor Author

boyuanzz commented Oct 9, 2020

Run Java PreCommit

Copy link
Contributor

@mxm mxm left a comment

Choose a reason for hiding this comment

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

Test failure, but looks good otherwise.

@boyuanzz
Copy link
Contributor Author

boyuanzz commented Oct 9, 2020

Test failure, but looks good otherwise.

Thanks for the quick approval! The spark unit tests were complaining mocking ambiguous stageBundleFactory.getBundle() method.

@boyuanzz
Copy link
Contributor Author

boyuanzz commented Oct 9, 2020

Run Python_PVR_Flink PreCommit

2 similar comments
@boyuanzz
Copy link
Contributor Author

boyuanzz commented Oct 9, 2020

Run Python_PVR_Flink PreCommit

@boyuanzz
Copy link
Contributor Author

boyuanzz commented Oct 9, 2020

Run Python_PVR_Flink PreCommit

@boyuanzz
Copy link
Contributor Author

boyuanzz commented Oct 9, 2020

Flink test failures are cased by #13014

@boyuanzz
Copy link
Contributor Author

Run Python_PVR_Flink PreCommit

@mxm
Copy link
Contributor

mxm commented Oct 12, 2020

Test Result (1 failure / +1)

    apache_beam.runners.portability.flink_runner_test.FlinkRunnerTestStreaming.test_register_finalizations

https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Phrase/16/

This is not expected?

@boyuanzz
Copy link
Contributor Author

Run Python_PVR_Flink PreCommit

@boyuanzz
Copy link
Contributor Author

Test Result (1 failure / +1)

    apache_beam.runners.portability.flink_runner_test.FlinkRunnerTestStreaming.test_register_finalizations

https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Phrase/16/

This is not expected?

This is not expected. It seems like the test is kind of flaky .

@boyuanzz
Copy link
Contributor Author

Thanks for your quick review! I'm going to merge this PR.

@boyuanzz boyuanzz merged commit c0755cd into apache:master Oct 12, 2020
@boyuanzz boyuanzz deleted the flink branch October 14, 2020 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants