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

[FLINK-6982] [guava] Integrate flink-shaded-guava-18 #4503

Closed
wants to merge 11 commits into from

Conversation

zentol
Copy link
Contributor

@zentol zentol commented Aug 9, 2017

What is the purpose of the change

This PR integrates the shaded guava dependency from flink-shaded. Basically, replace all usages of guava with the shaded guava dependency and remove all traces of the original dependency.

Brief change log

  • replace all guava dependencies with flink-shaded-guava
  • replace all guava imports
  • modify illegal import checkstyle rule to forbid unshaded guava imports
  • add suppression for IllegalImport rule for cassandra&kinesis
  • add check in travis watchdog that no unshaded guava classes are present in flink-dist

Verifying this change

The commits that replace existing guava usages are straight-forward.

Special care should be given to the last commit that removes the root pom shading of guava. Several modules still transitively rely on vanilla guava (for example cassandra) and have to do their own shading now. Some of these modules already did that (like cassandra of flink-shaded-hadoop2), but others didn't and relied on the fail-safe configuration of the root pom.

  • check that compilation works
  • check artifacts for inclusion of shaded guava
  • check artifacts for exclusion of unshaded guava
  • check that we are not exposing a vanilla guava dependency with maven
  • start a cluster and run some examples

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (yes)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

@StephanEwen
Copy link
Contributor

Looks pretty good, all in all!

+1 from my side

@zentol
Copy link
Contributor Author

zentol commented Aug 14, 2017

merging,

zentol added a commit to zentol/flink that referenced this pull request Aug 14, 2017
zentol added a commit to zentol/flink that referenced this pull request Aug 14, 2017
@asfgit asfgit closed this in 8dfb9d0 Aug 14, 2017
@zentol zentol deleted the 6982 branch August 14, 2017 16:17
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.

3 participants