Skip to content

IHS-154 Deprecate using raise_for_error = False #508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Parent: Merge develop into infrahub-develop
merged 10 commits into from
Sep 1, 2025
Merged

IHS-154 Deprecate using raise_for_error = False #508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
88d0217
Fix GraphQLError being raised when `raise_for_error` is `False`
gmazoyer Aug 25, 2025
003fb3a
Add simple test for coverage
gmazoyer Aug 25, 2025
b78d6ac
Mark `raise_for_error` as deprecated
gmazoyer Aug 26, 2025
25a32a2
Add changelog record
gmazoyer Aug 26, 2025
261a13e
Make `raise_for_error` optional
gmazoyer Aug 27, 2025
afe5372
Improve docstring
gmazoyer Aug 27, 2025
1de55f6
Improve docstring
gmazoyer Aug 27, 2025
d7befe2
Fix typo
gmazoyer Aug 27, 2025
494298f
Make more `raise_for_error` optional
gmazoyer Aug 28, 2025
74dffe1
Tweak changelog record
gmazoyer Aug 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/493.changed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecate the use of `raise_for_error=False` across several methods, using a try/except pattern is preferred.
83 changes: 53 additions & 30 deletions infrahub_sdk/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"""

Expand Down Expand Up @@ -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).
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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 {})
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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.

Expand All @@ -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.
"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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.

Expand All @@ -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.
"""
Expand Down Expand Up @@ -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).
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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 {})
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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.

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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.

Expand Down