use tempfile.TemporaryDirectory#337
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
|
Looks like Windows and python3.7 are not happy with this, https://bugs.python.org/issue26660 presumably. if and when we eventually get to do this, should probably remove the unit test case also - it wouldn't make much sense to have a unit test for tempfile function. |
|
Re: implementing this now, one option is python-poetry/poetry#5522 (comment) |
4bedfd5 to
e48e797
Compare
|
Kudos, SonarCloud Quality Gate passed! |
ab2682a to
64d7c86
Compare
|
You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab. |
|
rebased on #566 so that we no longer need to care about Windows python 3.7 bugs |
7ecf628 to
21caebb
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
2919f80 to
1f96d36
Compare
6f33977 to
c6e9cb6
Compare
|
Kudos, SonarCloud Quality Gate passed! |
c6e9cb6 to
8e58a44
Compare
|
Kudos, SonarCloud Quality Gate passed! |
8e58a44 to
33307e9
Compare
|
33307e9 to
431fd3c
Compare
431fd3c to
bc9dcc3
Compare
|
now partner to python-poetry/poetry#7680 |
Reviewer's GuideRefactors temporary directory handling to use tempfile.TemporaryDirectory (with ignore_cleanup_errors=True) directly throughout the codebase and tests, while removing the now-redundant helper utilities and their tests. Sequence diagram for wheel build metadata generation using TemporaryDirectorysequenceDiagram
participant WheelBuilder
participant tempfile
participant FileSystem
WheelBuilder->>tempfile: TemporaryDirectory(ignore_cleanup_errors=True)
activate tempfile
tempfile-->>WheelBuilder: temp_dir_path
WheelBuilder->>WheelBuilder: prepare_metadata(temp_dir_path)
WheelBuilder->>FileSystem: write dist_info metadata into temp_dir_path
deactivate tempfile
tempfile->>FileSystem: cleanup temp_dir_path (ignore cleanup errors)
Updated class diagram for wheel building and helper utilitiesclassDiagram
class WheelBuilder {
_metadata_directory
build()
prepare_metadata(destination)
_copy_file_scripts(zip_file)
_copy_dist_info(zip_file, metadata_directory)
}
class TemporaryDirectory {
ignore_cleanup_errors
__enter__()
__exit__(exc_type, exc_value, traceback)
}
WheelBuilder ..> TemporaryDirectory : uses
class HelpersBefore {
temporary_directory(args, kwargs)
robust_rmtree(path, max_timeout)
_on_rm_error(func, path, exc_info)
}
class HelpersAfter {
combine_unicode(string)
module_name(name)
parse_requires(requires)
readme_content_type(path)
}
HelpersBefore <|-- HelpersAfter : refactor removes temp and rmtree helpers
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- There’s quite a bit of repetition of
TemporaryDirectory(ignore_cleanup_errors=True)across tests and wheel building; consider introducing a small local helper/context manager in the test suite (and possibly in builders) to reduce duplication and make future changes to the tempdir behavior easier. - In places like
tests/masonry/test_api.py, you repeatedly wraptmp_dirwithPath(tmp_dir)after theTemporaryDirectorycontext; assigningtmp_path = Path(tmp_dir)once per block would simplify the code and avoid redundant conversions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s quite a bit of repetition of `TemporaryDirectory(ignore_cleanup_errors=True)` across tests and wheel building; consider introducing a small local helper/context manager in the test suite (and possibly in builders) to reduce duplication and make future changes to the tempdir behavior easier.
- In places like `tests/masonry/test_api.py`, you repeatedly wrap `tmp_dir` with `Path(tmp_dir)` after the `TemporaryDirectory` context; assigning `tmp_path = Path(tmp_dir)` once per block would simplify the code and avoid redundant conversions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
bc9dcc3 to
cd6923b
Compare
cd6923b to
c1fd014
Compare













cf python-poetry/poetry#5522
but note that there's a testcase in this repository:
poetry-core/tests/utils/test_helpers.py
Line 90 in 8c75c1a
Summary by Sourcery
Replace custom temporary directory and removal helpers with direct use of the standard library’s TemporaryDirectory across the codebase and tests.
Enhancements:
Tests: