HdfsDataSegmentPusher: Properly include scheme, host in output path if necessary. #3577
Conversation
…f necessary. Fixes apache#3576.
|
👍 |
| @@ -150,7 +151,7 @@ private DataSegment createDescriptorFile(DataSegment segment, Path outDir, final | |||
|
|
|||
| private ImmutableMap<String, Object> makeLoadSpec(Path outFile) | |||
| { | |||
| return ImmutableMap.<String, Object>of("type", "hdfs", "path", outFile.toString()); | |||
| return ImmutableMap.<String, Object>of("type", "hdfs", "path", outFile.toUri().toString()); | |||
There was a problem hiding this comment.
why was this not needed before ? It wasoutFile.toString() before #3494 and used to work.
FYI - I see this note in the toString() method of Path class -
// we can't use uri.toString(), which escapes everything, because we want
// illegal characters unescaped in the string, for glob processing, etc.
There was a problem hiding this comment.
It helps the new unit test to pass; without it, file:///x/y/z gets translated to file:/x/y/z.
I think that comment on Path actually indicates that adding toUri is a good thing. We're adding a path to a single file and don't want glob processing or any other special processing.
|
👍 |
…f necessary. (apache#3577) Fixes apache#3576.
|
@gianm will this always put "hdfs://host:port" in the load spec even if same was not specified in the "storageDirectory" |
|
@himanshug if the storageDirectory has no scheme (just /path/to/blah), it will stay that way in the loadSpec. See the UTs, one of them is for a storageDirectory with no scheme. |
|
@gianm that sounds good, thanks. |
…f necessary. (apache#3577) Fixes apache#3576.
Fixes #3576.