flux-mini: --setattr: place keys in attributes.system by default with and default values to 1

doc/man1/flux-mini.rst

@@ -365,12 +365,14 @@ OTHER OPTIONS
    otherwise VAL is interpreted as a string. If VAL is not set, then the
    default value is 1. See SHELL OPTIONS below.
 
-**--setattr=KEY=VAL**
+**--setattr=KEY[=VAL]**
    Set jobspec attribute. Keys may include periods to denote hierarchy.
-   VAL may be valid JSON (bare values, objects, or arrays), otherwise VAL
-   is interpreted as a string. If KEY starts with a ``^`` character, then
-   VAL is interpreted as a file, which must be valid JSON, to use as the
-   attribute value.
+   If KEY does not begin with ``system.``, ``user.``, or ``.``, then
+   ``system.`` is assumed.  VAL is optional and may be valid JSON (bare
+   values, objects, or arrays), otherwise VAL is interpreted as a string. If
+   VAL is not set, then the default value is 1.  If KEY starts with a ``^``
+   character, then VAL is interpreted as a file, which must be valid JSON,
+   to use as the attribute value.
 
 **--begin-time=DATETIME**
    Convenience option for setting a ``begin-time`` dependency for a job.

src/cmd/flux-mini.py

@@ -410,6 +410,30 @@ def __str__(self):
         return " ".join(result)
 
 
