IHS-154 Deprecate using raise_for_error = False (#508)

Author: gmazoyer

changelog/493.changed.md

@@ -0,0 +1 @@
+Deprecate the use of `raise_for_error=False` across several methods, using a try/except pattern is preferred.

infrahub_sdk/client.py

@@ -4,6 +4,7 @@
 import copy
 import logging
 import time
+import warnings
 from collections.abc import Coroutine, MutableMapping
 from functools import wraps
 from time import sleep
@@ -105,6 +106,15 @@ def wrapper(client: InfrahubClientSync, *args: Any, **kwargs: Any) -> httpx.Resp
     return wrapper
 
 
+def raise_for_error_deprecation_warning(value: bool | None) -> None:
+    if value is not None:
+        warnings.warn(
+            "Using `raise_for_error` is deprecated, use `try/except` to handle errors.",
+            DeprecationWarning,
+            stacklevel=1,
+        )
+
+
 class BaseClient:
     """Base class for InfrahubClient and InfrahubClientSync"""
 
@@ -881,7 +891,7 @@ async def execute_graphql(
         branch_name: str | None = None,
         at: str | Timestamp | None = None,
         timeout: int | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
         tracker: str | None = None,
     ) -> dict:
         """Execute a GraphQL query (or mutation).
@@ -893,13 +903,17 @@ async def execute_graphql(
             branch_name (str, optional): Name of the branch on which the query will be executed. Defaults to None.
             at (str, optional): Time when the query should be executed. Defaults to None.
             timeout (int, optional): Timeout in second for the query. Defaults to None.
-            raise_for_error (bool, optional): Flag to indicate that we need to raise an exception if the response has some errors. Defaults to True.
+            raise_for_error (bool | None, optional): Deprecated. Controls only HTTP status handling.
+                - None (default) or True: HTTP errors raise via resp.raise_for_status().
+                - False: HTTP errors are not automatically raised. Defaults to None.
+
         Raises:
-            GraphQLError: _description_
+            GraphQLError: When the GraphQL response contains errors.
 
         Returns:
-            _type_: _description_
+            dict: The GraphQL data payload (response["data"]).
         """
+        raise_for_error_deprecation_warning(value=raise_for_error)
 
         branch_name = branch_name or self.default_branch
         url = self._graphql_url(branch_name=branch_name, at=at)
@@ -922,7 +936,7 @@ async def execute_graphql(
             try:
                 resp = await self._post(url=url, payload=payload, headers=headers, timeout=timeout)
 
-                if raise_for_error:
+                if raise_for_error in (None, True):
                     resp.raise_for_status()
 
                 retry = False
@@ -1099,8 +1113,10 @@ async def query_gql_query(
         at: str | None = None,
         timeout: int | None = None,
         tracker: str | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
     ) -> dict:
+        raise_for_error_deprecation_warning(value=raise_for_error)
+
         url = f"{self.address}/api/query/{name}"
         url_params = copy.deepcopy(params or {})
         headers = copy.copy(self.headers or {})
@@ -1145,7 +1161,7 @@ async def query_gql_query(
             timeout=timeout or self.default_timeout,
         )
 
-        if raise_for_error:
+        if raise_for_error in (None, True):
             resp.raise_for_status()
 
         return decode_json(response=resp)
@@ -1155,7 +1171,7 @@ async def get_diff_summary(
         branch: str,
         timeout: int | None = None,
         tracker: str | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
     ) -> list[NodeDiff]:
         query = get_diff_summary_query()
         response = await self.execute_graphql(
@@ -1220,7 +1236,7 @@ async def allocate_next_ip_address(
         branch: str | None = ...,
         timeout: int | None = ...,
         tracker: str | None = ...,
-        raise_for_error: bool = ...,
+        raise_for_error: bool | None = ...,
     ) -> SchemaType: ...
 
     @overload
@@ -1265,7 +1281,7 @@ async def allocate_next_ip_address(
         branch: str | None = ...,
         timeout: int | None = ...,
         tracker: str | None = ...,
-        raise_for_error: bool = ...,
+        raise_for_error: bool | None = ...,
     ) -> CoreNode | None: ...
 
     async def allocate_next_ip_address(
@@ -1279,7 +1295,7 @@ async def allocate_next_ip_address(
         branch: str | None = None,
         timeout: int | None = None,
         tracker: str | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
     ) -> CoreNode | SchemaType | None:
         """Allocate a new IP address by using the provided resource pool.
 
@@ -1292,7 +1308,7 @@ async def allocate_next_ip_address(
             branch (str, optional): Name of the branch to allocate from. Defaults to default_branch.
             timeout (int, optional): Flag to indicate whether to populate the store with the retrieved nodes.
             tracker (str, optional): The offset for pagination.
-            raise_for_error (bool, optional): The limit for pagination.
+            raise_for_error (bool, optional): Deprecated, raise an error if the HTTP status is not 2XX.
         Returns:
             InfrahubNode: Node corresponding to the allocated resource.
         """
@@ -1367,7 +1383,7 @@ async def allocate_next_ip_prefix(
         branch: str | None = ...,
         timeout: int | None = ...,
         tracker: str | None = ...,
-        raise_for_error: bool = ...,
+        raise_for_error: bool | None = ...,
     ) -> SchemaType: ...
 
     @overload
@@ -1415,7 +1431,7 @@ async def allocate_next_ip_prefix(
         branch: str | None = ...,
         timeout: int | None = ...,
         tracker: str | None = ...,
-        raise_for_error: bool = ...,
+        raise_for_error: bool | None = ...,
     ) -> CoreNode | None: ...
 
     async def allocate_next_ip_prefix(
@@ -1430,7 +1446,7 @@ async def allocate_next_ip_prefix(
         branch: str | None = None,
         timeout: int | None = None,
         tracker: str | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
     ) -> CoreNode | SchemaType | None:
         """Allocate a new IP prefix by using the provided resource pool.
 
@@ -1444,7 +1460,7 @@ async def allocate_next_ip_prefix(
             branch: Name of the branch to allocate from. Defaults to default_branch.
             timeout: Flag to indicate whether to populate the store with the retrieved nodes.
             tracker: The offset for pagination.
-            raise_for_error: The limit for pagination.
+            raise_for_error (bool, optional): Deprecated, raise an error if the HTTP status is not 2XX.
         Returns:
             InfrahubNode: Node corresponding to the allocated resource.
         """
@@ -1625,7 +1641,7 @@ def execute_graphql(
         branch_name: str | None = None,
         at: str | Timestamp | None = None,
         timeout: int | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
         tracker: str | None = None,
     ) -> dict:
         """Execute a GraphQL query (or mutation).
@@ -1637,13 +1653,18 @@ def execute_graphql(
             branch_name (str, optional): Name of the branch on which the query will be executed. Defaults to None.
             at (str, optional): Time when the query should be executed. Defaults to None.
             timeout (int, optional): Timeout in second for the query. Defaults to None.
-            raise_for_error (bool, optional): Flag to indicate that we need to raise an exception if the response has some errors. Defaults to True.
+            raise_for_error (bool | None, optional): Deprecated. Controls only HTTP status handling.
+                - None (default) or True: HTTP errors raise via `resp.raise_for_status()`.
+                - False: HTTP errors are not automatically raised.
+              GraphQL errors always raise `GraphQLError`. Defaults to None.
+
         Raises:
-            GraphQLError: When an error occurs during the execution of the GraphQL query or mutation.
+            GraphQLError: When the GraphQL response contains errors.
 
         Returns:
-            dict: The result of the GraphQL query or mutation.
+            dict: The GraphQL data payload (`response["data"]`).
         """
+        raise_for_error_deprecation_warning(value=raise_for_error)
 
         branch_name = branch_name or self.default_branch
         url = self._graphql_url(branch_name=branch_name, at=at)
@@ -1666,7 +1687,7 @@ def execute_graphql(
             try:
                 resp = self._post(url=url, payload=payload, headers=headers, timeout=timeout)
 
-                if raise_for_error:
+                if raise_for_error in (None, True):
                     resp.raise_for_status()
 
                 retry = False
@@ -2238,8 +2259,10 @@ def query_gql_query(
         at: str | None = None,
         timeout: int | None = None,
         tracker: str | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
     ) -> dict:
+        raise_for_error_deprecation_warning(value=raise_for_error)
+
         url = f"{self.address}/api/query/{name}"
         url_params = copy.deepcopy(params or {})
         headers = copy.copy(self.headers or {})
@@ -2283,7 +2306,7 @@ def query_gql_query(
             timeout=timeout or self.default_timeout,
         )
 
-        if raise_for_error:
+        if raise_for_error in (None, True):
             resp.raise_for_status()
 
         return decode_json(response=resp)
@@ -2293,7 +2316,7 @@ def get_diff_summary(
         branch: str,
         timeout: int | None = None,
         tracker: str | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
     ) -> list[NodeDiff]:
         query = get_diff_summary_query()
         response = self.execute_graphql(
@@ -2358,7 +2381,7 @@ def allocate_next_ip_address(
         branch: str | None = ...,
         timeout: int | None = ...,
         tracker: str | None = ...,
-        raise_for_error: bool = ...,
+        raise_for_error: bool | None = ...,
     ) -> SchemaTypeSync: ...
 
     @overload
@@ -2403,7 +2426,7 @@ def allocate_next_ip_address(
         branch: str | None = ...,
         timeout: int | None = ...,
         tracker: str | None = ...,
-        raise_for_error: bool = ...,
+        raise_for_error: bool | None = ...,
     ) -> CoreNodeSync | None: ...
 
     def allocate_next_ip_address(
@@ -2417,7 +2440,7 @@ def allocate_next_ip_address(
         branch: str | None = None,
         timeout: int | None = None,
         tracker: str | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
     ) -> CoreNodeSync | SchemaTypeSync | None:
         """Allocate a new IP address by using the provided resource pool.
 
@@ -2501,7 +2524,7 @@ def allocate_next_ip_prefix(
         branch: str | None = ...,
         timeout: int | None = ...,
         tracker: str | None = ...,
-        raise_for_error: bool = ...,
+        raise_for_error: bool | None = ...,
     ) -> SchemaTypeSync: ...
 
     @overload
@@ -2549,7 +2572,7 @@ def allocate_next_ip_prefix(
         branch: str | None = ...,
         timeout: int | None = ...,
         tracker: str | None = ...,
-        raise_for_error: bool = ...,
+        raise_for_error: bool | None = ...,
     ) -> CoreNodeSync | None: ...
 
     def allocate_next_ip_prefix(
@@ -2564,7 +2587,7 @@ def allocate_next_ip_prefix(
         branch: str | None = None,
         timeout: int | None = None,
         tracker: str | None = None,
-        raise_for_error: bool = True,
+        raise_for_error: bool | None = None,
     ) -> CoreNodeSync | SchemaTypeSync | None:
         """Allocate a new IP prefix by using the provided resource pool.