Skip to content

ARROW-5466: [Java][CI] Dockerize Java CI, run all JDK builds in single Travis entry #4761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

ARROW-5466: [Java][CI] Dockerize Java CI, run all JDK builds in single Travis entry #4761

wants to merge 4 commits into from

Conversation

@wesm
Copy link
Member

@wesm wesm commented Jun 30, 2019 •
edited
Loading

Since OpenJDK9 has been superseded by OpenJDK11, it is not available in package repositories, so I'm not sure it's worth maintaining a build for this.

I have pushed a pre-built Docker image for this to

https://cloud.docker.com/u/ursalab/repository/docker/ursalab/arrow-ci-java-all-jdks

@codecov-io
Copy link

codecov-io commented Jun 30, 2019

Codecov Report

Merging #4761 into master will increase coverage by 1.94%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4761      +/-   ##
==========================================
+ Coverage   86.44%   88.39%   +1.94%     
==========================================
  Files         992      908      -84     
  Lines      138020   114089   -23931     
  Branches     1418     1418              
==========================================
- Hits       119309   100845   -18464     
+ Misses      18349    12882    -5467     
  Partials      362      362
Impacted Files Coverage Δ
cpp/src/arrow/util/thread-pool-test.cc 97.66% <0%> (-0.94%) ⬇️
rust/arrow/src/error.rs
rust/parquet/src/util/test_common/page_util.rs
rust/arrow/src/ipc/gen/SparseTensor.rs
rust/parquet/src/util/memory.rs
rust/arrow/src/compute/array_ops.rs
rust/parquet/src/record/api.rs
rust/datafusion/src/sql/parser.rs
rust/arrow/src/json/reader.rs
...t/datafusion/src/optimizer/projection_push_down.rs
... and 77 more

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 c29462c...cb53833. Read the comment docs.

@wesm
Copy link
Member Author

wesm commented Jun 30, 2019

Well that was relatively painless

@wesm wesm requested review from BryanCutler and pravindra June 30, 2019 19:06
xhochy
xhochy approved these changes Jun 30, 2019
View reviewed changes
Copy link
Member

@xhochy xhochy left a comment

Choose a reason for hiding this comment

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

+1, LGTM

kou
kou reviewed Jun 30, 2019
View reviewed changes
docker-compose.yml Outdated
Copy link
Member

@kou kou Jun 30, 2019

Choose a reason for hiding this comment

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

Should we use ursalabs (the last s is missing) for organization name on Docker Hub?

Copy link
Member Author

@wesm wesm Jun 30, 2019

Choose a reason for hiding this comment

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

It seems that this name is already taken on Docker Hub. I don't know who owns it

Copy link
Member

@kou kou Jul 1, 2019

Choose a reason for hiding this comment

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

Oh. I got it.

java/Dockerfile.all-jdks Outdated
Copy link
Member

@kou kou Jun 30, 2019

Choose a reason for hiding this comment

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

We can install both openjdk-8-jdk and openjdk-11-jdk without this:

Copy link
Member Author

@wesm wesm Jun 30, 2019

Choose a reason for hiding this comment

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

OK, thanks

ci/docker_build_java.sh Outdated
Copy link
Member

@kou kou Jun 30, 2019

Choose a reason for hiding this comment

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

How about adding --delete rsync option or removing ${arrow_src} before rsync?
If there are some built files with OpenJDK 8, OpenJDK 11 build may be conflicted.

Copy link
Member Author

@wesm wesm Jun 30, 2019

Choose a reason for hiding this comment

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

OK, will do

pravindra
pravindra approved these changes Jul 1, 2019
View reviewed changes
Copy link
Contributor

@pravindra pravindra left a comment

Choose a reason for hiding this comment

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

lgtm

@wesm wesm force-pushed the java-dockerify branch from cb53833 to cf568a2 Compare July 1, 2019 22:27
@wesm
Copy link
Member Author

wesm commented Jul 1, 2019

Will merge this once the build runs

BryanCutler
BryanCutler approved these changes Jul 1, 2019
View reviewed changes
Copy link
Member

@BryanCutler BryanCutler left a comment

Choose a reason for hiding this comment

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

Looks good, I just had a couple minor questions

.travis.yml
- name: "Java w/ OpenJDK 11"
language: java
- name: "Java OpenJDK8 and OpenJDK11"
language: cpp
Copy link
Member

@BryanCutler BryanCutler Jul 1, 2019

Choose a reason for hiding this comment

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

Should language be Java?

Copy link
Member Author

@wesm wesm Jul 2, 2019

Choose a reason for hiding this comment

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

This one doesn't matter b/c it's Java

ci/docker_build_java.sh Outdated

pushd $arrow_src/java
mvn -B -DskipTests -Drat.skip=true install
mvn -B -DskipTests -Drat.skip=true install
Copy link
Member

@BryanCutler BryanCutler Jul 1, 2019

Choose a reason for hiding this comment

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

Just to confirm, this is build only and you don't want tests run?

Copy link
Member Author

@wesm wesm Jul 2, 2019

Choose a reason for hiding this comment

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

Ah, thanks for catching. Yeah we need to run the tests. I'll fix

@wesm wesm closed this in e02ec4e Jul 2, 2019
@wesm wesm deleted the java-dockerify branch July 2, 2019 04:35
kou pushed a commit that referenced this pull request Jul 4, 2019
@wesm @kou
ARROW-5466: [Java][CI] Dockerize Java CI, run all JDK builds in singl…
a148982 
…e Travis entry

Since OpenJDK9 has been superseded by OpenJDK11, it is not available in package repositories, so I'm not sure it's worth maintaining a build for this.

I have pushed a pre-built Docker image for this to

https://cloud.docker.com/u/ursalab/repository/docker/ursalab/arrow-ci-java-all-jdks

Author: Wes McKinney <wesm+git@apache.org>

Closes #4761 from wesm/java-dockerify and squashes the following commits:

8b81037 <Wes McKinney> Actually run Java unit tests
cf568a2 <Wes McKinney> Code review feedback
6cb5e3f <Wes McKinney> Build Javadoc in docker-compose job
d9c3900 <Wes McKinney> Run all Java builds in a single Dockerized Travis CI entry
kszucs pushed a commit that referenced this pull request Jul 22, 2019
@wesm @kszucs
ARROW-5466: [Java][CI] Dockerize Java CI, run all JDK builds in singl…
a3242fb 
…e Travis entry

Since OpenJDK9 has been superseded by OpenJDK11, it is not available in package repositories, so I'm not sure it's worth maintaining a build for this.

I have pushed a pre-built Docker image for this to

https://cloud.docker.com/u/ursalab/repository/docker/ursalab/arrow-ci-java-all-jdks

Author: Wes McKinney <wesm+git@apache.org>

Closes #4761 from wesm/java-dockerify and squashes the following commits:

8b81037 <Wes McKinney> Actually run Java unit tests
cf568a2 <Wes McKinney> Code review feedback
6cb5e3f <Wes McKinney> Build Javadoc in docker-compose job
d9c3900 <Wes McKinney> Run all Java builds in a single Dockerized Travis CI entry
@asfimport asfimport mentioned this pull request Jul 2, 2019
Closed
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
@wesm @kou
ARROW-5466: [Java][CI] Dockerize Java CI, run all JDK builds in singl…
d105d1b 
…e Travis entry

Since OpenJDK9 has been superseded by OpenJDK11, it is not available in package repositories, so I'm not sure it's worth maintaining a build for this.

I have pushed a pre-built Docker image for this to

https://cloud.docker.com/u/ursalab/repository/docker/ursalab/arrow-ci-java-all-jdks

Author: Wes McKinney <wesm+git@apache.org>

Closes apache#4761 from wesm/java-dockerify and squashes the following commits:

8b81037 <Wes McKinney> Actually run Java unit tests
cf568a2 <Wes McKinney> Code review feedback
6cb5e3f <Wes McKinney> Build Javadoc in docker-compose job
d9c3900 <Wes McKinney> Run all Java builds in a single Dockerized Travis CI entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kou kou kou left review comments

@xhochy xhochy xhochy approved these changes

@pravindra pravindra pravindra approved these changes

@BryanCutler BryanCutler BryanCutler approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@wesm @codecov-io @kou @xhochy @pravindra @BryanCutler