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

view-job-records --jobid: accept all Flux job ID formats #566

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

cmoussa1
Copy link
Member

@cmoussa1 cmoussa1 commented Jan 25, 2025

As noted in #564, viewing a job record with view-job-records --jobid only accepts decimal formats of job IDs. However, Flux offers multiple formats for job IDs, and thus, the --jobid argument should also be able to handle such formats.


This PR adds the use of the flux.job.JobID() class to convert any Flux job ID format passed-in to a decimal format when looking up a job record in the flux-accounting job-archive.

I've also added in some basic tests that call view-job-records --jobid with different ID formats.

Fixes #564

@cmoussa1 cmoussa1 added improvement Upgrades to an already existing feature commands related to the flux-accounting commands/bindings labels Jan 25, 2025
@cmoussa1 cmoussa1 requested a review from ryanday36 January 25, 2025 00:52
As noted in flux-framework#564, viewing a job record with view-job-records --jobid
only accepts decimal formats of job IDs. However, Flux offers multiple
formats for Job IDs, and thus, the --jobid argument should also be able
to handle such formats.

Use the flux.job.JobID() class to convert any Flux job ID format to a
decimal format when looking up a job record in the flux-accounting
job-archive.
Problem: There are no tests for calling view-job-records --jobid where
the format of --jobid is in multiple formats, such as f58, hex, dothex,
etc.

Add some basic tests.
Copy link
Contributor

@ryanday36 ryanday36 left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks Chris!

@cmoussa1
Copy link
Member Author

Thanks @ryanday36! I just rebased to catch up after #567. Will set MWP here

@mergify mergify bot merged commit 0640f48 into flux-framework:master Jan 27, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commands related to the flux-accounting commands/bindings improvement Upgrades to an already existing feature merge-when-passing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

view-job-records -j only takes numeric jobid
2 participants