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

THDFSFile fixes #94

Merged
merged 10 commits into from
Jun 21, 2017
Merged

Conversation

evgeny-boger
Copy link
Contributor

This set of patches makes THDFSFile work again.
It also enables CMake build and allows linking against libhdfs3 (experimental native HDFS client implementation).

Kind of major change: HDFS URLs are now absolute instead of relative as it was before. I.e. one have to use "hdfs:///user/username/dir1/file2.root" notation to access file in the home directory.
This makes HDFS URLs somewhat standard in the sense that they could be used interchangeably between ROOT and Hadoop API and command-line utilities.

@evgeny-boger evgeny-boger changed the title Feature/hdfs fixes THDFSFile fixes Sep 27, 2015
@pcanal
Copy link
Member

pcanal commented Oct 1, 2015

Kind of major change: HDFS URLs are now absolute

Is this change necessary? Main reasons to ask are
a) it is somewhat backward incompatible (however I have no idea how many user might be relying on this)
b) without it, the user can still decide to use absolute path to have the "could be used interchangeably between ROOT and Hadoop API " part.

@evgeny-boger
Copy link
Contributor Author

Well, it isn't necessary.

Without this change user have to specify the absolute path by adding one more / in front of it. E.g.

hdfs://host:port//user/username/dir/file

(note the extra slash after host).

I personally think that's kind of weird.

Anyway, the problem is that there is no way (to my knowledge) to point to relative path in Hadoop using hdfs:// prefix. So Hadoop uses single slash to point to absolute url. This URL will be parsed incorrectly by current THDFSFile by assuming that the path is relative to user directory.

The absolute URLs with extra leading slashes are, however, correctly parsed by Hadoop. So there is a compatibility to some extent.

@pcanal
Copy link
Member

pcanal commented May 28, 2017

@smithdh Would you be able to review (and rebase) this PR?

@phsft-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@vgvassilev
Copy link
Member

@smithdh ping...

@smithdh
Copy link
Contributor

smithdh commented Jun 21, 2017

Hello @evgeny-boger Could you select "allow edits from maintainers" for this PR (I think this may not be set at the moment), so that I can rebase?
Thank you, David

@evgeny-boger
Copy link
Contributor Author

@smithdh, done

@smithdh smithdh force-pushed the feature/hdfs_fixes branch from 4a00643 to 41488bd Compare June 21, 2017 11:38
@vgvassilev
Copy link
Member

vgvassilev commented Jun 21, 2017

@phsft-bot build!

@smithdh and @evgeny-boger thanks rebasing! Could you fix the clang-format issues?

@phsft-bot
Copy link
Collaborator

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

@smithdh
Copy link
Contributor

smithdh commented Jun 21, 2017

hi @vgvassilev Yes, certainly.

@vgvassilev
Copy link
Member

@phsft-bot build!

@phsft-bot
Copy link
Collaborator

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

@vgvassilev vgvassilev merged commit 45f61ce into root-project:master Jun 21, 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