Skip to content

Add management command to report total quotas - #325

Open
marcoagonzales007 wants to merge 2 commits into
nerc-project:mainfrom
marcoagonzales007:report-total-quotas
Open

marcoagonzales007 wants to merge 2 commits into
nerc-project:mainfrom
marcoagonzales007:report-total-quotas

Conversation

@marcoagonzales007

Copy link
Copy Markdown

References #86

Adds a report_total_quotasmanagement command that sums all the quota values across active allocations.

Supports --cloud-type, --project-id, --per-project, and --format json|csv.

Quotas names are read from each resource's Available Quota Resources attribute
Rather than Allocation_Quota_attributes which only contains Quota_GPU now

Also added to unit tests to test against the command.

@QuanMPhm QuanMPhm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Apologies. I should have given you more details before sending you off with the issue. This particular task is old and requires a few design decisions to be made (i.e what filtering options should be available? What should the output format be?) For a task with many possible solutions like this, I would have asked that you implement the minimum set of features to complete the goal outlined in the initial issue, which was to give a sum of allocation quotas in the CLI. The minimum set would have just allowed giving a report across all allocations, and with one output format, similar to the draft that Justin original proposed.

A minimum set would minimize the work you need to do, and minimize the review burden. It also reduces the likelihood that parts of your work will be discarded, since some reviewers might find some of your features unnecessary (as I will do below). Again, this is my fault for not giving guidance beforehand. You were not expected to be aware of these considerations.

That being said, I believe your current PR adds more features than was originally asked for. I would suggest just implement one output format, and allow just two cli arguments:

  • cloud-type for user to decide quotas for which service should be listed
  • project-id to limit showing quotas for just a single project.

In addition, I would probably ask @joachimweyl and @knikolla if they have any opinions on what design decisions to make, since they are more aware than I am regarding what the demands of a Coldfront admin might be.

Going forward, for tasks involving more design decisions like this, I ask that you give me at least a brief proposal on the design of your solution before the PR.

Other than that, your PR does show that you have gained a good understanding of this codebase. Good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants