flux-framework / flux-core

core services for the Flux resource management framework
GNU Lesser General Public License v3.0
159 stars 49 forks source link

python: Update default Jobspec setattr() path #6053

Closed chu11 closed 5 days ago

chu11 commented 6 days ago

Problem: By default the Jobspec setattr() function sets keys to attributes.key. The default --setattr option in command line options like flux-submit default to set keys to attributes.system.key. This inconsistency can be confusing.

Default Jobspec setattr() to set keys to "attributes.system.key". Adjust command line --setattr option accordingly for tweak.

Fixes #6042


Just throwing up this WIP in case there are any initial objections

chu11 commented 6 days ago

For some strange reason the inception builder is failing and I believe not ok 17 - attach: reports job shell Killed if job shell is killed is the culprit. I can't imagine my change affecting this test. Adding some dumb debugging commits.

Edit: hmmm, my recent debugs made the fail disappear ... hopefully it's not something racy

chu11 commented 5 days ago

re-pushed with tests and removed WIP. I decided not to update any of the setattr("system.foo", bar) calls lingering around flux b/c I think the "system.foo" makes it a bit more clear what we're setting/configuring.

chu11 commented 5 days ago

re-pushed per comments above

codecov[bot] commented 5 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.36%. Comparing base (122975f) to head (9629f6b).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #6053 +/- ## =========================================== + Coverage 54.47% 83.36% +28.89% =========================================== Files 471 519 +48 Lines 76409 83817 +7408 =========================================== + Hits 41622 69876 +28254 + Misses 34787 13941 -20846 ``` | [Files](https://app.codecov.io/gh/flux-framework/flux-core/pull/6053?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework) | Coverage Δ | | |---|---|---| | [src/bindings/python/flux/cli/base.py](https://app.codecov.io/gh/flux-framework/flux-core/pull/6053?src=pr&el=tree&filepath=src%2Fbindings%2Fpython%2Fflux%2Fcli%2Fbase.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-c3JjL2JpbmRpbmdzL3B5dGhvbi9mbHV4L2NsaS9iYXNlLnB5) | `95.64% <100.00%> (ø)` | | | [src/bindings/python/flux/job/Jobspec.py](https://app.codecov.io/gh/flux-framework/flux-core/pull/6053?src=pr&el=tree&filepath=src%2Fbindings%2Fpython%2Fflux%2Fjob%2FJobspec.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-c3JjL2JpbmRpbmdzL3B5dGhvbi9mbHV4L2pvYi9Kb2JzcGVjLnB5) | `90.08% <100.00%> (+56.31%)` | :arrow_up: | ... and [437 files with indirect coverage changes](https://app.codecov.io/gh/flux-framework/flux-core/pull/6053/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework)