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

Jump to bottom

feat: add odp rest api manager #398

Merged
merged 9 commits into from
Aug 12, 2022
Merged

feat: add odp rest api manager #398

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f6a767f
feat: add odp rest api manager
Mat001 Aug 8, 2022
0d254cb
fix: fix linting, str type
Mat001 Aug 8, 2022
92099d4
fix white space
Mat001 Aug 8, 2022
c9db45a
addressed PR comments
Mat001 Aug 10, 2022
94e546b
moved helper test funciton to base.py
Mat001 Aug 10, 2022
14a1f82
fix graphql tests becasue helper method moved to base.py
Mat001 Aug 10, 2022
74b2ff8
remove unnecessary url parsing exceptions
Mat001 Aug 11, 2022
0d541b5
remove print statement
Mat001 Aug 11, 2022
0c669af
fixed type hints
Mat001 Aug 11, 2022
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
42 changes: 24 additions & 18 deletions optimizely/odp/zaius_rest_api_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from typing import Optional, List

import requests
from requests.exceptions import RequestException, ConnectionError, Timeout, JSONDecodeError, InvalidURL
from requests.exceptions import RequestException, ConnectionError, Timeout

from optimizely import logger as optimizely_logger
from optimizely.helpers.enums import Errors, OdpRestApiConfig
Expand All @@ -43,7 +43,7 @@ class ZaiusRestApiManager:
def __init__(self, logger: Optional[optimizely_logger.Logger] = None):
self.logger = logger or optimizely_logger.NoOpLogger()

def sendOdpEvents(self, api_key: str, api_host: str, events: List[OdpEvent]) -> Optional[bool]:
def send_odp_events(self, api_key: str, api_host: str, events: List[OdpEvent]) -> bool:
"""
Dispatch the event being represented by the OdpEvent object.

Expand All @@ -55,34 +55,40 @@ def sendOdpEvents(self, api_key: str, api_host: str, events: List[OdpEvent]) ->
Returns:
retry is True - if network or server error (5xx), otherwise False
"""
can_retry: bool = True
should_retry: bool = False
url = f'{api_host}/v3/events'
request_headers = {'content-type': 'application/json', 'x-api-key': api_key}

try:
payload_dict = json.dumps(events)
except TypeError as err:
self.logger.error(Errors.ODP_EVENT_FAILED.format(err))
return should_retry

try:
response = requests.post(url=url,
headers=request_headers,
data=json.dumps(events),
data=payload_dict,
timeout=OdpRestApiConfig.REQUEST_TIMEOUT)

response.raise_for_status()
can_retry = False

except (ConnectionError, Timeout):
self.logger.error(Errors.ODP_EVENT_FAILED.format('network error'))
# we do retry, can_retry = True
except JSONDecodeError:
self.logger.error(Errors.ODP_EVENT_FAILED.format('JSON decode error'))
can_retry = False
except InvalidURL:
self.logger.error(Errors.ODP_EVENT_FAILED.format('invalid URL'))
can_retry = False
# retry on network errors
should_retry = True
except RequestException as err:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think so, but can you confirm that this catches all other exceptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jaeopt yes, confirm. We use the same exception handling in event_dispatcher and fetch_datafile:
https://github.com/optimizely/python-sdk/blob/master/optimizely/event_dispatcher.py#L56
https://github.com/optimizely/python-sdk/blob/master/optimizely/config_manager.py#L367

It'll catch all these: https://requests.readthedocs.io/en/latest/_modules/requests/exceptions/

if 400 <= err.response.status_code < 500:
self.logger.error(Errors.ODP_EVENT_FAILED.format(err))
can_retry = False
if err.response is not None:
if 400 <= err.response.status_code < 500:
# log 4xx
self.logger.error(Errors.ODP_EVENT_FAILED.format(err.response.text))
else:
# log 5xx
self.logger.error(Errors.ODP_EVENT_FAILED.format(err))
# retry on 500 exceptions
should_retry = True
else:
# log exceptions without response body (i.e. invalid url)
self.logger.error(Errors.ODP_EVENT_FAILED.format(err))
# we do retry, can_retry = True
finally:
return can_retry

return should_retry
17 changes: 17 additions & 0 deletions tests/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

import json
import unittest
from typing import Optional

from requests import Response

from optimizely import optimizely

Expand All @@ -28,6 +31,20 @@ def assertStrictTrue(self, to_assert):
def assertStrictFalse(self, to_assert):
self.assertIs(to_assert, False)

def fake_server_response(self, status_code: int = None, content: Optional[str] = None,
url: str = None) -> Optional[Response]:
"""Mock the server response."""
response = Response()

