-
-
Notifications
You must be signed in to change notification settings - Fork 269
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
Add status messages for artifact installation #4005
Add status messages for artifact installation #4005
Conversation
|
||
print_lock = Base.ReentrantLock() # for non-fancyprint printing | ||
|
||
# name -> (state, bar) where state is :ready, :running, :done, or :failed | ||
download_states = Dict{String, DownloadState}() |
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.
Artifact names are not globally unique, so I changed the key to the artifact tree hash.
This is what I get on my Windows laptop, where installing and unpacking are much slower. status.messages.-.Made.with.Clipchamp.mp4 |
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.
Looks great
dstate.state = :failed | ||
rethrow() |
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.
Should we set progress status to failed
here and keep failed jobs around in the list until all complete/failed?
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.
I tried this but got strange formatting when many artifacts failed to install.
Installing artifacts ╸━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/83
XML2 failed
Qt6Wayland failed
Gettext failed
Xorg_xcb_util_wm failed
Libuuid failed
EarCut failed
Xorg_xcb_util failed
Vulkan_Loader failed
Installing artifacts ╸━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/83
XML2 failed
Qt6Wayland failed
Gettext failed
Installing artifacts ╸━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/83
Installing artifacts ╸━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/83
XML2 failed
Installing artifacts ╸━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/83
XML2 failed
Installing artifacts ╸━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0/83
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.
Just need to change the counting of rows to include to include :failed
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.
Is it an issue if more rows than can fit in the terminal are printed?
It's unclear to me what "installing" refers to here. The whole process of downloading, unpacking etc is installing to me. Could that be made more precise? |
Maybe the "Downloading artifacts" should be changed to "Installing artifacts" to show that it encompasses the whole procedure. |
Here is what the new version looks like. screencast.-.Made.with.Clipchamp.mp4 |
This PR replaces the artifact download progress bar with a status message if an installation step takes over half a second.
Here the unpacking step for Qt6Base takes over half a second.
This is required for #4001, where installing may take up to 60 seconds on some systems.
screencast.mp4