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

support specification of file permissions in --add-file submission option #6505

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Dec 13, 2024

This PR adds support for specifying file permissions in the --add-file option in Flux submission command. (Presently, all files added with content directly provided on the command like get the default permissions of 0600)

Example:

$ flux submit --add-file=test:700='#!/bin/sh\ntrue\n' -n1 {{tmpdir}}/test

Problem: If perms is passed as None in Jobspec.add_file(), then
the default of 0600 is overridden. It would be better to use the
default in this case.

Set the default value of perms to None in the function arguments,
and if perms is None use the default of 0o600.
Problem: A file added via the cli submission --add-file=NAME=DATA
option always gets the default permissions of 0600.

Allow the file name to contain a colon followed by permissions, e.g.

 --add-file=test:700="sleep 0\n"
Problem: The --add-file description in common/job-other-options.rst
doesn't include the ability to force file permissions.

Update the documentation.
Problem: No tests in the testsuite ensure that file permissions may
be specified in the submission cli `--add-file` option.

Add a couple test to t2710-python-cli-submit.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!

Definitely not worth holding this up, but I wonder if there is a loose python module out there that supports chmod(1)'s "symbolic mode" (chmod +x, etc)?

@grondo
Copy link
Contributor Author

grondo commented Dec 13, 2024

I did look for something, but only found solutions for parsing the ls output style e.g. -rwxr-x--- which didn't seem that useful. For now I just wanted to get basic ability to specify perms succinctly on the command line.

@grondo
Copy link
Contributor Author

grondo commented Dec 13, 2024

Thanks I've set MWP.

@mergify mergify bot merged commit 2ddf0d5 into flux-framework:master Dec 13, 2024
35 checks passed
Copy link

codecov bot commented Dec 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.63%. Comparing base (8f575da) to head (967063d).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6505      +/-   ##
==========================================
- Coverage   83.64%   83.63%   -0.02%     
==========================================
  Files         525      525              
  Lines       87684    87693       +9     
==========================================
- Hits        73345    73341       -4     
- Misses      14339    14352      +13     
Files with missing lines Coverage Δ
src/bindings/python/flux/cli/base.py 95.73% <100.00%> (+0.04%) ⬆️
src/bindings/python/flux/job/Jobspec.py 90.12% <100.00%> (+0.04%) ⬆️

... and 13 files with indirect coverage changes

@grondo grondo deleted the add-file-perms branch December 13, 2024 18:05
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