Skip to content

Commit 0b2b1f4

Browse files
committed
job-list: consistently return job attributes
Problem: The job-list module was inconsistent in its return of job data to the user. In some cases, "defaults" were returned when data was never initialized. In other cases, the data is simply not returned to the user. Solution: Consistently do not return data to the user when it is not available / not set. The attributes adjusted are: name ntasks nnodes ranks nodelist expiration Adjust tests accordingly. In the Python JobInfo class, set display defaults for name and ntasks.
1 parent 4bffe00 commit 0b2b1f4

File tree

5 files changed

+38
-29
lines changed

5 files changed

+38
-29
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ class JobInfo:
199199
"t_cleanup": 0.0,
200200
"t_inactive": 0.0,
201201
"expiration": 0.0,
202+
"name": "",
203+
"ntasks": "",
202204
"nnodes": "",
203205
"priority": "",
204206
"ranks": "",

src/modules/job-list/job_state.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,10 @@ static struct job *job_create (struct list_ctx *ctx, flux_jobid_t id)
147147
job->id = id;
148148
job->state = FLUX_JOB_STATE_NEW;
149149
job->userid = FLUX_USERID_UNKNOWN;
150+
job->ntasks = -1;
151+
job->nnodes = -1;
150152
job->urgency = -1;
153+
job->expiration = -1.0;
151154
job->wait_status = -1;
152155
/* pending jobs that are not yet assigned a priority shall be
153156
* listed after those who do, so we set the job priority to MIN */

src/modules/job-list/job_util.c

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -79,40 +79,42 @@ static int store_attr (struct job *job,
7979
val = json_integer (job->state);
8080
}
8181
else if (!strcmp (attr, "name")) {
82-
/* potentially NULL if jobspec invalid */
83-
if (job->name)
84-
val = json_string (job->name);
85-
else
86-
val = json_string ("");
82+
/* job->name potentially NULL if jobspec invalid */
83+
if (!job->name)
84+
return 0;
85+
val = json_string (job->name);
8786
}
8887
else if (!strcmp (attr, "ntasks")) {
88+
/* job->ntasks potentially < 0 if jobspec invalid */
89+
if (job->ntasks < 0)
90+
return 0;
8991
val = json_integer (job->ntasks);
9092
}
9193
else if (!strcmp (attr, "nnodes")) {
92-
if (!(job->states_mask & FLUX_JOB_STATE_RUN))
94+
/* job->nnodes potentially < 0 if R invalid */
95+
if (!(job->states_mask & FLUX_JOB_STATE_RUN)
96+
|| job->nnodes < 0)
9397
return 0;
9498
val = json_integer (job->nnodes);
9599
}
96100
else if (!strcmp (attr, "ranks")) {
97-
if (!(job->states_mask & FLUX_JOB_STATE_RUN))
101+
/* job->ranks potentially NULL if R invalid */
102+
if (!(job->states_mask & FLUX_JOB_STATE_RUN)
103+
|| !job->ranks)
98104
return 0;
99-
/* potentially NULL if R invalid */
100-
if (job->ranks)
101-
val = json_string (job->ranks);
102-
else
103-
val = json_string ("");
105+
val = json_string (job->ranks);
104106
}
105107
else if (!strcmp (attr, "nodelist")) {
106-
if (!(job->states_mask & FLUX_JOB_STATE_RUN))
108+
/* job->nodelist potentially NULL if R invalid */
109+
if (!(job->states_mask & FLUX_JOB_STATE_RUN)
110+
|| !job->nodelist)
107111
return 0;
108-
/* potentially NULL if R invalid */
109-
if (job->nodelist)
110-
val = json_string (job->nodelist);
111-
else
112-
val = json_string ("");
112+
val = json_string (job->nodelist);
113113
}
114114
else if (!strcmp (attr, "expiration")) {
115-
if (!(job->states_mask & FLUX_JOB_STATE_RUN))
115+
/* job->expiration potentially < 0 if R invalid */
116+
if (!(job->states_mask & FLUX_JOB_STATE_RUN)
117+
|| job->expiration < 0)
116118
return 0;
117119
val = json_real (job->expiration);
118120
}

t/t2260-job-list.t

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,6 +1162,7 @@ test_expect_success HAVE_JQ 'create illegal jobspec with empty command array' '
11621162
cat hostname.json | $jq ".tasks[0].command = []" > bad_jobspec.json
11631163
'
11641164

1165+
# note that ntasks will not be set if jobspec invalid
11651166
test_expect_success HAVE_JQ 'flux job list works on job with illegal jobspec' '
11661167
jobid=`flux job submit bad_jobspec.json | flux job id` &&
11671168
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' '
11741175
done &&
11751176
test "$i" -lt "5" &&
11761177
flux job list --states=inactive | grep $jobid > list_illegal_jobspec.out &&
1177-
cat list_illegal_jobspec.out | $jq -e ".name == \"\"" &&
1178-
cat list_illegal_jobspec.out | $jq -e ".ntasks == 0"
1178+
cat list_illegal_jobspec.out | $jq -e ".name == null" &&
1179+
cat list_illegal_jobspec.out | $jq -e ".ntasks == null"
11791180
'
11801181

11811182
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' '
12111212
done &&
12121213
test "$i" -lt "5" &&
12131214
flux job list --states=inactive | grep $jobid > list_illegal_R.out &&
1214-
cat list_illegal_R.out | $jq -e ".ranks == \"\"" &&
1215-
cat list_illegal_R.out | $jq -e ".nnodes == 0" &&
1216-
cat list_illegal_R.out | $jq -e ".nodelist == \"\""
1215+
cat list_illegal_R.out | $jq -e ".ranks == null" &&
1216+
cat list_illegal_R.out | $jq -e ".nnodes == null" &&
1217+
cat list_illegal_R.out | $jq -e ".nodelist == null" &&
1218+
cat list_illegal_R.out | $jq -e ".expiration == null"
12171219
'
12181220

12191221
test_expect_success HAVE_JQ,NO_CHAIN_LINT 'flux job list-ids works on job with illegal R' '

t/t2800-jobs-cmd.t

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,8 +1012,7 @@ test_expect_success HAVE_JQ 'create illegal jobspec with empty command array' '
10121012
'
10131013

10141014
# to avoid potential racyness, wait up to 5 seconds for job to appear
1015-
# in job list. note that how a jobspec is illegal can affect what
1016-
# values below are set vs not set.
1015+
# in job list. note that ntasks will not be set if jobspec invalid
10171016
test_expect_success HAVE_JQ 'flux jobs works on job with illegal jobspec' '
10181017
jobid=`flux job submit bad_jobspec.json` &&
10191018
fj_wait_event $jobid clean &&
@@ -1026,7 +1025,7 @@ test_expect_success HAVE_JQ 'flux jobs works on job with illegal jobspec' '
10261025
done &&
10271026
test "$i" -lt "5" &&
10281027
flux jobs -no "{name},{ntasks}" $jobid > list_illegal_jobspec.out &&
1029-
echo ",0" > list_illegal_jobspec.exp &&
1028+
echo "," > list_illegal_jobspec.exp &&
10301029
test_cmp list_illegal_jobspec.out list_illegal_jobspec.exp
10311030
'
10321031

@@ -1050,8 +1049,9 @@ test_expect_success HAVE_JQ 'flux jobs works on job with illegal R' '
10501049
i=$((i + 1))
10511050
done &&
10521051
test "$i" -lt "5" &&
1053-
flux jobs -no "{ranks},{nnodes},{nodelist}" $jobid > list_illegal_R.out &&
1054-
echo ",0," > list_illegal_R.exp &&
1052+
flux jobs -no "{ranks},{nnodes},{nodelist},{expiration}" $jobid \
1053+
> list_illegal_R.out &&
1054+
echo ",,,0.0" > list_illegal_R.exp &&
10551055
test_cmp list_illegal_R.out list_illegal_R.exp
10561056
'
10571057

0 commit comments

Comments
 (0)