Skip to content

[BEAM-8471] Put back FlinkJobServerDriver.fromParams to fix post commit - #9911

Merged
tweise merged 1 commit into
apache:masterfrom
tweise:BEAM-8471.postCommitFailure
Oct 29, 2019
Merged

[BEAM-8471] Put back FlinkJobServerDriver.fromParams to fix post commit#9911
tweise merged 1 commit into
apache:masterfrom
tweise:BEAM-8471.postCommitFailure

Conversation

@tweise

Copy link
Copy Markdown
Contributor

https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/testReport/org.apache.beam.sdk/PipelineTest/testTupleInjectionTransform/


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.
  • 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)

LangSDKApexDataflowFlinkGearpumpSamzaSpark
GoBuild Status------Build Status------Build Status
JavaBuild StatusBuild StatusBuild StatusBuild Status
Build Status
Build Status
Build StatusBuild StatusBuild Status
Build Status
PythonBuild Status
Build Status
Build Status
Build Status
---Build Status
Build Status
Build Status
Build Status
------Build Status
XLang---------Build Status---------

Pre-Commit Tests Status (on master branch)

---JavaPythonGoWebsite
Non-portableBuild StatusBuild Status
Build Status
Build StatusBuild Status
Portable---Build Status------

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

@tweise
tweise requested review from mxm and removed request for mxmOctober 28, 2019 23:04
@tweise

Copy link
Copy Markdown
ContributorAuthor

follow-up from #9872

@tweise

Copy link
Copy Markdown
ContributorAuthor

Run Java Flink PortableValidatesRunner Streaming

@tweise
tweise requested a review from ibzibOctober 28, 2019 23:32
@ibzib

Copy link
Copy Markdown

Any reason we can't change TestPortableRunner to use a proper interface, ie JobServerDriver? It'd definitely be less fragile than a reflection-based pseudo-interface.

@tweise

Copy link
Copy Markdown
ContributorAuthor

Any reason we can't change TestPortableRunner to use a proper interface, ie JobServerDriver? It'd definitely be less fragile than a reflection-based pseudo-interface.

Definitely worthwhile to investigate as follow-up. I don't like that fromXXX spaghetti and was trying to reduce it a bit. Will still need reflection to instantiate the class though.

@ibzib

Copy link
Copy Markdown

Will still need reflection to instantiate the class though.

Right, but we could add fromParams to the JobServerDriver interface. Then in TestPortableRunner we could narrow from Object to JobServerDriver.

@ibzibibzib left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Anyway, your changes LGTM. I don't think we need to change too much to improve the interface stuff, but if you don't want to do that here I can do it in a follow-up.

@tweise

Copy link
Copy Markdown
ContributorAuthor

Anyway, your changes LGTM. I don't think we need to change too much to improve the interface stuff, but if you don't want to do that here I can do it in a follow-up.

Go for it. What I did was to make something else work, it would be good to do a focussed cleanup.

@ibzib

Copy link
Copy Markdown

Filed https://issues.apache.org/jira/browse/BEAM-8509 as a follow-up.

Sign up for freeto 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

@tweise@ibzib