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

[SPARK-3647] Add more exceptions to Guava relocation. #2496

Closed
wants to merge 1 commit into from

Conversation

vanzin
Copy link
Contributor

@vanzin vanzin commented Sep 22, 2014

Guava's Optional refers to some package private classes / methods, and
when those are relocated the code stops working, throwing exceptions.
So add the affected classes to the exception list too, and add a unit
test.

(Note that this unit test only really makes sense in maven, since we
don't relocate in the sbt build. Also, JavaAPISuite doesn't seem to
be run by "mvn test" - I had to manually add command line options to
enable it.)

Guava's Optional refers to some package private classes / methods, and
when those are relocated the code stops working, throwing exceptions.
So add the affected classes to the exception list too, and add a unit
test.

(Note that this unit test only really makes sense in maven, since we
don't relocate in the sbt build. Also, JavaAPISuite doesn't seem to
be run by "mvn test" - I had to manually add command line options to
enable it.)
@vanzin
Copy link
Contributor Author

vanzin commented Sep 22, 2014

I verified that the test triggers the issue without the pom changes.

@SparkQA
Copy link

SparkQA commented Sep 22, 2014

QA tests have started for PR 2496 at commit 84f58d7.

  • This patch merges cleanly.

@pwendell
Copy link
Contributor

LGTM - I looked at the Optional Source code and in the current version these are the only two other classes that are referenced.

@SparkQA
Copy link

SparkQA commented Sep 22, 2014

QA tests have finished for PR 2496 at commit 84f58d7.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 22, 2014

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20669/

@pwendell
Copy link
Contributor

Thanks @vanzin for contributing this!

@asfgit asfgit closed this in 8dfe79f Sep 23, 2014
@vanzin vanzin deleted the SPARK-3647 branch September 23, 2014 21:37
markhamstra pushed a commit to markhamstra/spark that referenced this pull request Oct 3, 2014
Guava's Optional refers to some package private classes / methods, and
when those are relocated the code stops working, throwing exceptions.
So add the affected classes to the exception list too, and add a unit
test.

(Note that this unit test only really makes sense in maven, since we
don't relocate in the sbt build. Also, JavaAPISuite doesn't seem to
be run by "mvn test" - I had to manually add command line options to
enable it.)

Author: Marcelo Vanzin <vanzin@cloudera.com>

Closes apache#2496 from vanzin/SPARK-3647 and squashes the following commits:

84f58d7 [Marcelo Vanzin] [SPARK-3647] Add more exceptions to Guava relocation.

Conflicts:
	assembly/pom.xml
	core/pom.xml
	core/src/test/java/org/apache/spark/JavaAPISuite.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants