Skip to content

Add rich based spinner#13451

Merged
ichard26 merged 2 commits into
pypa:mainfrom
ichard26:feat/rich-spinners
Jul 17, 2025
Merged

Add rich based spinner#13451
ichard26 merged 2 commits into
pypa:mainfrom
ichard26:feat/rich-spinners

Conversation

@ichard26

@ichard26 ichard26 commented Jun 27, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Broken out of #13450. Our legacy spinners can't be used for that PR because they require that their spin() method is called manually (and thus are only really functional with the subprocess utility). Instead, let rich handle the updates in the background for us.

Can be tested manually like so:

import time
import logging.config  # this is a bug with pip's logging stack
from pip._internal.cli.spinners import open_rich_spinner
from pip._internal.utils.logging import setup_logging

setup_logging(0, no_color=False, user_log_file=None)
with open_rich_spinner("doing work"):
		time.sleep(2)
try:
		with open_rich_spinner("doing work"):
				1/0
except BaseException:
		pass
try:
		with open_rich_spinner("doing work"):
				raise KeyboardInterrupt
except BaseException:
		pass

@ichard26 ichard26 added skip news Does not need a NEWS file entry (eg: trivial changes) C: output Related to what pip prints labels Jun 27, 2025
@ichard26
Add rich based spinner
ea7634a 
The benefit of this spinner over our legacy spinners is that Rich will
update and render the spinner automatically for us. This is much nicer
than having to call .spin() manually.
@ichard26 ichard26 force-pushed the feat/rich-spinners branch from af966e4 to ea7634a Compare June 27, 2025 23:29
@ichard26 ichard26 mentioned this pull request Jun 28, 2025
Merged
notatallshaw
notatallshaw approved these changes Jul 17, 2025
View reviewed changes

@notatallshaw notatallshaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

I would be tempted to rename PipRichSpinner to _PipRichSpinner to make it clear it shouldn't be accessed externally.

Also because the PipRichSpinner is running in a thread, if somehow something other than Live called render or finish there could be thread safety issues, but because the context manager doesn't even return the Live object I think this should never happen.

@ichard26 ichard26 enabled auto-merge (squash) July 17, 2025 17:37
@ichard26

ichard26 commented Jul 17, 2025

Copy link
Copy Markdown
Member Author

There are two ways thread safety issues can arise:

  • Multiple threads are calling render() or finish() directly, as you've mentioned, the API makes this impossible
  • Multiple threads are writing to the terminal at once, leading to interwoven output. This can be avoided as long as the same console instance is used globally. This is easy to achieve using the get_console() helper that I added in PR Installation progress bar ✨ #13220.

In addition, this code is going to be rolled out in a opt-in basis so there are unexpected issues, they can be addressed at a later point.

Thanks for the review!

@ichard26 ichard26 merged commit f07a4cd into pypa:main Jul 17, 2025
29 checks passed
@ichard26 ichard26 deleted the feat/rich-spinners branch July 17, 2025 18:03
sepehr-rs pushed a commit to sepehr-rs/pip that referenced this pull request Jul 19, 2025
@ichard26 @sepehr-rs
Add rich based spinner (pypa#13451)
c270658 
The benefit of this spinner over our legacy spinners is that Rich will
update and render the spinner automatically for us. This is much nicer
than having to call .spin() manually.
pelson pushed a commit to pelson/pip that referenced this pull request Jul 20, 2025
@ichard26 @pelson
Add rich based spinner (pypa#13451)
e5829f5 
The benefit of this spinner over our legacy spinners is that Rich will
update and render the spinner automatically for us. This is much nicer
than having to call .spin() manually.
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@notatallshaw notatallshaw notatallshaw approved these changes

Assignees

No one assigned

Labels

C: output Related to what pip prints skip news Does not need a NEWS file entry (eg: trivial changes)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ichard26 @notatallshaw