Skip to content
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.

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

Require Java 8 and include some Java 8 dependencies. #3914

Merged
merged 4 commits into from
Feb 14, 2017
Merged

Conversation

gianm
Copy link
Contributor

@gianm gianm commented Feb 7, 2017

Patch for #3746, #3745, #3710, #3595.

  • Upgrade Jetty to 9.3.16.v20170120.
  • Upgrade DataSketches to 0.8.4.
  • Bundle caffeine-cache by default.
  • Still target Java 7 when compiling base Druid classes.

Our hadoop gauntlet tests don't pass with this, because they are running on Java 7. But hopefully they will pass after upgrading that test environment to Java 8. In the meantime I'm doing this as a WIP PR just so people can see the changes and try it in their own environment if they want.

- Upgrade Jetty to 9.3.16.v20170120.
- Upgrade DataSketches to 0.8.4.
- Bundle caffeine-cache by default.
- Still target Java 7 when compiling base Druid classes.
@gianm gianm added this to the 0.10.0 milestone Feb 7, 2017
@gianm
Copy link
Contributor Author

gianm commented Feb 14, 2017

Shall we also disable travis builds for jdk7? They, of course, do not pass after applying this patch.

@jon-wei
Copy link
Contributor

jon-wei commented Feb 14, 2017

I tested this PR for hadoop compatibility with:

  • sequenceiq hadoop docker image 2.3.0 to 2.7.3
  • cloudera quickstart docker image latest
  • hortonworks hdp 2.5 docker image
  • an EMR 5.0.0 cluster

all running Java 8 on the hadoop cluster side, running a batch ingestion task on the wikiticker data, those all passed

@gianm
Copy link
Contributor Author

gianm commented Feb 14, 2017

Thanks @jon-wei! In that case I'll take off the WIP tag and this is ready for review.

@gianm gianm changed the title [WIP] Require Java 8 and include some Java 8 dependencies. Require Java 8 and include some Java 8 dependencies. Feb 14, 2017
Copy link
Contributor

@drcrallen drcrallen left a comment

Choose a reason for hiding this comment

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

Travis needs fixing, but other than that, can the changes here be limited to only what is required to get java 8 the default in the current codebase, and leave the "things that get better when we allow java8" in a separate PR?

@@ -38,7 +38,7 @@
<dependency>
<groupId>com.yahoo.datasketches</groupId>
<artifactId>sketches-core</artifactId>
<version>0.4.1</version>
<version>0.8.4</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really germane to java 8?

@drcrallen
Copy link
Contributor

Discussed in dev sync. @gianm was going to update travis yaml before merge

@gianm
Copy link
Contributor Author

gianm commented Feb 14, 2017

Removed jdk7 from travis.yml.

@fjy
Copy link
Contributor

fjy commented Feb 14, 2017

👍

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.

4 participants