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-6081] Support fetching http/https uris in driver runner. #4832

Closed
wants to merge 1 commit into from

Conversation

tnachen
Copy link
Contributor

@tnachen tnachen commented Feb 28, 2015

Currently if passed uris such as http/https, it won't able to fetch them as it only calls HadoopFs get.
This fix utilizes the existing util method to fetch remote uris as well.

@tnachen
Copy link
Contributor Author

tnachen commented Feb 28, 2015

@andrewor14

@SparkQA
Copy link

SparkQA commented Feb 28, 2015

Test build #28123 has finished for PR 4832 at commit aa52cd6.

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

@tnachen
Copy link
Contributor Author

tnachen commented Feb 28, 2015

Test failure doesn't look anything to do with my change?

@srowen
Copy link
Member

srowen commented Feb 28, 2015

Unrelated it seems. This would require a JIRA. I'm not sure about how important this is vs passing the SecurityManager around one more layer, but in any event the change is not big. I defer to others for an opinion.

@tnachen tnachen changed the title Support fetching remote uris in driver runner. [SPARK-6081] Support fetching http/https uris in driver runner. Feb 28, 2015
@tnachen
Copy link
Contributor Author

tnachen commented Feb 28, 2015

I'm curious too about SecurityManager, let me know what makes sense about it.

@andrewor14
Copy link
Contributor

retest this please

@@ -468,6 +468,7 @@ private[spark] object Utils extends Logging {
in: InputStream,
destFile: File,
fileOverwrite: Boolean): Unit = {
logDebug(s"Creating temp file in ${destFile.getParentFile.getAbsolutePath}")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should add this

@andrewor14
Copy link
Contributor

Looks good.

@SparkQA
Copy link

SparkQA commented Mar 25, 2015

Test build #29126 has finished for PR 4832 at commit aa52cd6.

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

@andrewor14
Copy link
Contributor

I'm merging this into master after fixing the comments myself. Thanks.

@asfgit asfgit closed this in 320bca4 Apr 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants