-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Return file size along with count on cache purge or removal #13108
Conversation
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.
Why not keep both? It can be helpful to know how many wheels were removed.
You should also use utils.misc.format_size()
as reading something like 10,182,123 bytes of files removed
is not exactly user friendly...
Perhaps something like this?
bytes_removed = 0
for filename in files:
bytes_removed += os.stat(filename).st_size
os.unlink(filename)
logger.verbose("Removed %s", filename)
logger.info("Files removed: %s (%s)", len(files), format_size(bytes_removed))
Restore file count to cache purge message
Good call on |
cache purge
cache purge
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.
It isn't useful for pip cache purge
, yes, but it can helpful when you're trying to remove only a few cached wheels, say with pip cache remove six
.
I'll leave this open in case other maintainers have differing opinions, but otherwise, this looks good to me! Thank you!
news/12176.feature.rst
Outdated
@@ -0,0 +1 @@ | |||
Pip now returns the size, rather than the number, of files cleared on ``pip cache purge`` |
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.
This should be updated to note that it's alongside the count and affects both purge
and remove
.
cache purge
Congrats on your first contribution! @charwick |
Fixes #12176