deploy: Add a spinner #848
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In some cases I wasn't seeing this output, and I think it's because we hadn't `drop()`'d the bar. Since it's in scope it's cleaner to use its `println` method. Signed-off-by: Colin Walters <[email protected]>
First, this operation was absolutely not asynchronous and we need to spawn it in a helper thread on general principle. Doing that is unfortunately VERY hacky because we need to clone a lot of things and especially there's an ergonomics hit here since `Deployment` is incorrectly `!Send`. But the main motivation is this operation can be slow sometimes, so let's show progress output so the admin knows we're not blocked. Closes: bootc-dev#842 Signed-off-by: Colin Walters <[email protected]>
omertuc
reviewed
Nov 5, 2024
Contributor
omertuc
left a comment
omertuc
left a comment
There was a problem hiding this comment.
looks good except for one question
Contributor
|
/lgtm |
Contributor
|
Oh and FYI, IDK if you care but you added |
Things we do via this mechanism can take some time; let's log the elapsed time for greater visibility both to the human output *and* log to the journal. Signed-off-by: Colin Walters <[email protected]>
cgwalters
force-pushed
the
spinner-deploy
branch
from
adc37ab to
daeafea
Compare
Collaborator
Author
I dropped that |
Collaborator
Author
This repository doesn't use Prow, so to merge it needs a Github approval state |
omertuc
approved these changes
Nov 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
deploy: Use bar.println
In some cases I wasn't seeing this output, and I think it's
because we hadn't
drop()'d the bar. Since it's in scopeit's cleaner to use its
printlnmethod.Signed-off-by: Colin Walters [email protected]
deploy: Add a spinner
First, this operation was absolutely not asynchronous
and we need to spawn it in a helper thread on general
principle. Doing that is unfortunately VERY hacky
because we need to clone a lot of things and especially
there's an ergonomics hit here since
Deploymentisincorrectly
!Send.But the main motivation is this operation can be slow sometimes,
so let's show progress output so the admin knows we're not blocked.
Closes: #842
Signed-off-by: Colin Walters [email protected]
utils: Print and log time for async_task_with_spinner
Things we do via this mechanism can take some time; let's
log the elapsed time for greater visibility both to the
human output and log to the journal.
Signed-off-by: Colin Walters [email protected]