Skip to content

Commit

Permalink
add-user: make --username and --bank required
Browse files Browse the repository at this point in the history
Problem: The --username and --bank arguments for add-user should be
required arguments, but they are currently treated as optional
arguments. This means that if someone calls add-user without passing
--username or --bank, the corresponding add_user() function will try
to run the function with None values. Requirements should be added for
both of these arguments at the top level so that the call fails if
either one of these is not present.

Add required=True to both the --username and --bank arguments in
add-user. Specify which arguments are being passed where in
flux-account-service.py. Add two tests to t1007-flux-account-users.t to
make sure errors are raised when either one of these is missing from the
add-user call.
  • Loading branch information
cmoussa1 committed Nov 11, 2024
1 parent a29e5a1 commit da87159
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 10 deletions.
20 changes: 10 additions & 10 deletions src/cmd/flux-account-service.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,16 +171,16 @@ def view_user(self, handle, watcher, msg, arg):
def add_user(self, handle, watcher, msg, arg):
try:
val = u.add_user(
self.conn,
msg.payload["username"],
msg.payload["bank"],
msg.payload["userid"],
msg.payload["shares"],
msg.payload["max_running_jobs"],
msg.payload["max_active_jobs"],
msg.payload["max_nodes"],
msg.payload["queues"],
msg.payload["projects"],
conn=self.conn,
username=msg.payload["username"],
bank=msg.payload["bank"],
uid=msg.payload["userid"],
shares=msg.payload["shares"],
max_running_jobs=msg.payload["max_running_jobs"],
max_active_jobs=msg.payload["max_active_jobs"],
max_nodes=msg.payload["max_nodes"],
queues=msg.payload["queues"],
projects=msg.payload["projects"],
)

payload = {"add_user": val}
Expand Down
2 changes: 2 additions & 0 deletions src/cmd/flux-account.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ def add_add_user_arg(subparsers):
"--username",
help="username",
metavar="USERNAME",
required=True,
)
subparser_add_user.add_argument(
"--userid",
Expand All @@ -77,6 +78,7 @@ def add_add_user_arg(subparsers):
"--bank",
help="bank to charge jobs against",
metavar="BANK",
required=True,
)
subparser_add_user.add_argument(
"--shares",
Expand Down
10 changes: 10 additions & 0 deletions t/t1007-flux-account-users.t
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ test_expect_success 'add some queues to the DB' '
flux account add-queue special --priority=99999
'

test_expect_success 'call add-user without specifying a username' '
test_must_fail flux account add-user --bank=A > error.out 2>&1 &&
grep "add-user: error: the following arguments are required: --username" error.out
'

test_expect_success 'call add-user without specifying a bank' '
test_must_fail flux account add-user --username=user5011 > error.out 2>&1 &&
grep "add-user: error: the following arguments are required: --bank" error.out
'

test_expect_success 'trying to add an association that already exists should raise an IntegrityError' '
test_must_fail flux account add-user --username=user5011 --userid=5011 --bank=A > already_exists.out 2>&1 &&
grep "association user5011,A already active in association_table" already_exists.out
Expand Down

0 comments on commit da87159

Please sign in to comment.