add CLIPlugin to accept arbitrary R shapes/JSON on the command-line by wihobbs · Pull Request #7252 · flux-framework/flux-core

wihobbs · 2025-12-19T20:11:33Z

This PR takes the excellent work done by @sam-maloney for RFC#46 and extends it into a command-line plugin that users can opt in to by setting FLUX_CLI_PLUGINPATH. This was originally suggested by @grondo as a way for us to test/get user feedback on the ergonomics of specifying shape on the command line, and make progress toward incorporating that option into our standard command line.

The plugin is relatively straightforward and the testing just references examples from the corresponding RFC. I know this is coming up right before many 🌴s, so no rush on reviews, but I wanted to get it to a stopping point before my own holiday begins :).

src/bindings/python/flux/shape/parser.py

src/bindings/python/flux/cli_jobspec/parser.py

wihobbs · 2026-02-04T22:08:25Z

@sam-maloney was kind enough to do a quick review on 1/8/26 and commented in Slack:

I made some minor modifications to the commit in my branch to hopefully address the Python linting errors and CodeQL problems, so you can incorporate that easily enough. (@wihobbs: ✅)

As for testing, my thinking was that if it could handle all the examples in RFC14 then that was a reasonably good start for testing valid coverage. I appreciate that you've added some invalid test cases, and I'm sure edge cases will come up as they always do, but I don't have much specific to suggest for now.

And as for naming, I have no issue at all with removing "CLI". I just picked something, and the CLI was what I was thinking about at the time, but a short form certainly could be useful in other contexts, so a more general name would be great. Whether that is "shape" or "compressed_resources" or "rstring", etc. doesn't really matter to me, depending on whether it is preferred to have it short or verbose!

grondo

Nice work! Just a few comments or questions to address inline. Thanks!

src/bindings/python/flux/shape/plugin/shape.py

t/t2718-python-cli-job-shape.t

grondo

It is getting to the point where @sam-maloney's --resource-shape option will be really useful so I made another pass on this one.

I'm also wondering if we should install the plugin so it is picked up by default, instead of requiring users to modify their plugin path to get the feature.

src/bindings/python/flux/Makefile.am

src/bindings/python/flux/shape/plugin/shape.py

t/python/t0041-job-shape-parser.py

t/Makefile.am

t/t2718-python-cli-job-shape.t

garlick · 2026-03-28T13:49:54Z

This would indeed be handy for toy scheduler experiments. Ping?

wihobbs · 2026-03-30T15:36:50Z

... I have no idea why this closed.

Edit: p.s. this is not ready for another review, a few more modifications needed, but will be soon.

Problem: There is no parser for the CLI Jobspec Resource String Syntax proposed in RFC 46. Add a flux.shape.parser Python module which implements a parser using the 'ply' module (mimicking the constraint parser). For testing, the parser may be run from the command line with flux python -m flux.shape.parser [OPTIONS] -- EXPRESSION

Problem: currently, users have no way on the CLI of utilizing the experimental parser in RFC 46 for specifying a job shape or a resource set they'd like to use via JSON. This causes users to defer to cumbersome piping and JSON mangling when they want to submit jobs of arbitrary resource shapes on the command line. Add a /plugin subdirectory to src/bindings/python/flux/shape for and a plugin for specifying `--resources-json` or `--resources-shape` for specifying resources on the command line beyond the standard ones supported.

Problem: there are examples of valid job shapes listed in RFC 46 but there is currently no unit test for the python jobspec shape parser. Add a unit test with the examples in RFC 46.

Problem: the jobspec shape/JSON plugin is currently not tested on the command line. Add a sharness test.

Problem: the RFC 46 resource shape parser implementation needs comprehensive test coverage to ensure correct parsing of the various syntax elements including resource counts, ranges, labels, attributes, nested resources, and resource lists. Add t0039-shape-parser.t to test the flux.shape.parser module as a standalone CLI tool. The tests cover basic parsing, range syntax, idsets, attributes, nested resources, resource lists, output formats (JSON and YAML), command-line options, error handling for invalid syntax, and all value types (strings, booleans, numbers, arrays). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Problem: the closing quote handler in the shape parser lexer was returning the quote token even when the quote_empty flag was set, causing an extra token to be emitted after empty quoted strings. Return None instead of the token when quote_empty is not set, since the QSTRING tokens have already been returned by the QSTRING_END state handler. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

wihobbs · 2026-03-30T19:35:39Z

This is ready for another review. I would like to get an ack from @sam-maloney before we merge, particularly on the testing and 501c14e, since it modifies his contribution and was Claude-generated. On balance, it looks reasonable to me, but Sam is the expert on the parser. If he's good with that, I will fold it up into the initial commit.

As for

I'm also wondering if we should install the plugin so it is picked up by default, instead of requiring users to modify their plugin path to get the feature.

that will depend on #7244, which I plan to go work on now.

codecov · 2026-03-31T14:23:24Z

Codecov Report

❌ Patch coverage is 96.17834% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.77%. Comparing base (d6f2129) to head (501c14e).
⚠️ Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
src/bindings/python/flux/shape/parser.py	96.18%	11 Missing ⚠️
src/bindings/python/flux/shape/plugin/shape.py	96.15%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7252      +/-   ##
==========================================
+ Coverage   83.72%   83.77%   +0.04%     
==========================================
  Files         568      570       +2     
  Lines       95551    95865     +314     
==========================================
+ Hits        80000    80310     +310     
- Misses      15551    15555       +4

Files with missing lines	Coverage Δ
src/bindings/python/flux/shape/plugin/shape.py	`96.15% <96.15%> (ø)`
src/bindings/python/flux/shape/parser.py	`96.18% <96.18%> (ø)`

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

grondo · 2026-03-31T14:30:48Z

src/bindings/python/flux/shape/plugin/shape.py

+                except TypeError:
+                    jobspec.jobspec["resources"] = data
+        else:
+            pass


Is the

else: pass

unnecessary here?

Also, just a suggestion: The function would be easier to read if it used args.shape and args.json directly instead of assigning to temporary local variables s and j.

wihobbs · 2026-03-31T15:30:06Z

I was able to reproduce the el8 issue locally:

Generating LALR tables
Traceback (most recent call last):
  File "/usr/lib64/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib64/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/usr/src/src/bindings/python/flux/shape/parser.py", line 666, in <module>
    print(yaml.dump(result, sort_keys=False))
  File "/usr/lib64/python3.6/site-packages/yaml/__init__.py", line 200, in dump
    return dump_all([data], stream, Dumper=Dumper, **kwds)
TypeError: dump_all() got an unexpected keyword argument 'sort_keys'
not ok 13 - flux.shape.parser --yaml produces yaml output
#
#		cat >expected10 <<-EOF &&
#		- type: slot
#		  count: 4
#		  label: task
#		  with:
#		  - type: node
#		    count: 1
#
#		EOF
#		$parser --yaml -- slot=4/node >output10 &&
#		test_cmp expected10 output10
#

According to the pyyaml changelog, support was added for the sort_keys option in v5.1. Unfortunately, the most recent rocky linux 8 package is 3.12.

I tried removing that key, but it does change the output:

--- expected10	2026-03-31 15:20:23.384955001 +0000
+++ output10	2026-03-31 15:20:23.489955001 +0000
@@ -1,7 +1,6 @@
-- type: slot
-  count: 4
+- count: 4
   label: task
+  type: slot
   with:
-  - type: node
-    count: 1
+  - {count: 1, type: node}

Perhaps that's not so bad, and would most likely be consistent across all distros? We could always add the key back in one glorious day when we finally no longer are tied to Python 3.6.8...

wihobbs · 2026-03-31T15:32:23Z

We could vendor pyyaml-5.1, or build it ourselves with pip. It appears to support python 3.6 according to the same changelog. However, if we do something to the el8 container, we'll have to remember and find a similar solution for all the TOSS 4 systems.

grondo · 2026-03-31T15:42:32Z

I don't think we'd want to vendor a python package just for testing purposes.

Perhaps there is a workaround for sorting the output without sort_keys. A search online suggests using collections.OrderedDict before dumping to get sorted output.

github-advanced-security bot found potential problems Dec 19, 2025

View reviewed changes

src/bindings/python/flux/shape/parser.py Fixed Show fixed Hide fixed

src/bindings/python/flux/cli_jobspec/parser.py Fixed Show fixed Hide fixed

wihobbs requested a review from trws January 7, 2026 22:40

wihobbs force-pushed the cli-jobspec-resources-parser branch 2 times, most recently from 25f1bbd to bd2e57b Compare January 12, 2026 20:41

wihobbs requested a review from grondo January 12, 2026 20:42

wihobbs force-pushed the cli-jobspec-resources-parser branch from bd2e57b to a432a63 Compare January 15, 2026 20:23

grondo reviewed Feb 5, 2026

View reviewed changes

src/bindings/python/flux/shape/plugin/shape.py Outdated Show resolved Hide resolved

t/t2718-python-cli-job-shape.t Show resolved Hide resolved

wihobbs force-pushed the cli-jobspec-resources-parser branch from a432a63 to 3d3172e Compare February 5, 2026 23:22

wihobbs mentioned this pull request Feb 10, 2026

ply has been abandoned, should we consider alternatives? #7375

Open

grondo reviewed Mar 20, 2026

View reviewed changes

wihobbs mentioned this pull request Mar 23, 2026

gaps for allowing jobspecs that are not V1? #7482

Open

wihobbs closed this Mar 30, 2026

wihobbs force-pushed the cli-jobspec-resources-parser branch from 3d3172e to db89f76 Compare March 30, 2026 15:36

wihobbs reopened this Mar 30, 2026

wihobbs force-pushed the cli-jobspec-resources-parser branch 3 times, most recently from 6683056 to 6e90be0 Compare March 30, 2026 18:50

sam-maloney and others added 6 commits March 30, 2026 12:27

t: add python unit test for ShapeParser

2e72321

Problem: there are examples of valid job shapes listed in RFC 46 but there is currently no unit test for the python jobspec shape parser. Add a unit test with the examples in RFC 46.

t: add tests for jobspec shape cli plugin

63ed551

Problem: the jobspec shape/JSON plugin is currently not tested on the command line. Add a sharness test.

wihobbs force-pushed the cli-jobspec-resources-parser branch from 6e90be0 to 501c14e Compare March 30, 2026 19:27

wihobbs requested a review from grondo March 30, 2026 19:35

grondo reviewed Mar 31, 2026

View reviewed changes

Conversation

wihobbs commented Dec 19, 2025

Uh oh!

Uh oh!

Uh oh!

wihobbs commented Feb 4, 2026

Uh oh!

grondo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

grondo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

garlick commented Mar 28, 2026

Uh oh!

wihobbs commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wihobbs commented Mar 30, 2026

Uh oh!

codecov bot commented Mar 31, 2026

Codecov Report

Uh oh!

grondo Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wihobbs commented Mar 31, 2026

Uh oh!

wihobbs commented Mar 31, 2026

Uh oh!

grondo commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

grondo left a comment •

edited

Loading

wihobbs commented Mar 30, 2026 •

edited

Loading

grondo Mar 31, 2026 •

edited

Loading