plugin: add project validation and annotation #294

cmoussa1 · 2022-10-21T20:38:21Z

Background

Mentioned in #293, there is a requirement to be able to associate jobs with "projects," which allow multiple users who belong in different banks to submit jobs under one entity. Currently, the flux-accounting database supports adding projects and adding associations to certain projects, but this information is not relayed to the multi-factor priority plugin, so there is no way to associate a job with a project.

This PR adds project support in the priority plugin. The two big components that make up this PR are 1) providing some validation for projects if one is specified during job.validate, and 2) updating the jobspec with the project name in the PRIORITY event of a job.

Projects (along with the default project) for each association are now sent via flux account-priority-update to the plugin's internal map, where they are unpacked and stored in each association's bank_info object. The project information for each association is also added to the information returned in the plugin.query callback.

When an association specifies a project during job submission with --setattr=system.project=my_project, the project name is checked to both exist and be a valid project for the user to submit jobs under. Failure in either of these checks results in a job rejection with an appropriate message as to why the job was rejected.

If a user/bank submits a job under a default project, a jobspec-update event is posted under attributes.system.project in the PRIORITY event to add the user/bank's default project. The project name can then be found in the job's eventlog:

{"timestamp": 1666106606.9670131,"name":"jobspec-update","context":{"attributes.system.project":"my_default_project"}}

cmoussa1 · 2022-12-06T20:53:53Z

So that was odd! My python-lint check was failing with an error message saying it could not locate Python 3.6. From some quick and rough Google searches it looks like it might have something to do with the latest version of Ubuntu being used (I was using ubuntu-latest): actions/setup-python#162. I updated the version number to match flux-core's Ubuntu version for the python-lint check, and now it works as excepted, so I think I'll move the most recent commit on this PR to another branch and post a separate, small fix PR.

garlick · 2023-05-15T16:23:19Z

Just skimming a bit, but should the jobspec-update event only be posted when the project is not present in the jobspec already?

cmoussa1 · 2023-05-15T20:11:26Z

@garlick, ah, you're absolutely right; thanks for catching that! If a user specifies a project on the command line the project will be present in the jobspec already, so there won't be a need to add it manually via jobspec-update. It should only be added if a user doesn't specify one. Perhaps that can be done in priority_cb () instead of in validate_cb (). I'm about to push up some changes to this PR to only add a project via jobspec-update only if if a user doesn't specify one on the command line.

cmoussa1 · 2023-05-16T15:53:07Z

OK, I've pushed up some changes to this PR to only post a jobspec-update if a user/bank submits a job under a default project (thanks for pointing that out yesterday!). With that, I'll open it up to some initial review whenever convenient. 😄

cmoussa1 · 2023-07-25T21:12:07Z

@grondo if you have some time later this week or next, could you give this PR a look?

grondo

@cmoussa1 - I made a first pass at this PR. Sorry it took so long!

A few comments inline. I also had some general questions:

Is the use of projects now a requirement? What happens at a site where no projects are configured at all
Do we need to add attributes.system.project to the official list of system attributes in RFC 14?
Does job-list need to support project as a job attribute it can return (and so job-archive can store this attribute)

Sorry if all this has been discussed before, I may need a refresher on the use case for job "project" attribute!

grondo · 2023-08-08T21:18:55Z

src/cmd/flux-account-priority-update.py

+
+    data = {"data": bulk_p_data}
+
+    flux.Flux().rpc("job-manager.mf_priority.rec_p_update", json.dumps(data)).get()


Just as an FYI - the Python RPC class will take care of encoding a non-string payload with json.dumps() if the argument is not string, so the json.dumps(data) is unnecessary here. It doesn't cause a problem though.

Ah, I did not know this, thanks for pointing this out! There are other parts in this script that also wrap data with json.dumps(). I should open a separate PR later that drops it.

Yeah, it doesn't cause a problem to call json.dumps() first, but it would probably be good to clean up in the future.

grondo · 2023-08-08T21:28:03Z

src/plugins/mf_priority.cpp

@@ -168,15 +168,24 @@ static int get_queue_info (
 }


-static void split_string (char *queues, struct bank_info *b)
+static void split_string (char flag, char *list, struct bank_info *b)


I think it might make more sense for this function to take two args, a string to split on ,, and the vector onto which to push the results, rather than passing the whole struct bank_info * and using a flag to determine which member will receive the new values.

You might then rename the function split_string_and_push_back()

This way, in the future if there is another list on which you want to use this function, it won't require any changes.
E.g. the new calling pattern would be

split_string_and_push_back (projects, &b->projects);

(actually I may have gotten the C++ wrong, but you get the idea)

Also, callers should be updated in the same commit if possible to avoid a commit that breaks the build (This is useful if anyone ever needs to use git bisect on the codebase)

I think I see what you're saying! Adjusting the parameters of the function to just take a string to split on , and the vector to push the results cleans the code up quite considerably (no having to pass funky single-character flags); instead it can just look like this:

split_string_and_push_back (projects, &b->projects);

and the function only needs one while loop. Thanks for the suggestion!

I can make this change as well as add the callers of this function to the same commit (sorry about that!!).

grondo · 2023-08-08T21:29:37Z

src/plugins/mf_priority.cpp


        // split queues comma-delimited string and add it to b->queues vector
        b->queues.clear ();
-        split_string (queues, b);
+        split_string ('q', queues, b);


As noted before, this change should have gone with the previous commit.

grondo · 2023-08-08T21:31:39Z

src/plugins/mf_priority.cpp

+    if (flux_respond (h, msg, NULL) < 0)
+        flux_log_error (h, "flux_respond");


The "success" response should be delayed until the processing is successful. That is, I'd move this to right before the return

grondo · 2023-08-08T21:32:39Z

src/plugins/mf_priority.cpp

+
+        if (json_unpack_ex (el, &error, 0, "{s:s}",
+                            "project", &project) < 0)
+            flux_log (h, LOG_ERR, "mf_priority unpack: %s", error.text);


Should there be a goto error; on this failure? Or is the intent to just ignore errors unpacking individual projects?

Oh, nope, that's my mistake - it should probably respond with an error if it fails to unpack any projects sent by the flux-accounting DB because that could affect job validation with a specified project, so there should be a goto error; here. I will fix this!

grondo · 2023-08-08T21:34:04Z

src/plugins/mf_priority.cpp

@@ -1046,7 +1104,8 @@ extern "C" int flux_plugin_init (flux_plugin_t *p)
    if (flux_plugin_register (p, "mf_priority", tab) < 0
        || flux_jobtap_service_register (p, "rec_update", rec_update_cb, p)
        || flux_jobtap_service_register (p, "reprioritize", reprior_cb, p)
-        || flux_jobtap_service_register (p, "rec_q_update", rec_q_cb, p) < 0)
+        || flux_jobtap_service_register (p, "rec_q_update", rec_q_cb, p)
+        || flux_jobtap_service_register (p, "rec_p_update", rec_p_cb, p) < 0)


I know you already have req_q_update but my old eyes are finding it very difficult to see the difference between the p and q here, so I was just wondering if perhaps the service should be rec_project_update?

Yeah absolutely, sorry for the confusion! I was trying to stay under the 80 character limit here but it can be hard for me to look at too, so I should just break it up 👍 will fix this

Ah! I can sympathize with that! However, no problem breaking the function across multiple lines!

grondo · 2023-08-08T21:37:00Z

t/t1015-mf-priority-urgency.t

-	flux.Flux().rpc("job-manager.mf_priority.rec_update", json.dumps(bulk_user_data)).get()
 	flux.Flux().rpc("job-manager.mf_priority.rec_q_update", json.dumps(bulk_queue_data)).get()
 	EOF
-'
-
-test_expect_success 'send the user information to the plugin' '


These removed lines look suspiciously like an accident, since this commit is mainly about moving test JSON input to separate files. Just a note to double check that you didn't accidentally remove a test and/or test RPC.

Thanks for checking. This should be fine, as the JSON payload that stored all of the user information was moved to a separate file, but I think I made it more confusing because in this commit, I re-ordered the test that sent the user information to the plugin to before sending the queue information. Not sure why I did that. 🤦

I think if I just leave that test that sends user information in the same spot, it should fix the commit from looking like I removed a couple tests. I will fix this!

Ah, ok, thanks for the clarification. I figured everything was good since tests were passing, but just thought I'd check!

grondo · 2023-08-08T21:48:21Z

src/plugins/mf_priority.cpp

+    // add project name to jobspec
+    json_t *project_name = json_object ();
+    if (!project_name)
+        return -1;
+
+    if (json_object_set_new (project_name,
+                             "attributes.system.project",
+                             json_string (project)) < 0) {
+        json_decref (project_name);
+        return -1;
+    }
+
+    // post jobspec-update event
+    if (flux_jobtap_event_post_pack (p, FLUX_JOBTAP_CURRENT_JOB,
+                                         "jobspec-update", "O",
+                                         project_name) < 0) {
+        json_decref (project_name);
+        return -1;
+    }


I'd suggest just using flux_jobtap_event_post_pack() to pack the project name instead of separately constructing the object, e.g.

// post jobspec-update event if (flux_jobtap_event_post_pack (p, FLUX_JOBTAP_CURRENT_JOB, "jobspec-update", "{s:{s:{s:s}}}", "attributes", "system", "project", project) < 0) { return -1; }

Then you do not have to worry about the memory for the temporary JSON object.

Oh neat! I didn't know you could do this; this cleans up the function a lot. Thanks for the suggestion!!

@cmoussa1: D'oh! I gave bad advice here by not paying attention and forgetting how the jobpsec-update event works. Sorry! You can ignore this comment. My suggestion should have been:

// post jobspec-update event if (flux_jobtap_event_post_pack (p, FLUX_JOBTAP_CURRENT_JOB, "jobspec-update", "{s:s}", "attributes.system.project", project) < 0) { return -1; }

I apologize for the confusion 🤦

omg! i think this worked, and it solved the issue of having to place the call to jobspec-update in a place like job.state.run and instead it can go in job.validate where it would make more sense. This feels like a eureka moment! 😆 Thanks for catching this and don't worry about it at all, I'm glad it worked 🙌

I'll make the appropriate changes to the commit and force push in a bit.

grondo · 2023-08-08T22:09:18Z

src/plugins/mf_priority.cpp

+            if (project == NULL) {
+                project = const_cast <char *>
+                    (bank_it->second.def_project.c_str ());
+                if (update_jobspec_project (p, userid, bank, project) < 0) {
+                    flux_jobtap_raise_exception (p, FLUX_JOBTAP_CURRENT_JOB,
+                                                "mf_priority", 0,
+                                                "failed to update jobspec "
+                                                " with project name");
+
+                    return -1;
+                }
+            }
+        }
+    }
+
+    // if using a default project, add it to jobspec
+    if (project == NULL) {
+        if (update_jobspec_project (p, userid, bank, project) < 0) {
+            flux_jobtap_raise_exception (p, FLUX_JOBTAP_CURRENT_JOB,
+                                         "mf_priority", 0, "failed to update "
+                                         "jobspec with project name");


is there any way to combine this duplicated code? I might need help understanding why update_jobspec_project() needs to be called two different places like this here, sorry!

Ah, you're so right! Thanks for pointing this out. I don't know why I have this code duplicated; it doesn't need to be. I think I must have thought that if a job is submitted and held in PRIORITY because a user is waiting for their information to be added to the plugin, it required a special case in priority_cb () to add the project name, but that's not the case. Sorry for the confusion!

I'll remove this duplicate code and add a test which shows that the project name gets updated in the case where a user's job is held in PRIORITY as the plugin waits for (and then receives) information from the flux-accounting DB.

grondo · 2023-08-08T22:42:35Z

Perhaps that can be done in priority_cb () instead of in validate_cb (). I'm about to push up some changes to this PR to only add a project via jobspec-update only if if a user doesn't specify one on the command line.

A job could enter the PRIORITY state multiple times, so would this mean the jobspec-update could unnecessarily occur multiple times? Also, if you delay the jobspec-update until PRIORITY, then the job would be without an assigned project until it passes the dependency state, which might be unexpected.

We may want to think about how to issue the jobspec-update event during job.validate as it was before, perhaps after the job has been fully validated?

grondo · 2023-08-08T22:45:00Z

Hm, I don't why the pr-validator thinks all your commits have empty bodies, but I'll look into it and get that action fixed.

cmoussa1 · 2023-09-12T17:09:36Z

Yes, the first set of tests in t1028-mf-priority-projects.t should demonstrate this case:

test_expect_success 'submit a job under a user before plugin gets updated' '
	job0=$(flux python ${SUBMIT_AS} 1003 hostname) &&
	test $(flux jobs -no {state} ${job0}) = PRIORITY
'

test_expect_success 'add user to flux-accounting DB and to plugin; job transitions to RUN' '
	flux account -p ${DB_PATH} add-user --username=user1003 --userid=1003 --bank=account1 &&
	flux account-priority-update -p ${DB_PATH} &&
	test $(flux jobs -no {state} ${job0}) = RUN
'

test_expect_success 'check that project gets updated for submitted job' '
	flux job info $job0 eventlog > eventlog.out &&
	grep "{\"attributes.system.project\":\"\*\"}" eventlog.out &&
	flux job cancel $job0
'

In the above, a job gets submitted before any data is loaded to the plugin. The user is then added to the flux-accounting DB and the DB info is sent to the plugin, the job transitions to RUN, and the project name is checked in the job's main eventlog to look for the default project name.

grondo · 2023-09-14T23:37:40Z

t/t1028-mf-priority-projects.t

+test_expect_success 'submit a job under a project that does not exist' '
+	test_must_fail flux python ${SUBMIT_AS} 1001 --setattr=system.project=projectFOO \
+		hostname > project_dne.out 2>&1 &&
+	test_debug "project_dne.out" &&


Just noticed this while trying to run this test with -d -v -- test_debug() takes a command to run, not a file. Since there's no command project_dne.out, this line causes a failure. Change to

test_debug "cat project_dne.out" &&

here and in the other use of test_debug in this sharness test. (Also perhaps audit the rest of the testsuite)

grondo · 2023-09-14T23:54:25Z

t/t1028-mf-priority-projects.t

+	flux account-priority-update -p ${DB_PATH} &&
+	test $(flux jobs -no {state} ${job0}) = RUN


There's a slight race condition here (and actually above in the check for PRIORITY) because of the eventual consistency properties of job-list. It might be safer to check the eventlog and ensure the expected event has been posted.

E.g. instead of checking that the job is in the PRIORITY state above, check that the last event in the eventlog is depend.

and instead of checking for the RUN state here, wait for an alloc event...

grondo · 2023-09-15T00:07:47Z

The user is then added to the flux-accounting DB and the DB info is sent to the plugin, the job transitions to RUN, and the project name is checked in the job's main eventlog to look for the default project name.

Sorry, still getting clarification here!

In this test though the project name of * (is this an empty project name or something?) is added immediately when the job is submitted. When the accounting database is updated, the user still has no project information, so when the database info is sent to the priority plugin it doesn't need to update anything. What about users that do have a default project, doesn't the plugin have to assign these default projects to pending jobs in PRIOIRITY after the first update?

I apologize if I'm still not getting it and I appreciate your patience!

cmoussa1 · 2023-09-22T20:48:26Z

No need to apologize, I think I am doing a poor job explaining the requirements here, so I'm sorry! Here is the requirement for how each job needs to be associated with a project, taken from conversation with @ryanday36 in #255 (feel free to correct me I am misunderstanding this Ryan):

If a user is added to the DB without specifying any projects, a default project * is added for the user automatically, and jobs submitted without specifying a project will fall under *; every user who is added to the DB belongs to the * project, but will only run jobs under * if they do not already have another default project.

If a user is added to the DB with a specified project name, any job submitted without specifying a project name will fall under that project name.

Does that explanation make sense? So, I believe you are absolutely right, in the event where a user does have a default project, the plugin has to assign this project to pending jobs while the plugin waits for flux-accounting information. I was able to confirm that this was not yet supported with the most current push of this branch by submitting a job under a user/bank before the plugin gets any data, adding the user with a specific default project, updating the plugin, and seeing that the project name for the user/bank does not get updated to the correct default project.

To fix this, I can place a check in job.state.priority when the information for the job is being updated to reflect the loaded flux-accounting information to update the project name to the user/bank's default if it is not *.

I also wrote a set of tests that submits two jobs from two different users before the plugin gets any information (so both jobs are pending). One user gets added with no projects specified, and the other user gets added with one project specified (which becomes their default). Once the plugin gets loaded with the flux-accounting DB information, the jobs get updated. The job from the user/bank with no projects specified keeps its project as *, but the job from the user/bank with a specified default project gets updated to use the newly found default project name. I can add those tests to the sharness test file that's created in this PR as well as include a comment describing how the project assignment works in either case.

Let me know if this explanation does or does not make sense. I'll push up these changes (as well as the changes based on your feedback above) shortly.

grondo · 2023-09-22T21:27:28Z

Ah, ok. I guess my naive question would be why not just not set a project when no project is set instead of using *?

cmoussa1 · 2023-09-22T21:28:45Z

If I am understanding your question correctly, * is the name of the project that gets set when no project is set.

grondo · 2023-09-22T21:30:09Z

Yeah, sorry I meant just don't set attributes.system.project when no project is set. This might reduce the number of necessary jobspec-update events.

cmoussa1 · 2023-09-22T21:32:24Z

Oh, whoops. I misunderstood your question, sorry. My understanding is that we want a default whether a user/bank has a default project or not. Here is a comment from @ryanday36 in #255 when projects were first being added to the flux-accounting DB:

No, we want both kinds of defaults :) If no project is specified when the user is created, giving them '*' for their project and default project is what we want.

grondo · 2023-09-22T21:35:27Z

Ok, thanks! I didn't read that issue carefully enough! 🤦

grondo

Just a couple other comments I had while looking at this again. Not a full review.

grondo · 2023-09-22T21:49:08Z

src/plugins/mf_priority.cpp

    while (s_stream.good ()) {
        std::string substr;
        getline (s_stream, substr, ','); // get string delimited by comma
-        b->queues.push_back (substr);
+            vec->push_back (substr);


Is there a stray indent here?

grondo · 2023-09-22T21:53:29Z

src/plugins/mf_priority.cpp

+    b->projects = { "*" };
+    b->def_project = "*";


When the flux-accounting info is missing, shouldn't the project be treated like the bank info, i.e. be somehow recorded as unknown? That is, if the bank info is missing, we don't know if the default project should be "*" since the real default should be set in the accounting database. In fact, should the default project be something encoded in the database and left our of the plugin itself (that way other sites could make their own decision what a default project should be, or if there should even be a default)?

That's a great question. I can change this assignment to be set to indicate that the project for the user/bank's job is unknown until it gets its information - just re-ran the tests after changing these values to something like "DNE", which is the same as the bank information, and I think everything still works as expected. 👍

In the user table in the flux-accounting DB, the default project for any user/bank when being added is * (if one is not specified at user creation time; the default project can be changed later if desired):

""" projects tinytext DEFAULT '*' NOT NULL ON CONFLICT REPLACE DEFAULT '*', default_project tinytext DEFAULT '*' NOT NULL ON CONFLICT REPLACE DEFAULT '*', """

and * is added as a valid project name to the DB when the DB is first created:

conn.execute( """ CREATE TABLE IF NOT EXISTS project_table ( project_id integer PRIMARY KEY AUTOINCREMENT, project tinytext NOT NULL, usage real DEFAULT 0.0 NOT NULL );""" ) conn.execute("INSERT INTO project_table (project) VALUES ('*')")

Since this is the case, maybe we don't even need to assign values for project and def_project here? Removing these assignments should also be fine (at least, the the testsuite passes). What do you think? Sorry if I am misunderstanding you.

Yes, that would look more correct to me. If the default is already set in the database, then the code should use that default and not a copy, since that might be redundant and also could cause the values to diverge if someone updates the database and forget to do the same in the plugin. This is why it is usually bad practice to copy a default value into more than one place.

Of course, I'm not all that familiar with the code here, so maybe it made more sense in this situation, so I was just posing the question.

I think you are right and it was a good question to ask, so I'll go ahead and make this change here. I'll remove the assignment of project and default_project. 👍

just pushed up a change (really just dropped this commit) that added this assignment of a project/default project in the case where a user/bank submits a job before any information is loaded to the plugin.

Problem: The split_string () helper function can only accept a list of queues to parse when unpacking user/bank information from the flux-accounting DB, but there also exists a need to be able to parse a list of projects as well. Since both lists can be parsed the same way, this helper function should be expanded. Expand the split_string () helper function to accept either a list of queues or a list of proects to parse through when unpacking user/bank information.

Add an unpack of the "projects" and "def_project" columns for each association when unpacking them in the rec_update_cb () function. Add a new callback to unpack information from the project_table and store it in a vector of projects to be used for project validation and annotation.

Add information from the project_table and the projects column in the association_table to the bulk update script that sends user, bank, and queue information to the multi-factor priority plugin.

Add "active", "projects", and "def_project" key-value pairs to the fake payloads sent to the multi-factor priority plugin in a number of the sharness tests.

Problem: The sample user payloads sent to the plugin grow longer than wanted in the sharness test files with the addition of the "project" and "default_project" fields. For the tests that have these sample payloads, create a new folder in the t/ directory that stores these JSON payloads permanently in a file instead of creating them within the test.

Add project validation to the validate_cb () function, which checks that if a project is specified by an association when a job is submitted, it: 1) is a valid project to submit jobs under, and 2) is a valid project for the association to submit jobs under. If either one of these checks fail, the job is rejected with a message including the project name.

Add a helper function to mf_priority.cpp that adds the project name for a user/bank's job to the jobspec via jobspec-update under attributes.system.project.

Problem: If a user submits a job under a default project, there is no way to tell which project they submitted their job under because it does not get posted to jobspec. Add a jobspec-update of the user/bank's default project name when the job is in the job.new callback. In the event where a user/bank submits a job before any information is loaded to the priority plugin and their job is held in PRIORITY state, add a jobspec-update of the user/bank's default project name in job.state.priority IF their default project name is not "*" ("*" is the default project name if a user is added to the database with no projects).

Add the projects and def_project fields to the list of information returned for each association in the plugin.query callback. Add the projects and def_project key-value pairs to the expected files for t1019-mf-priority-info-fetch.t.

Problem: There are no tests for project validation and annotation in the multi-factor priority plugin. Add some tests.

Problem: Now that the priority jobtap plugin supports jobspec-updates of both banks and projects, the "jobspec-update" event in the eventlog holds both of these updates on one line, which breaks the assumption in t1029-mf-priority-default-bank.t that the jobspec-update of a user's default bank will be surrounded by brackets {}. Remove the outer brackets from the tests in t1029-mf-priority-default-bank.t that grep for a certain bank.

codecov · 2023-09-26T18:48:18Z

