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

view-user: create new AssociationFormatter subclass for viewing associations #527

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cmoussa1
Copy link
Member

@cmoussa1 cmoussa1 commented Nov 6, 2024

Problem

The view_user() function manually defines its own print functionality for viewing information about an association in the
flux-accounting database, but the Python bindings have its own formatting class that does this. It's also tedious to add new formatting features to the view_user() function.


This PR creates a new subclass of AccountingFormatter called AssociationFormatter. It has a unique error message when the user cannot be found in the association_table, but otherwise inherits the default JSON and table outputs that the AccountingFormatter class already provides. Thus, the various helper functions used to create output for the view_user() functions could be removed.

I've edited a number of tests in the testsuite that check for specific output from the view-user command, as well as added unit tests for the view_user() function.

@cmoussa1 cmoussa1 added improvement Upgrades to an already existing feature low priority items that can be worked on at a later date labels Nov 6, 2024
@cmoussa1 cmoussa1 requested a review from wihobbs November 6, 2024 20:53
@cmoussa1 cmoussa1 changed the title view-user: use AccountingFormatter class view-user: create new AssociationFormatter subclass for viewing associations Nov 6, 2024
@cmoussa1 cmoussa1 force-pushed the restructure.association.views branch from bf976aa to d9fd90c Compare November 6, 2024 23:59
Problem: The view_user() function manually defines its own print
functionality for viewing information about an association in the
flux-accounting database, but the Python bindings have its own
formatting class that does this.

Create a new subclass of AccountingFormatter called
AssociationFormatter. Adjust the initilization of the
AccountingFormatter to accept a custom error message when the query
fails to find any results.

Adjust the view_user() function to use the new AssociationFormatter
class when formatting the output. Remove the --json optional argument
in favor of printing in JSON by default. Add a --fields optional
argument to allow the user to customize their output, similar to the
list-banks command.
Problem: The expected output of the view-user command is out-of-date
with the use of the new formatter class in the view_user() function.

Adjust the expected output in the testsuite to account for the new
format provided by the formatter class.
Problem: There are a few tests in the sharness testsuite that pass
--json when calling view-user, but this optional argument is no longer
valid as the default output of view-user is in JSON.

Adjust the view-user --json calls in the testsuite to account for the
removal of --json.
Problem: There are no unit tests for checking the output of
view_user().

Add some tests.
@cmoussa1 cmoussa1 force-pushed the restructure.association.views branch from d9fd90c to 8e62574 Compare November 20, 2024 01:08
Copy link

codecov bot commented Nov 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.40%. Comparing base (73a0ef9) to head (8e62574).
Report is 9 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #527   +/-   ##
=======================================
  Coverage   82.40%   82.40%           
=======================================
  Files          20       20           
  Lines        1478     1478           
=======================================
  Hits         1218     1218           
  Misses        260      260           
---- 🚨 Try these New Features:

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant