| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
I like the idea in principle, but existing code using onerror will stop working, because onerror arg is accepted but silently ignored. I think a retro-compatible solution to this could be to call onexc AND onerror for a certain period of time. I'm not sure what's the current policy re. deprecations, but I suppose there can be a transitional time (2 releases? 3?) during which using onerror arg results in DeprecationWarning and then it's finally removed. Something like (not tested): try:
...
except OSError as err:
if onerror is not None:
warnings.warn("'onerror' argument is deprecated and will be removed in XXX. Use 'onexc' argument instead.",
DeprecationWarning)
onerror(..., ..., sys.exc_info())
if onexc is not None:
onerror(..., ..., err) |
Sorry, something went wrong.
It will still work, because onerror gets wrapped by onexc in Lib/shutil.py:708-714. Note that I didn't remove any tests, so all previous functionality is still there. |
Sorry, something went wrong.
|
Oh you're right sorry. I misread your patch. |
Sorry, something went wrong.
|
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again. |
Sorry, something went wrong.
|
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again. |
Sorry, something went wrong.
|
Thanks for making the requested changes! @giampaolo: please review the changes made to this pull request. |
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot wasm32-wasi 3.x has failed when building commit d51a6dc. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/all/#builders/1046/builds/1597 Failed tests:
Failed subtests:
Summary of the results of the build (if available): == Tests result: FAILURE == 325 tests OK. 10 slowest tests:
1 test failed: 107 tests skipped: Total duration: 8 min 9 sec Click to see traceback logsTraceback (most recent call last):
File "/Lib/test/test_shutil.py", line 532, in test_both_onerror_and_onexc
self.assertTrue(onexc_called)
AssertionError: False is not true
Traceback (most recent call last):
File "/Lib/shutil.py", line 762, in rmtree
return _rmtree_unsafe(path, onexc)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Lib/shutil.py", line 583, in _rmtree_unsafe
onexc(os.scandir, path, err)
File "/Lib/shutil.py", line 580, in _rmtree_unsafe
with os.scandir(path) as scandir_it:
^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 44] No such file or directory: '@test_42_tmpæ'
|
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot AMD64 Debian root 3.x has failed when building commit d51a6dc. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/all/#builders/345/builds/4352 Failed tests:
Failed subtests:
Summary of the results of the build (if available): == Tests result: FAILURE then FAILURE == 411 tests OK. 10 slowest tests:
1 test failed: 21 tests skipped: 1 re-run test: Total duration: 39 min 34 sec Click to see traceback logsTraceback (most recent call last):
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 725, in rmtree
onexc(os.lstat, path, err)
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 723, in rmtree
orig_st = os.lstat(path, dir_fd=dir_fd)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '@test_3079655_tmpæ'
Traceback (most recent call last):
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 725, in rmtree
onexc(os.lstat, path, err)
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 723, in rmtree
orig_st = os.lstat(path, dir_fd=dir_fd)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '@test_3084007_tmpæ'
Traceback (most recent call last):
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_shutil.py", line 532, in test_both_onerror_and_onexc
self.assertTrue(onexc_called)
AssertionError: False is not true
|
Sorry, something went wrong.
|
@iritkatriel looks like this broke some build bots. Traceback (most recent call last):
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 725, in rmtree
onexc(os.lstat, path, err)
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 723, in rmtree
orig_st = os.lstat(path, dir_fd=dir_fd)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '@test_3079655_tmpæ'
Traceback (most recent call last):
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 725, in rmtree
onexc(os.lstat, path, err)
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 723, in rmtree
orig_st = os.lstat(path, dir_fd=dir_fd)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '@test_3084007_tmpæ'
Traceback (most recent call last):
File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_shutil.py", line 532, in test_both_onerror_and_onexc
self.assertTrue(onexc_called)
AssertionError: False is not true
|
Sorry, something went wrong.
|
I see what I did - I copied part of this test from another test, but I didn't copy the skip instructions that make it not run where it doesn't work. Will fix. |
Sorry, something went wrong.
|
Ignore my experiment with the 1st buildbot report. |
Sorry, something went wrong.
|
Hey, why does onexc accept three arguments (function, path, exc) rather than just the exception? If it accepted only one argument, then:
The only thing lost would be the function value, which seems like an unreasonably detailed insight into the internals of rmtree(). In fact, rmtree() already bends the truth about the function that actually failed - for example os.path.islink is given if we fail a junction check on Windows. And no internal code looks at the value of function. |
Sorry, something went wrong.
Because that's what onerror did. This PR was just about replacing exc_info by exc. Feel free to suggest additional changes in a separate issue. |
Sorry, something went wrong.
|
Righto, thanks. I've logged #103218. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
onexc expects an exception instead of exc_info. This is part of the larger effort to move on from exc_info.
Fixes #102828.