jorisvandenbossche commented on code in PR #12848:
URL: https://github.com/apache/arrow/pull/12848#discussion_r851327053
##########
python/pyarrow/fs.py:
##########
@@ -346,18 +346,24 @@ def create_dir(self, path, recursive):
def delete_dir(self, path):
self.fs.rm(path, recursive=True)
- def _delete_dir_contents(self, path):
- for subpath in self.fs.listdir(path, detail=False):
+ def _delete_dir_contents(self, path, missing_dir_ok):
+ try:
+ subpaths = self.fs.listdir(path, detail=False)
+ except FileNotFoundError:
+ if missing_dir_ok:
+ return
+ raise
+ for subpath in subpaths:
if self.fs.isdir(subpath):
self.fs.rm(subpath, recursive=True)
elif self.fs.isfile(subpath):
self.fs.rm(subpath)
- def delete_dir_contents(self, path):
+ def delete_dir_contents(self, path, missing_dir_ok):
Review Comment:
I suppose this will break existing implementations, because our C++ code
will now pass this keyword, while existing handlers won't yet accept that
keyword in this method like above.
Now, that will _always_ happen if we add new keywords, and we want to be
able to add new keywords (so not saying we shouldn't do this change). But
therefore wondering:
- Should we recommend adding `**kwargs` (that get ignored) to all methods on
the Handler class to avoid such breakage? (of course, that would ignore the
keyword initially when we add one, so also result in unexpected behaviour for
the user. So that is maybe not better as initially raising an error until the
handler gets updated for the latest arrow version)
- In theory we could only pass the keyword in the C++ code (in
`PyFileSystem::DeleteDirContents`) to the handler method _if_ it has a
non-default value, and otherwise not pass it (and set a default on the line
above). Of course that complicates the C++ code with an if/else block (and
introduces a risk of introducing inconsistent defaults at some point if one
would change).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
