Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
108 changes: 108 additions & 0 deletions UnleashClient/_transport.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
"""
HTTP layer under the Unleash API client: sends one request and returns the
response.
"""

from dataclasses import dataclass, field
from typing import Any, Mapping, Optional, Protocol, runtime_checkable

import requests
from requests.adapters import HTTPAdapter
from urllib3 import Retry

from UnleashClient.constants import REQUEST_TIMEOUT


@dataclass(frozen=True, kw_only=True)
class Request:
"""
HTTP request.

:param method: ``"GET"`` or ``"POST"``.
:param url: Absolute url, without the query string.
:param headers: Headers to send.
:param params: Query string parameters.
:param body: Request body, or None to send none.
"""

method: str
url: str
headers: Mapping[str, str]
params: Mapping[str, str] = field(default_factory=dict)
body: Optional[bytes] = None


@dataclass(frozen=True, kw_only=True)
class Response:
"""
HTTP response. Every status is a response, not an error: deciding what
a status means is the API client's job.

:param status: HTTP status code.
:param headers: Response headers, looked up case-insensitively.
:param body: Raw response body.
"""

status: int
headers: Mapping[str, str]
body: bytes
Comment on lines +46 to +48

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is a legit concern, but transport is private at this point, and the case insensivity problem is fixed in #473, #473 fixes it in the API client rather than in Response



@runtime_checkable
class Transport(Protocol):
"""Sends requests for the sync client."""

def execute(self, request: Request) -> Response:
"""
Send ``request`` once and return the response, whatever its status.

:raises Exception: only when no response arrived, e.g. a connection
error or a timeout.
"""


class RequestsTransport:
"""
Sends requests with ``requests``, on a fresh session for every request.

:param timeout: Seconds to wait for the connection and for each read.
:param _retries: GET requests are retried this many times on a connection
error or a 500, 502 or 504, by urllib3.
:param _request_kwargs: extra keyword arguments for every ``requests`` call.
"""

def __init__(
self,
*,
timeout: float = REQUEST_TIMEOUT,
# these two are are going away too
_retries: Optional[int] = None,
_request_kwargs: Optional[Mapping[str, Any]] = None,
) -> None:
self._timeout = timeout
self._retries = _retries
self._request_kwargs = dict(_request_kwargs or {})

def execute(self, request: Request) -> Response:
kwargs: dict = {"headers": dict(request.headers), "timeout": self._timeout}
if request.params:
kwargs["params"] = dict(request.params)
if request.body is not None:
kwargs["data"] = request.body

with requests.Session() as session:
if request.method == "GET" and self._retries is not None:
adapter = HTTPAdapter(
max_retries=Retry(
total=self._retries, status_forcelist=[500, 502, 504]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

legit, and this is no behavior change at this point, urllib3's Retry raises RetryError once retries run out on a 5xx
in #472 removes the old retry mechanism and for both clients the new shared retry returns the last response after it ran out

)
)
session.mount("https://", adapter)
session.mount("http://", adapter)

resp = session.request(
request.method, request.url, **kwargs, **self._request_kwargs
)
return Response(
status=resp.status_code, headers=resp.headers, body=resp.content
)
90 changes: 50 additions & 40 deletions UnleashClient/_unleash_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
from typing import Any, Dict, NamedTuple, Optional, Tuple

import requests
from requests import Response
from requests.adapters import HTTPAdapter
from requests.exceptions import InvalidHeader, InvalidSchema, InvalidURL, MissingSchema
from urllib3 import Retry

from UnleashClient._event_dispatcher import _EventDispatcher
from UnleashClient._headers import _HeaderFactory
from UnleashClient._transport import Request, RequestsTransport, Response, Transport
from UnleashClient.config import PollingOptions, UnleashConfig
from UnleashClient.constants import FEATURES_URL, METRICS_URL, REGISTER_URL
from UnleashClient.events import (
Expand Down Expand Up @@ -78,31 +76,47 @@ def _normalized_url(url: str, path: str) -> str:
return f"{url}{path}"


def _text(resp: Response) -> str:
return resp.body.decode("utf-8", errors="replace")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i am not entirely sure how much of a real life scenario this is, request also uses utf-8 be default and I think unleash server also sends utf-8
@ppicom maybe you can help me out with the legitimacy of this?
if yes, I will fix it in a separate PR



def _log_resp_info(resp: Response) -> None:
LOGGER.debug("HTTP status code: %s", resp.status_code)
LOGGER.debug("HTTP status code: %s", resp.status)
LOGGER.debug("HTTP headers: %s", resp.headers)
LOGGER.debug("HTTP content: %s", resp.text)
LOGGER.debug("HTTP content: %s", _text(resp))


class _UnleashApiClient:
"""
Sends the SDK's requests to the Unleash server: feature fetches, client
registration and metrics submission.
Talks to the Unleash API: feature fetches, client registration and metrics
submission. Builds each request and decides what its response means; the
transport only sends it.

:param config: Client configuration.
:param headers: Builds the header set each endpoint needs.
:param events: Receives ``FETCH_COMPLETED`` after every feature fetch.
:param transport: Sends the requests. Defaults to a :class:`RequestsTransport`
built from ``config.http``.
"""

def __init__(
self,
config: UnleashConfig,
headers: _HeaderFactory,
events: Optional[_EventDispatcher] = None,
transport: Optional[Transport] = None,
) -> None:
self._config: UnleashConfig = config
self._headers: _HeaderFactory = headers
self._events: Optional[_EventDispatcher] = events

self._transport: Transport = transport or RequestsTransport(
timeout=config.http.timeout,
_retries=config.http.retries,
# this is going to go away soon, just like the whole httpOptions in the client constructor
_request_kwargs=config.http.options,
)

# pylint: disable=broad-except
def fetch_features(self, etag: str = "", *, options: PollingOptions) -> FetchResult:
"""
Expand Down Expand Up @@ -146,31 +160,24 @@ def _fetch_features(
if options.project_name:
base_params = {"project": options.project_name}

adapter = HTTPAdapter(
max_retries=Retry(
total=config.http.retries, status_forcelist=[500, 502, 504]
)
)
with requests.Session() as session:
session.mount("https://", adapter)
session.mount("http://", adapter)
resp = session.get(
base_url,
resp = self._transport.execute(
Request(
method="GET",
url=base_url,
headers={
**self._headers.polling(options),
**request_specific_headers,
},
params=base_params,
timeout=config.http.timeout,
**config.http.options,
)
)

http_status_code = resp.status_code
if resp.status_code not in [200, 304]:
http_status_code = resp.status
if resp.status not in [200, 304]:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client feature fetch failed due to unexpected HTTP status code: %s",
resp.status_code,
resp.status,
)
raise Exception(
"Unleash Client feature fetch failed!"
Expand All @@ -180,13 +187,14 @@ def _fetch_features(
if "etag" in resp.headers.keys():
fetched_etag = resp.headers["etag"]

if resp.status_code == 304:
if resp.status == 304:
return (
FetchResult(None, fetched_etag, not_modified=True),
FetchStatus.NOT_CHANGED,
)

return FetchResult(resp.text, fetched_etag), _fetched_status(resp.text)
raw_state = _text(resp)
return FetchResult(raw_state, fetched_etag), _fetched_status(raw_state)
except Exception as exc:
LOGGER.exception(
"Unleash Client feature fetch failed due to exception: %s", exc
Expand All @@ -212,19 +220,20 @@ def register(self, payload: Dict[str, Any]) -> bool:
LOGGER.info("Registering unleash client with unleash @ %s", config.url)
LOGGER.info("Registration request information: %s", payload)

resp = requests.post(
_normalized_url(config.url, REGISTER_URL),
data=json.dumps(payload),
headers=self._headers.base(),
timeout=config.http.timeout,
**config.http.options,
resp = self._transport.execute(
Request(
method="POST",
url=_normalized_url(config.url, REGISTER_URL),
headers=self._headers.base(),
body=json.dumps(payload).encode("utf-8"),
)
)

if resp.status_code not in {200, 202}:
if resp.status not in {200, 202}:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client registration failed due to unexpected HTTP status code: %s",
resp.status_code,
resp.status,
)
return False

Expand Down Expand Up @@ -255,19 +264,20 @@ def send_metrics(self, payload: Dict[str, Any]) -> bool:
LOGGER.info("Sending messages to with unleash @ %s", config.url)
LOGGER.info("unleash metrics information: %s", payload)

resp = requests.post(
_normalized_url(config.url, METRICS_URL),
data=json.dumps(payload),
headers=self._headers.metrics(),
timeout=config.http.timeout,
**config.http.options,
resp = self._transport.execute(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is also legit and also fixed in a later PR
i tried to keep changes as small as possible, so some PRs are kind of intermediary states :/

register() and send_metrics() log and return False, and fetch reports UNAVAILABLE in case of any error from execute()

Request(
method="POST",
url=_normalized_url(config.url, METRICS_URL),
headers=self._headers.metrics(),
body=json.dumps(payload).encode("utf-8"),
)
)

if resp.status_code != 202:
if resp.status != 202:
_log_resp_info(resp)
LOGGER.warning(
"Unleash Client metrics submission due to unexpected HTTP status code: %s",
resp.status_code,
resp.status,
)
return False

Expand Down
Loading
Loading