-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-14630 Contract Tests to verify create, mkdirs and rename under a file is forbidden #533
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
HADOOP-14630 Contract Tests to verify create, mkdirs and rename under a file is forbidden #533
Conversation
@@ -524,9 +524,17 @@ Create a directory and all its parents | |||
#### Preconditions | |||
|
|||
|
|||
The path must either be a directory or not exist | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
if exists(FS, p) and not isDir(FS, p) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
||
No ancestor may be a file | ||
|
||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
forall d = ancestors(FS, p) : | ||
if exists(FS, d) and not isDir(FS, d) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
@@ -566,6 +574,12 @@ Writing to or overwriting a directory must fail. | |||
|
|||
if isDir(FS, p) : raise {FileAlreadyExistsException, FileNotFoundException, IOException} | |||
|
|||
No ancestor may be a file | |||
|
|||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
forall d = ancestors(FS, p) : | ||
if exists(FS, d) and not isDir(FS, d) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Whitespace is from an error file. I deny (direct) responsibility
|
10fd31d
to
2615fb3
Compare
@@ -524,9 +524,17 @@ Create a directory and all its parents | |||
#### Preconditions | |||
|
|||
|
|||
The path must either be a directory or not exist | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
if exists(FS, p) and not isDir(FS, p) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
||
No ancestor may be a file | ||
|
||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
forall d = ancestors(FS, p) : | ||
if exists(FS, d) and not isDir(FS, d) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
@@ -566,6 +574,12 @@ Writing to or overwriting a directory must fail. | |||
|
|||
if isDir(FS, p) : raise {FileAlreadyExistsException, FileNotFoundException, IOException} | |||
|
|||
No ancestor may be a file | |||
|
|||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
forall d = ancestors(FS, p) : | ||
if exists(FS, d) and not isDir(FS, d) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
💔 -1 overall
This message was automatically generated. |
@@ -524,9 +524,17 @@ Create a directory and all its parents | |||
#### Preconditions | |||
|
|||
|
|||
The path must either be a directory or not exist | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
if exists(FS, p) and not isDir(FS, p) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
||
No ancestor may be a file | ||
|
||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
forall d = ancestors(FS, p) : | ||
if exists(FS, d) and not isDir(FS, d) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
@@ -566,6 +574,12 @@ Writing to or overwriting a directory must fail. | |||
|
|||
if isDir(FS, p) : raise {FileAlreadyExistsException, FileNotFoundException, IOException} | |||
|
|||
No ancestor may be a file | |||
|
|||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
forall d = ancestors(FS, p) : | ||
if exists(FS, d) and not isDir(FS, d) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
… a file is forbidden
…nting createNonRecursive Change-Id: If826ad1d893c8c9733d6259cb5d7e4d986df49e4
…leSystemMetricsSystem; intermittent failure during testing Change-Id: I884de58f555908fbd867f82a078e04bf86849413
…ion if the direct parent is a file Change-Id: Id53f1245068a078d682c37391b618b9de64db52a
this has happened in 2 places now; I'm going to make it something which stores can declare they do Change-Id: I60aa2d68cd2f9f2e511972b6c799d10348d82578
…d a new switch to allow filesystems to delcare they permit this. S3A contact XML adds the flag; Swift does not -it does check, at a cost in performance. Change-Id: Ifd40c6a58f6ac8cc9eeb83168e0449de59313dfd tested: s3a, abfs, swift. my ADL login is no longer valid.
2615fb3
to
fac64e5
Compare
|
||
#### Preconditions | ||
|
||
|
||
The path must either be a directory or not exist | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
if exists(FS, p) and not isDir(FS, p) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
||
No ancestor may be a file | ||
|
||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
@@ -577,14 +584,20 @@ Writing to or overwriting a directory must fail. | |||
|
|||
if isDir(FS, p) : raise {FileAlreadyExistsException, FileNotFoundException, IOException} | |||
|
|||
No ancestor may be a file | |||
|
|||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
💔 -1 overall
This message was automatically generated. |
|
||
#### Preconditions | ||
|
||
|
||
The path must either be a directory or not exist | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
if exists(FS, p) and not isDir(FS, p) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
||
No ancestor may be a file | ||
|
||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
@@ -577,14 +584,20 @@ Writing to or overwriting a directory must fail. | |||
|
|||
if isDir(FS, p) : raise {FileAlreadyExistsException, FileNotFoundException, IOException} | |||
|
|||
No ancestor may be a file | |||
|
|||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
💔 -1 overall
This message was automatically generated. |
|
||
#### Preconditions | ||
|
||
|
||
The path must either be a directory or not exist | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
if exists(FS, p) and not isDir(FS, p) : | ||
raise [ParentNotDirectoryException, FileAlreadyExistsException, IOException] | ||
|
||
No ancestor may be a file | ||
|
||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
@@ -577,14 +584,20 @@ Writing to or overwriting a directory must fail. | |||
|
|||
if isDir(FS, p) : raise {FileAlreadyExistsException, FileNotFoundException, IOException} | |||
|
|||
No ancestor may be a file | |||
|
|||
forall d = ancestors(FS, p) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace:end of line
💔 -1 overall
This message was automatically generated. |
…g partition while choosing. Author: Aditya Toomula <atoomula@linkedin.com> Reviewers: Jagadish<jagadish@apache.org> Closes apache#533 from atoomula/chooser
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 once rebased again and a QA. Do you think we can backport this to all 2.10+ branches?
expectMkdirsUnderFileFails("mkdirs() file/dir", | ||
grandparent, child); | ||
|
||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need another mkdirs(child)
test here? I thought it's already tested in above expectMkdirsUnderFileFails()
.
Is this for verbose logging output? I mean,
handleRelaxedException(action,
"ParentNotDirectoryException",
e);
v.s.
handleRelaxedException("creating a file under a subdirectory of a file ",
"FileAlreadyExistsException",
e);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just being curious about deeper creation .. those object stores are so troublesome here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think this test testMkdirUnderFileSubdir
makes perfect sense. In this test, I was assuming the whole try-catch
clause is having the same logic (except exception handling) as the call to expectMkdirsUnderFileFails()
in LoC 376 in this method.
thx for the merge -no obvious issues with a backport |
merged; closing. Thanks for looking @ this |
HADOOP-14630. Contract Tests to verify create, mkdirs and rename under a file is forbidden