-
Notifications
You must be signed in to change notification settings - Fork 196
Fix semantics of tiledb_vfs_create_dir
on local filesystem.
#5579
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
Conversation
f9b7a73
to
d3b62d0
Compare
e4a886f
to
a31223f
Compare
tiledb_vfs_create_dir
on local filesystem.
a31223f
to
63781f6
Compare
63781f6
to
728e9b4
Compare
if (status != 0) { | ||
auto err = errno; | ||
if (err == EEXIST) { | ||
// Do not fail if directory already existed. |
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.
This is the documented behavior, so I'm treating this change as a bug fix.
TileDB/tiledb/api/c_api/vfs/vfs_api_external.h
Lines 229 to 230 in 639322f
* - On all other backends, if the directory exists, the function | |
* just succeeds without doing anything. |
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.
LGTM! The example makes sense, and I also played around a bit in TileDB-Py before and after this change and everything works fine.
auto status = mkdir(path.c_str(), directory_permissions_); | ||
if (status != 0) { | ||
auto err = errno; | ||
if (err == EEXIST) { |
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.
I would prefer to check whether err != EEXIST
and throw the error in that case, rather than returning when err == EEXIST
.
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.
I think the current way is better; it indicates that throwing on an error is the general case, and ignoring the error is the special case.
Fixes CORE-268.
Tests added. The Windows implementation already creates parent directories.
TYPE: BUG
DESC: Fixed the
tiledb_vfs_create_dir
function to automatically create non-existent parent directories on POSIX.TYPE: BUG
DESC: Fixed the
tiledb_vfs_create_dir
function to not fail when the directory already exists.