adding SDK changes for ACLP APIs by pmajali · Pull Request #528 · linode/linode_api4-python

pmajali · 2025-04-11T06:20:23Z

📝 Description

The PR adds Monitor API for ACLP Product

✔️ How to Test

import os
import logging
from linode_api4 import LinodeClient

logger = logging.getLogger(__name__)

client = LinodeClient(os.environ.get("MY_PERSONAL_ACCESS_TOKEN"))
client.base_url = "https://api.linode.com/v4beta"


print("Listing services supported by ACLP")
services = client.monitor.supported_services()
for service in services:
        print("Label:", service.label)

How do I run the relevant unit/integration tests?

Run unit test as follows
pytest test/unit

Run integration test as follows
make testint TEST_SUITE=monitor

yec-akamai · 2025-04-16T19:40:05Z

@pmajali Thank you for starting contributing to python SDK! Do you mind filling the PR description and test steps?

linode_api4/groups/__init__.py

linode_api4/objects/__init__.py

test/integration/models/monitor/test_monitor.py

test/unit/objects/monitor_test.py

yec-akamai

Can you fix the PR given the suggestion from code scanning?

linode_api4/groups/monitor.py

linode_api4/objects/monitor.py

linode_api4/groups/__init__.py

linode_api4/objects/monitor.py

linode_api4/groups/monitor.py

linode_api4/objects/monitor.py

linode_api4/groups/monitor.py

linode_api4/objects/monitor.py

yec-akamai

You'll have to sign all of you commits (make sure each commit has the green Verified tag) before merging this PR. To save troubles in the end, I'd suggest to do it sooner than later. You can look for github official docs to add signatures to all your commits

Co-authored-by: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com>

linode_api4/groups/__init__.py

linode_api4/objects/__init__.py

yec-akamai

Thanks for addressing the comments! A few things to note:

yec-akamai · 2025-05-13T20:16:15Z

linode_api4/groups/monitor_service.py

+    def list_monitor_dashboards(self, *filters) -> list[MonitorDashboard]:
+        """
+        Returns a list of dashboards.
+
+            dashboards = client.monitor_service.list_monitor_dashboards()
+            dashboard = client.load(MonitorDashboard, 1)
+
+        .. note:: This endpoint is in beta. This will only function if base_url is set to `https://api.linode.com/v4beta`.
+
+        API Documentation: https://techdocs.akamai.com/linode-api/reference/get-dashboards-all
+
+        :param filters: Any number of filters to apply to this query.
+                        See :doc:`Filtering Collections</linode_api4/objects/filtering>`
+                        for more details on filtering.
+
+        :returns: A list of Dashboards.
+        :rtype: PaginatedList of Dashboard
+        """
+
+        return self.client._get_and_filter(MonitorDashboard, *filters)
+
+    def list_dashboards_by_service(
+        self, service_type: str, *filters
+    ) -> list[MonitorDashboard]:
+        """
+        Returns a list of dashboards for a particular service.
+
+            dashboard_by_service = client.monitor_service.list_dashboards_by_service(service_type="dbaas")
+
+        .. note:: This endpoint is in beta. This will only function if base_url is set to `https://api.linode.com/v4beta`.
+
+        API Documentation: https://techdocs.akamai.com/linode-api/reference/get-dashboards
+
+        :param service_type: The service type to get dashboards for.
+        :type service_type: str
+        :param filters: Any number of filters to apply to this query.
+                        See :doc:`Filtering Collections</linode_api4/objects/filtering>`
+                        for more details on filtering.
+
+        :returns: Dashboards filtered by Service Type.
+        :rtype: PaginatedList of the Dashboards
+        """
+
+        return self.client._get_and_filter(
+            MonitorDashboard,
+            *filters,
+            endpoint=f"/monitor/services/{service_type}/dashboards",
+        )


I wonder if it makes more sense to combine these two functions, since they're serving the same purpose to list dashboards and returning the same type.

How about we just having one function dashboards(...), and take an optional parameter service_type? In this case if service_type is provided by user, we will call the listing dashboards by service_type endpoint; if not, we will can the basic listing endpoint. The function header should looks like:

def dashborads( self, service_type: Optional[str], *filters ) -> PaginatedList: # implement the two endpoints

We could have one function but both the endpoints have different implementation and fall under different api_endpoint group, so it could be misleading.
for dashboards we have - /monitor/dashboards/{id}
for service dashboard - /monitor/services/{service_type}/dashboards

linode_api4/groups/monitor_service.py

yec-akamai · 2025-05-13T20:20:00Z

linode_api4/groups/monitor_service.py

+        return self.client._get_and_filter(
+            MonitorService,
+            *filters,
+            endpoint=f"/monitor/services/{service_type}",


I checked the api doc, and it looks GET /monitor/services/{service_type} is not a listing endpoint

This endpoint should ideally return the details about a service. But the return type is a list.

Oh interesting, do you mean that even GET /monitor/services/{service_type} supposed to get a single service, but the response is a list, right?

linode_api4/groups/monitor_service.py

yec-akamai · 2025-05-16T20:54:08Z

linode_api4/objects/monitor_service.py

+class MonitorService(Base):
+    """
+    Represents a single service type.
+    API Documentation: https://techdocs.akamai.com/linode-api/reference/get-monitor-services
+
+    """
+
+    api_endpoint = "/monitor/services/{service_type}"
+    id_attribute = "service_type"
+    properties = {
+        "service_type": Property(ServiceType, identifier=True),
+        "label": Property(),
+    }


Since GET /monitor/services/{service_type} returns a list, it can't be populated by the default _api_get() in Base. I think you can build MonitorService into a JSONObject as well, so user won't accidentally try to get this object from default approach

updated to JSONObject

yec-akamai · 2025-05-20T19:55:30Z

linode_api4/groups/monitor.py

+        :param service_type: The service type to create token for.
+        :type service_type: str
+        :param entity_ids: The list of entity IDs for which the token is valid.
+        :type entity_ids: list of int


It seems entity_ids is not necessary a list of int and could be any id types based on the linodego change: linode/linodego#746 Can you make the change and test it correspondingly as well?

updated type to Any

yec-akamai

Nice work, tests passed locally! Just change the entity_ids type then should be good to go

lgarber-akamai

Looks great and all tests are passing on my end. Thanks for the contribution!

yec-akamai

Nice work! Thanks for the contribution!

pmajali requested a review from a team as a code owner April 11, 2025 06:20

pmajali requested review from jriddle-linode and lgarber-akamai and removed request for a team April 11, 2025 06:20

lgarber-akamai added the new-feature for new features in the changelog. label Apr 11, 2025

github-advanced-security bot found potential problems Apr 16, 2025

View reviewed changes

yec-akamai self-requested a review April 18, 2025 15:27

yec-akamai reviewed Apr 18, 2025

View reviewed changes

linode_api4/groups/monitor.py Outdated Show resolved Hide resolved

lgarber-akamai reviewed Apr 21, 2025

View reviewed changes

linode_api4/groups/monitor.py Outdated Show resolved Hide resolved

lgarber-akamai reviewed Apr 21, 2025

View reviewed changes

linode_api4/objects/monitor.py Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems May 5, 2025

View reviewed changes

linode_api4/groups/__init__.py Fixed Show fixed Hide fixed

yec-akamai requested changes May 8, 2025

View reviewed changes

yec-akamai reviewed May 8, 2025

View reviewed changes

pmajali and others added 12 commits May 13, 2025 20:35

adding monitor APIs

40b0057

updating doc

9069a2c

updating tests

55bd63c

updating lint errors

347b07e

Updating method name

5d0b6a5

Co-authored-by: Lena Garber <114949949+lgarber-akamai@users.noreply.github.com>

updating code with review comments

c37bee7

sorting imports

295a48b

updating with make format changes

75478f7

updating with review comments

c40ffa4

review updates

5a40161

updating docstring

ed02e83

Merge branch 'dev' into monitor-api

e2d401f

github-advanced-security bot found potential problems May 13, 2025

View reviewed changes

linode_api4/groups/__init__.py Fixed Show fixed Hide fixed

linode_api4/objects/__init__.py Fixed Show fixed Hide fixed

yec-akamai requested changes May 13, 2025

View reviewed changes

updating func names

ed066d7

PR comments

bc8ef27

yec-akamai reviewed May 16, 2025

View reviewed changes

pmajali added 2 commits May 19, 2025 17:21

review comments

cf185a3

adding __all__ module

faa55d3

yec-akamai reviewed May 20, 2025

View reviewed changes

yec-akamai requested review from lgarber-akamai and yec-akamai May 20, 2025 20:31

updating unittest and entity_id to any

bc317a0

lgarber-akamai approved these changes May 29, 2025

View reviewed changes

yec-akamai approved these changes Jun 2, 2025

View reviewed changes

yec-akamai merged commit 0b1b2af into linode:dev Jun 2, 2025
12 checks passed

Conversation

pmajali commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description

✔️ How to Test

Uh oh!

yec-akamai commented Apr 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yec-akamai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yec-akamai left a comment • 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!

Uh oh!

Uh oh!

yec-akamai left a comment

Choose a reason for hiding this comment

Uh oh!

yec-akamai May 13, 2025

Choose a reason for hiding this comment

Uh oh!

pmajali May 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yec-akamai May 13, 2025

Choose a reason for hiding this comment

Uh oh!

pmajali May 14, 2025

Choose a reason for hiding this comment

Uh oh!

yec-akamai May 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yec-akamai May 16, 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!

pmajali May 19, 2025

Choose a reason for hiding this comment

Uh oh!

yec-akamai May 20, 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!

pmajali May 21, 2025

Choose a reason for hiding this comment

Uh oh!

yec-akamai left a comment

Choose a reason for hiding this comment

Uh oh!

lgarber-akamai left a comment

Choose a reason for hiding this comment

Uh oh!

yec-akamai left a comment

Choose a reason for hiding this comment

pmajali commented Apr 11, 2025 •

edited

Loading

yec-akamai left a comment •

edited

Loading

yec-akamai May 16, 2025 •

edited

Loading

yec-akamai May 20, 2025 •

edited

Loading