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-6144] [core] Fix addFile when source files are on "hdfs:" #4894

Closed
wants to merge 3 commits into from

Conversation

vanzin
Copy link
Contributor

@vanzin vanzin commented Mar 4, 2015

The code failed in two modes: it complained when it tried to re-create a directory that already existed, and it was placing some files in the wrong parent directory. The patch fixes both issues.

@vanzin
Copy link
Contributor Author

vanzin commented Mar 4, 2015

@trystanleftwich I'm not trying to steal your thunder :-). It's just that this is pretty urgent given the release schedule. You'll be given full credit for the change.

I tested this patch with both files and directories being distributed with SparkContext.addFile, and it works as I expect it to.

@SparkQA
Copy link

SparkQA commented Mar 4, 2015

Test build #28269 has started for PR 4894 at commit 58266aa.

  • This patch merges cleanly.

@@ -624,7 +624,8 @@ private[spark] object Utils extends Logging {
case _ =>
val fs = getHadoopFileSystem(uri, hadoopConf)
val path = new Path(uri)
fetchHcfsFile(path, new File(targetDir, path.getName), fs, conf, hadoopConf, fileOverwrite)
fetchHcfsFile(path, targetDir, fs, conf, hadoopConf, fileOverwrite,
Some(filename))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can bump this to the previous line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh actually this doesn't follow the convention for named params (filename = Some(filename)). I'll fix it, and in that case it doesn't fit in the previous line...

@andrewor14
Copy link
Contributor

LGTM provided that tests pass

@SparkQA
Copy link

SparkQA commented Mar 4, 2015

Test build #28270 has started for PR 4894 at commit 100b3a1.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Mar 4, 2015

Test build #28269 has finished for PR 4894 at commit 58266aa.

  • 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/28269/
Test PASSed.

@andrewor14
Copy link
Contributor

I'm merging this into master and 1.3. Thanks @vanzin and @trystanleftwich for your fixes.

asfgit pushed a commit that referenced this pull request Mar 4, 2015
The code failed in two modes: it complained when it tried to re-create a directory that already existed, and it was placing some files in the wrong parent directory. The patch fixes both issues.

Author: Marcelo Vanzin <vanzin@cloudera.com>
Author: trystanleftwich <trystan@atscale.com>

Closes #4894 from vanzin/SPARK-6144 and squashes the following commits:

100b3a1 [Marcelo Vanzin] Style fix.
58266aa [Marcelo Vanzin] Fix fetchHcfs file for directories.
91733b7 [trystanleftwich] [SPARK-6144]When in cluster mode using ADD JAR with a hdfs:// sourced jar will fail

(cherry picked from commit 3a35a0d)
Signed-off-by: Andrew Or <andrew@databricks.com>
@asfgit asfgit closed this in 3a35a0d Mar 4, 2015
@SparkQA
Copy link

SparkQA commented Mar 4, 2015

Test build #28270 has finished for PR 4894 at commit 100b3a1.

  • 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/28270/
Test PASSed.

@vanzin vanzin deleted the SPARK-6144 branch March 6, 2015 00:42
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.

4 participants