+def parse_jobspec_keyval(label, keyval):
+    """Parse a key[=value] option as used with --setopt and --setattr
+
+    Supports ^key=filename to load JSON object from a file
+    """
+    # Split into key, val with a default of 1 if no val given:
+    key, val = (keyval.split("=", 1) + [1])[:2]
+
+    # Support key prefix of ^ to load value from a file
+    if key.startswith("^"):
+        key = key.lstrip("^")
+        with open(val) as filep:
+            try:
+                val = json.load(filep)
+            except (json.JSONDecodeError, TypeError) as exc:
+                raise ValueError(f"{label}: {val}: {exc}") from exc
+    else:
+        try:
+            val = json.loads(val)
+        except (json.JSONDecodeError, TypeError):
+            pass
+    return key, val
+
+
 class MiniCmd:
     """
     MiniCmd is the base class for all flux-mini subcommands
@@ -457,10 +481,11 @@ def create_parser(exclude_io=False):
         parser.add_argument(
             "--setattr",
             action="append",
-            help="Set job attribute ATTR to VAL (multiple use OK). "
-            + "If ATTR starts with ^, then VAL is a file containing valid "
-            + "JSON which will be used as the value of the attribute.",
-            metavar="ATTR=VAL",
+            help="Set job attribute ATTR. An optional value is supported "
+            + " with ATTR=VAL (default VAL=1). If ATTR starts with ^, "
+            + "then VAL is a file containing valid JSON which will be used "
+            + "as the value of the attribute. (multiple use OK)",
+            metavar="ATTR",
         )
         parser.add_argument(
             "--dependency",
@@ -603,12 +628,7 @@ def jobspec_create(self, args):
 
         if args.setopt is not None:
             for keyval in args.setopt:
-                # Split into key, val with a default for 1 if no val given:
-                key, val = (keyval.split("=", 1) + [1])[:2]
-                try:
-                    val = json.loads(val)
-                except (json.JSONDecodeError, TypeError):
-                    pass
+                key, val = parse_jobspec_keyval("--setopt", keyval)
                 jobspec.setattr_shell_option(key, val)
 
         if args.debug_emulate:
@@ -620,23 +640,20 @@ def jobspec_create(self, args):
 
         if args.setattr is not None:
             for keyval in args.setattr:
-                tmp = keyval.split("=", 1)
-                if len(tmp) != 2:
-                    raise ValueError("--setattr: Missing value for attr " + keyval)
-                key = tmp[0]
-                if key.startswith("^"):
-                    key = key.strip("^")
-                    with open(tmp[1]) as filep:
-                        try:
-                            val = json.load(filep)
-                        except (json.JSONDecodeError, TypeError) as exc:
-                            raise ValueError(f"--setattr: {tmp[1]}: {exc}")
-                else:
-                    try:
-                        val = json.loads(tmp[1])
-                    except (json.JSONDecodeError, TypeError):
-                        val = tmp[1]
-                jobspec.setattr(key, val)
+                key, val = parse_jobspec_keyval("--setattr", keyval)
+
+                #  If key does not explicitly start with ".", "system."
+                #   or "user.", then "system." is implied. This is a
+                #   meant to be a usability enhancement since almost all
+                #   uses of --setattr will target attributes.system.
+                #
+                if not key.startswith((".", "user.", "system.")):
+                    key = "system." + key
+
+                #  Allow key to begin with "." which simply forces the key
+                #   to start at the top level (since .system is not applied
+                #   due to above conditional)
+                jobspec.setattr(key.lstrip("."), val)
 
         return jobspec
 

t/t2110-job-ingest-validator.t

@@ -160,7 +160,7 @@ test_expect_success 'job-ingest: load multiple validators' '
 	ingest_module reload validator-plugins=feasibility,jobspec
 '
 test_expect_success 'job-ingest: jobs that fail either validator are rejected' '
-	test_must_fail flux mini submit --setattr=foo=bar hostname &&
+	test_must_fail flux mini submit --setattr=.foo=bar hostname &&
 	test_must_fail flux mini submit -n 4568 hostname
 '
 test_expect_success 'job-ingest: validator unexpected exit is handled' '

t/t2700-mini-cmd.t

@@ -126,16 +126,23 @@ test_expect_success 'flux mini submit --time-limit=4-00:30:00 fails' '
 '
 
 test_expect_success HAVE_JQ 'flux mini submit --setattr works' '
-	flux mini submit --dry-run \
+	flux mini submit --env=-* --dry-run \
 		--setattr user.meep=false \
 		--setattr user.foo=\"xxx\" \
 		--setattr user.foo2=yyy \
+		--setattr foo \
+		--setattr .test=a \
+		--setattr test2=b \
 		--setattr system.bar=42 hostname >attr.out &&
-	test $(jq ".attributes.user.meep" attr.out) = "false" &&
-	test $(jq ".attributes.user.foo" attr.out) = "\"xxx\"" &&
-	test $(jq ".attributes.user.foo2" attr.out) = "\"yyy\"" &&
-	test $(jq ".attributes.system.bar" attr.out) = "42"
+	jq -e ".attributes.user.meep == false" attr.out &&
+	jq -e ".attributes.user.foo == \"xxx\"" attr.out &&
+	jq -e ".attributes.user.foo2 == \"yyy\"" attr.out &&
+	jq -e ".attributes.system.foo == 1" attr.out &&
+	jq -e ".attributes.test == \"a\"" attr.out &&
+	jq -e ".attributes.system.test2 == \"b\"" attr.out &&
+	jq -e ".attributes.system.bar == 42" attr.out
 '
+
 test_expect_success HAVE_JQ 'flux mini submit --setattr=^ATTR=VAL works' '
 	cat | jq -S . >attr.json <<-EOF &&
 	[
@@ -153,14 +160,6 @@ test_expect_success HAVE_JQ 'flux mini submit --setattr=^ATTR=VAL works' '
 	    jq -S .attributes.user.foo > attrout.json &&
 	test_cmp attr.json attrout.json
 '
-test_expect_success 'flux mini submit --setattr fails without value' '
-	test_expect_code 1 \
-		flux mini submit --dry-run \
-		--setattr foo \
-		hostname >attr_fail.out 2>&1 &&
-	test_debug "cat attr_fail.out" &&
-	grep "Missing value for attr foo" attr_fail.out
-'
 test_expect_success HAVE_JQ 'flux mini submit --setattr=^ detects bad JSON' '
 	cat <<-EOF > bad.json &&
 	[ { "foo":"value",