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

jobtap: add flux_jobtap_jobspec_update_id_pack() #6500

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Dec 10, 2024

This PR addresses #5957 by adding a function that posts a jobspec-update event for an arbitrary jobid.

The function assumes the target job is not in an active callback, i.e. it is being updated asynchronously, so some of the work of flux_jobtap_josbpec_update_pack(), like accumulating updates until the current callback is complete, can be skipped. Thus the function is really just a wrapper around event_post_pack().

Problem: There's no interface to update the jobspec of a job
outside of a jobtap callback for that job.

Add flux_jobtap_jobspec_update_id_pack(). This function just does
some nominal checks on the target job, then wraps event_post_pack().
Problem: The jobtap API call flux_jobtap_jobspec_update_id_pack()
is not tested.

Add it to the testing in t2212-job-manager-plugins.t.
Copy link
Member

@jameshcorbett jameshcorbett left a comment

Choose a reason for hiding this comment

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

LGTM!

@grondo
Copy link
Contributor Author

grondo commented Dec 11, 2024

Thanks! I'll set MWP.

@mergify mergify bot merged commit 7d4d63b into flux-framework:master Dec 11, 2024
34 of 35 checks passed
Copy link

codecov bot commented Dec 11, 2024

Codecov Report

Attention: Patch coverage is 80.64516% with 6 lines in your changes missing coverage. Please review.

Project coverage is 83.62%. Comparing base (0af65c5) to head (c5d9fda).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
src/modules/job-manager/jobtap.c 80.64% 6 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6500   +/-   ##
=======================================
  Coverage   83.61%   83.62%           
=======================================
  Files         524      524           
  Lines       87623    87654   +31     
=======================================
+ Hits        73267    73301   +34     
+ Misses      14356    14353    -3     
Files with missing lines Coverage Δ
src/modules/job-manager/jobtap.c 84.71% <80.64%> (-0.11%) ⬇️

... and 11 files with indirect coverage changes

@grondo grondo deleted the issue#5957 branch December 11, 2024 00:18
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