Add management command to report total quotas - #325
marcoagonzales007 wants to merge 2 commits into
Conversation
QuanMPhm
left a comment
There was a problem hiding this comment.
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-typefor user to decide quotas for which service should be listedproject-idto 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!
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 ResourcesattributeRather than
Allocation_Quota_attributeswhich only containsQuota_GPUnowAlso added to unit tests to test against the command.