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

job-list: support getting job project and bank #5413

Merged
merged 5 commits into from
Sep 19, 2023

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Aug 25, 2023

With the jobspec-update support from #5408 and the upcoming flux update support in #5409, this was half-easy to add so decided to add it (issues #4697 and #4698).

This is dependent on #5409 going in of course.

It's worth noting that nothing in flux-accounting does this yet, so it has no practical use. We may want to hold off on eventual merge until flux-accounting merging some stuff in for this.

@chu11 chu11 force-pushed the issue4697_4698_project_bank branch from 9a66c50 to 738e16a Compare September 1, 2023 03:40
@chu11 chu11 changed the title WIP job-list: support getting job project and bank job-list: support getting job project and bank Sep 1, 2023
@chu11
Copy link
Member Author

chu11 commented Sep 1, 2023

with #5408 and #5409 now in, rebased and removed WIP.

Note that it may be a bit premature to merge until flux-accounting supports in their plugins (such as #301). But there is nothing wrong with adding it as is for when that support comes from flux-accounting later.

Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

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

This LGTM.

Problem: For systems that add project/bank accounting information
in jobspecs, it would be convenient if users could look up that
information via job-list / flux-jobs.

Solution: Add support to job-list to get project/bank from a job.

Fixes flux-framework#4697
Fixes flux-framework#4698
Problem: There is no coverage for getting project/bank information
in job-list.

Add coverage in t2260-job-list.t.
Problem: job-list now supports getting a job's project/bank,
but flux-jobs does not support it.

Support getting a job's project/bank via flux-jobs.
Problem: There is no coverage for the project/bank fields in flux-jobs.

Add coverage in t2800-jobs-cmd.t.
Problem: The new project/bank fields in flux-jobs are not
documented.

Add them in flux-jobs(1).
@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #5413 (9340d86) into master (ec8aece) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #5413      +/-   ##
==========================================
+ Coverage   83.59%   83.62%   +0.02%     
==========================================
  Files         480      480              
  Lines       80782    80788       +6     
==========================================
+ Hits        67533    67555      +22     
+ Misses      13249    13233      -16     
Files Changed Coverage Δ
src/bindings/python/flux/job/info.py 92.89% <ø> (ø)
src/modules/job-list/job-list.c 79.01% <ø> (ø)
src/modules/job-list/job_data.c 94.83% <ø> (ø)
src/modules/job-list/job_util.c 92.15% <100.00%> (+0.32%) ⬆️

... and 10 files with indirect coverage changes

@mergify mergify bot merged commit 87a06dd into flux-framework:master Sep 19, 2023
30 checks passed
@chu11 chu11 deleted the issue4697_4698_project_bank branch September 19, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants