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

flux-account.py: get rid of dictionary initialization #512

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

cmoussa1
Copy link
Member

Problem

flux-account.py manually creates the dictionary of arguments that is sent via RPC to the flux-accounting service, but since the service already unpacks these arguments, flux-account.py can just create a dictionary of the arguments using vars() and send the dictionary without having to define each key-value pair before sending the RPC.


This PR changes flux-account.py to just use vars() on the parsed args to create a dictionary before sending the RPC to the appropriate flux-accounting command.

@cmoussa1 cmoussa1 added improvement Upgrades to an already existing feature low priority items that can be worked on at a later date labels Oct 16, 2024
@cmoussa1 cmoussa1 force-pushed the clean.flux-account branch 2 times, most recently from 5798718 to c9c0656 Compare October 23, 2024 18:04
Problem: flux-account.py manually creates the dictionary of arguments
that is sent via RPC to the flux-accounting service, but since the
service already unpacks these arguments, flux-account.py can just
create a dictionary of the arguments using vars() and send the entire
dictionary.

Use vars() on the parsed args to create a dictionary before sending
the RPC to the appropriate flux-accounting command.

Move the .split() call of the "fields" optional argument to the
flux-accounting service instead of in flux-account.py.
@cmoussa1 cmoussa1 requested a review from wihobbs October 23, 2024 18:31
Copy link
Member

@wihobbs wihobbs left a comment

Choose a reason for hiding this comment

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

Nice improvement! LGTM

@cmoussa1
Copy link
Member Author

Thanks! Setting MWP here

@mergify mergify bot merged commit 4e82ad6 into flux-framework:master Oct 24, 2024
13 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 low priority items that can be worked on at a later date merge-when-passing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants