Allow creating and modifying allocations with backwards-compatibility #52

QuanMPhm · 2025-04-25T20:02:27Z

Closes #47. After digging around DRF's documentation on serialization, it turned out the code changes would be a bit more complicated than I expected. I will submit a draft for now to demonstrate my current direction.

My draft allows creating new allocations through api/allocations by posting a json payload like this:

{
    'allocationattribute_set': [
        {'allocation_attribute_type': 'OpenShift Limit on CPU Quota',
        'value': '4'},
        {'allocation_attribute_type': 'OpenShift Limit on RAM Quota (MiB)',
        'value': '4096'},
    ],
    'project_id': 3,
    'resources_id': [1]
}

The output from the allocations API has been changed slightly. It now outputs this:

{
    "id": 31025,
    "project": {
        "id": 3,
        "title": "NERC-TESTING-1",
        "pi": "[[email protected]](mailto:[email protected])",
        "description": "khkjhkl\r\njjhhjk",
        "field_of_science": "Polar Ocean and Climate Systems",
        "status": "New"
    },
    "description": "",
    "resource": {
        "name": "microshift",
        "resource_type": "OpenShift"
    },
    "status": 1,
    "allocationattribute_set": [
        {
            "allocation_attribute_type": "Allocated Project Name",
            "value": "nerc-testing-1-dc43c5"
        },
        {
            "allocation_attribute_type": "Allocated Project ID",
            "value": "nerc-testing-1-dc43c5"
        },
        ....
    ]
}

I will implement the "update" feature once given the green light on this approach.

QuanMPhm · 2025-09-11T16:50:04Z

Note-to-self: Look at the REST API introduced by upstream Coldfront to see if it meets the requirements of this issue

QuanMPhm · 2025-09-15T14:17:06Z

@knikolla After reviewing the new Coldfront API in v1.1.7, I believe it does not meet our requirements, but it will cause a small conflict. The new API is explicitly read-only. It will cause a conflict because the API's URLs are under <domain_name>/api/, which is the same subdomain our plugin API is under. I could change our plugin's URL to something else. Otherwise that's the only concern I found

knikolla · 2025-09-15T14:22:51Z

@knikolla After reviewing the new Coldfront API in v1.1.7, I believe it does not meet our requirements, but it will cause a small conflict. The new API is explicitly read-only. It will cause a conflict because the API's URLs are under <domain_name>/api/, which is the same subdomain our plugin API is under. I could change our plugin's URL to something else. Otherwise that's the only concern I found

While it's under the same URL, it need to be explicitly enabled to register itself under it, so I think we're okay for now. No need to change away from api/.

QuanMPhm · 2025-09-15T15:45:28Z

@knikolla @naved001 Do you have any opinion about this draft? Should I proceed with this?

knikolla · 2025-09-15T17:09:22Z

src/coldfront_plugin_api/serializers.py

