Skip to content

Implement java precommit dataflow examples tests to run on java 11#10882

Closed
pawelpasterz wants to merge 1 commit intoapache:masterfrom
pawelpasterz:java-precommit-dataflow-examples-java11
Closed

Implement java precommit dataflow examples tests to run on java 11#10882
pawelpasterz wants to merge 1 commit intoapache:masterfrom
pawelpasterz:java-precommit-dataflow-examples-java11

Conversation

@pawelpasterz
Copy link
Contributor

Implement PreCommit Dataflow Examples to run on java 11


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 Apex Dataflow Flink Gearpump Samza Spark
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
Python Build 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)

--- Java Python Go Website
Non-portable 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.

@mwalenia
Copy link
Member

run seed job

@mwalenia
Copy link
Member

Run Java_Examples_Dataflow_Java11 PreCommit

@mwalenia
Copy link
Member

run seed job

@mwalenia
Copy link
Member

Run java precommit

@mwalenia
Copy link
Member

Run Java_Examples_Dataflow PreCommit

@mwalenia
Copy link
Member

Run Java_Examples_Dataflow_Java11 PreCommit

@mwalenia
Copy link
Member

LGTM, when the tests finish, I'll merge it. Thanks for your contribution!

@mwalenia
Copy link
Member

Run Java PreCommit

3 similar comments
@mwalenia
Copy link
Member

Run Java PreCommit

@mwalenia
Copy link
Member

Run Java PreCommit

@mwalenia
Copy link
Member

Run Java PreCommit

@mwalenia
Copy link
Member

run java precommit

@pawelpasterz pawelpasterz force-pushed the java-precommit-dataflow-examples-java11 branch from e137696 to b8aad00 Compare February 18, 2020 11:53
@mwalenia
Copy link
Member

retest this please

1 similar comment
@mwalenia
Copy link
Member

retest this please

@pawelpasterz pawelpasterz force-pushed the java-precommit-dataflow-examples-java11 branch from b8aad00 to 67b3b73 Compare February 19, 2020 06:09
@kkucharc
Copy link
Contributor

run seed job

@kkucharc
Copy link
Contributor

retest this please

@pawelpasterz pawelpasterz force-pushed the java-precommit-dataflow-examples-java11 branch from 67b3b73 to 52e97c8 Compare February 19, 2020 11:54
@kkucharc
Copy link
Contributor

Run Java PreCommit

@aaltay
Copy link
Member

aaltay commented Feb 20, 2020

/cc: @kennknowles

@mwalenia
Copy link
Member

@kennknowles do you have anything to add to the review, or can we merge this?

@aaltay
Copy link
Member

aaltay commented Feb 26, 2020

@kennknowles or @Ardagan - could either one of you review this PR please?

@kennknowles
Copy link
Member

Sorry - got this mixed up with a similar PR and lost track.

Copy link
Member

@kennknowles kennknowles left a comment

Choose a reason for hiding this comment

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

Naively, the idea is:

  1. Build and publish Beam main jars with Java 8
  2. Build and test with Java 11

I think this PR is on the right path but maybe some more research into how to make this robust?

Copy link
Member

Choose a reason for hiding this comment

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

Could this step be a publish?

Copy link
Member

Choose a reason for hiding this comment

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

And then this step point to the deps from the prior step?

Copy link
Member

Choose a reason for hiding this comment

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

Doing these exclusions seems a fragile. Is there a way to indicate that some dependencies are provided from published cache and not built?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kennknowles thanks for all comments, first I thought I will answer to all your comments but then I decided to gather it all in just one. I can see there is lot of knowledge that is missing to me right now, can we contact outside this pr scope? (ex email?) I would very appreciate if you will share some knowledge with me. What do you think?

Copy link
Contributor

@Ardagan Ardagan left a comment

Choose a reason for hiding this comment

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

The PR looks valid for me overall. However there's a concern regarding the approach.

Current approach requires someone to maintain the list of exclusions that adds technical debt. Also once we migrate to Java11 we'll have similar issue since we'd want to verify we can still run Java8 pipelines. That would worsen the situation.

Do we have any plan on how to deal with further support of these tests?

Another concern is inline: do we only run tests with Java11 or actually compile test pipelines with Java11?
It would be great if we can add tests that show which version of JVM is used for execution as well as which version of Java was used for compiling tests.
For the first, we can look into ManagementFactory.getRuntimeMXBean().getVmVersion().
Not positive on the best approach for second though.

@pawelpasterz pawelpasterz force-pushed the java-precommit-dataflow-examples-java11 branch from 52e97c8 to b562775 Compare March 6, 2020 08:12
@mwalenia
Copy link
Member

mwalenia commented Mar 6, 2020

retest this please

@mwalenia
Copy link
Member

mwalenia commented Mar 6, 2020

run seed job

@mwalenia
Copy link
Member

mwalenia commented Mar 6, 2020

Run Java_Examples_Dataflow_Java11 PreCommit

2 similar comments
@mwalenia
Copy link
Member

mwalenia commented Mar 6, 2020

Run Java_Examples_Dataflow_Java11 PreCommit

@mwalenia
Copy link
Member

mwalenia commented Mar 6, 2020

Run Java_Examples_Dataflow_Java11 PreCommit

@mwalenia
Copy link
Member

mwalenia commented Mar 6, 2020

Run Java PreCommit

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.

6 participants