Merge pull request #4327 from chu11/job_list_consistent_returns

job-list: consistently return job attributes

Author: mergify[bot]

src/bindings/python/flux/job/info.py

@@ -199,6 +199,8 @@ class JobInfo:
         "t_cleanup": 0.0,
         "t_inactive": 0.0,
         "expiration": 0.0,
+        "name": "",
+        "ntasks": "",
         "nnodes": "",
         "priority": "",
         "ranks": "",

src/modules/job-list/job_state.c

@@ -147,7 +147,10 @@ static struct job *job_create (struct list_ctx *ctx, flux_jobid_t id)
     job->id = id;
     job->state = FLUX_JOB_STATE_NEW;
     job->userid = FLUX_USERID_UNKNOWN;
+    job->ntasks = -1;
+    job->nnodes = -1;
     job->urgency = -1;
+    job->expiration = -1.0;
     job->wait_status = -1;
     /* pending jobs that are not yet assigned a priority shall be
      * listed after those who do, so we set the job priority to MIN */

src/modules/job-list/job_util.c

@@ -79,40 +79,42 @@ static int store_attr (struct job *job,
         val = json_integer (job->state);
     }
     else if (!strcmp (attr, "name")) {
-        /* potentially NULL if jobspec invalid */
-        if (job->name)
-            val = json_string (job->name);
-        else
-            val = json_string ("");
+        /* job->name potentially NULL if jobspec invalid */
+        if (!job->name)
+            return 0;
+        val = json_string (job->name);
     }
     else if (!strcmp (attr, "ntasks")) {
+        /* job->ntasks potentially < 0 if jobspec invalid */
+        if (job->ntasks < 0)
+            return 0;
         val = json_integer (job->ntasks);
     }
     else if (!strcmp (attr, "nnodes")) {
-        if (!(job->states_mask & FLUX_JOB_STATE_RUN))
+        /* job->nnodes potentially < 0 if R invalid */
+        if (!(job->states_mask & FLUX_JOB_STATE_RUN)
+            || job->nnodes < 0)
             return 0;
         val = json_integer (job->nnodes);
     }
     else if (!strcmp (attr, "ranks")) {
-        if (!(job->states_mask & FLUX_JOB_STATE_RUN))
+        /* job->ranks potentially NULL if R invalid */
+        if (!(job->states_mask & FLUX_JOB_STATE_RUN)
+            || !job->ranks)
             return 0;
-        /* potentially NULL if R invalid */
-        if (job->ranks)
-            val = json_string (job->ranks);
-        else
-            val = json_string ("");
+        val = json_string (job->ranks);
     }
     else if (!strcmp (attr, "nodelist")) {
-        if (!(job->states_mask & FLUX_JOB_STATE_RUN))
+        /* job->nodelist potentially NULL if R invalid */
+        if (!(job->states_mask & FLUX_JOB_STATE_RUN)
+            || !job->nodelist)
             return 0;
-        /* potentially NULL if R invalid */
-        if (job->nodelist)
-            val = json_string (job->nodelist);
-        else
-            val = json_string ("");
+        val = json_string (job->nodelist);
     }
     else if (!strcmp (attr, "expiration")) {
-        if (!(job->states_mask & FLUX_JOB_STATE_RUN))
+        /* job->expiration potentially < 0 if R invalid */
+        if (!(job->states_mask & FLUX_JOB_STATE_RUN)
+            || job->expiration < 0)
             return 0;
         val = json_real (job->expiration);
     }

t/t2260-job-list.t

@@ -1162,6 +1162,7 @@ test_expect_success HAVE_JQ 'create illegal jobspec with empty command array' '
         cat hostname.json | $jq ".tasks[0].command = []" > bad_jobspec.json
 '
 
+# note that ntasks will not be set if jobspec invalid
 test_expect_success HAVE_JQ 'flux job list works on job with illegal jobspec' '
         jobid=`flux job submit bad_jobspec.json | flux job id` &&
         fj_wait_event $jobid clean >/dev/null &&
@@ -1174,8 +1175,8 @@ test_expect_success HAVE_JQ 'flux job list works on job with illegal jobspec' '
         done &&
         test "$i" -lt "5" &&
         flux job list --states=inactive | grep $jobid > list_illegal_jobspec.out &&
-        cat list_illegal_jobspec.out | $jq -e ".name == \"\"" &&
-        cat list_illegal_jobspec.out | $jq -e ".ntasks == 0"
+        cat list_illegal_jobspec.out | $jq -e ".name == null" &&
+        cat list_illegal_jobspec.out | $jq -e ".ntasks == null"
 '
 
 test_expect_success HAVE_JQ,NO_CHAIN_LINT 'flux job list-ids works on job with illegal jobspec' '
@@ -1211,9 +1212,10 @@ test_expect_success HAVE_JQ 'flux job list works on job with illegal R' '
         done &&
         test "$i" -lt "5" &&
         flux job list --states=inactive | grep $jobid > list_illegal_R.out &&
-        cat list_illegal_R.out | $jq -e ".ranks == \"\"" &&
-        cat list_illegal_R.out | $jq -e ".nnodes == 0" &&
-        cat list_illegal_R.out | $jq -e ".nodelist == \"\""
+        cat list_illegal_R.out | $jq -e ".ranks == null" &&
+        cat list_illegal_R.out | $jq -e ".nnodes == null" &&
+        cat list_illegal_R.out | $jq -e ".nodelist == null" &&
+        cat list_illegal_R.out | $jq -e ".expiration == null"
 '
 
 test_expect_success HAVE_JQ,NO_CHAIN_LINT 'flux job list-ids works on job with illegal R' '

t/t2800-jobs-cmd.t

@@ -1012,8 +1012,7 @@ test_expect_success HAVE_JQ 'create illegal jobspec with empty command array' '
 '
 
 # to avoid potential racyness, wait up to 5 seconds for job to appear
-# in job list.  note that how a jobspec is illegal can affect what
-# values below are set vs not set.
+# in job list.  note that ntasks will not be set if jobspec invalid
 test_expect_success HAVE_JQ 'flux jobs works on job with illegal jobspec' '
         jobid=`flux job submit bad_jobspec.json` &&
         fj_wait_event $jobid clean &&
@@ -1026,7 +1025,7 @@ test_expect_success HAVE_JQ 'flux jobs works on job with illegal jobspec' '
         done &&
         test "$i" -lt "5" &&
         flux jobs -no "{name},{ntasks}" $jobid > list_illegal_jobspec.out &&
-        echo ",0" > list_illegal_jobspec.exp &&
+        echo "," > list_illegal_jobspec.exp &&
         test_cmp list_illegal_jobspec.out list_illegal_jobspec.exp
 '
 
@@ -1050,8 +1049,9 @@ test_expect_success HAVE_JQ 'flux jobs works on job with illegal R' '
                 i=$((i + 1))
         done &&
         test "$i" -lt "5" &&
-        flux jobs -no "{ranks},{nnodes},{nodelist}" $jobid > list_illegal_R.out &&
-        echo ",0," > list_illegal_R.exp &&
+        flux jobs -no "{ranks},{nnodes},{nodelist},{expiration}" $jobid \
+            > list_illegal_R.out &&
+        echo ",,,0.0" > list_illegal_R.exp &&
         test_cmp list_illegal_R.out list_illegal_R.exp
 '