From 743f87818bf47028422cbfbb90aa69e871c05637 Mon Sep 17 00:00:00 2001 From: "M.Hamza Khan" Date: Wed, 6 Nov 2019 13:59:23 +0300 Subject: [PATCH 1/5] Bug fix: can now disable retrying of the sdk. (See #21) --- opsgenie_sdk/api_client.py | 14 ++++++++------ templates/api_client.mustache | 14 ++++++++------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/opsgenie_sdk/api_client.py b/opsgenie_sdk/api_client.py index 8d2c835..6f13f12 100644 --- a/opsgenie_sdk/api_client.py +++ b/opsgenie_sdk/api_client.py @@ -77,13 +77,12 @@ def __init__(self, configuration=None, header_name=None, header_value=None, self.configuration = configuration self.pool_threads = pool_threads - self.retrying = tenacity.Retrying(stop=tenacity.stop_after_attempt(configuration.retry_count), + self.retrying = tenacity.Retrying(stop=self.should_retry_stop, wait=tenacity.wait_random_exponential(multiplier=configuration.back_off, max=configuration.retry_max_delay, min=configuration.retry_delay), - retry=(tenacity.retry_if_result(self.is_retry_enabled) and - ((tenacity.retry_if_exception_type(RetryableException)) | - (tenacity.retry_if_exception_type(HTTPError))))) + retry=(tenacity.retry_if_exception_type(RetryableException) | + (tenacity.retry_if_exception_type(HTTPError)))) self.rest_client = rest.RESTClientObject(configuration, retrying=self.retrying) self.default_headers = {} @@ -104,8 +103,11 @@ def __del__(self): self._pool.join() self._pool = None - def is_retry_enabled(self): - return self.configuration.retry_enabled + def should_retry_stop(self, retry_state): + if self.configuration.retry_enabled and retry_state.attempt_number <= self.configuration.retry_count: + return False + + return True @property def pool(self): diff --git a/templates/api_client.mustache b/templates/api_client.mustache index 56573b6..8b8d4fc 100755 --- a/templates/api_client.mustache +++ b/templates/api_client.mustache @@ -71,13 +71,12 @@ class ApiClient(object): self.configuration = configuration self.pool_threads = pool_threads - self.retrying = tenacity.Retrying(stop=tenacity.stop_after_attempt(configuration.retry_count), + self.retrying = tenacity.Retrying(stop=self.should_retry_stop, wait=tenacity.wait_random_exponential(multiplier=configuration.back_off, max=configuration.retry_max_delay, min=configuration.retry_delay), - retry=(tenacity.retry_if_result(self.is_retry_enabled) and - ((tenacity.retry_if_exception_type(RetryableException)) | - (tenacity.retry_if_exception_type(HTTPError))))) + retry=(tenacity.retry_if_exception_type(RetryableException) | + (tenacity.retry_if_exception_type(HTTPError)))) self.rest_client = rest.RESTClientObject(configuration, retrying=self.retrying) self.default_headers = {} @@ -98,8 +97,11 @@ class ApiClient(object): self._pool.join() self._pool = None - def is_retry_enabled(self): - return self.configuration.retry_enabled + def should_retry_stop(self, retry_state): + if self.configuration.retry_enabled and retry_state.attempt_number <= self.configuration.retry_count: + return False + + return True @property def pool(self): From 17eb1638a990f4683f15a9da6ce7888318b4c431 Mon Sep 17 00:00:00 2001 From: "M.Hamza Khan" Date: Wed, 6 Nov 2019 14:31:43 +0300 Subject: [PATCH 2/5] Removed the unused retries parameter from the Configuration object to avoid confusion --- opsgenie_sdk/configuration.py | 2 -- templates/configuration.mustache | 2 -- 2 files changed, 4 deletions(-) diff --git a/opsgenie_sdk/configuration.py b/opsgenie_sdk/configuration.py index 8792800..8f08793 100644 --- a/opsgenie_sdk/configuration.py +++ b/opsgenie_sdk/configuration.py @@ -116,8 +116,6 @@ def __init__(self): self.proxy_headers = None # Safe chars for path_param self.safe_chars_for_path_param = '' - # Adding retries to override urllib3 default value 3 - self.retries = None self.metrics_transaction_id = None diff --git a/templates/configuration.mustache b/templates/configuration.mustache index 4b16f8c..8ff6fa8 100755 --- a/templates/configuration.mustache +++ b/templates/configuration.mustache @@ -117,8 +117,6 @@ class Configuration(six.with_metaclass(TypeWithDefault, object)): self.proxy_headers = None # Safe chars for path_param self.safe_chars_for_path_param = '' - # Adding retries to override urllib3 default value 3 - self.retries = None self.metrics_transaction_id = None From fa534d291f2c8d29a7ab929963559037996e7c7a Mon Sep 17 00:00:00 2001 From: "M.Hamza Khan" Date: Wed, 6 Nov 2019 16:24:38 +0300 Subject: [PATCH 3/5] Publishing sdk metrics whenever an exception is thrown while attempting to call the api --- opsgenie_sdk/api_client.py | 31 ++++++++++++++++++++++++------- templates/api_client.mustache | 31 ++++++++++++++++++++++++------- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/opsgenie_sdk/api_client.py b/opsgenie_sdk/api_client.py index 6f13f12..4acf5f5 100644 --- a/opsgenie_sdk/api_client.py +++ b/opsgenie_sdk/api_client.py @@ -194,13 +194,30 @@ def __call_api( url = _host + resource_path # perform request and return response - response_data = self.retrying.call(fn=self.request, method=method, url=url, - query_params=query_params, - headers=header_params, - post_params=post_params, - body=body, - _preload_content=_preload_content, - _request_timeout=_request_timeout) + try: + response_data = self.retrying.call(fn=self.request, method=method, url=url, + query_params=query_params, + headers=header_params, + post_params=post_params, + body=body, + _preload_content=_preload_content, + _request_timeout=_request_timeout) + except Exception as exception: + self._sdk_request_details = { + "query_params": query_params, + "headers": header_params, + "post_params": post_params, + "body": body, + "_preload_content": _preload_content, + "_request_timeout": _request_timeout + } + self.sdk_metric_publisher.build_metric(transaction_id=config.metrics_transaction_id, + duration=datetime.datetime.now() - self._request_start_time, + resource_path=url, error_type=type(exception), + error_message=str(exception), + sdk_request_details=self._sdk_request_details, + sdk_result_details="An Exception Was Thrown!") + raise exception self.last_response = response_data diff --git a/templates/api_client.mustache b/templates/api_client.mustache index 8b8d4fc..ff4ef8b 100755 --- a/templates/api_client.mustache +++ b/templates/api_client.mustache @@ -191,13 +191,30 @@ class ApiClient(object): url = _host + resource_path # perform request and return response - response_data = {{#asyncio}}await {{/asyncio}}{{#tornado}}yield {{/tornado}}self.retrying.call(fn=self.request, method=method, url=url, - query_params=query_params, - headers=header_params, - post_params=post_params, - body=body, - _preload_content=_preload_content, - _request_timeout=_request_timeout) + try: + response_data = {{#asyncio}}await {{/asyncio}}{{#tornado}}yield {{/tornado}}self.retrying.call(fn=self.request, method=method, url=url, + query_params=query_params, + headers=header_params, + post_params=post_params, + body=body, + _preload_content=_preload_content, + _request_timeout=_request_timeout) + except Exception as exception: + self._sdk_request_details = { + "query_params": query_params, + "headers": header_params, + "post_params": post_params, + "body": body, + "_preload_content": _preload_content, + "_request_timeout": _request_timeout + } + self.sdk_metric_publisher.build_metric(transaction_id=config.metrics_transaction_id, + duration=datetime.datetime.now() - self._request_start_time, + resource_path=url, error_type=type(exception), + error_message=str(exception), + sdk_request_details=self._sdk_request_details, + sdk_result_details="An Exception Was Thrown!") + raise exception self.last_response = response_data From 6e9aadd05f6fb61b40b62d914aa0e4856448f0a0 Mon Sep 17 00:00:00 2001 From: "M.Hamza Khan" Date: Wed, 6 Nov 2019 16:25:36 +0300 Subject: [PATCH 4/5] bug fix: request field of http_metrics is being correctly populated now --- opsgenie_sdk/rest.py | 6 ++++-- templates/rest.mustache | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/opsgenie_sdk/rest.py b/opsgenie_sdk/rest.py index 574595b..efbc759 100644 --- a/opsgenie_sdk/rest.py +++ b/opsgenie_sdk/rest.py @@ -238,6 +238,8 @@ def request(self, method, url, query_params=None, headers=None, logger.debug("response body: %s", r.data) data = self.decodeResponse(r.data) + http_metrics_request = [('method', method), ('url', url), ('query_params', query_params), ('headers', headers), + ('body', body), ('post_params', post_params)] if "message" not in data: self.http_metric.build_metric(transaction_id=self.configuration.metrics_transaction_id, duration=data["took"], @@ -246,7 +248,7 @@ def request(self, method, url, query_params=None, headers=None, error=False, status=r.status, status_code=r.status, - request=query_params) + request=http_metrics_request) else: self.http_metric.build_metric(transaction_id=self.configuration.metrics_transaction_id, duration=data["took"], @@ -255,7 +257,7 @@ def request(self, method, url, query_params=None, headers=None, error=True, status=r.status, status_code=r.status, - request=query_params) + request=http_metrics_request) should_retry = self.__checkHttpCode__(r.status) if should_retry: diff --git a/templates/rest.mustache b/templates/rest.mustache index ed4d282..3b2b805 100755 --- a/templates/rest.mustache +++ b/templates/rest.mustache @@ -229,6 +229,8 @@ class RESTClientObject(object): logger.debug("response body: %s", r.data) data = self.decodeResponse(r.data) + http_metrics_request = [('method', method), ('url', url), ('query_params', query_params), ('headers', headers), + ('body', body), ('post_params', post_params)] if "message" not in data: self.http_metric.build_metric(transaction_id=self.configuration.metrics_transaction_id, duration=data["took"], @@ -237,7 +239,7 @@ class RESTClientObject(object): error=False, status=r.status, status_code=r.status, - request=query_params) + request=http_metrics_request) else: self.http_metric.build_metric(transaction_id=self.configuration.metrics_transaction_id, duration=data["took"], @@ -246,7 +248,7 @@ class RESTClientObject(object): error=True, status=r.status, status_code=r.status, - request=query_params) + request=http_metrics_request) should_retry = self.__checkHttpCode__(r.status) if should_retry: From 6b8f8969d05ab82fad2b42f1869adb83c1779eea Mon Sep 17 00:00:00 2001 From: "M.Hamza Khan" Date: Wed, 6 Nov 2019 17:08:45 +0300 Subject: [PATCH 5/5] bug fix: respecting retry_max_delay parameter of the Configuration object now (see #21) --- opsgenie_sdk/api_client.py | 5 ++--- opsgenie_sdk/configuration.py | 2 +- templates/api_client.mustache | 5 ++--- templates/configuration.mustache | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/opsgenie_sdk/api_client.py b/opsgenie_sdk/api_client.py index 4acf5f5..fa112eb 100644 --- a/opsgenie_sdk/api_client.py +++ b/opsgenie_sdk/api_client.py @@ -79,8 +79,7 @@ def __init__(self, configuration=None, header_name=None, header_value=None, self.retrying = tenacity.Retrying(stop=self.should_retry_stop, wait=tenacity.wait_random_exponential(multiplier=configuration.back_off, - max=configuration.retry_max_delay, - min=configuration.retry_delay), + max=configuration.retry_delay), retry=(tenacity.retry_if_exception_type(RetryableException) | (tenacity.retry_if_exception_type(HTTPError)))) @@ -104,7 +103,7 @@ def __del__(self): self._pool = None def should_retry_stop(self, retry_state): - if self.configuration.retry_enabled and retry_state.attempt_number <= self.configuration.retry_count: + if self.configuration.retry_enabled and retry_state.attempt_number <= self.configuration.retry_count and retry_state.seconds_since_start <= self.configuration.retry_max_delay: return False return True diff --git a/opsgenie_sdk/configuration.py b/opsgenie_sdk/configuration.py index 8f08793..a07ccc9 100644 --- a/opsgenie_sdk/configuration.py +++ b/opsgenie_sdk/configuration.py @@ -80,7 +80,7 @@ def __init__(self): # Retry count self.retry_count = 5 # Delay time between attempts - self.retry_delay = 0 + self.retry_delay = 30 # Maximum amount of delay self.retry_max_delay = 60 # Multiplier applied to delay between attempts diff --git a/templates/api_client.mustache b/templates/api_client.mustache index ff4ef8b..5b9013d 100755 --- a/templates/api_client.mustache +++ b/templates/api_client.mustache @@ -73,8 +73,7 @@ class ApiClient(object): self.retrying = tenacity.Retrying(stop=self.should_retry_stop, wait=tenacity.wait_random_exponential(multiplier=configuration.back_off, - max=configuration.retry_max_delay, - min=configuration.retry_delay), + max=configuration.retry_delay), retry=(tenacity.retry_if_exception_type(RetryableException) | (tenacity.retry_if_exception_type(HTTPError)))) @@ -98,7 +97,7 @@ class ApiClient(object): self._pool = None def should_retry_stop(self, retry_state): - if self.configuration.retry_enabled and retry_state.attempt_number <= self.configuration.retry_count: + if self.configuration.retry_enabled and retry_state.attempt_number <= self.configuration.retry_count and retry_state.seconds_since_start <= self.configuration.retry_max_delay: return False return True diff --git a/templates/configuration.mustache b/templates/configuration.mustache index 8ff6fa8..c559bb6 100755 --- a/templates/configuration.mustache +++ b/templates/configuration.mustache @@ -81,7 +81,7 @@ class Configuration(six.with_metaclass(TypeWithDefault, object)): # Retry count self.retry_count = 5 # Delay time between attempts - self.retry_delay = 0 + self.retry_delay = 30 # Maximum amount of delay self.retry_max_delay = 60 # Multiplier applied to delay between attempts