Codecov Report

Merging #294 (036d549) into master (5be0ffb) will decrease coverage by 0.19%.
The diff coverage is 75.82%.

❗ Current head 036d549 differs from pull request most recent head 09063e0. Consider uploading reports for the commit 09063e0 to get more accurate results

@@            Coverage Diff             @@
##           master     #294      +/-   ##
==========================================
- Coverage   81.04%   80.85%   -0.19%     
==========================================
  Files          20       20              
  Lines        1419     1499      +80     
==========================================
+ Hits         1150     1212      +62     
- Misses        269      287      +18

Files	Coverage Δ
src/plugins/mf_priority.cpp	`78.57% <75.82%> (-0.19%)`	⬇️

cmoussa1 · 2024-01-17T17:15:37Z

I've been able to make some decent progress on adding an external class for user/bank (i.e accounting) information that I think would really clean up this PR, so I think I'll just close this and re-propose something else at a later date.

cmoussa1 added the new feature new feature label Oct 21, 2022

cmoussa1 force-pushed the add.projects.to.plugin branch from 1e11f56 to 6903551 Compare October 27, 2022 14:59

cmoussa1 force-pushed the add.projects.to.plugin branch 3 times, most recently from 1bef807 to 46c3125 Compare December 6, 2022 20:40

cmoussa1 force-pushed the add.projects.to.plugin branch from d940727 to 31b1e1a Compare December 7, 2022 16:04

This was referenced Dec 8, 2022

idea: record bank name in priority event #272

Closed

plugin: record bank name to jobspec when user submits under default bank #301

Merged

cmoussa1 force-pushed the add.projects.to.plugin branch from 31b1e1a to 6ce8502 Compare April 11, 2023 17:04

cmoussa1 changed the title ~~[WIP] plugin: add project validation and annotation~~ plugin: add project validation and annotation Apr 11, 2023

cmoussa1 marked this pull request as ready for review April 11, 2023 17:08

cmoussa1 force-pushed the add.projects.to.plugin branch from 6ce8502 to 9062a94 Compare May 10, 2023 19:18

