Skip to content

[BEAM-8509] TestPortableRunner uses JobServerDriver interface - #9914

Merged
mxm merged 1 commit into
apache:masterfrom
ibzib:jsd-interface
Oct 29, 2019
Merged

[BEAM-8509] TestPortableRunner uses JobServerDriver interface#9914
mxm merged 1 commit into
apache:masterfrom
ibzib:jsd-interface

Conversation

@ibzib

Copy link
Copy Markdown

Unfortunately, due to Java language limitations we cannot define fromParams as an abstract static method. But at least start and stop are enforced by the type system now.

R: @tweise@mxm

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.

Unfortunately, due to Java language limitations we cannot define fromParams as an abstract static method. But at least start and stop are enforced by the type system now.
@ibzib

Copy link
Copy Markdown
Author

Run Java Flink PortableValidatesRunner Batch

mxm
mxm approved these changes Oct 29, 2019

@mxmmxm 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.

LGTM

try {
jobServerDriver =
InstanceBuilder.ofType(jobServerDriverClass)
.fromFactoryMethod("fromParams")

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.

We should watch out for #9872 because the method name/usage changes there.

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.

Oh didn't see that this was a follow-up to #9911 :)

@mxm
mxm merged commit 7047a05 into apache:masterOct 29, 2019
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

@ibzib@mxm