DI-26927 fixed review comments from API v4 team

srbhaakamai · srbhaakamai · commit 546d3fbdb773 · 2025-09-18T13:50:58.000+05:30
diff --git a/linode_api4/groups/monitor.py b/linode_api4/groups/monitor.py
@@ -155,7 +155,6 @@ def get_alert_definitions(
         self,
         *filters,
         service_type: Optional[str] = None,
-        alert_id: Optional[int] = None,
     ) -> Union[PaginatedList, AlertDefinition]:
         """
         Retrieve one or more alert definitions.
@@ -167,7 +166,7 @@ def get_alert_definitions(
 
         Examples:
             alert_definitions = client.monitor.get_alert_definitions()
-            alert_definition = client.monitor.get_alert_definitions(service_type="dbaas", alert_id=1234)
+            alert_definition = client.load(AlertDefinition, alert_id,"dbaas")
             alert_definitions_for_service = client.monitor.get_alert_definitions(service_type="dbaas")
 
         .. note:: This endpoint is in beta and requires using the v4beta base URL.
@@ -193,19 +192,9 @@ def get_alert_definitions(
         :rtype: Union[AlertDefinition, PaginatedList[AlertDefinition]]
         """
 
-        if alert_id is not None and service_type is None:
-            raise ValueError(
-                "service_type must be provided when alert_id is specified"
-            )
-
         endpoint = "/monitor/alert-definitions"
         if service_type:
             endpoint = f"/monitor/services/{service_type}/alert-definitions"
-            if alert_id:
-                endpoint = f"{endpoint}/{alert_id}"
-                # Requesting a single object
-                alert_json = self.client.get(endpoint)
-                return AlertDefinition(self.client, alert_id, alert_json)
 
         # Requesting a list
         return self.client._get_and_filter(
@@ -304,7 +293,7 @@ def create_alert_definition(
                 json=result,
             )
 
-        return AlertDefinition(self.client, result["id"], result)
+        return AlertDefinition(self.client, result["id"],service_type, result)
 
     def update_alert_definition(
         self,
@@ -375,26 +364,4 @@ def update_alert_definition(
             data=params,
         )
 
-        return AlertDefinition(self.client, result["id"], result)
-
-    def delete_alert_definition(
-        self, service_type: str, alert_id: int
-    ) -> None:
-        """
-        Delete an alert definition.
-
-        .. note:: This endpoint is in beta and requires using the v4beta base URL.
-
-        API Documentation: https://techdocs.akamai.com/linode-api/reference/delete-alert-definition
-
-        :param service_type: Service type of the alert definition to delete
-                             (e.g. ``"dbaas"``).
-        :type service_type: str
-        :param alert_id: ID of the alert definition to delete.
-        :type alert_id: int
-
-        :returns: None
-        """
-        self.client.delete(
-            f"/monitor/services/{service_type}/alert-definitions/{alert_id}"
-        )
+        return AlertDefinition(self.client, result["id"], service_type, result)
diff --git a/linode_api4/objects/monitor.py b/linode_api4/objects/monitor.py
@@ -1,4 +1,5 @@
 __all__ = [
+       "AlertType",
     "MonitorDashboard",
     "MonitorMetricsDefinition",
     "MonitorService",
@@ -8,14 +9,14 @@
     "TriggerConditions",
     "AlertChannel",
     "AlertDefinition",
-    "AlertType",
     "AlertChannelEnvelope",
 ]
 from dataclasses import dataclass, field
 from enum import Enum
 from typing import List, Literal, Optional, Union
 
 from linode_api4.objects.base import Base, Property
+from linode_api4.objects import DerivedBase
 from linode_api4.objects.serializable import JSONObject, StrEnum
 
 
@@ -288,69 +289,53 @@ class AlertChannelEnvelope(JSONObject):
     type: str = ""
     url: str = ""
 
-class AlertDefinition(Base):
+@dataclass
+class AlertType(Enum):
     """
-    Represents an alert definition for a monitor service.
+    Enumeration of alert origin types used by alert definitions.
 
-    This object models the JSON returned by the Monitor API's alert-definition
-    endpoints. Alert definitions describe a named condition (one or more rules)
-    that, when met, will trigger notifications via configured alert channels.
-
-    Typical usage:
-      alert = client.monitor.alert_definitions(service_type="dbaas", alert_id=1234)
-      alerts = client.monitor.alert_definitions(service_type="dbaas")
-
-    Important fields encoded in the API response include:
-      - id: int - Unique identifier for the alert definition.
-      - label: str - Human-readable name for the alert.
-      - severity: str/int - Severity level (e.g. "warning", "critical").
-      - type: str - "user" or "system" indicating origin.
-      - service_type: str - The service type the alert applies to (e.g. "dbaas").
-      - status: str - Current status of the alert definition (e.g. "active").
-      - rule_criteria: dict - The set of rules evaluated to determine whether
-        an alert should trigger (see :class:`RuleCriteria`).
-      - trigger_conditions: dict - Evaluation configuration (see :class:`TriggerConditions`).
-      - alert_channels: list - A list of lightweight channel envelopes
-        (:class:`AlertChannelEnvelope`) used to notify when the alert fires.
-      - entity_ids: list - Optional list of entity IDs this alert targets.
-      - created / updated / updated_by / created_by - Auditing metadata.
-
-    Note: The API returns a key named "class" which is a reserved word in
-    Python. When populating this object the value is stored on the attribute
-    ``_class``.
+    Values:
+      - SYSTEM: Alerts that originate from the system (built-in or platform-managed).
+      - USER: Alerts created and managed by users (custom alerts).
+
+    The API uses this value in the `type` field of alert-definition responses.
+    This enum can be used to compare or validate the `type` value when
+    processing alert definitions.
+    """
+    SYSTEM = "system"
+    USER = "user"
+
+class AlertDefinition(DerivedBase):
+    """
+    Represents an alert definition for a monitor service.
 
     API Documentation: https://techdocs.akamai.com/linode-api/reference/get-alert-definition
     """
-    id: int
-    label: str
-    severity: int
-    _type: str
-    service_type: str
-    status: str
-    has_more_resources: bool
-    rule_criteria: list[Rule]
-    trigger_conditions: TriggerConditions
-    alert_channels: list[AlertChannelEnvelope]
-    created: str
-    updated: str
-    updated_by: str
-    created_by: str
-    entity_ids: list[str] = None
-    description: str = None
-    _class: str = None
-   
-
-    def __init__(self, client, id, json=None):
-        super().__init__(client, id, json)
-
-    def _populate(self, json):
-        """
-        Populates this object with data from a JSON dictionary.
-        """
-        reserved = {"class": "_class", "type": "_type"}
-        for json_key, json_value in json.items():
-            attr = reserved.get(json_key, json_key)
-            setattr(self, attr, json_value)
+    
+    api_endpoint = "/monitor/services/{service}/alert-definitions/{id}"
+    derived_url_path = "alert-definitions"
+    parent_id_name =  "service"
+    id_attribute = "id"
+
+    properties = {
+        "id": Property(identifier=True),
+        "label": Property(),
+        "severity": Property(),
+        "type": Property(AlertType),
+        "service_type": Property(mutable=True),
+        "status": Property(mutable=True),
+        "has_more_resources": Property(mutable=True),
+        "rule_criteria": Property(RuleCriteria),
+        "trigger_conditions": Property(TriggerConditions),
+        "alert_channels": Property(List[AlertChannelEnvelope]),
+        "created": Property(is_datetime=True),
+        "updated": Property(is_datetime=True),
+        "updated_by": Property(),
+        "created_by": Property(),
+        "entity_ids": Property(List[str]),
+        "description": Property(),
+        "_class": Property("class"),
+    }
 
 
 @dataclass
@@ -367,22 +352,6 @@ class ChannelContent(JSONObject):
     """
     email: EmailChannelContent = None
     # Other channel types like 'webhook', 'slack' could be added here as Optional fields.
-
-@dataclass
-class AlertType(Enum):
-    """
-    Enumeration of alert origin types used by alert definitions.
-
-    Values:
-      - SYSTEM: Alerts that originate from the system (built-in or platform-managed).
-      - USER: Alerts created and managed by users (custom alerts).
-
-    The API uses this value in the `type` field of alert-definition responses.
-    This enum can be used to compare or validate the `type` value when
-    processing alert definitions.
-    """
-    SYSTEM = "system"
-    USER = "user"
     
 class AlertChannel(Base):
     """
@@ -391,43 +360,22 @@ class AlertChannel(Base):
     notifications (for example: email lists, webhooks, PagerDuty, Slack, etc.).
 
     This class maps to the Monitor API's `/monitor/alert-channels` resource
-    and is used by the SDK to list, load, and inspect channels. Typical fields
-    returned by the API include:
-      - id: int - Unique identifier for the channel
-      - label: str - Human readable label for the channel
-      - type / channel_type: str - Channel type ("email", "webhook", "pagerduty", ...)
-      - url: str - URL or destination associated with the channel (when applicable)
-      - content: dict - Channel-specific configuration block (e.g. email addresses)
-      - created / updated / created_by / updated_by - Auditing metadata
-
-    Note: The exact shape of the `content` block varies by channel type; the
-    SDK exposes `ChannelContent` / `EmailChannelContent` dataclasses for common
-    cases but callers may receive raw dicts from the API in some responses.
+    and is used by the SDK to list, load, and inspect channels.
     """
+
     api_endpoint = "/monitor/alert-channels/"
-    id: int
-    alerts: List[AlertChannelEnvelope]
-    label: str
-    channel_type: str
-    content: ChannelContent
-    created: str
-    created_by: str
-    _type: AlertType
-    updated: str
-    updated_by: str
-
-    @property
-    def type(self):
-        return self._type
-
-    @type.setter
-    def type(self, value):
-        self._type = value
-
-    def _populate(self, json):
-        """
-        Populates this object with data from a JSON dictionary.
-        """
-        for key, value in json.items():
-            if hasattr(self, key):
-                setattr(self, key, value)
+    id_attribute = "id"
+
+    properties = {
+        "id": Property(identifier=True),
+        "label": Property(),
+        "type": Property("channel_type"),
+        "channel_type": Property(),
+        "content": Property(ChannelContent),
+        "created": Property(is_datetime=True),
+        "updated": Property(is_datetime=True),
+        "created_by": Property(),
+        "updated_by": Property(),
+        "url": Property(),
+        # Add other fields as needed
+    }
diff --git a/test/integration/models/monitor/test_monitor.py b/test/integration/models/monitor/test_monitor.py
@@ -4,6 +4,7 @@
 
 from linode_api4 import ApiError, LinodeClient
 from linode_api4.objects import (
+    AlertDefinition,
     MonitorDashboard,
     MonitorMetricsDefinition,
     MonitorService,
@@ -173,23 +174,14 @@ def test_integration_create_get_update_delete_alert_definition(
         assert created.id
         assert getattr(created, "label", None) == label
 
-        # Fetch by id
-        fetched = client.monitor.get_alert_definitions(
-            service_type=service_type, alert_id=created.id
-        )
-        assert fetched.id == created.id
-        assert getattr(fetched, "label", None) == label
-
         # Wait for server-side processing to complete (status transitions)
         timeout = 120
         interval = 10
         start = time.time()
         while getattr(created, "status", None) == "in progress" and (time.time() - start) < timeout:
             time.sleep(interval)
             try:
-                created = client.monitor.alert_definitions(
-                    service_type=service_type, alert_id=created.id
-                )
+                created = client.load(AlertDefinition,created.id,service_type)
             except Exception:
                 # transient errors while polling; continue until timeout
                 pass
@@ -208,17 +200,18 @@ def test_integration_create_get_update_delete_alert_definition(
     finally:
         if created:
             # Best-effort cleanup; allow transient errors.
-            alert_update_interval = 120 # max time alert should take to update
+            alert_update_interval = 60 # max time alert should take to update
             try:
                 time.sleep(alert_update_interval) 
-                client.monitor.delete_alert_definition(service_type, created.id)
+                delete_alert = client.load(AlertDefinition,created.id,service_type)
+                delete_alert.delete()
             except Exception:
                 
                 pass
 
             # confirm it's gone (if API returns 404 or raises)
             try:
-                client.monitor.alert_definitions(service_type=service_type, alert_id=created.id)
+                client.load(AlertDefinition,created.id,service_type)
                 # If no exception, fail explicitly
                 assert False, "Alert definition still retrievable after delete"
             except ApiError:
