-
Notifications
You must be signed in to change notification settings - Fork 28.4k
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-6300][Spark Core] sc.addFile(path) does not support the relative path. #4993
Conversation
merge lastest spark
merge lastest spark
merge lastest spark
merge lastest spark
Can one of the admins verify this patch? |
Thanks for taking this up @DoingDone9. Would it be possible to add a test for this? |
val schemeCorrectedPath = uri.getScheme match { | ||
case null | "local" => "file:" + uri.getPath | ||
case null | "local" => "file:" + file.getCanonicalPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For scoping, probably better to move new File(path)
into the match block.
Also, we should include the "file" scheme in the case as well.
I have added the test, please test it. @sryza |
ok to test |
Test build #28516 has started for PR 4993 at commit
|
Test build #28516 has finished for PR 4993 at commit
|
Test PASSed. |
@@ -1093,7 +1093,7 @@ class SparkContext(config: SparkConf) extends Logging with ExecutorAllocationCli | |||
def addFile(path: String, recursive: Boolean): Unit = { | |||
val uri = new URI(path) | |||
val schemeCorrectedPath = uri.getScheme match { | |||
case null | "local" => "file:" + uri.getPath | |||
case null | "local" => "file:" + new File(path).getCanonicalPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be slightly more robust as new File(path).getCanonicalFile.toURI.toString
. Then the "file:"
can go away too.
Test build #28546 has started for PR 4993 at commit
|
I have changed it. Please test it. @srowen |
Test build #28547 has started for PR 4993 at commit
|
Test build #28546 has finished for PR 4993 at commit
|
Test PASSed. |
Test build #28547 has finished for PR 4993 at commit
|
Test PASSed. |
test("addFile works") { | ||
val file = File.createTempFile("someprefix", "somesuffix") | ||
val absolutePath = file.getAbsolutePath | ||
val file1 = File.createTempFile("someprefix1", "somesuffix1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't use the temp dir you created below.
if (!gotten1.exists()) { | ||
throw new SparkException("file doesn't exist : " + absolutePath1) | ||
} | ||
if (!gotten2.exists()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same; feels like you can wrap up all these checks in a local function that you can just apply to both files rather than repeating code.
I think this is OK to go actually. The refactoring of the common code isn't a big deal, and, the temp dir issue is something I'm addressing in bulk in SPARK-6338 right now anyway, so I could touch it up along with that logical change. |
…ve path. when i run cmd like that sc.addFile("../test.txt"), it did not work and throwed an exception: java.lang.IllegalArgumentException: java.net.URISyntaxException: Relative path in absolute URI: file:../test.txt at org.apache.hadoop.fs.Path.initialize(Path.java:206) at org.apache.hadoop.fs.Path.<init>(Path.java:172) ........ ....... Caused by: java.net.URISyntaxException: Relative path in absolute URI: file:../test.txt at java.net.URI.checkPath(URI.java:1804) at java.net.URI.<init>(URI.java:752) at org.apache.hadoop.fs.Path.initialize(Path.java:203) Author: DoingDone9 <799203320@qq.com> Closes #4993 from DoingDone9/relativePath and squashes the following commits: ee375cd [DoingDone9] Update SparkContextSuite.scala d594e16 [DoingDone9] Update SparkContext.scala 0ff3fa8 [DoingDone9] test for add file dced8eb [DoingDone9] Update SparkContext.scala e4a13fe [DoingDone9] getCanonicalPath 161cae3 [DoingDone9] Merge pull request #4 from apache/master c87e8b6 [DoingDone9] Merge pull request #3 from apache/master cb1852d [DoingDone9] Merge pull request #2 from apache/master c3f046f [DoingDone9] Merge pull request #1 from apache/master (cherry picked from commit 00e730b) Signed-off-by: Sean Owen <sowen@cloudera.com>
…ve path. when i run cmd like that sc.addFile("../test.txt"), it did not work and throwed an exception: java.lang.IllegalArgumentException: java.net.URISyntaxException: Relative path in absolute URI: file:../test.txt at org.apache.hadoop.fs.Path.initialize(Path.java:206) at org.apache.hadoop.fs.Path.<init>(Path.java:172) ........ ....... Caused by: java.net.URISyntaxException: Relative path in absolute URI: file:../test.txt at java.net.URI.checkPath(URI.java:1804) at java.net.URI.<init>(URI.java:752) at org.apache.hadoop.fs.Path.initialize(Path.java:203) Author: DoingDone9 <799203320@qq.com> Closes apache#4993 from DoingDone9/relativePath and squashes the following commits: ee375cd [DoingDone9] Update SparkContextSuite.scala d594e16 [DoingDone9] Update SparkContext.scala 0ff3fa8 [DoingDone9] test for add file dced8eb [DoingDone9] Update SparkContext.scala e4a13fe [DoingDone9] getCanonicalPath 161cae3 [DoingDone9] Merge pull request #4 from apache/master c87e8b6 [DoingDone9] Merge pull request #3 from apache/master cb1852d [DoingDone9] Merge pull request #2 from apache/master c3f046f [DoingDone9] Merge pull request #1 from apache/master
when i run cmd like that sc.addFile("../test.txt"), it did not work and throwed an exception:
java.lang.IllegalArgumentException: java.net.URISyntaxException: Relative path in absolute URI: file:../test.txt
at org.apache.hadoop.fs.Path.initialize(Path.java:206)
at org.apache.hadoop.fs.Path.(Path.java:172)
........
.......
Caused by: java.net.URISyntaxException: Relative path in absolute URI: file:../test.txt
at java.net.URI.checkPath(URI.java:1804)
at java.net.URI.(URI.java:752)
at org.apache.hadoop.fs.Path.initialize(Path.java:203)