Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd: add pop-db as a flux account command #487

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

cmoussa1
Copy link
Member

Problem

The pop-db command is located in a separate file in the src/cmd/ directory and thus requires a different method of calling it than the other flux-accounting commands, i.e you need to write flux account-pop-db instead of flux account pop-db.


This PR moves this file to the directory containing the other Python bindings for flux-accounting and adds it as a regular command like the other bindings so that you can just call it like flux account pop-db.

It removes the methods and --path command line argument that deal with establishing a connection to the flux-accounting DB since the systemd service will handle establishing that connection.

I've also added both pop-db and export-db as commands that are tested in t1026-flux-account-perms.t to ensure that they require admin privileges to be run.

Finally, it adjusts the calls in t1009-pop-db.t and t1016-export-db.t and top-level README for pop-db to account for the change.

Problem: The "pop-db" command is located in a separate file in the
src/cmd/ directory and thus requires a different method of calling it
than the other flux-accounting commands, i.e you need to write "flux
account-pop-db" instead of "flux account pop-db".

Move this file to the directory containing the other Python bindings for
flux-accounting and add it as a regular command like the other bindings
so that you can just call it like "flux account pop-db".

Remove the methods and command line argument that deal with establishing
a connection to the flux-accounting DB since the systemd service will
handle establishing that connection.

Adjust the calls in the sharness tests and top-level README for pop-db
to account for the change.
Problem: t1026-flux-account-perms.t goes through all of the flux account
commands that require admin privileges to run and makes sure that they
cannot be accessed by regular users, but it does not test the
"export-db" or "pop-db" commands.

Add both of these commands to the test suite to ensure they require
admin privileges to be run.
@cmoussa1 cmoussa1 added the improvement Upgrades to an already existing feature label Aug 29, 2024
@cmoussa1 cmoussa1 requested a review from jameshcorbett August 29, 2024 17:31
Copy link
Member

@jameshcorbett jameshcorbett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@cmoussa1
Copy link
Member Author

thanks! setting MWP here

@mergify mergify bot merged commit 88ec3b0 into flux-framework:master Aug 29, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Upgrades to an already existing feature merge-when-passing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants