Skip to content

Commit

Permalink
delete-bank: add --force option
Browse files Browse the repository at this point in the history
Problem: The delete-bank command does not actually delete a row from
the bank_table but instead sets the 'active' column for a row to
0. There might be a use case where an admin would actually want to
remove a row from the table.

Add a new --force optional argument to the delete-bank command which
will actually remove the row from the bank_table.

Add a function description to delete_bank() to describe what the
function is doing and add a note about the --force option.
  • Loading branch information
cmoussa1 committed Jan 30, 2025
1 parent 76dc475 commit ca012bc
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 7 deletions.
28 changes: 22 additions & 6 deletions src/bindings/python/fluxacct/accounting/bank_subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,27 @@ def view_bank(conn, bank, tree=False, users=False, parsable=False, cols=None):
raise ValueError(f"view-bank: {exc}")


def delete_bank(conn, bank):
def delete_bank(conn, bank, force=False):
"""
Deactivate a bank row in the bank_table by setting its 'active' status to 0.
If force=True, actually remove the bank row from the bank_table. If the bank contains
multiple sub-banks and associations, either disable or actually remove those rows as
well.
Args:
conn: The SQLite Connection object
bank: the name of the bank
force: an option to actually remove the row from the bank_table instead of
just setting the 'active' column to 0.
"""
cursor = conn.cursor()
if force:
sql_stmt = "DELETE FROM bank_table WHERE bank=?"
else:
sql_stmt = "UPDATE bank_table SET active=0 WHERE bank=?"

try:
cursor.execute("UPDATE bank_table SET active=0 WHERE bank=?", (bank,))
cursor.execute(sql_stmt, (bank,))

# helper function to traverse the bank table and disable all of its sub banks
def get_sub_banks(bank):
Expand All @@ -190,13 +206,13 @@ def get_sub_banks(bank):
FROM association_table WHERE bank=?
"""
for assoc_row in cursor.execute(select_assoc_stmt, (bank,)):
u.delete_user(conn, username=assoc_row[0], bank=assoc_row[1])
u.delete_user(
conn, username=assoc_row[0], bank=assoc_row[1], force=force
)
# else, disable all of its sub banks and continue traversing
else:
for row in result:
cursor.execute(
"UPDATE bank_table SET active=0 WHERE bank=?", (row[0],)
)
cursor.execute(sql_stmt, (row[0],))
get_sub_banks(row[0])

get_sub_banks(bank)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/flux-account-service.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ def add_bank(self, handle, watcher, msg, arg):

def delete_bank(self, handle, watcher, msg, arg):
try:
val = b.delete_bank(self.conn, msg.payload["bank"])
val = b.delete_bank(self.conn, msg.payload["bank"], msg.payload["force"])

payload = {"delete_bank": val}

Expand Down
11 changes: 11 additions & 0 deletions src/cmd/flux-account.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,17 @@ def add_delete_bank_arg(subparsers):
help="bank name",
metavar="BANK",
)
subparser_delete_bank.add_argument(
"--force",
action="store_const",
const=True,
default=False,
help=(
"actually remove bank from bank_table (WARNING: removing a row from "
"the bank_table can affect a bank and its users' fair-share value; "
"proceed with caution)"
),
)


def add_edit_bank_arg(subparsers):
Expand Down

0 comments on commit ca012bc

Please sign in to comment.