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-1209 [CORE] (Take 2) SparkHadoop{MapRed,MapReduce}Util should not use package org.apache.hadoop #3048

Closed
wants to merge 3 commits into from

Conversation

srowen
Copy link
Member

@srowen srowen commented Nov 1, 2014

@andrewor14 Another try at SPARK-1209, to address #2814 (comment)

I successfully tested with mvn -Dhadoop.version=1.0.4 -DskipTests clean package; mvn -Dhadoop.version=1.0.4 test I assume that is what failed Jenkins last time. I also tried -Dhadoop.version1.2.1 and -Phadoop-2.4 -Pyarn -Phive for more coverage.

So this is why the class was put in org.apache.hadoop to begin with, I assume. One option is to leave this as-is for now and move it only when Hadoop 1.0.x support goes away.

This is the other option, which adds a call to force the constructor to be public at run-time. It's probably less surprising than putting Spark code in org.apache.hadoop, but, does involve reflection. A SecurityManager might forbid this, but it would forbid a lot of stuff Spark does. This would also only affect Hadoop 1.0.x it seems.

@SparkQA
Copy link

SparkQA commented Nov 1, 2014

Test build #22695 has started for PR 3048 at commit 0d48f4b.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 1, 2014

Test build #22695 has finished for PR 3048 at commit 0d48f4b.

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

@AmplabJenkins
Copy link

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

@andrewor14
Copy link
Contributor

LGTM. @pwendell we should consider merging this into 1.2. Can you look at the changes here?

@pwendell
Copy link
Contributor

Cool let's pull this in and see how it goes.

asfgit pushed a commit that referenced this pull request Nov 10, 2014
…ot use package org.apache.hadoop

andrewor14 Another try at SPARK-1209, to address #2814 (comment)

I successfully tested with `mvn -Dhadoop.version=1.0.4 -DskipTests clean package; mvn -Dhadoop.version=1.0.4 test` I assume that is what failed Jenkins last time. I also tried `-Dhadoop.version1.2.1` and `-Phadoop-2.4 -Pyarn -Phive` for more coverage.

So this is why the class was put in `org.apache.hadoop` to begin with, I assume. One option is to leave this as-is for now and move it only when Hadoop 1.0.x support goes away.

This is the other option, which adds a call to force the constructor to be public at run-time. It's probably less surprising than putting Spark code in `org.apache.hadoop`, but, does involve reflection. A `SecurityManager` might forbid this, but it would forbid a lot of stuff Spark does. This would also only affect Hadoop 1.0.x it seems.

Author: Sean Owen <sowen@cloudera.com>

Closes #3048 from srowen/SPARK-1209 and squashes the following commits:

0d48f4b [Sean Owen] For Hadoop 1.0.x, make certain constructors public, which were public in later versions
466e179 [Sean Owen] Disable MIMA warnings resulting from moving the class -- this was also part of the PairRDDFunctions type hierarchy though?
eb61820 [Sean Owen] Move SparkHadoopMapRedUtil / SparkHadoopMapReduceUtil from org.apache.hadoop to org.apache.spark

(cherry picked from commit f8e5732)
Signed-off-by: Patrick Wendell <pwendell@gmail.com>
@asfgit asfgit closed this in f8e5732 Nov 10, 2014
@srowen srowen deleted the SPARK-1209 branch November 11, 2014 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants