[
https://issues.apache.org/jira/browse/HADOOP-15860?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16725538#comment-16725538
]
Sean Mackrory edited comment on HADOOP-15860 at 12/20/18 2:59 AM:
------------------------------------------------------------------
So there's a checkstyle issue to fix (space before the '(') and a number of
other minor whitespace issues like inconsistent spacing before '{'s. Can you
also revisit the exception messages? Currently they're not entirely correct.
For example, I'd make the following changes:
* "Cannot create a Path with names that end with a dot" -> "ABFS does not allow
files or directories to end with a dot"
* "Path does not have a trailing period." -> "Attempt to create file that ended
with a dot should throw IllegalArgumentException"
Also, it occurred to me that mkdirs is recursive. Someone could pass
"/dir1/dir2./dir3/" when none of them exist and we wouldn't stop them. In
mkdirs we should check each .parent() up to root.
When playing around with these tests to see what happened in various cases I
got a couple of exceptions for files already existing. There's 2 reasons for
that:
* It might be that one test run was hitting data from the previous run. Let's
replace our use of "new Path()" with "path()". It's a helper function in the
parent classes that gives you a path inside a randomized subdirectory to help
prevent this. All tests should actually do this, but let's just fix the new
ones for now.
* Let's add a comment about the actual problem here. (a) Microsoft says you
shouldn't do this, and (b) if you do it anyway, /this./path. and /this/path are
treated as identical in some cases. This is potentially problematic if anyone
messes with the tests since the 2 paths in each of your tests would be equals.
So let's have a clear comment about this and I'll elaborate in the commit
message too.
was (Author: mackrorysd):
So there's a checkstyle issue to fix (space before the '(') and a number of
other minor whitespace issues like inconsistent spacing before '{'s. Can you
also revisit the exception messages? Currently they're not entirely correct.
For example, I'd make the following changes:
* "Cannot create a Path with names that end with a dot" -> "ABFS does not allow
files or directories to end with a dot"
* "Path does not have a trailing period." -> "Attempt to create file that ended
with a dot should throw IllegalArgumentException"
Also, it occurred to me that mkdirs is recursive. Someone could pass
"/dir1/dir2./dir3/" when none of them exist and we wouldn't stop them.
When playing around with these tests to see what happened in various cases I
got a couple of exceptions for files already existing. There's 2 reasons for
that:
* It might be that one test run was hitting data from the previous run. Let's
replace our use of "new Path()" with "path()". It's a helper function in the
parent classes that gives you a path inside a randomized subdirectory to help
prevent this. All tests should actually do this, but let's just fix the new
ones for now.
* Let's add a comment about the actual problem here. (a) Microsoft says you
shouldn't do this, and (b) if you do it anyway, /this./path. and /this/path are
treated as identical in some cases. This is potentially problematic if anyone
messes with the tests since the 2 paths in each of your tests would be equals.
So let's have a clear comment about this and I'll elaborate in the commit
message too.
> ABFS: Throw IllegalArgumentException when Directory/File name ends with a
> period(.)
> -----------------------------------------------------------------------------------
>
> Key: HADOOP-15860
> URL: https://issues.apache.org/jira/browse/HADOOP-15860
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/adl
> Affects Versions: 3.2.0
> Reporter: Sean Mackrory
> Assignee: Shweta
> Priority: Major
> Attachments: HADOOP-15860.001.patch, HADOOP-15860.002.patch,
> trailing-periods.patch
>
>
> If you create a directory with a trailing period (e.g. '/test.') the period
> is silently dropped, and will be listed as simply '/test'. '/test.test'
> appears to work just fine.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
