[BEAM-59] Add getFilename method to ResourceId#2754
[BEAM-59] Add getFilename method to ResourceId#2754vikkyrk wants to merge 4 commits intoapache:masterfrom vikkyrk:resource_name
Conversation
|
R: @dhalperi |
| } | ||
|
|
||
| @Override | ||
| public String getFilename() { |
There was a problem hiding this comment.
if it can return null, add @Nullable
| * @return a string representing the name of file or directory, or null if there are zero | ||
| * components. | ||
| */ | ||
| String getFilename(); |
| } | ||
|
|
||
| @Override | ||
| public String getFilename() { |
| "abc"); | ||
| assertEquals(toResourceIdentifier("gs://my_bucket/abc/xyz.txt").getFilename(), | ||
| "xyz.txt"); | ||
|
|
| return gcsPath.getBucket(); | ||
| } else { | ||
| return gcsPath.getFileName().toString(); | ||
| } |
There was a problem hiding this comment.
I feel like this should return null for <= 1 and never return bucket -- gs://bucket/ is equivalent to / , right?
There was a problem hiding this comment.
Yeah I was in two minds but included the bucket as it is part of the hierarchical component. I was going with anything after the the scheme should be included.
There was a problem hiding this comment.
On deeper thought, I think bucket should be treated like / and omitted here. Quick poll around the office -- everyone agreed.
Also, build didn't pass?
|
Look into the build error? Also, please title PR with |
|
Done. (It was |
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.