From 41eb9c1d863aa738ca07ff793966afaad07c3259 Mon Sep 17 00:00:00 2001 From: Serhii Khalymon Date: Wed, 7 Aug 2019 18:23:09 +0300 Subject: [PATCH 1/9] feat: add to_rfc1123_datetime helper function; create datetime_utils --- .../applitools/common/config/configuration.py | 5 +- .../applitools/common/utils/__init__.py | 5 ++ .../applitools/common/utils/datetime_utils.py | 63 +++++++++++++++++++ .../applitools/common/utils/general_utils.py | 22 ------- 4 files changed, 70 insertions(+), 25 deletions(-) create mode 100644 eyes_common/applitools/common/utils/datetime_utils.py diff --git a/eyes_common/applitools/common/config/configuration.py b/eyes_common/applitools/common/config/configuration.py index a471d1887..c2370fec9 100644 --- a/eyes_common/applitools/common/config/configuration.py +++ b/eyes_common/applitools/common/config/configuration.py @@ -9,7 +9,7 @@ from applitools.common.geometry import RectangleSize from applitools.common.match import ImageMatchSettings, MatchLevel from applitools.common.server import FailureReports, SessionType -from applitools.common.utils import argument_guard, general_utils +from applitools.common.utils import UTC, argument_guard from applitools.common.utils.json_utils import JsonInclude __all__ = ("BatchInfo", "Configuration") @@ -30,8 +30,7 @@ class BatchInfo(object): metadata={JsonInclude.THIS: True}, ) # type: Optional[Text] started_at = attr.ib( - factory=lambda: datetime.now(general_utils.UTC), - metadata={JsonInclude.THIS: True}, + factory=lambda: datetime.now(UTC), metadata={JsonInclude.THIS: True} ) # type: Union[datetime, Text] sequence_name = attr.ib( init=False, diff --git a/eyes_common/applitools/common/utils/__init__.py b/eyes_common/applitools/common/utils/__init__.py index 6a39fff27..1bd75e20b 100644 --- a/eyes_common/applitools/common/utils/__init__.py +++ b/eyes_common/applitools/common/utils/__init__.py @@ -11,6 +11,11 @@ urlsplit, urlunsplit, ) +from .datetime_utils import ( # type: ignore # noqa + UTC, + current_time_in_rfc1123, + to_rfc1123_datetime, +) from .general_utils import cached_property # noqa __all__ = compat.__all__ + ("image_utils", "argument_guard") # noqa diff --git a/eyes_common/applitools/common/utils/datetime_utils.py b/eyes_common/applitools/common/utils/datetime_utils.py new file mode 100644 index 000000000..c74701c68 --- /dev/null +++ b/eyes_common/applitools/common/utils/datetime_utils.py @@ -0,0 +1,63 @@ +from datetime import datetime, timedelta, tzinfo +from typing import Text + +__all__ = ("UTC", "to_rfc1123_datetime", "current_time_in_rfc1123") + + +class _UtcTz(tzinfo): + """ + A UTC timezone class which is tzinfo compliant. + """ + + _ZERO = timedelta(0) + + def utcoffset(self, dt): + return _UtcTz._ZERO + + def tzname(self, dt): + return "UTC" + + def dst(self, dt): + return _UtcTz._ZERO + + +UTC = _UtcTz() + + +def to_rfc1123_datetime(dt): + # type: (datetime) -> Text + """Return a string representation of a date according to RFC 1123 + (HTTP/1.1). + + The supplied date must be in UTC. + + """ + weekday = ["Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"][dt.weekday()] + month = [ + "Jan", + "Feb", + "Mar", + "Apr", + "May", + "Jun", + "Jul", + "Aug", + "Sep", + "Oct", + "Nov", + "Dec", + ][dt.month - 1] + return "%s, %02d %s %04d %02d:%02d:%02d GMT" % ( + weekday, + dt.day, + month, + dt.year, + dt.hour, + dt.minute, + dt.second, + ) + + +def current_time_in_rfc1123(): + # type: () -> Text + return to_rfc1123_datetime(datetime.now(UTC)) diff --git a/eyes_common/applitools/common/utils/general_utils.py b/eyes_common/applitools/common/utils/general_utils.py index 74fdf5f9a..27d659dde 100644 --- a/eyes_common/applitools/common/utils/general_utils.py +++ b/eyes_common/applitools/common/utils/general_utils.py @@ -4,7 +4,6 @@ import itertools import time import typing -from datetime import timedelta, tzinfo from applitools.common import logger @@ -21,27 +20,6 @@ T = typing.TypeVar("T") -class _UtcTz(tzinfo): - """ - A UTC timezone class which is tzinfo compliant. - """ - - _ZERO = timedelta(0) - - def utcoffset(self, dt): - return _UtcTz._ZERO - - def tzname(self, dt): - return "UTC" - - def dst(self, dt): - return _UtcTz._ZERO - - -# Constant representing UTC -UTC = _UtcTz() - - def use_default_if_none_factory(default_obj, obj): def default(attr_name): val = getattr(obj, attr_name) From 95dbaee9da7cdd89ca7a5f5eff555a4f9471e6a0 Mon Sep 17 00:00:00 2001 From: Serhii Khalymon Date: Thu, 8 Aug 2019 12:30:06 +0300 Subject: [PATCH 2/9] feat: add long_requests with check response --- eyes_core/applitools/core/server_connector.py | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/eyes_core/applitools/core/server_connector.py b/eyes_core/applitools/core/server_connector.py index 7626ca33a..18ee2cbf0 100644 --- a/eyes_core/applitools/core/server_connector.py +++ b/eyes_core/applitools/core/server_connector.py @@ -17,6 +17,7 @@ from applitools.common.test_results import TestResults from applitools.common.utils import ( argument_guard, + datetime_utils, gzip_compress, image_utils, json_utils, @@ -85,15 +86,42 @@ def request(self, method, url_resource, **kwargs): def long_request(self, method, url_resource, **kwargs): headers = kwargs["headers"].copy() - headers["Eyes-Expect"] = "202-accepted" + headers["Eyes-Expect"] = "202+location" + headers["Eyes-Date"] = datetime_utils.current_time_in_rfc1123() + kwargs["headers"] = headers + response = self.request(method, url_resource, **kwargs) + return self._long_request_check_status(response) + + def _long_request_check_status(self, response): + if response.status_code == 200: + # request ends successful + return response + elif response.status_code == 202: + # long request here; calling received url to know that request was processed + url = response.headers["location"] + response = self._long_request_loop(url) + return self._long_request_check_status(response) + elif response.status_code == 201: + # delete url that was used before + url = response.headers["location"] + return self.request( + requests.delete, + url, + headers={"Eyes-Date": datetime_utils.current_time_in_rfc1123()}, + ) + elif response.status_code == 410: + raise EyesError("The server task has gone.") + else: + raise EyesError("Unknown error during long request: {}".format(response)) + + def _long_request_loop(self, url): for delay in self.request_delay(): - # Sending the current time of the request (in RFC 1123 format) - headers["Eyes-Date"] = time.strftime( - "%a, %d %b %Y %H:%M:%S GMT", time.gmtime() + response = self.request( + requests.get, + url, + headers={"Eyes-Date": datetime_utils.current_time_in_rfc1123()}, ) - kwargs["headers"] = headers - response = self.request(method, url_resource, **kwargs) - if response.status_code != 202: + if response.status_code != 200: return response logger.debug("Still running... Retrying in {}s".format(delay)) else: From 364cdd6501d064882b5236a5c4b991a88219a088 Mon Sep 17 00:00:00 2001 From: Serhii Khalymon Date: Thu, 8 Aug 2019 12:32:36 +0300 Subject: [PATCH 3/9] refactor: simplify request handling --- eyes_core/applitools/core/server_connector.py | 157 ++++++++---------- 1 file changed, 66 insertions(+), 91 deletions(-) diff --git a/eyes_core/applitools/core/server_connector.py b/eyes_core/applitools/core/server_connector.py index 18ee2cbf0..d700db233 100644 --- a/eyes_core/applitools/core/server_connector.py +++ b/eyes_core/applitools/core/server_connector.py @@ -5,6 +5,7 @@ import typing from struct import pack +import attr import requests from requests import Response from requests.packages import urllib3 # noqa @@ -47,28 +48,27 @@ __all__ = ("ServerConnector",) +@attr.s class _RequestCommunicator(object): LONG_REQUEST_DELAY_SEC = 2 MAX_LONG_REQUEST_DELAY_SEC = 10 LONG_REQUEST_DELAY_MULTIPLICATIVE_INCREASE_FACTOR = 1.5 - def __init__(self, timeout_sec, headers, api_key, endpoint_uri): - # type: (int, Dict, Text, Text) -> None - self.timeout_sec = timeout_sec - self.headers = headers.copy() - self.api_key = api_key - self.endpoint_uri = endpoint_uri + headers = attr.ib() # type: Dict + timeout_sec = attr.ib(default=None) # type: int + api_key = attr.ib(default=None) # type: Text + server_url = attr.ib(default=None) # type: Text - def request(self, method, url_resource, **kwargs): + def request(self, method, url_resource, use_api_key=True, **kwargs): if url_resource is not None: # makes URL relative url_resource = url_resource.lstrip("/") - url_resource = urljoin(self.endpoint_uri, url_resource) + url_resource = urljoin(self.server_url, url_resource) params = {} - if self.api_key: + if use_api_key: params["apiKey"] = self.api_key params.update(kwargs.get("params", {})) - headers = kwargs.get("headers", self.headers) + headers = kwargs.get("headers", self.headers).copy() timeout = kwargs.get("timeout", self.timeout_sec) response = method( url_resource, @@ -85,7 +85,7 @@ def request(self, method, url_resource, **kwargs): return response def long_request(self, method, url_resource, **kwargs): - headers = kwargs["headers"].copy() + headers = kwargs.get("headers", self.headers).copy() headers["Eyes-Expect"] = "202+location" headers["Eyes-Date"] = datetime_utils.current_time_in_rfc1123() kwargs["headers"] = headers @@ -142,50 +142,6 @@ def request_delay( raise StopIteration -class _Request(object): - """ - Class for fetching data from - """ - - def __init__(self, com): - self._com = com # type: _RequestCommunicator - - def post(self, url_resource=None, long_query=False, **kwargs): - # type: (str, bool, **Any) -> requests.Response - func = self._com.long_request if long_query else self._com.request - return func(requests.post, url_resource, **kwargs) - - def put(self, url_resource=None, long_query=False, **kwargs): - # type: (str, bool, **Any) -> requests.Response - func = self._com.long_request if long_query else self._com.request - return func(requests.put, url_resource, **kwargs) - - def get(self, url_resource=None, long_query=False, **kwargs): - # type: (str, bool, **Any) -> requests.Response - func = self._com.long_request if long_query else self._com.request - return func(requests.get, url_resource, **kwargs) - - def delete(self, url_resource=None, long_query=False, **kwargs): - # type: (str, bool, **Any) -> requests.Response - func = self._com.long_request if long_query else self._com.request - return func(requests.delete, url_resource, **kwargs) - - -def create_request_factory(headers): - class RequestFactory(object): - def __init__(self): - self._com = None - - def create(self, api_key, server_url, timeout_sec): - # server_url could be updated - self._com = _RequestCommunicator( - timeout_sec, headers, api_key, endpoint_uri=server_url - ) - return _Request(self._com) - - return RequestFactory() - - def prepare_match_data(match_data): # type: (MatchWindowData) -> bytes screenshot64 = match_data.app_output.screenshot64 @@ -220,12 +176,7 @@ class ServerConnector(object): RENDER_STATUS = "/render-status" RENDER = "/render" - api_key = None # type: Optional[Text] - timeout_sec = None # type: Optional[float] - server_url = None # type: Optional[Text] _is_session_started = False - _request = None # type: Optional[_Request] - _render_request = None # type: Optional[_Request] def __init__(self): # type: () -> None @@ -235,31 +186,41 @@ def __init__(self): :param server_url: The url of the Applitools server. """ self._render_info = None # type: Optional[RenderingInfo] - self._request_factory = create_request_factory( - headers=ServerConnector.DEFAULT_HEADERS - ) + self._com = _RequestCommunicator(headers=ServerConnector.DEFAULT_HEADERS) - def _validate_api_key(self): - if self.api_key is None: + def update_config(self, conf): + if conf.api_key is None: raise EyesError( "API key not set! Log in to https://applitools.com to obtain your" " API Key and use 'api_key' to set it." ) + self._com.server_url = conf.server_url + self._com.api_key = conf.api_key + self._com.timeout_sec = conf.timeout / 1000.0 - def update_config(self, conf): - self.server_url = conf.server_url - self.api_key = conf.api_key - self._validate_api_key() - self.timeout_sec = conf.timeout / 1000.0 - - self._request = self._request_factory.create( - server_url=self.server_url, - api_key=self.api_key, - timeout_sec=self.timeout_sec, - ) - self._render_request = self._request_factory.create( - server_url=self.server_url, api_key=None, timeout_sec=self.timeout_sec - ) + @property + def server_url(self): + return self._com.server_url + + @server_url.setter + def server_url(self, value): + self._com.server_url = value + + @property + def api_key(self): + return self._com.api_key + + @api_key.setter + def api_key(self, value): + self._com.api_key = value + + @property + def timeout(self): + return self._com.timeout_sec * 1000 # ms + + @timeout.setter + def timeout(self, value): + self._com.timeout_sec = value / 1000.0 @property def is_session_started(self): @@ -278,7 +239,9 @@ def start_session(self, session_start_info): """ logger.debug("start_session called.") data = json_utils.to_json(session_start_info) - response = self._request.post(url_resource=self.API_SESSIONS_RUNNING, data=data) + response = self._com.long_request( + requests.post, url_resource=self.API_SESSIONS_RUNNING, data=data + ) running_session = json_utils.attr_from_response(response, RunningSession) running_session.is_new_session = response.status_code == requests.codes.created self._is_session_started = True @@ -300,9 +263,9 @@ def stop_session(self, running_session, is_aborted, save): raise EyesError("Session not started") params = {"aborted": is_aborted, "updateBaseline": save} - response = self._request.delete( + response = self._com.long_request( + requests.delete, url_resource=urljoin(self.API_SESSIONS_RUNNING, running_session.id), - long_query=True, params=params, headers=ServerConnector.DEFAULT_HEADERS, ) @@ -336,7 +299,8 @@ def match_window(self, running_session, match_data): headers = ServerConnector.DEFAULT_HEADERS.copy() headers["Content-Type"] = "application/octet-stream" # TODO: allow to send images as base64 - response = self._request.post( + response = self._com.long_request( + requests.post, url_resource=urljoin(self.API_SESSIONS_RUNNING, running_session.id), data=data, headers=headers, @@ -359,7 +323,8 @@ def post_dom_snapshot(self, dom_json): headers["Content-Type"] = "application/octet-stream" dom_bytes = gzip_compress(dom_json.encode("utf-8")) - response = self._request.post( + response = self._com.request( + requests.post, url_resource=urljoin(self.API_SESSIONS_RUNNING, "data"), data=dom_bytes, headers=headers, @@ -374,7 +339,9 @@ def render_info(self): logger.debug("render_info() called.") headers = ServerConnector.DEFAULT_HEADERS.copy() headers["Content-Type"] = "application/json" - response = self._request.get(self.RENDER_INFO_PATH, headers=headers) + response = self._com.request( + requests.get, self.RENDER_INFO_PATH, headers=headers + ) if not response.ok: raise EyesError( "Cannot get render info: \n Status: {}, Content: {}".format( @@ -397,7 +364,9 @@ def render(self, *render_requests): headers["X-Auth-Token"] = self._render_info.access_token data = json_utils.to_json(render_requests) - response = self._render_request.post(url, headers=headers, data=data) + response = self._com.request( + requests.post, url, use_api_key=False, headers=headers, data=data + ) if response.ok or response.status_code == 404: return json_utils.attr_from_response(response, RunningRender) raise EyesError( @@ -426,8 +395,10 @@ def render_put_resource(self, running_render, resource): url = urljoin( self._render_info.service_url, self.RESOURCES_SHA_256 + resource.hash ) - response = self._render_request.put( + response = self._com.request( + requests.put, url, + use_api_key=False, headers=headers, data=content, params={"render-id": running_render.render_id}, @@ -450,10 +421,10 @@ def download_resource(self, url): headers["Accept-Encoding"] = "identity" response = requests.get( - url, headers=headers, timeout=self.timeout_sec, verify=False + url, headers=headers, timeout=self._com.timeout_sec, verify=False ) if response.status_code == 406: - response = requests.get(url, timeout=self.timeout_sec, verify=False) + response = requests.get(url, timeout=self._com.timeout_sec, verify=False) response.raise_for_status() return response @@ -467,8 +438,12 @@ def render_status_by_id(self, render_id): headers["Content-Type"] = "application/json" headers["X-Auth-Token"] = self._render_info.access_token url = urljoin(self._render_info.service_url, self.RENDER_STATUS) - response = self._render_request.post( - url, headers=headers, data=json.dumps([render_id]) + response = self._com.request( + requests.post, + url, + use_api_key=False, + headers=headers, + data=json.dumps([render_id]), ) if not response.ok: raise EyesError( From 758664aee496e1f555f7f3720f718788a271882d Mon Sep 17 00:00:00 2001 From: Serhii Khalymon Date: Thu, 8 Aug 2019 13:08:01 +0300 Subject: [PATCH 4/9] test: update server_connector tests --- tests/unit/conftest.py | 6 ------ tests/unit/eyes_core/test_server_connector.py | 12 ++++++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index db23807b5..8d0910dea 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -72,13 +72,7 @@ def configured_connector(custom_eyes_server): @pytest.fixture(scope="function") def started_connector(configured_connector): - configured_connector._request = configured_connector._request_factory.create( - server_url=configured_connector.server_url, - api_key=configured_connector.api_key, - timeout_sec=configured_connector.timeout_sec, - ) configured_connector._is_session_started = True - return configured_connector diff --git a/tests/unit/eyes_core/test_server_connector.py b/tests/unit/eyes_core/test_server_connector.py index 10e3914b4..76e9e506b 100644 --- a/tests/unit/eyes_core/test_server_connector.py +++ b/tests/unit/eyes_core/test_server_connector.py @@ -3,8 +3,6 @@ from typing import Any import pytest -from mock import patch - from applitools.common import ( AppEnvironment, AppOutput, @@ -24,6 +22,7 @@ from applitools.common.utils.json_utils import attr_from_json from applitools.common.visual_grid import RenderingInfo from applitools.core import ServerConnector +from mock import patch API_KEY = "TEST-API-KEY" CUSTOM_EYES_SERVER = "http://custom-eyes-server.com" @@ -110,7 +109,7 @@ def mocked_requests_post(*args, **kwargs): _request_check(*args, **kwargs) url = args[0] if url == RUNNING_SESSION_URL: - return MockResponse(RUNNING_SESSION_DATA_RESPONSE, 201) + return MockResponse(RUNNING_SESSION_DATA_RESPONSE, 200) elif url == urljoin(RUNNING_SESSION_URL, RUNNING_SESSION_DATA_RESPONSE_ID): return MockResponse('{"asExpected": true}', 200) elif url == RUNNING_SESSION_DATA_URL: @@ -304,7 +303,12 @@ def test_request_with_changed_values(configured_connector): ) configured_connector.update_config(conf) - with patch("requests.post") as mocked_post: + with patch( + "requests.post", + side_effect=lambda *args, **kwargs: MockResponse( + RUNNING_SESSION_DATA_RESPONSE, 200 + ), + ) as mocked_post: with patch( "applitools.core.server_connector.json_utils.attr_from_response", return_value=RUNNING_SESSION_OBJ, From ebdc6eed3d7e0eccfe15b9877aaebba2f3b362de Mon Sep 17 00:00:00 2001 From: Serhii Khalymon Date: Thu, 8 Aug 2019 13:42:22 +0300 Subject: [PATCH 5/9] fix: start session should be regular request * in response from start session statuses could be 200 or 201 --- eyes_core/applitools/core/server_connector.py | 2 +- tests/unit/eyes_core/test_server_connector.py | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/eyes_core/applitools/core/server_connector.py b/eyes_core/applitools/core/server_connector.py index d700db233..21be2e95d 100644 --- a/eyes_core/applitools/core/server_connector.py +++ b/eyes_core/applitools/core/server_connector.py @@ -239,7 +239,7 @@ def start_session(self, session_start_info): """ logger.debug("start_session called.") data = json_utils.to_json(session_start_info) - response = self._com.long_request( + response = self._com.request( requests.post, url_resource=self.API_SESSIONS_RUNNING, data=data ) running_session = json_utils.attr_from_response(response, RunningSession) diff --git a/tests/unit/eyes_core/test_server_connector.py b/tests/unit/eyes_core/test_server_connector.py index 76e9e506b..1a137d912 100644 --- a/tests/unit/eyes_core/test_server_connector.py +++ b/tests/unit/eyes_core/test_server_connector.py @@ -109,7 +109,7 @@ def mocked_requests_post(*args, **kwargs): _request_check(*args, **kwargs) url = args[0] if url == RUNNING_SESSION_URL: - return MockResponse(RUNNING_SESSION_DATA_RESPONSE, 200) + return MockResponse(RUNNING_SESSION_DATA_RESPONSE, 201) elif url == urljoin(RUNNING_SESSION_URL, RUNNING_SESSION_DATA_RESPONSE_ID): return MockResponse('{"asExpected": true}', 200) elif url == RUNNING_SESSION_DATA_URL: @@ -303,12 +303,7 @@ def test_request_with_changed_values(configured_connector): ) configured_connector.update_config(conf) - with patch( - "requests.post", - side_effect=lambda *args, **kwargs: MockResponse( - RUNNING_SESSION_DATA_RESPONSE, 200 - ), - ) as mocked_post: + with patch("requests.post") as mocked_post: with patch( "applitools.core.server_connector.json_utils.attr_from_response", return_value=RUNNING_SESSION_OBJ, From 612e132c338c819fd990e25973ae3cc8d4855549 Mon Sep 17 00:00:00 2001 From: Serhii Khalymon Date: Thu, 8 Aug 2019 13:47:24 +0300 Subject: [PATCH 6/9] refactor: use requests codes instead of numbers --- eyes_core/applitools/core/server_connector.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/eyes_core/applitools/core/server_connector.py b/eyes_core/applitools/core/server_connector.py index 21be2e95d..e14101184 100644 --- a/eyes_core/applitools/core/server_connector.py +++ b/eyes_core/applitools/core/server_connector.py @@ -93,15 +93,15 @@ def long_request(self, method, url_resource, **kwargs): return self._long_request_check_status(response) def _long_request_check_status(self, response): - if response.status_code == 200: + if response.status_code == requests.codes.ok: # request ends successful return response - elif response.status_code == 202: + elif response.status_code == requests.codes.accepted: # long request here; calling received url to know that request was processed url = response.headers["location"] response = self._long_request_loop(url) return self._long_request_check_status(response) - elif response.status_code == 201: + elif response.status_code == requests.codes.created: # delete url that was used before url = response.headers["location"] return self.request( @@ -109,7 +109,7 @@ def _long_request_check_status(self, response): url, headers={"Eyes-Date": datetime_utils.current_time_in_rfc1123()}, ) - elif response.status_code == 410: + elif response.status_code == requests.codes.gone: raise EyesError("The server task has gone.") else: raise EyesError("Unknown error during long request: {}".format(response)) @@ -121,7 +121,7 @@ def _long_request_loop(self, url): url, headers={"Eyes-Date": datetime_utils.current_time_in_rfc1123()}, ) - if response.status_code != 200: + if response.status_code != requests.codes.ok: return response logger.debug("Still running... Retrying in {}s".format(delay)) else: @@ -367,7 +367,7 @@ def render(self, *render_requests): response = self._com.request( requests.post, url, use_api_key=False, headers=headers, data=data ) - if response.ok or response.status_code == 404: + if response.ok or response.status_code == requests.codes.not_found: return json_utils.attr_from_response(response, RunningRender) raise EyesError( "ServerConnector.render - unexpected status ({})\n\tcontent{}".format( @@ -423,7 +423,7 @@ def download_resource(self, url): response = requests.get( url, headers=headers, timeout=self._com.timeout_sec, verify=False ) - if response.status_code == 406: + if response.status_code == requests.codes.not_acceptable: response = requests.get(url, timeout=self._com.timeout_sec, verify=False) response.raise_for_status() return response From e8dab42b8929356579b49dffa79b1dcd4f39c0a8 Mon Sep 17 00:00:00 2001 From: Serhii Khalymon Date: Thu, 8 Aug 2019 16:57:00 +0300 Subject: [PATCH 7/9] refactor: simplify _long_request_loop mechanism --- eyes_core/applitools/core/server_connector.py | 49 ++++++++----------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/eyes_core/applitools/core/server_connector.py b/eyes_core/applitools/core/server_connector.py index e14101184..2a0204246 100644 --- a/eyes_core/applitools/core/server_connector.py +++ b/eyes_core/applitools/core/server_connector.py @@ -1,6 +1,7 @@ from __future__ import absolute_import import json +import math import time import typing from struct import pack @@ -50,9 +51,9 @@ @attr.s class _RequestCommunicator(object): - LONG_REQUEST_DELAY_SEC = 2 - MAX_LONG_REQUEST_DELAY_SEC = 10 - LONG_REQUEST_DELAY_MULTIPLICATIVE_INCREASE_FACTOR = 1.5 + LONG_REQUEST_DELAY_SEC = 2 # type: int + MAX_LONG_REQUEST_DELAY_SEC = 10 # type: int + LONG_REQUEST_DELAY_MULTIPLICATIVE_INCREASE_FACTOR = 1.5 # type: float headers = attr.ib() # type: Dict timeout_sec = attr.ib(default=None) # type: int @@ -114,32 +115,22 @@ def _long_request_check_status(self, response): else: raise EyesError("Unknown error during long request: {}".format(response)) - def _long_request_loop(self, url): - for delay in self.request_delay(): - response = self.request( - requests.get, - url, - headers={"Eyes-Date": datetime_utils.current_time_in_rfc1123()}, - ) - if response.status_code != requests.codes.ok: - return response - logger.debug("Still running... Retrying in {}s".format(delay)) - else: - raise requests.Timeout("Couldn't process request") - - @staticmethod - def request_delay( - first_delay=LONG_REQUEST_DELAY_SEC, - step_factor=LONG_REQUEST_DELAY_MULTIPLICATIVE_INCREASE_FACTOR, - max_delay=MAX_LONG_REQUEST_DELAY_SEC, - ): - delay = _RequestCommunicator.LONG_REQUEST_DELAY_SEC # type: Num - while True: - yield delay - time.sleep(first_delay) - delay = delay * step_factor - if delay > max_delay: - raise StopIteration + def _long_request_loop(self, url, delay=LONG_REQUEST_DELAY_SEC): + delay = min( + self.MAX_LONG_REQUEST_DELAY_SEC, + math.floor(delay * self.LONG_REQUEST_DELAY_MULTIPLICATIVE_INCREASE_FACTOR), + ) + logger.debug("Still running... Retrying in {}s".format(delay)) + + time.sleep(delay) + response = self.request( + requests.get, + url, + headers={"Eyes-Date": datetime_utils.current_time_in_rfc1123()}, + ) + if response.status_code != requests.codes.ok: + return response + return self._long_request_loop(url, delay) def prepare_match_data(match_data): From ac60f8df614b580185ed6365e88ddb4913d7780a Mon Sep 17 00:00:00 2001 From: Serhii Khalymon Date: Thu, 8 Aug 2019 17:38:08 +0300 Subject: [PATCH 8/9] refactor: change `location` to capitalised --- eyes_core/applitools/core/server_connector.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eyes_core/applitools/core/server_connector.py b/eyes_core/applitools/core/server_connector.py index 2a0204246..03bcd3cb9 100644 --- a/eyes_core/applitools/core/server_connector.py +++ b/eyes_core/applitools/core/server_connector.py @@ -99,12 +99,12 @@ def _long_request_check_status(self, response): return response elif response.status_code == requests.codes.accepted: # long request here; calling received url to know that request was processed - url = response.headers["location"] + url = response.headers["Location"] response = self._long_request_loop(url) return self._long_request_check_status(response) elif response.status_code == requests.codes.created: # delete url that was used before - url = response.headers["location"] + url = response.headers["Location"] return self.request( requests.delete, url, From fe9299db8a3bdc0cc9cdf69befcbc35dcf4407ed Mon Sep 17 00:00:00 2001 From: Serhii Khalymon Date: Thu, 8 Aug 2019 17:40:12 +0300 Subject: [PATCH 9/9] test: add test for long_request --- tests/unit/eyes_core/test_server_connector.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/unit/eyes_core/test_server_connector.py b/tests/unit/eyes_core/test_server_connector.py index 1a137d912..bc7b23b20 100644 --- a/tests/unit/eyes_core/test_server_connector.py +++ b/tests/unit/eyes_core/test_server_connector.py @@ -2,6 +2,8 @@ import os from typing import Any +import requests + import pytest from applitools.common import ( AppEnvironment, @@ -35,6 +37,8 @@ RUNNING_SESSION_URL = urljoin(CUSTOM_EYES_SERVER, API_SESSIONS_RUNNING) RUNNING_SESSION_DATA_URL = urljoin(RUNNING_SESSION_URL, "data") RENDER_INFO_PATH_URL = urljoin(CUSTOM_EYES_SERVER, RENDER_INFO_PATH) +LONG_REQUEST_URL = urljoin(CUSTOM_EYES_SERVER, "/one") +LONG_REQUEST_RESPONSE_URL = urljoin(CUSTOM_EYES_SERVER, "/second") RENDER_INFO_URL = "https://render-wus.applitools.com" RENDER_INFO_AT = "Some Token" @@ -94,6 +98,8 @@ def mocked_requests_delete(*args, **kwargs): url = args[0] if url == urljoin(RUNNING_SESSION_URL, RUNNING_SESSION_DATA_RESPONSE_ID): return MockResponse(STOP_SESSION_DATA, 200) + elif url == LONG_REQUEST_RESPONSE_URL: + return MockResponse({}, 200) return MockResponse(None, 404) @@ -102,6 +108,10 @@ def mocked_requests_get(*args, **kwargs): url = args[0] if url == RENDER_INFO_PATH_URL: return MockResponse(RENDERING_INFO_DATA, 200) + if url == LONG_REQUEST_URL: + return MockResponse(None, 202, {"Location": LONG_REQUEST_RESPONSE_URL}) + if url == LONG_REQUEST_RESPONSE_URL: + return MockResponse(None, 201, {"Location": LONG_REQUEST_RESPONSE_URL}) return MockResponse(None, 404) @@ -315,6 +325,13 @@ def test_request_with_changed_values(configured_connector): assert new_server_url in mocked_post.call_args[0][0] +def test_long_request(configured_connector): + with patch("requests.get", side_effect=mocked_requests_get): + with patch("requests.delete", side_effect=mocked_requests_delete): + r = configured_connector._com.long_request(requests.get, LONG_REQUEST_URL) + assert r.status_code == 200 + + def test_get_rendering_info(started_connector): with patch("requests.get", side_effect=mocked_requests_get): render_info = started_connector.render_info()