gh-102828: add onexc arg to shutil.rmtree. Deprecate onerror.#102829
gh-102828: add onexc arg to shutil.rmtree. Deprecate onerror.#102829iritkatriel merged 4 commits into
Conversation
|
I like the idea in principle, but existing code using 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 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: |
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. |
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 If it accepted only one argument, then:
The only thing lost would be the |
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.
onexc expects an exception instead of exc_info. This is part of the larger effort to move on from exc_info.
Fixes #102828.