You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by zsxwing <gi...@git.apache.org> on 2015/12/09 00:31:04 UTC
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
GitHub user zsxwing opened a pull request:
https://github.com/apache/spark/pull/10208
[SPARK-12220][Core]Make Utils.fetchFile support files that contain special characters
This PR encodes and decodes the file name to fix the issue.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zsxwing/spark uri
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/10208.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #10208
----
commit 56a35a3aff88ed24e370823368e63ba09832dce1
Author: Shixiong Zhu <sh...@databricks.com>
Date: 2015-12-08T23:27:19Z
Make Utils.fetchFile support files that contain special characters
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47118093
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
--- End diff --
I think `Utils.resolveURI` already covers this functionality.
(It's "URI" and "URL" not "uri" and "url".)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164136279
This feels hacky to be partly encoding URIs with ad-hoc methods. I understand the problem but this seems to be making the project code more complex. The methods seems like overkill since you're just calling a single (tested) JDK method. _shrug_
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164601031
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164978352
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47767/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163705815
**[Test build #47529 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47529/consoleFull)** for PR 10208 at commit [`2c31643`](https://github.com/apache/spark/commit/2c3164386040b5051e0332652cff9d2052b90cdb).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165263095
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47847/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47190317
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
+ require(!fileName.contains("/") && !fileName.contains("\\"))
+ // `file` and `localhost` are not used. Just to prevent URI from parsing `fileName` as
+ // scheme or host. The prefix "/" is required because URI doesn't accept a relative path.
+ // We should remove it after we get the raw path.
+ new URI("file", null, "localhost", -1, "/" + fileName, null, null).getRawPath.substring(1)
+ }
+
+ /**
+ * Get the file name from uri's raw path and decode it. The raw path of uri must not end with "/".
+ */
+ def decodeFileNameInURI(uri: URI): String = {
+ val rawPath = uri.getRawPath
+ assert(!rawPath.endsWith("/"))
+ val rawFileName = rawPath.split("/").last
+ new URI("file:///" + rawFileName).getPath.substring(1)
--- End diff --
I created a method here so that I can write unit tests for this one to confirm the desired behavior. For the security issue of `%2F`, I think the server should take care of it, since the attacker can also use such special URI without Spark.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164951102
**[Test build #47767 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47767/consoleFull)** for PR 10208 at commit [`28fa737`](https://github.com/apache/spark/commit/28fa73723d5dcea50fc66c01b70c37b7677849a7).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163058509
LGTM pending tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163355964
**[Test build #47437 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47437/consoleFull)** for PR 10208 at commit [`2c31643`](https://github.com/apache/spark/commit/2c3164386040b5051e0332652cff9d2052b90cdb).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164539539
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47664/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165222257
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/10208
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164555796
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165226020
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47846/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165504341
OK. I think this pretty fine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164577354
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47675/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47118241
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
+ require(!fileName.contains("/") && !fileName.contains("\\"))
+ // `file` and `localhost` are not used. Just to prevent URI from parsing `fileName` as
+ // scheme or host. The prefix "/" is required because URI doesn't accept a relative path.
+ // We should remove it after we get the raw path.
+ new URI("file", null, "localhost", -1, "/" + fileName, null, null).getRawPath.substring(1)
+ }
+
+ /**
+ * Get the file name from uri's raw path and decode it. The raw path of uri must not end with "/".
+ */
+ def decodeFileNameInURI(uri: URI): String = {
+ val rawPath = uri.getRawPath
+ assert(!rawPath.endsWith("/"))
+ val rawFileName = rawPath.split("/").last
+ new URI("file:///" + rawFileName).getPath.substring(1)
--- End diff --
I don't get this function -- the URI would already have methods to return the 'non-raw' path right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163352232
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47679327
--- Diff: core/src/main/scala/org/apache/spark/HttpFileServer.scala ---
@@ -20,6 +20,7 @@ package org.apache.spark
import java.io.File
import com.google.common.io.Files
+import org.apache.commons.httpclient.util.URIUtil
--- End diff --
Before you consider doing more work to change again, I'd love to get another opinion if possible. We won't stall too long on this in any event. I suppose I preferred your original change, yes, to this one, just because it avoids any new subtle dependency entanglement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47208123
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
+ require(!fileName.contains("/") && !fileName.contains("\\"))
+ // `file` and `localhost` are not used. Just to prevent URI from parsing `fileName` as
+ // scheme or host. The prefix "/" is required because URI doesn't accept a relative path.
+ // We should remove it after we get the raw path.
+ new URI("file", null, "localhost", -1, "/" + fileName, null, null).getRawPath.substring(1)
+ }
+
+ /**
+ * Get the file name from uri's raw path and decode it. The raw path of uri must not end with "/".
+ */
+ def decodeFileNameInURI(uri: URI): String = {
+ val rawPath = uri.getRawPath
+ assert(!rawPath.endsWith("/"))
+ val rawFileName = rawPath.split("/").last
+ new URI("file:///" + rawFileName).getPath.substring(1)
--- End diff --
Sure, but isn't that an argument that you don't need this method (or at least you don't need more than `new URI("file:/" + rawFileName).getPath.substring(1)`)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47260233
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
--- End diff --
> Meh, at this point why not just new URI("file:/" + fileName).getRawPath.substring(1) and keep it simple?
The parameter of `URI(String)` must be an encoded path. But here `fileName` is what we want to encode.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by 3ourroom <gi...@git.apache.org>.
Github user 3ourroom commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47625904
--- Diff: core/src/main/scala/org/apache/spark/HttpFileServer.scala ---
@@ -20,6 +20,7 @@ package org.apache.spark
import java.io.File
import com.google.common.io.Files
+import org.apache.commons.httpclient.util.URIUtil
--- End diff --
NAVER - http://www.naver.com/
--------------------------------------------
3ourroom@naver.com 님께 보내신 메일 <Re: [spark] [SPARK-12220][Core]Make Utils.fetchFile support files that contain special characters (#10208)> 이 다음과 같은 이유로 전송 실패했습니다.
--------------------------------------------
받는 사람이 회원님의 메일을 수신차단 하였습니다.
--------------------------------------------
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163733865
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164578310
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47143928
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
--- End diff --
I don't think that file name is a problem in a URL though. `:` is not reserved in that part. You'd have to send this to `java.net.URI` "manually" like you've done, or else use `file:/abc:xyz`. Then it's doing the same thing; your method wouldn't escape the colon either.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163733716
**[Test build #47529 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47529/consoleFull)** for PR 10208 at commit [`2c31643`](https://github.com/apache/spark/commit/2c3164386040b5051e0332652cff9d2052b90cdb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47625393
--- Diff: core/src/main/scala/org/apache/spark/HttpFileServer.scala ---
@@ -20,6 +20,7 @@ package org.apache.spark
import java.io.File
import com.google.common.io.Files
+import org.apache.commons.httpclient.util.URIUtil
--- End diff --
This only accidentally works since the library is a transitive dependency of core. I'm hesitant to make it a direct dependency and make core yet bigger, but in theory it already is (indirectly). It's not otherwise used in Spark though.
I know we've been around a lot on this; I had though using the JDK URI class directly was simplest rather than drag in a dependency. If anything I was questioning wrapping it up in a method, but, that's not that bad.
Any other opinions?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47673951
--- Diff: core/src/main/scala/org/apache/spark/HttpFileServer.scala ---
@@ -20,6 +20,7 @@ package org.apache.spark
import java.io.File
import com.google.common.io.Files
+import org.apache.commons.httpclient.util.URIUtil
--- End diff --
> If anything I was questioning wrapping it up in a method, but, that's not that bad.
Do you suggest that reverting my last commit?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164557116
**[Test build #47675 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47675/consoleFull)** for PR 10208 at commit [`8734464`](https://github.com/apache/spark/commit/8734464eb66e961b25795aa93b3e8e86ea5fef73).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164577349
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47145539
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
--- End diff --
That's right, but the existing method already does just that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164581161
**[Test build #47683 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47683/consoleFull)** for PR 10208 at commit [`8734464`](https://github.com/apache/spark/commit/8734464eb66e961b25795aa93b3e8e86ea5fef73).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165004101
**[Test build #47786 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47786/consoleFull)** for PR 10208 at commit [`28fa737`](https://github.com/apache/spark/commit/28fa73723d5dcea50fc66c01b70c37b7677849a7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164041345
@srowen further comments?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47260472
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
+ require(!fileName.contains("/") && !fileName.contains("\\"))
+ // `file` and `localhost` are not used. Just to prevent URI from parsing `fileName` as
+ // scheme or host. The prefix "/" is required because URI doesn't accept a relative path.
+ // We should remove it after we get the raw path.
+ new URI("file", null, "localhost", -1, "/" + fileName, null, null).getRawPath.substring(1)
+ }
+
+ /**
+ * Get the file name from uri's raw path and decode it. The raw path of uri must not end with "/".
+ */
+ def decodeFileNameInURI(uri: URI): String = {
+ val rawPath = uri.getRawPath
+ assert(!rawPath.endsWith("/"))
+ val rawFileName = rawPath.split("/").last
+ new URI("file:///" + rawFileName).getPath.substring(1)
--- End diff --
> Sure, but isn't that an argument that you don't need this method (or at least you don't need more than new URI("file:/" + rawFileName).getPath.substring(1))?
the uri here may contains more than a file name, such as `file:/abc/xyz`. But it should return only `xyz`. So `new URI("file:/" + rawFileName).getPath.substring(1))` doesn't work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163733868
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47529/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164524295
@srowen Just found `org.apache.commons.httpclient.util.URIUtil` has already supported URI decode/encode features and updated my PR with it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47723889
--- Diff: core/src/main/scala/org/apache/spark/HttpFileServer.scala ---
@@ -20,6 +20,7 @@ package org.apache.spark
import java.io.File
import com.google.common.io.Files
+import org.apache.commons.httpclient.util.URIUtil
--- End diff --
Okey. Explicitly added it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163074811
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164978281
**[Test build #47767 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47767/consoleFull)** for PR 10208 at commit [`28fa737`](https://github.com/apache/spark/commit/28fa73723d5dcea50fc66c01b70c37b7677849a7).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165321243
@srowen do the latest changes LGTY?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164600933
**[Test build #47683 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47683/consoleFull)** for PR 10208 at commit [`8734464`](https://github.com/apache/spark/commit/8734464eb66e961b25795aa93b3e8e86ea5fef73).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163704361
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164539532
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163074814
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47373/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164978350
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164982463
**[Test build #47786 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47786/consoleFull)** for PR 10208 at commit [`28fa737`](https://github.com/apache/spark/commit/28fa73723d5dcea50fc66c01b70c37b7677849a7).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165004179
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47786/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164577260
**[Test build #47675 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47675/consoleFull)** for PR 10208 at commit [`8734464`](https://github.com/apache/spark/commit/8734464eb66e961b25795aa93b3e8e86ea5fef73).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47129606
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
--- End diff --
`Utils.resolveURI` cannot handle some file names, such as `abc:xyz`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165262501
**[Test build #47847 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47847/consoleFull)** for PR 10208 at commit [`2c31643`](https://github.com/apache/spark/commit/2c3164386040b5051e0332652cff9d2052b90cdb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47129458
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
+ require(!fileName.contains("/") && !fileName.contains("\\"))
+ // `file` and `localhost` are not used. Just to prevent URI from parsing `fileName` as
+ // scheme or host. The prefix "/" is required because URI doesn't accept a relative path.
+ // We should remove it after we get the raw path.
+ new URI("file", null, "localhost", -1, "/" + fileName, null, null).getRawPath.substring(1)
+ }
+
+ /**
+ * Get the file name from uri's raw path and decode it. The raw path of uri must not end with "/".
+ */
+ def decodeFileNameInURI(uri: URI): String = {
+ val rawPath = uri.getRawPath
+ assert(!rawPath.endsWith("/"))
+ val rawFileName = rawPath.split("/").last
+ new URI("file:///" + rawFileName).getPath.substring(1)
--- End diff --
Here I want to split the raw path before decoding it, so that it can handle uris that contain `%2F`. I think some http links may contain `%2F`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165215923
I suppose the decision is what we want to maintain more: our own encode/decode logic, or potential dependency conflicts. If we test the encode/decode logic well then we probably don't need to maintain it much, but at the same time it's good to use a trusted library in case there's something we missed. Though I agree the dependency thing could be more difficult to maintain. I'm OK one way or the other, so we can just do it ourselves as @srowen prefers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163058416
**[Test build #47373 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47373/consoleFull)** for PR 10208 at commit [`56a35a3`](https://github.com/apache/spark/commit/56a35a3aff88ed24e370823368e63ba09832dce1).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165004177
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47697316
--- Diff: core/src/main/scala/org/apache/spark/HttpFileServer.scala ---
@@ -20,6 +20,7 @@ package org.apache.spark
import java.io.File
import com.google.common.io.Files
+import org.apache.commons.httpclient.util.URIUtil
--- End diff --
Should we consider explicitly adding that dependency? If it's already done somewhere else then it would be best that we don't re-implement the same thing and potentially introduce our own bugs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164526176
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163074749
**[Test build #47373 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47373/consoleFull)** for PR 10208 at commit [`56a35a3`](https://github.com/apache/spark/commit/56a35a3aff88ed24e370823368e63ba09832dce1).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164531611
yeah if it doesn't mean adding a dependency on HttpClient into core -- it may happen to come in transitively already
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163056143
CC @vanzin
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163352107
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47144942
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
--- End diff --
We don't need to encode `colon`. The purpose here is: assume we have a uri prefix: `spark://localhost:1234/` and a file name: `abc xyz`, and want to concat the prefix and the file name to `spark://localhost:1234/abc%20xyz` so that we can pass it to `java.net.URI(String)`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47208027
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
--- End diff --
Yes, it must be `file:/abc:xyz` since you need to tell it it needs to be treated as a file name. Meh, at this point why not just `new URI("file:/" + fileName).getRawPath.substring(1)` and keep it simple?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47146241
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
+ require(!fileName.contains("/") && !fileName.contains("\\"))
+ // `file` and `localhost` are not used. Just to prevent URI from parsing `fileName` as
+ // scheme or host. The prefix "/" is required because URI doesn't accept a relative path.
+ // We should remove it after we get the raw path.
+ new URI("file", null, "localhost", -1, "/" + fileName, null, null).getRawPath.substring(1)
+ }
+
+ /**
+ * Get the file name from uri's raw path and decode it. The raw path of uri must not end with "/".
+ */
+ def decodeFileNameInURI(uri: URI): String = {
+ val rawPath = uri.getRawPath
+ assert(!rawPath.endsWith("/"))
+ val rawFileName = rawPath.split("/").last
+ new URI("file:///" + rawFileName).getPath.substring(1)
--- End diff --
I see -- `URI` handles all that just fine as far as I can tell but there's no way to tell that the final element in a path is a string containing a slash, instead of two path elements separated by a slash, since the class has no way to access path elements.
I think you don't need to go to a URI, back to string, back to URI here. This may not even need a method for the one-liner: `new URI("file:///" + rawFileName).getPath.substring(1)` used one place.
As an aside, some app servers will be picky about serving URIs with %2F in some places, since this has been used in the past for some security exploits, to disguise a cheeky request for some local file URL that would otherwise be caught by (faulty) logic in the app that's not thinking about escaped sequences and trying to handle raw paths. I think Tomcat won't for example. It may be overkill but might even be worth considering conservatively rejecting such a URI anyway.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-163352110
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47435/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165226014
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164981091
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10208#discussion_r47190051
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -331,6 +331,30 @@ private[spark] object Utils extends Logging {
}
/**
+ * A file name may contain some invalid url characters, such as " ". This method will convert the
+ * file name to a raw path accepted by `java.net.URI(String)`.
+ *
+ * Note: the file name must not contain "/" or "\"
+ */
+ def encodeFileNameToURIRawPath(fileName: String): String = {
--- End diff --
If I pass `abc:xyz` to URI.resolveURI, it doesn't work:
```
scala> Utils.resolveURI("abc:xyz").getRawPath
res0: String = null
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-164601032
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47683/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165263089
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165530476
Great. Merging to master and 1.6
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165222160
Okey. I just removed my last 2 commits.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165022316
The problem is, nothing else in Spark uses HttpClient. I don't think it's worth it just for this, especially since there's essentially a one-liner using JDK methods. I agree that, if we could reuse a significant set of methods from HttpClient and retire some Spark code, that'd make it worth it. That's a bit of a separate issue. Still there, I know HttpClient versions can be a bit of a problem since lots of things depend on various versions of 3.x and 4.x, and it's easy to hit version compatibility problems.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12220][Core]Make Utils.fetchFile suppor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10208#issuecomment-165227079
**[Test build #47847 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47847/consoleFull)** for PR 10208 at commit [`2c31643`](https://github.com/apache/spark/commit/2c3164386040b5051e0332652cff9d2052b90cdb).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org