if status_code:
response.status_code = status_code
if content:
response._content = content.encode('utf-8')
if url:
response.url = url

return response

def setUp(self, config_dict='config_dict'):
self.config_dict = {
'revision': '42',
Expand Down
26 changes: 0 additions & 26 deletions tests/helpers_for_tests.py
View file
Open in desktop

This file was deleted.

33 changes: 16 additions & 17 deletions tests/test_odp_zaius_graphql_api_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

from requests import exceptions as request_exception

from tests.helpers_for_tests import fake_server_response
from optimizely.helpers.enums import OdpGraphQLApiConfig
from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager
from . import base
Expand Down Expand Up @@ -50,7 +49,7 @@ def test_fetch_qualified_segments__valid_request(self):
def test_fetch_qualified_segments__success(self):
with mock.patch('requests.post') as mock_request_post:
mock_request_post.return_value = \
fake_server_response(status_code=200, content=self.good_response_data)
self.fake_server_response(status_code=200, content=self.good_response_data)

api = ZaiusGraphQLApiManager()
response = api.fetch_segments(api_key=self.api_key,
Expand All @@ -65,7 +64,7 @@ def test_fetch_qualified_segments__node_missing(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = \
fake_server_response(status_code=200, content=self.node_missing_response_data)
self.fake_server_response(status_code=200, content=self.node_missing_response_data)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand All @@ -81,8 +80,8 @@ def test_fetch_qualified_segments__mixed_missing_keys(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = \
fake_server_response(status_code=200,
content=self.mixed_missing_keys_response_data)
self.fake_server_response(status_code=200,
content=self.mixed_missing_keys_response_data)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand All @@ -97,7 +96,7 @@ def test_fetch_qualified_segments__mixed_missing_keys(self):
def test_fetch_qualified_segments__success_with_empty_segments(self):
with mock.patch('requests.post') as mock_request_post:
mock_request_post.return_value = \
fake_server_response(status_code=200, content=self.good_empty_response_data)
self.fake_server_response(status_code=200, content=self.good_empty_response_data)

api = ZaiusGraphQLApiManager()
response = api.fetch_segments(api_key=self.api_key,
Expand All @@ -112,8 +111,8 @@ def test_fetch_qualified_segments__invalid_identifier(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = \
fake_server_response(status_code=200,
content=self.invalid_identifier_response_data)
self.fake_server_response(status_code=200,
content=self.invalid_identifier_response_data)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand All @@ -129,7 +128,7 @@ def test_fetch_qualified_segments__other_exception(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = \
fake_server_response(status_code=200, content=self.other_exception_response_data)
self.fake_server_response(status_code=200, content=self.other_exception_response_data)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand All @@ -145,7 +144,7 @@ def test_fetch_qualified_segments__bad_response(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = \
fake_server_response(status_code=200, content=self.bad_response_data)
self.fake_server_response(status_code=200, content=self.bad_response_data)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand All @@ -161,7 +160,7 @@ def test_fetch_qualified_segments__name_invalid(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = \
fake_server_response(status_code=200, content=self.name_invalid_response_data)
self.fake_server_response(status_code=200, content=self.name_invalid_response_data)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand All @@ -176,8 +175,8 @@ def test_fetch_qualified_segments__name_invalid(self):
def test_fetch_qualified_segments__invalid_key(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = fake_server_response(status_code=200,
content=self.invalid_edges_key_response_data)
mock_request_post.return_value = self.fake_server_response(status_code=200,
content=self.invalid_edges_key_response_data)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand All @@ -192,8 +191,8 @@ def test_fetch_qualified_segments__invalid_key(self):
def test_fetch_qualified_segments__invalid_key_in_error_body(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = fake_server_response(status_code=200,
content=self.invalid_key_for_error_response_data)
mock_request_post.return_value = self.fake_server_response(status_code=200,
content=self.invalid_key_for_error_response_data)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand Down Expand Up @@ -223,7 +222,7 @@ def test_fetch_qualified_segments__network_error(self):
def test_fetch_qualified_segments__400(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = fake_server_response(status_code=403, url=self.api_host)
mock_request_post.return_value = self.fake_server_response(status_code=403, url=self.api_host)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand All @@ -243,7 +242,7 @@ def test_fetch_qualified_segments__400(self):
def test_fetch_qualified_segments__500(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = fake_server_response(status_code=500, url=self.api_host)
mock_request_post.return_value = self.fake_server_response(status_code=500, url=self.api_host)

api = ZaiusGraphQLApiManager(logger=mock_logger)
api.fetch_segments(api_key=self.api_key,
Expand Down
Loading