-
Notifications
You must be signed in to change notification settings - Fork 9
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
Fix bug #308 #309
base: main
Are you sure you want to change the base?
Fix bug #308 #309
Conversation
|
||
_TEMP_DIR: Optional[tempfile.TemporaryDirectory] = None | ||
TEMP_DIR_NAME = Optional[None] | ||
|
||
TEMP_DIR_NAME: Optional[None | str | PosixPath] = None |
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.
Optional[Union[str,PosixPath]]
. sorry still at 3.8 :-)
@@ -114,7 +114,8 @@ def redefine_temp_location(path: str) -> str: | |||
_clear() | |||
|
|||
# cleanup old temporary directory | |||
shutil.rmtree(TEMP_DIR_NAME, ignore_errors=True) | |||
if TEMP_DIR_NAME is not None: |
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.
interestingly, in python 3.9, the unit tests don't fail here
apparently the ignore_errors
kicks in here
shutil.py:721
try:
orig_st = os.lstat(path)
except Exception:
onerror(os.lstat, path, sys.exc_info())
return
could you run this unit test with 3.12?
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.
indeed, 3.11 works fine but in 3.12 I get the same error as you do
change log in docs don't reflect that change in behaviour https://docs.python.org/3/library/shutil.html#shutil.rmtree
should we file an issue to python?
@@ -114,7 +114,8 @@ def redefine_temp_location(path: str) -> str: | |||
_clear() | |||
|
|||
# cleanup old temporary directory | |||
shutil.rmtree(TEMP_DIR_NAME, ignore_errors=True) | |||
if TEMP_DIR_NAME is not None: | |||
shutil.rmtree(TEMP_DIR_NAME, ignore_errors=True) |
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 the rmtree at all? doesn't tempdir take care of this?
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.
if you decide to keep it please add a comment that in python 3.12 , ignore_errors
does not work if None
is passed
fixes #308