-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: better run feedback for scripts #8023
Conversation
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 like this!
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.
lgtm, pending @DaniPopes
@@ -160,14 +156,17 @@ pub struct BundledState { | |||
|
|||
impl BundledState { | |||
pub async fn wait_for_pending(mut self) -> Result<Self> { | |||
let progress = ScriptProgress::default(); | |||
let progress_ref = &progress; |
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.
nit: I don't think this tmp var is needed?
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.
if using just progress
in closure it tries to move the progress
var itself into the closure and fails to compile
error[E0507]: cannot move out of `progress`, a captured variable in an `FnMut` closure
--> crates/script/src/broadcast.rs:165:45
|
159 | let progress = ScriptProgress::default();
| -------- captured outer variable
...
165 | .map(|(sequence_idx, sequence)| async move {
| __________________--------------------------_^
| | |
| | captured by this `FnMut` closure
166 | | let rpc_url = sequence.rpc_url();
167 | | let provider = Arc::new(get_http_provider(rpc_url));
168 | | progress.wait_for_pending(sequence_idx, sequence, &provider).await
| | --------
| | |
| | variable moved due to use in coroutine
| | move occurs because `progress` has type `ScriptProgress`, which does not implement the `Copy` trait
169 | | })
| |_____________^ `progress` is moved here
Motivation
Closes #8017
Solution
Adds
ScriptProgress
managing multipleSequenceProgress
instances drawing progress bars for sequences.Current structure should allow managing progress bars for concurrently sent sequences which is something I want to bring back at some point as #6271 was a bit of an overkill imo (we can still deploy in parallel if no chain ids overlap)
Example output
2024-06-01.02-44-16.mp4