resolving review comments

Author: mawasthy-lgtm

linode_api4/groups/monitor.py

@@ -451,6 +451,8 @@ def channel_create(
         :rtype: AlertChannel
 
         .. note::
+           If you need to obtain a single :class:`AlertChannel`, use :meth:`LinodeClient.load`.
+           Example: ``client.load(AlertChannel, channel_id)``.
            For updating an alert channel, use the ``save()`` method on the :class:`AlertChannel` object.
            For deleting an alert channel, use the ``delete()`` method directly on the :class:`AlertChannel` object.
         """
@@ -470,27 +472,6 @@ def channel_create(
 
         return AlertChannel(self.client, result["id"], result)
 
-    def alert_channel(self, channel_id: int) -> AlertChannel:
-        """
-        Retrieve a specific notification channel definition details by its channel ID.
-
-        Returns an :class:`AlertChannel` object for the specified channel ID.
-        The channel object contains all configuration details for the notification
-        destination (e.g., email lists, webhooks, etc.).
-
-        .. note:: This endpoint is in beta and requires using the v4beta base URL.
-
-        API Documentation: https://techdocs.akamai.com/linode-api/reference/get-notification-channel
-
-        :param channel_id: The ID of the alert channel to retrieve.
-        :type channel_id: int
-
-        :returns: The requested :class:`AlertChannel` object.
-        :rtype: AlertChannel
-        :raises ApiError: if the requested channel could not be loaded.
-        """
-        return self.client.load(AlertChannel, channel_id)
-
     def alert_channel_alerts(self, channel_id: int, *filters) -> PaginatedList:
         """
         Retrieve all alerts associated with a specific alert channel.
@@ -499,8 +480,6 @@ def alert_channel_alerts(self, channel_id: int, *filters) -> PaginatedList:
         specified alert channel. This allows you to see which alert definitions
         are configured to notify this specific channel.
 
-        .. note:: This endpoint is in beta and requires using the v4beta base URL.
-
         API Documentation: https://techdocs.akamai.com/linode-api/reference/get-notification-channel-alerts
 
         :param channel_id: The ID of the alert channel to retrieve alerts for.
@@ -512,14 +491,13 @@ def alert_channel_alerts(self, channel_id: int, *filters) -> PaginatedList:
         :rtype: PaginatedList[AlertDefinition]
         """
         endpoint = f"/monitor/alert-channels/{channel_id}/alerts"
+
+        # Build filter dict if filters provided
         parsed_filters = None
         if filters:
-            if len(filters) > 1:
-                parsed_filters = and_(
-                    *filters
-                ).dct  # pylint: disable=no-value-for-parameter
-            else:
-                parsed_filters = filters[0].dct
+            parsed_filters = (
+                and_(*filters).dct if len(filters) > 1 else filters[0].dct
+            )
 
         response_json = self.client.get(endpoint, filters=parsed_filters)
 
@@ -529,20 +507,18 @@ def alert_channel_alerts(self, channel_id: int, *filters) -> PaginatedList:
                 json=response_json,
             )
 
-        # For each alert definition in the response, extract the service_type
-        # and use it as the parent_id when creating AlertDefinition objects
-        result = []
-        for obj in response_json.get("data", []):
-            if "id" in obj and "service_type" in obj:
-                alert = AlertDefinition.make_instance(
-                    obj["id"],
-                    self.client,
-                    parent_id=obj["service_type"],
-                    json=obj,
-                )
-                result.append(alert)
-
-        # Return paginated list with pagination metadata from response
+        # Create AlertDefinition objects with proper parent_id (service_type)
+        result = [
+            AlertDefinition.make_instance(
+                obj["id"],
+                self.client,
+                parent_id=obj["service_type"],
+                json=obj,
+            )
+            for obj in response_json.get("data", [])
+            if "id" in obj and "service_type" in obj
+        ]
+
         return PaginatedList(
             self.client,
             endpoint[1:],

test/integration/models/monitor/test_monitor.py

@@ -245,25 +245,36 @@ def test_integration_create_get_update_delete_alert_definition(
     label = f"{label}-{int(time.time())}"
     description = "E2E alert created by SDK integration test"
 
-    # Pick an existing alert channel to attach to the definition; skip if none
-    channels = list(
-        client.monitor.alert_channels()
-    )  # TODO: create channel instead of relying on pre-existing one
-    if not channels:
-        pytest.skip(
-            "No alert channels available on account for creating alert definitions"
-        )
+    # Get valid users to create an alert channel for the alert definition
+    users = list(client.account.users())
+    if len(users) == 0:
+        pytest.skip("No account users available for creating alert channels")
+
+    # Use the first user for the alert channel
+    usernames = [users[0].username]
 
     created = None
+    created_channel = None
 
     try:
+        # Create a new alert channel for this test
+        created_channel = client.monitor.channel_create(
+            label=f"{get_test_label()}-channel-{int(time.time())}",
+            channel_type="email",
+            details=ChannelDetails(
+                email=EmailDetails(
+                    recipient_type="user",
+                    usernames=usernames,
+                )
+            ),
+        )
         # Create the alert definition using API-compliant top-level fields
         created = client.monitor.create_alert_definition(
             service_type=service_type,
             label=label,
             severity=1,
             description=description,
-            channel_ids=[channels[0].id],
+            channel_ids=[created_channel.id],
             rule_criteria=rule_criteria,
             trigger_conditions=trigger_conditions,
         )
@@ -293,6 +304,15 @@ def test_integration_create_get_update_delete_alert_definition(
                 AlertDefinition, created.id, service_type
             )
             delete_alert.delete()
+        if created_channel:
+            # Clean up the created channel
+            try:
+                created_channel.delete()
+            except Exception as e:
+                # Log but don't fail if cleanup fails
+                print(
+                    f"Warning: Failed to delete channel {created_channel.id}: {e}"
+                )
 
 
 def test_alert_definition_entities(test_linode_client):
@@ -337,8 +357,7 @@ def test_integration_create_get_update_delete_alert_channel(test_linode_client):
     working end-to-end against the actual API.
     """
     client = test_linode_client
-    label = get_test_label() + "-e2e-channel"
-    label = f"{label}-{int(time.time())}"
+    label = "pythonsdk-alert-channel-test"
 
     created_channel = None
 
@@ -413,31 +432,6 @@ def test_integration_create_get_update_delete_alert_channel(test_linode_client):
                 )
 
 
-def test_integration_alert_channel(test_linode_client):
-    """Test retrieving a single alert channel by ID.
-
-    This test fetches an existing alert channel and verifies that all
-    expected properties are populated correctly.
-    """
-    client = test_linode_client
-
-    # Get an existing alert channel to test with
-    channels = list(client.monitor.alert_channels())
-    if len(channels) == 0:
-        pytest.skip("No alert channels available on account for testing")
-
-    channel_id = channels[0].id
-
-    # Test the alert_channel() method
-    fetched_channel = client.monitor.alert_channel(channel_id)
-
-    assert isinstance(fetched_channel, AlertChannel)
-    assert fetched_channel.id == channel_id
-    assert fetched_channel.label is not None
-    assert fetched_channel.channel_type is not None
-    assert fetched_channel.details is not None
-
-
 def test_integration_alert_channel_alerts(test_linode_client):
     """Test retrieving alerts associated with a specific alert channel.
 

test/unit/groups/monitor_api_test.py

@@ -258,50 +258,6 @@ def test_create_update_delete_alert_channel(self):
             assert mock_delete.call_url == channel_url
             assert result is True
 
-    def test_alert_channel(self):
-        """
-        Test retrieval of a specific alert channel by ID.
-        Verifies the alert_channel method returns a single AlertChannel object.
-        """
-        channel_id = 123
-        channel_url = f"/monitor/alert-channels/{channel_id}"
-
-        channel_response = {
-            "id": channel_id,
-            "label": "alert notification channel",
-            "type": "user",
-            "channel_type": "email",
-            "details": {
-                "email": {
-                    "usernames": ["admin-user1", "admin-user2"],
-                    "recipient_type": "user",
-                }
-            },
-            "alerts": {
-                "url": f"{channel_url}/alerts",
-                "type": "alerts-definitions",
-                "alert_count": 2,
-            },
-            "created": "2024-01-01T00:00:00",
-            "updated": "2024-01-01T00:00:00",
-            "created_by": "tester",
-            "updated_by": "tester",
-        }
-
-        with self.mock_get(channel_response) as mock_get:
-            channel = self.client.monitor.alert_channel(channel_id=channel_id)
-
-            assert mock_get.call_url == channel_url
-            assert isinstance(channel, AlertChannel)
-            assert channel.id == channel_id
-            assert channel.label == "alert notification channel"
-            assert channel.channel_type == "email"
-            assert channel.details.email.usernames == [
-                "admin-user1",
-                "admin-user2",
-            ]
-            assert channel.alerts.alert_count == 2
-
     def test_alert_channel_alerts(self):
         """
         Test retrieval of alerts associated with a specific alert channel.