Allow dynamic quota creation and removal #287
Conversation
| defaults={"value": json.dumps(new_quota_dict)}, | ||
| ) | ||
|
|
||
| # TODO (Quan): Dict update allows migration of existing quotas. This is fine? |
There was a problem hiding this comment.
@knikolla @jtriley This is a pre-existing feature, so I assume the answer is yes. Just to make sure.
| "OpenStack Storage", | ||
| openstack_nese_storage_rate, | ||
| ) | ||
| # TODO (Quan): An illustration of how billing could be simplified. Shuold I follow with this? |
There was a problem hiding this comment.
@knikolla I couldn't do the same refactoring for the Openshift allocations because different storages have their own rates. I could have refactored the code further to circumvent that issue, but I didn't want the PR to be too long.
| }, | ||
| ) | ||
|
|
||
| # TODO (Quan): What happens when a quota is removed? Should the attribute be removed from Coldfront? |
There was a problem hiding this comment.
@knikolla @jtriley @joachimweyl This also has implications for billing storage. This test case is failing here since I would like people's consensus on desired behavior.
While not entirely clear, it seems the recent Pandas relase (3.0.0) changed how casting to decimal types works, causing some invoicing code to throw errors, specifically calls to read_csv() Seperating loading of the CSV and casting seems to fix this
QuanMPhm
force-pushed
the
ops_1391/final
branch
5 times, most recently
from
9aa62a7 to
b3c58d8
Compare
QuanMPhm
force-pushed
the
ops_1391/final
branch
from
b3c58d8 to
35273aa
Compare
|
@knikolla I addressed all your suggestions on Slack except one:
May I ask that I implement this feature in a subsequent PR, to prevent this PR from bloating even more? If not, I will implement this after I receive answers for my questions above. |
|
What is the impact of this omission? |
|
@joachimweyl The impact will be that to change the display names of attributes (the names that users will see in the Coldfront UI, i.e |
|
Makes sense to me. |
2 participants
Closes nerc-project/operations#1391. This is how I would suggest to review this PR.
Two CLI commands have been added,
add_quota_to_resource.pyandremove_quota_from_resource.py. I would suggest understanding those two commands first. These commands allow us to dynamically add/remove quotas instead of having them hard-coded as they are currently done. These commands don't impact the quota objects in the clusters, nor the quota attributes in allocations. Their full impact is illustrated when used within the typical user workflow, or in tandem withvalidate_allocations.py. I would now suggest checking the changes tofunctional/openshift/test_allocations.pyto see the full implications of this PR. The other functional test cases only contain minor changes.Afterwards,
tasks.py,validate_allocations.py, and the allocator base and subclasses should be reviewed. They are the main consumers of quota information. All other changes relatively minor.This is a draft for now since I have some questions, and the tests are failing. I just wanted people to know my general direction with this feature.
I will wait for people's feedback before continuing work on this PR, since I assume substantial feedback will be given.