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

shell: generate fatal error if block taskmap scheme has an argument #5730

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Feb 10, 2024

This simple PR fixes #5723: If a user inadvertently specifies --taskmap=block:arg when they meant --taskmap=other_scheme:arg, the shell currently silently ignores arg and silently continues with the block scheme. Since an argument is not expected or allowed with the block taskmap scheme, the shell should generate an error in this case.

Fixes #5723

Problem: If --taskmap=block:foo is used, the job shell silently
ignores the value 'foo'. This could lead to a job running with
the wrong taskmap if the user intended some other taskmap scheme.
It would be better to abort and notify the user something's wrong.

Check if a value is specified with `--taskmap=block` and abort the
shell if so.
Problem: None of the shell taskmap tests ensure that an argument
specified with the block scheme is rejected by the shell.

Add a test to t2616-job-shell-taskmap.t.
Copy link
Member

@garlick garlick 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 Feb 14, 2024

Thanks!

Copy link

codecov bot commented Feb 14, 2024

Codecov Report

Merging #5730 (9161029) into master (5fe073b) will decrease coverage by 0.01%.
The diff coverage is 80.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5730      +/-   ##
==========================================
- Coverage   83.46%   83.46%   -0.01%     
==========================================
  Files         487      487              
  Lines       83130    83133       +3     
==========================================
  Hits        69385    69385              
- Misses      13745    13748       +3     
Files Coverage Δ
src/shell/shell.c 78.71% <80.00%> (-0.05%) ⬇️

... and 9 files with indirect coverage changes

@mergify mergify bot merged commit 35c2c10 into flux-framework:master Feb 14, 2024
34 of 35 checks passed
@grondo grondo deleted the issue#5723 branch February 14, 2024 23:56
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.

shell taskmap block scheme ignores its arguments
2 participants