Skip to content

[qob,batch] Add cost to BatchProgressBar#14876

Merged
hail-ci-robot merged 4 commits into
hail-is:mainfrom
jigold:cost-progress-bar-2
May 16, 2025
Merged

[qob,batch] Add cost to BatchProgressBar#14876
hail-ci-robot merged 4 commits into
hail-is:mainfrom
jigold:cost-progress-bar-2

Conversation

@jigold

@jigold jigold commented May 5, 2025

Copy link
Copy Markdown
Contributor

Change Description

Adds cost information to the progress bar

Security Assessment

  • This change has no security impact

Impact Description

Client-side change only

(Reviewers: please confirm the security impact before approving)

@jigold

jigold commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

Can someone copy and paste the error here? Or get me the test failure in a different way if it's long?

@jigold

jigold commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

This looks like it's passing now :)

@chrisvittal chrisvittal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks Jackie! The only failures you're getting now are lint errors, you'll need to ruff format your code. Then it should be fully green.

@jigold

jigold commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

check-hail-fast is telling me everything is broken. Can you give me the specific error?

@chrisvittal

Copy link
Copy Markdown
Collaborator
ruff format hail --diff
--- hail/python/hailtop/batch_client/aioclient.py
+++ hail/python/hailtop/batch_client/aioclient.py
@@ -507,7 +507,9 @@
         else:
             description += url
 
-        with progress.with_task(description, total=status['n_jobs'], disable=disable_progress_bar, cost=status['cost']) as progress_task:
+        with progress.with_task(
+            description, total=status['n_jobs'], disable=disable_progress_bar, cost=status['cost']
+        ) as progress_task:
             while True:
                 status = await self.status()
                 progress_task.update(None, total=status['n_jobs'], completed=status['n_completed'], cost=status['cost'])
@@ -706,7 +708,10 @@
             while True:
                 status = await self.status()
                 progress_task.update(
-                    None, total=status['n_jobs'] - starting_job + 1, completed=status['n_completed'] - starting_job + 1, cost=status['cost']
+                    None,
+                    total=status['n_jobs'] - starting_job + 1,
+                    completed=status['n_completed'] - starting_job + 1,
+                    cost=status['cost'],
                 )
                 if status['complete']:
                     return status

1 file would be reformatted, 461 files already formatted

@jigold

jigold commented May 8, 2025

Copy link
Copy Markdown
Contributor Author

Can someone check whether this worked now? It looks like there needs to be 2 approvals now?

@jigold

jigold commented May 15, 2025

Copy link
Copy Markdown
Contributor Author

Could someone please post the new error? Thanks!

@chrisvittal

Copy link
Copy Markdown
Collaborator

Just lints. Fixed.

@patrick-schultz patrick-schultz 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 should be a nice qol improvement. Thanks Jackie!

@hail-ci-robot
hail-ci-robot merged commit ad03662 into hail-is:main May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants