Skip to content
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

♻️ Slight changes to get_client and to_internal_data #47

Merged
merged 2 commits into from
Nov 29, 2024
Merged

♻️ Slight changes to get_client and to_internal_data #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78a8324
:recycle: Slight changes to get_client and to_internal_data
stevenbal Nov 28, 2024
83d4d60
:bug: If no Notifications client is configured, ignore checks
stevenbal Nov 28, 2024
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
21 changes: 12 additions & 9 deletions vng_api_common/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
"""

import logging
from typing import Any, Optional

from django.conf import settings
from django.utils.module_loading import import_string

from ape_pie import APIClient
from requests import JSONDecodeError, RequestException, Response
Expand All @@ -18,6 +14,10 @@ class ClientError(RuntimeError):
pass


class NoServiceConfigured(RuntimeError):
pass


# TODO: use more approriate method name
def to_internal_data(response: Response) -> dict | list | None:
try:
Expand All @@ -33,7 +33,6 @@ def to_internal_data(response: Response) -> dict | list | None:
raise
raise ClientError(response_json if response_json is not None else {}) from exc

assert response_json
return response_json


Expand All @@ -56,18 +55,22 @@ def request(
return super().request(method, url, *args, **kwargs)


def get_client(url: str) -> Client | None:
def get_client(
url: str, raise_exceptions: bool = False, **client_kwargs
) -> Client | None:
"""
Get a client instance for the given URL.
If no suitable client is found, ``None`` is returned.
"""
from zgw_consumers.client import build_client
from zgw_consumers.models import Service

service: Optional[Service] = Service.get_service(url)
service: Service | None = Service.get_service(url)

if not service:
logger.warning(f"No service configured for {url}")
return None
if raise_exceptions:
raise NoServiceConfigured(f"{url} API should be added to Service model")
return

return build_client(service, client_factory=Client)
return build_client(service, client_factory=Client, **client_kwargs)
3 changes: 3 additions & 0 deletions vng_api_common/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ def _test_nrc_config(check_autorisaties_subscription=True) -> list:
nrc_config = NotificationsConfig.get_solo()
nrc_client: Optional[Client] = NotificationsConfig.get_client()

if not nrc_client:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it might be nice to return early here, looking at the code it currently returns this code whenever no client was configured. Unless we want to have different behavior I think we should return that here too or remove this path which returns early.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this because I saw the change here: open-zaak/open-zaak@08c7ccd

Though I'm not sure if we need it at all?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed on slack: replaced it with the current behavior in OZ

return [((_("NRC"), _("Missing"), False))]

has_nrc_auth = nrc_client.auth is not None if nrc_client else False

if not nrc_config.notifications_api_service:
Expand Down