Skip to content

Commit

Permalink
Merge pull request #5687 from chu11/job_list_flux_error_t
Browse files Browse the repository at this point in the history 
job-list: misc cleanup
  • Loading branch information
@mergify
mergify[bot] authored Jan 18, 2024
2 parents d00494b + 2c4faa8 commit 74bd805
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 53 deletions.
3 changes: 3 additions & 0 deletions src/bindings/python/flux/job/list.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ class JobList:
not empty.
:user: Username or userid for which to fetch jobs. Default is all users.
:max_entries: Maximum number of jobs to return
:since: Limit jobs to those that have been active since a given timestamp.
:name: Limit jobs to those with a specific name.
:queue: Limit jobs to those submitted to a specific queue.
"""

# pylint: disable=too-many-instance-attributes
Expand Down
2 changes: 1 addition & 1 deletion src/modules/job-list/idsync.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ static void idsync_data_respond (struct idsync_ctx *isctx,
struct idsync_data *isd,
struct job *job)
{
job_list_error_t err;
flux_error_t err;
json_t *o;

if (!(o = job_to_json (job, isd->attrs, &err)))
Expand Down
23 changes: 6 additions & 17 deletions src/modules/job-list/job_util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,16 @@
#include <assert.h>

#include "src/common/libutil/errno_safe.h"
#include "src/common/libutil/errprintf.h"
#include "ccan/str/str.h"

#include "job-list.h"
#include "job_util.h"

void seterror (job_list_error_t *errp, const char *fmt, ...)
{
if (errp) {
va_list ap;
int saved_errno = errno;
va_start (ap, fmt);
(void) vsnprintf (errp->text, sizeof (errp->text), fmt, ap);
va_end (ap);
errno = saved_errno;
}
}

static int store_attr (struct job *job,
const char *attr,
json_t *o,
job_list_error_t *errp)
flux_error_t *errp)
{
json_t *val = NULL;

Expand Down Expand Up @@ -219,7 +208,7 @@ static int store_attr (struct job *job,
val = json_incref (grudgeset_tojson (job->dependencies));
}
else {
seterror (errp, "%s is not a valid attribute", attr);
errprintf (errp, "%s is not a valid attribute", attr);
errno = EINVAL;
return -1;
}
Expand All @@ -237,7 +226,7 @@ static int store_attr (struct job *job,
return 0;
}

int store_all_attr (struct job *job, json_t *o, job_list_error_t *errp)
int store_all_attr (struct job *job, json_t *o, flux_error_t *errp)
{
const char **ptr = job_attrs ();

Expand All @@ -260,7 +249,7 @@ int store_all_attr (struct job *job, json_t *o, job_list_error_t *errp)
* EPROTO - malformed attrs array
* ENOMEM - out of memory
*/
json_t *job_to_json (struct job *job, json_t *attrs, job_list_error_t *errp)
json_t *job_to_json (struct job *job, json_t *attrs, flux_error_t *errp)
{
json_t *val = NULL;
size_t index;
Expand All @@ -280,7 +269,7 @@ json_t *job_to_json (struct job *job, json_t *attrs, job_list_error_t *errp)
json_array_foreach (attrs, index, value) {
const char *attr = json_string_value (value);
if (!attr) {
seterror (errp, "attr has no string value");
errprintf (errp, "attr has no string value");
errno = EINVAL;
goto error;
}
Expand Down
9 changes: 1 addition & 8 deletions src/modules/job-list/job_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,7 @@

#include "job_data.h"

typedef struct {
char text[160];
} job_list_error_t;

void __attribute__((format (printf, 2, 3)))
seterror (job_list_error_t *errp, const char *fmt, ...);

json_t *job_to_json (struct job *job, json_t *attrs, job_list_error_t *errp);
json_t *job_to_json (struct job *job, json_t *attrs, flux_error_t *errp);

#endif /* ! _FLUX_JOB_LIST_JOB_UTIL_H */

Expand Down
39 changes: 20 additions & 19 deletions src/modules/job-list/list.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <assert.h>

#include "src/common/libutil/errno_safe.h"
#include "src/common/libutil/errprintf.h"
#include "src/common/libczmqcontainers/czmq_containers.h"
#include "ccan/str/str.h"

Expand All @@ -30,7 +31,7 @@
#include "state_match.h"

json_t *get_job_by_id (struct job_state_ctx *jsctx,
job_list_error_t *errp,
flux_error_t *errp,
const flux_msg_t *msg,
flux_jobid_t id,
json_t *attrs,
Expand All @@ -44,7 +45,7 @@ json_t *get_job_by_id (struct job_state_ctx *jsctx,
* ENOMEM - out of memory
*/
int get_jobs_from_list (json_t *jobs,
job_list_error_t *errp,
flux_error_t *errp,
zlistx_t *list,
int max_entries,
json_t *attrs,
Expand Down Expand Up @@ -94,7 +95,7 @@ int get_jobs_from_list (json_t *jobs,
* ENOMEM - out of memory
*/
json_t *get_jobs (struct job_state_ctx *jsctx,
job_list_error_t *errp,
flux_error_t *errp,
int max_entries,
double since,
json_t *attrs,
Expand Down Expand Up @@ -250,7 +251,7 @@ void list_cb (flux_t *h, flux_msg_handler_t *mh,
const flux_msg_t *msg, void *arg)
{
struct list_ctx *ctx = arg;
job_list_error_t err;
flux_error_t err;
json_t *jobs;
json_t *attrs;
int max_entries;
Expand All @@ -266,7 +267,7 @@ void list_cb (flux_t *h, flux_msg_handler_t *mh,
"attrs", &attrs,
"since", &since,
"constraint", &constraint) < 0) {
seterror (&err, "invalid payload: %s", flux_msg_last_error (msg));
errprintf (&err, "invalid payload: %s", flux_msg_last_error (msg));
errno = EPROTO;
goto error;
}
Expand All @@ -291,31 +292,31 @@ void list_cb (flux_t *h, flux_msg_handler_t *mh,
}
}
if (max_entries < 0) {
seterror (&err, "invalid payload: max_entries < 0 not allowed");
errprintf (&err, "invalid payload: max_entries < 0 not allowed");
errno = EPROTO;
goto error;
}
if (since < 0.) {
seterror (&err, "invalid payload: since < 0.0 not allowed");
errprintf (&err, "invalid payload: since < 0.0 not allowed");
errno = EPROTO;
goto error;
}
if (!json_is_array (attrs)) {
seterror (&err, "invalid payload: attrs must be an array");
errprintf (&err, "invalid payload: attrs must be an array");
errno = EPROTO;
goto error;
}
if (!(c = list_constraint_create (constraint, &error))) {
seterror (&err,
"invalid payload: constraint object invalid: %s",
error.text);
errprintf (&err,
"invalid payload: constraint object invalid: %s",
error.text);
errno = EPROTO;
goto error;
}
if (!(statec = state_constraint_create (constraint, &error))) {
seterror (&err,
"invalid payload: constraint object invalid: %s",
error.text);
errprintf (&err,
"invalid payload: constraint object invalid: %s",
error.text);
errno = EPROTO;
goto error;
}
Expand Down Expand Up @@ -422,7 +423,7 @@ int check_id_valid (struct job_state_ctx *jsctx,
* ENOMEM - out of memory
*/
json_t *get_job_by_id (struct job_state_ctx *jsctx,
job_list_error_t *errp,
flux_error_t *errp,
const flux_msg_t *msg,
flux_jobid_t id,
json_t *attrs,
Expand Down Expand Up @@ -473,7 +474,7 @@ void list_id_cb (flux_t *h, flux_msg_handler_t *mh,
const flux_msg_t *msg, void *arg)
{
struct list_ctx *ctx = arg;
job_list_error_t err = {{0}};
flux_error_t err = {{0}};
json_t *job;
flux_jobid_t id;
json_t *attrs;
Expand All @@ -485,19 +486,19 @@ void list_id_cb (flux_t *h, flux_msg_handler_t *mh,
"id", &id,
"attrs", &attrs,
"state", &state) < 0) {
seterror (&err, "invalid payload: %s", flux_msg_last_error (msg));
errprintf (&err, "invalid payload: %s", flux_msg_last_error (msg));
errno = EPROTO;
goto error;
}

if (!json_is_array (attrs)) {
seterror (&err, "invalid payload: attrs must be an array");
errprintf (&err, "invalid payload: attrs must be an array");
errno = EPROTO;
goto error;
}

if (state && (state & ~valid_states)) {
seterror (&err, "invalid payload: invalid state specified");
errprintf (&err, "invalid payload: invalid state specified");
errno = EPROTO;
goto error;
}
Expand Down
70 changes: 62 additions & 8 deletions src/modules/job-list/test/match.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,16 @@ static void test_basic_userid (void)
while (tests->userid) {
struct job *job;
bool rv;
job = setup_job (tests->userid, NULL, NULL, 0, 0, 0.0, 0.0, 0.0, 0.0, 0.0);
job = setup_job (tests->userid,
NULL,
NULL,
0,
0,
0.0,
0.0,
0.0,
0.0,
0.0);
rv = job_match (job, c);
ok (rv == tests->expected,
"basic userid job match test #%d/#%d",
Expand Down Expand Up @@ -308,7 +317,16 @@ static void test_basic_name (void)
while (!tests->end) {
struct job *job;
bool rv;
job = setup_job (0, tests->name, NULL, 0, 0, 0.0, 0.0, 0.0, 0.0, 0.0);
job = setup_job (0,
tests->name,
NULL,
0,
0,
0.0,
0.0,
0.0,
0.0,
0.0);
rv = job_match (job, c);
ok (rv == tests->expected,
"basic name job match test #%d/#%d",
Expand Down Expand Up @@ -385,7 +403,16 @@ static void test_basic_queue (void)
while (!tests->end) {
struct job *job;
bool rv;
job = setup_job (0, NULL, tests->queue, 0, 0, 0.0, 0.0, 0.0, 0.0, 0.0);
job = setup_job (0,
NULL,
tests->queue,
0,
0,
0.0,
0.0,
0.0,
0.0,
0.0);
rv = job_match (job, c);
ok (rv == tests->expected,
"basic queue job match test #%d/#%d",
Expand Down Expand Up @@ -466,7 +493,16 @@ static void test_basic_states (void)
while (tests->state) {
struct job *job;
bool rv;
job = setup_job (0, NULL, NULL, tests->state, 0, 0.0, 0.0, 0.0, 0.0, 0.0);
job = setup_job (0,
NULL,
NULL,
tests->state,
0,
0.0,
0.0,
0.0,
0.0,
0.0);
rv = job_match (job, c);
ok (rv == tests->expected,
"basic states job match test #%d/#%d",
Expand Down Expand Up @@ -550,7 +586,16 @@ static void test_basic_results (void)
while (tests->state) { /* result can be 0, iterate on state > 0 */
struct job *job;
bool rv;
job = setup_job (0, NULL, NULL, tests->state, tests->result, 0.0, 0.0, 0.0, 0.0, 0.0);
job = setup_job (0,
NULL,
NULL,
tests->state,
tests->result,
0.0,
0.0,
0.0,
0.0,
0.0);
rv = job_match (job, c);
ok (rv == tests->expected,
"basic results job match test #%d/#%d",
Expand Down Expand Up @@ -1046,7 +1091,16 @@ static void test_basic_conditionals (void)
while (tests->userid) {
struct job *job;
bool rv;
job = setup_job (tests->userid, tests->name, NULL, 0, 0, 0.0, 0.0, 0.0, 0.0, 0.0);
job = setup_job (tests->userid,
tests->name,
NULL,
0,
0,
0.0,
0.0,
0.0,
0.0,
0.0);
rv = job_match (job, c);
ok (rv == tests->expected,
"basic conditionals job match test #%d/#%d",
Expand Down Expand Up @@ -1373,7 +1427,7 @@ struct realworld_constraint_test {
"batch",
FLUX_JOB_STATE_SCHED,
0,
0,
0.0,
false,
},
{
Expand All @@ -1382,7 +1436,7 @@ struct realworld_constraint_test {
"batch",
FLUX_JOB_STATE_RUN,
0,
0,
0.0,
false,
},
{
Expand Down
30 changes: 30 additions & 0 deletions src/modules/job-list/test/state_match.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,36 @@ struct state_match_constraint_test {
true,
},
},
/* cover every constraint operator
* - every test here should fail as we AND several impossible things
*/
{
"{ \"and\": \
[ \
{ \"userid\": [ 42 ] }, \
{ \"name\": [ \"foo\" ] }, \
{ \"queue\": [ \"foo\" ] }, \
{ \"states\": [ \"running\" ] }, \
{ \"results\": [ \"completed\" ] }, \
{ \"t_submit\": [ \">=500.0\" ] }, \
{ \"t_depend\": [ \">=100.0\" ] }, \
{ \"t_run\": [ \"<=100.0\" ] }, \
{ \"t_cleanup\": [ \">=100.0\" ] }, \
{ \"t_inactive\": [ \"<=100.0\" ] } \
] \
}",
{
false,
false,
false,
false,
false,
false,
false,
false,
false,
},
},
{
NULL,
{
Expand Down

0 comments on commit 74bd805

Please sign in to comment.