-
Notifications
You must be signed in to change notification settings - Fork 80
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
chore(weave): add rich progress bar to client flush #3828
Changes from 7 commits
195a6e1
c1d1acc
3e90c0c
49f4537
dce75f6
38d2e4b
adfb9e1
d6fcce2
3fccc1f
b94fda1
f6d537b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
"""Progress bar utilities for WeaveClient. | ||
|
||
This module provides functionality for displaying progress bars when flushing | ||
tasks in the WeaveClient. | ||
""" | ||
|
||
from typing import Callable, TypedDict | ||
|
||
from rich.console import Console | ||
from rich.progress import ( | ||
BarColumn, | ||
Progress, | ||
SpinnerColumn, | ||
TaskProgressColumn, | ||
TextColumn, | ||
TimeElapsedColumn, | ||
) | ||
|
||
|
||
class WeaveFlushStatus(TypedDict): | ||
gtarpenning marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""Status information about the current flush operation.""" | ||
|
||
# Current job counts | ||
main_jobs: int | ||
fastlane_jobs: int | ||
call_processor_jobs: int | ||
total_jobs: int | ||
|
||
# Tracking of completed jobs | ||
completed_since_last_update: int | ||
total_completed: int | ||
|
||
# Maximum number of jobs seen during this flush operation | ||
max_total_jobs: int | ||
|
||
# Whether there are any pending jobs | ||
has_pending_jobs: bool | ||
|
||
|
||
def create_progress_bar_callback() -> Callable[[WeaveFlushStatus], None]: | ||
"""Create a callback function that displays a progress bar for flush status. | ||
|
||
Returns: | ||
A callback function that can be passed to WeaveClient._flush. | ||
""" | ||
console = Console() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you check that it doesn't overwrite user logs? I'm not sure why but rich sometimes paints over my terminal which can be annoying There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The trace donut link gets printed while the bar is going, I think its fine. |
||
|
||
# Create a progress bar instance | ||
progress = Progress( | ||
SpinnerColumn(), | ||
TextColumn("[bold blue]{task.description}"), | ||
BarColumn(bar_width=None, complete_style="magenta"), | ||
TaskProgressColumn(), | ||
TimeElapsedColumn(), | ||
console=console, | ||
refresh_per_second=10, | ||
expand=True, | ||
transient=False, | ||
) | ||
|
||
# Start the progress display | ||
progress.start() | ||
|
||
# Create a task for tracking progress | ||
task_id = None | ||
|
||
def progress_callback(status: WeaveFlushStatus) -> None: | ||
"""Update the progress bar based on the flush status. | ||
|
||
Args: | ||
status: The current flush status. | ||
""" | ||
nonlocal task_id | ||
|
||
# If this is the first update, create the task | ||
if task_id is None: | ||
if status["max_total_jobs"] == 0: | ||
# No jobs to track, just return | ||
progress.stop() | ||
return | ||
|
||
# Print initial message | ||
if not progress.live.is_started: | ||
print(f"Flushing {status['max_total_jobs']} pending tasks...") | ||
|
||
# Create the task | ||
task_id = progress.add_task( | ||
"Flushing tasks", total=status["max_total_jobs"] | ||
) | ||
|
||
# If there are no more pending jobs, complete the progress bar | ||
if not status["has_pending_jobs"]: | ||
progress.update(task_id, completed=status["max_total_jobs"]) | ||
progress.stop() | ||
return | ||
|
||
# If new jobs were added, update the total | ||
if status["max_total_jobs"] > progress.tasks[task_id].total: | ||
progress.update(task_id, total=status["max_total_jobs"]) | ||
|
||
# Update progress bar with completed jobs | ||
if status["completed_since_last_update"] > 0: | ||
progress.update(task_id, advance=status["completed_since_last_update"]) | ||
|
||
# Format job details for description | ||
job_details = [] | ||
if status["main_jobs"] > 0: | ||
job_details.append(f"{status['main_jobs']} main") | ||
if status["fastlane_jobs"] > 0: | ||
job_details.append(f"{status['fastlane_jobs']} file-upload") | ||
if status["call_processor_jobs"] > 0: | ||
job_details.append(f"{status['call_processor_jobs']} call-batch") | ||
|
||
job_details_str = ", ".join(job_details) if job_details else "none" | ||
|
||
# Update progress bar description | ||
progress.update( | ||
task_id, | ||
description=f"Flushing tasks ({status['total_jobs']} remaining: {job_details_str})", | ||
) | ||
|
||
return progress_callback |
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 is not testing the pbar.
If you really want to test the pbar, you can capture redirect IO into a buf and confirm it looks the way you want
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.
hmmm yeah, the issue is actually more generating a test that actually requires flushing. not sure there is a trivial way of doing it... scratching my head over here
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.
There are a few ways, but maybe the easiest is to use a mock server that is slow to respond / only accepts 1 connection. Then you can queue up some jobs and see them go through 1-by-1