Skip to content
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

db: Add option to capture canceled jobs #1534

Merged
merged 8 commits into from
Mar 18, 2024
Merged

db: Add option to capture canceled jobs #1534

merged 8 commits into from
Mar 18, 2024

Conversation

V-FEXrt
Copy link
Contributor

@V-FEXrt V-FEXrt commented Mar 12, 2024

Adds --canceled which captures jobs that were canceled in the last build (usually by ctrl+c). This works similarly to --failed in that it will forget the list of canceled jobs after the next wake invocation.

@V-FEXrt V-FEXrt requested a review from colinschmidt March 12, 2024 20:27
Base automatically changed from db-simple-timeline to master March 14, 2024 15:58
Copy link
Contributor

@colinschmidt colinschmidt left a comment

Choose a reason for hiding this comment

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

LGTM

@V-FEXrt
Copy link
Contributor Author

V-FEXrt commented Mar 14, 2024

Need to run a test to verify that wakebox timed out jobs show up as canceled then I can land this

rm -f wake.db

# Use || true to ignore the expected non-0 return from timeout
timeout 1 ${WAKE} test || true
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need test Unit here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. test is exported as a CLI function so it expects a List String and with nothing is specified then Nil is passed

Comment on lines +437 to +440
bool is_db_inspection =
!clo.job_ids.empty() || !clo.output_files.empty() || !clo.input_files.empty() ||
!clo.labels.empty() || !clo.tags.empty() || clo.last_use || clo.last_exe || clo.failed ||
clo.tagdag || clo.timeline || clo.taguri || clo.simple || clo.simple_timeline || clo.canceled;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a linter for C++?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, just for formatting though. clang-format (version 12.0.X)

@V-FEXrt V-FEXrt merged commit 9e7ef2c into master Mar 18, 2024
13 checks passed
@V-FEXrt V-FEXrt deleted the db-canceled branch March 18, 2024 20:18
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.

2 participants