+        allocation = Allocation.objects.create(
+            project=validated_data["project"],
+            status=AllocationStatusChoice.objects.get(
+                name="Active"


Good question.

New would be the safe choice.

Otherwise allowing it to be provided as part of the API query during creation and default to New if missing.

Since it seems people want to use the API to create Coldfront allocations and have them also create projects on OpenShift and OpenStack, do I also signal allocation_activate if the status choice is Active?

Do not send a signal when an allocation is created in any state. If someone modifies an allocation to change status, then do send a signal. So if New -> Active = activate or Active -> Denied|Revoked = deactivate

This means if someone wants to use the API to create an allocation and have it create a project in the remote cluster, we want them to send 2 requests to the API?

Yes, that is fine.

knikolla · 2025-09-15T17:10:44Z

src/coldfront_plugin_api/serializers.py

+            "resource",
+            "resources_id",
+            "status",
+            "allocationattribute_set",


Is there any way to "rename" how this fields appear in the output? I think allocationattribute_set is too "internal implementation".

knikolla · 2025-09-15T17:13:37Z

src/coldfront_plugin_api/serializers.py

+            "project_id",
+            "description",
+            "resource",
+            "resources_id",


I don't like that there's both a resource and a resources_id. Can you make this work with just resource by for example accepting an API request to create a resource allocation with

'resource': { 'id': 3 }

QuanMPhm · 2025-09-18T18:52:58Z

src/coldfront_plugin_api/serializers.py

+        slug_field="name", queryset=AllocationStatusChoice.objects.all()
+    )
+
+    # TODO (Quan): What about default start/end dates? Default quantity? Description? Justification?


@joachimweyl @knikolla @naved001

Default the end date to a year from now, quantity to 1 and Description and Justification to empty strings.

QuanMPhm · 2025-09-18T18:53:09Z

src/coldfront_plugin_api/serializers.py

-    def get_resource(self, obj: Allocation) -> dict:
-        resource = obj.resources.first()
-        return {"name": resource.name, "resource_type": resource.resource_type.name}
+        # TODO (Quan): If the status is `Active`, do we fire the `activate_allocation`


@joachimweyl @knikolla @naved001

Responded in a different comment.

QuanMPhm · 2025-09-18T18:53:13Z

src/coldfront_plugin_api/serializers.py

+
+    def update(self, allocation: Allocation, validated_data):
+        """Only allow updating allocation attributes for now"""
+        # TODO (Quan) Do we want to allow updating any other allocation properties?


@joachimweyl @knikolla @naved001

I'm not convinced about the appropriate way to handle attribute changes. When done through the web dashboard they create a ChangeRequest, whereas modified from here they would modify the attribute directly. This is a flow that we haven't really dealt with as of now. I would say for now disallow changing attributes until we have a clear use case and workflow.

QuanMPhm · 2025-09-24T15:04:34Z

@joachimweyl @knikolla Since there is also a need to automate decomissioning projects. Do we want the API to implement a DELETE handler as well?

knikolla · 2025-09-24T20:22:24Z

@joachimweyl @knikolla Since there is also a need to automate decomissioning projects. Do we want the API to implement a DELETE handler as well?

Not at the moment.

QuanMPhm · 2025-09-30T17:21:39Z

Note-to-self: The branch containing work on the update function is in 47/modify_api_update

QuanMPhm · 2025-09-30T20:00:44Z

@knikolla @naved001 All of my questions have been resolved. I believe this PR is ready for review and approval

knikolla · 2025-10-03T20:50:52Z

src/coldfront_plugin_api/serializers.py

+        fields = ["id", "project", "description", "resources", "status", "attributes"]

-    resource = serializers.SerializerMethodField()
+    resources = ResourceSerializer(many=True)


Please keep resource as a single element in the API rather than a resources list. This is not something the Web Dashboard exposes when requesting allocations and is not something the API should expose.

When you save it to the database you can convert it into a list since that's what the model expects.

knikolla · 2025-10-03T21:00:40Z

src/coldfront_plugin_api/serializers.py

    project = ProjectSerializer()
-    attributes = serializers.SerializerMethodField()
-    status = serializers.SerializerMethodField()
+    attributes = AllocationAttributeSerializer(


This is going to break the invoicing scripts in a non-backwards compatible way, that's going to make it painful to update, as both the API AND the invoicing scripts will need to be updated at the same time.

May I suggest you either

preserve the previous format of the attributes field for both GET and create operations, OR

keep the previous attributes as is for GET operations and introduce a new field with a different name alongside it that appears in GET and supports create.

Version the API.

I will keep the API in a backwards compatible way. To do that, I will have to go against your comment here, since as far as I could tell, SerializerMethodField is read-only, and is the only way for the attributes list to be displayed the way we've had them. A new field requested_attributes will be added.

Same will go for the resource attribute

The more I tried looking for a solution that allows writing and reading through the same field, it seems technically possible, but would require very convoluted code that stretches the intended use of DRF

@knikolla Actually, the more spend on this, I'm now wondering if it's best to just version the API. I am fine with either making my PR backward-compatible, or to just version the API. May I have your opinion on which option you'd pick and why?

@QuanMPhm Make a proposal for how you would version the API.

QuanMPhm · 2025-10-07T20:01:51Z

Note to self: The branch containing an API versioning solution is 47/modify_api_version. The branch for backwards-compatibility is 47/modify_api_backward. Original solution's branch is 47/modify_api

To create an allocation, a JSON payload can be uploaded to `/api/allocations`: { "attributes": [ {"attribute_type": "OpenShift Limit on CPU Quota", "value": 8}, {"attribute_type": "OpenShift Limit on RAM Quota (MiB)", "value": 16}, ], "project": {"id": project.id}, "resources": [{"id": self.resource.id}], "status": "New", } Updating allocation status is done via a PATCH request to `/api/allocations/{id}` with a JSON payload: { "status": "Active" } Certain status transitions trigger signals: - New -> Active: allocation_activate - Active -> Denied: allocation_deactivate

QuanMPhm · 2025-10-09T18:40:26Z

@knikolla I've updated this PR to be backwards-compatible. #64 is the counterpart that uses API versioning. Both PRs are largely similar

QuanMPhm requested review from joachimweyl, knikolla, larsks and naved001 April 25, 2025 20:02

QuanMPhm mentioned this pull request Sep 10, 2025

Create a script to create allocations in Coldfront #60

Open

3 tasks

knikolla reviewed Sep 15, 2025

View reviewed changes

QuanMPhm force-pushed the 47/modify_api branch from 73b35ab to 06dbb3d Compare September 18, 2025 18:52

QuanMPhm requested a review from knikolla September 18, 2025 18:52

QuanMPhm commented Sep 18, 2025

View reviewed changes

QuanMPhm force-pushed the 47/modify_api branch 3 times, most recently from a37b1f4 to 4d2206d Compare September 30, 2025 20:00

QuanMPhm marked this pull request as ready for review September 30, 2025 20:00

knikolla mentioned this pull request Oct 1, 2025

Kristi does Code Reviews (Week of 2025-09-29) CCI-MOC/ops-issues#1621

Closed

9 tasks

knikolla requested changes Oct 3, 2025

View reviewed changes

QuanMPhm mentioned this pull request Oct 9, 2025

Allow creating and updating allocations with API versioning #64

Open

QuanMPhm changed the title ~~Allow creating and modifying allocations~~ Allow creating and modifying allocations with backwards-compatibility Oct 9, 2025

QuanMPhm force-pushed the 47/modify_api branch from 4d2206d to 8964950 Compare October 9, 2025 18:39

QuanMPhm mentioned this pull request Oct 9, 2025

Added script to create allocations through Coldfront API #65

Draft

QuanMPhm requested a review from knikolla November 4, 2025 15:49

Allow creating and modifying allocations with backwards-compatibility #52

Are you sure you want to change the base?

Allow creating and modifying allocations with backwards-compatibility #52

Conversation

QuanMPhm commented Apr 25, 2025

Uh oh!

QuanMPhm commented Sep 11, 2025

Uh oh!

QuanMPhm commented Sep 15, 2025

Uh oh!

knikolla commented Sep 15, 2025

Uh oh!

QuanMPhm commented Sep 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuanMPhm commented Sep 24, 2025

Uh oh!

knikolla commented Sep 24, 2025

Uh oh!

QuanMPhm commented Sep 30, 2025

Uh oh!

QuanMPhm commented Sep 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuanMPhm Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuanMPhm commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QuanMPhm commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

QuanMPhm Oct 5, 2025 •

edited

Loading

QuanMPhm commented Oct 7, 2025 •

edited

Loading