This was referenced May 12, 2023

flux-accounting: job usage reporting by project #350

Closed

usage: collate user/project usage for reporting #352

Closed

cmoussa1 force-pushed the add.projects.to.plugin branch from 9062a94 to 385a8ad Compare May 15, 2023 16:01

cmoussa1 force-pushed the add.projects.to.plugin branch 2 times, most recently from 7160e0b to 437fe00 Compare May 16, 2023 15:43

cmoussa1 requested a review from grondo May 16, 2023 15:53

cmoussa1 force-pushed the add.projects.to.plugin branch 2 times, most recently from 6a62eee to f42f3fd Compare July 25, 2023 16:27

cmoussa1 force-pushed the add.projects.to.plugin branch from f42f3fd to 3670a22 Compare August 8, 2023 21:26

grondo reviewed Aug 8, 2023

View reviewed changes

grondo reviewed Sep 14, 2023

View reviewed changes

cmoussa1 mentioned this pull request Sep 18, 2023

testsuite: fix tests that look at job state #390

Closed

cmoussa1 force-pushed the add.projects.to.plugin branch from 35cef73 to 2ba8c04 Compare September 22, 2023 21:21

grondo reviewed Sep 22, 2023

View reviewed changes

cmoussa1 added 6 commits September 25, 2023 09:35

priority-update: add projects info to update

c19bee2

Add information from the project_table and the projects column in the association_table to the bulk update script that sends user, bank, and queue information to the multi-factor priority plugin.

t: add active, projects, def_project cols

a14fdd1

Add "active", "projects", and "def_project" key-value pairs to the fake payloads sent to the multi-factor priority plugin in a number of the sharness tests.

cmoussa1 force-pushed the add.projects.to.plugin branch from 2ba8c04 to 7d01df6 Compare September 25, 2023 19:46

cmoussa1 added 5 commits September 26, 2023 11:43

plugin: add helper function to add project name

dc55621

Add a helper function to mf_priority.cpp that adds the project name for a user/bank's job to the jobspec via jobspec-update under attributes.system.project.

plugin_query: add projects, def_project

139c440

Add the projects and def_project fields to the list of information returned for each association in the plugin.query callback. Add the projects and def_project key-value pairs to the expected files for t1019-mf-priority-info-fetch.t.

t: add tests for project validation

9723d3b

Problem: There are no tests for project validation and annotation in the multi-factor priority plugin. Add some tests.

cmoussa1 force-pushed the add.projects.to.plugin branch from 7d01df6 to 09063e0 Compare September 26, 2023 18:43

cmoussa1 closed this Jan 17, 2024


		data = {"data": bulk_p_data}

		flux.Flux().rpc("job-manager.mf_priority.rec_p_update", json.dumps(data)).get()

		if (flux_respond (h, msg, NULL) < 0)
		flux_log_error (h, "flux_respond");

		flux account-priority-update -p ${DB_PATH} &&
		test $(flux jobs -no {state} ${job0}) = RUN

plugin: add project validation and annotation #294

plugin: add project validation and annotation #294

Conversation

cmoussa1 commented Oct 21, 2022 • edited Loading

Background

cmoussa1 commented Dec 6, 2022

garlick commented May 15, 2023

cmoussa1 commented May 15, 2023 • edited Loading

cmoussa1 commented May 16, 2023

cmoussa1 commented Jul 25, 2023

grondo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmoussa1 Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmoussa1 Aug 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grondo commented Aug 8, 2023

grondo commented Aug 8, 2023

cmoussa1 commented Sep 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grondo commented Sep 15, 2023 • edited Loading

cmoussa1 commented Sep 22, 2023

grondo commented Sep 22, 2023

cmoussa1 commented Sep 22, 2023

grondo commented Sep 22, 2023

cmoussa1 commented Sep 22, 2023

grondo commented Sep 22, 2023

grondo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 26, 2023

Codecov Report

cmoussa1 commented Jan 17, 2024

cmoussa1 commented Oct 21, 2022 •

edited

Loading

cmoussa1 commented May 15, 2023 •

edited

Loading

cmoussa1 Aug 9, 2023 •

edited

Loading

cmoussa1 Aug 11, 2023 •

edited

Loading

grondo commented Sep 15, 2023 •

edited

Loading