-
Notifications
You must be signed in to change notification settings - Fork 109
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 more err_tip for easier debugging #363
Conversation
Small change to make it a little easier to debug when something goes wrong.
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.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @blakehatch)
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.
All other bare 15 await? and 10 await.maps besides the two mentioned have underlying error reporting so no err_tip is needed.
Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @allada)
cas/worker/running_actions_manager.rs
line 366 at r1 (raw file):
} let (mut file_nodes, dir_entries, mut symlinks) = try_join3(
nit: err_tip could be added here for thorough debugging as try_join has no error reporting.
cas/worker/running_actions_manager.rs
line 516 at r1 (raw file):
&self.work_directory, )); let (command, _) = try_join(command_fut, download_to_directory_fut).await?;
nit: another try_join that might benefit from an error output.
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.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @blakehatch)
cas/worker/running_actions_manager.rs
line 366 at r1 (raw file):
Previously, blakehatch (Blake Hatch) wrote…
nit: err_tip could be added here for thorough debugging as try_join has no error reporting.
Adding an err_tip
here would not tell us which one failed. It'd just say one of them failed. We expect the underlying futures to provide the useful errors.
cas/worker/running_actions_manager.rs
line 516 at r1 (raw file):
Previously, blakehatch (Blake Hatch) wrote…
nit: another try_join that might benefit from an error output.
(same as above)
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.
Reviewed 1 of 1 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @allada)
Small change to make it a little easier to debug when something goes wrong.
Small change to make it a little easier to debug when something goes wrong.
Small change to make it a little easier to debug when something goes wrong.
This change is