Skip to content

TOREE-408: Add support for hdfs and s3 to AddJar.#125

Closed
rdblue wants to merge 2 commits into
apache:masterfrom
rdblue:TOREE-408-add-jar-support-hadoop-file-systems
Closed

TOREE-408: Add support for hdfs and s3 to AddJar.#125
rdblue wants to merge 2 commits into
apache:masterfrom
rdblue:TOREE-408-add-jar-support-hadoop-file-systems

Conversation

@rdblue

@rdblue rdblue commented Jun 10, 2017

Copy link
Copy Markdown
Contributor

No description provided.

@LK-Tmac1

Copy link
Copy Markdown

How to know the reason for the failure of the test? The details said 1 Test Failed for org.apache.toree.magic.builtin.AddJarSpec but no further explanations.

@jodersky

jodersky commented Jun 12, 2017

Copy link
Copy Markdown
Member

@liukun1016 It's a bit further up, line 3324.

rdblue added 2 commits June 18, 2017 14:32
URI removes the query fragment from the path component and file name is
based on the path. This test used to rely on the fact that the query
fragment was included in the file name returned by URL, but this is
unreliable. The URL could easily end with some other query param instead
of the file name, so the logic wasn't reliable.
@rdblue rdblue force-pushed the TOREE-408-add-jar-support-hadoop-file-systems branch from b1b436f to 4b2134d Compare June 18, 2017 21:44
@rdblue

rdblue commented Jun 18, 2017

Copy link
Copy Markdown
Contributor Author

Rebased on master and fixed the test failure.

The problem was with the test itself. The addJar code was returning the file name based on a query param instead of the URL's path. Since query params can be in any order and can't be relied upon for correct file names, the right thing to do is to get the filename from the path component. I updated the test to do that.

@rdblue

rdblue commented Jun 19, 2017

Copy link
Copy Markdown
Contributor Author

@kamir, I think this fixes #118 because URI correctly parses the file name without query params. Can you have a look?

@lresende

Copy link
Copy Markdown
Member

LGTM, merging if there are no more comments.

@asfgit asfgit closed this in 79d02e2 Jul 20, 2017
lresende pushed a commit to lresende/incubator-toree that referenced this pull request Oct 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants