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

ingest: require job queue if [queues] are configured #4616

Merged
merged 2 commits into from Sep 26, 2022

Conversation

garlick
Copy link
Member

@garlick garlick commented Sep 26, 2022

Problem: if queues are configured, but no default queue is configured, and a job is submitted without a queue, the job is ingested and allowed to fail later.

Modify the 'defaults' frobnicator plugin to reject such a job at ingest.

Problem: if queues are configured, but no default queue is configured,
and a job is submitted without a queue, the job is ingested and
allowed to fail later

Modify the 'defaults' frobnicator plugin to reject such a job at ingest.

Fix sharness limit tests that were using queue configurations that are now
considered invalid.

Fixes flux-framework#4614
Problem: there is no test coverage for the frobnicator defaults
plugin rejecing a job with no queue when queues are configured,
but no default queue is configured.

Add a test.
@garlick garlick changed the title ingest: require user to specify a queue if configured but no default ingest: require job queue if [queues] are configured Sep 26, 2022
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.

LGTM!

@codecov
Copy link

codecov bot commented Sep 26, 2022

Codecov Report

Merging #4616 (6c0349b) into master (0aacf0c) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4616      +/-   ##
==========================================
- Coverage   83.40%   83.40%   -0.01%     
==========================================
  Files         411      411              
  Lines       68803    68805       +2     
==========================================
+ Hits        57388    57389       +1     
- Misses      11415    11416       +1     
Impacted Files Coverage Δ
...gs/python/flux/job/frobnicator/plugins/defaults.py 91.52% <100.00%> (+0.29%) ⬆️
src/common/libsubprocess/fork.c 72.65% <0.00%> (-0.79%) ⬇️
src/common/libsubprocess/subprocess.c 87.89% <0.00%> (-0.30%) ⬇️
src/shell/output.c 76.70% <0.00%> (+0.15%) ⬆️
src/cmd/builtin/shutdown.c 87.85% <0.00%> (+0.93%) ⬆️

@garlick
Copy link
Member Author

garlick commented Sep 26, 2022

Thanks! Setting MWP.

@mergify mergify bot merged commit bbf5c49 into flux-framework:master Sep 26, 2022
@garlick garlick deleted the issue#4614 branch September 26, 2022 21:51
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.

None yet

2 participants