From 2e9bd59395e45aeee1a3804200883ce72d39b1dd Mon Sep 17 00:00:00 2001 From: William Swanson Date: Thu, 6 Mar 2025 15:55:26 -0600 Subject: [PATCH 1/3] Update COmanage API request timeouts Fix exponential backoff to also apply to wait times between making requests, and not just timeouts on the requests themselves. Also improve error messages to include more information about how the request went / why it failed. --- comanage_utils.py | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/comanage_utils.py b/comanage_utils.py index bf08385..f9c15ce 100644 --- a/comanage_utils.py +++ b/comanage_utils.py @@ -4,6 +4,7 @@ import re import sys import json +import time import urllib.error import urllib.request from ldap3 import Server, Connection, ALL, ALL_ATTRIBUTES, SAFE_SYNC @@ -27,9 +28,9 @@ TEST_LDAP_TARGET_ID = 9 -MIN_TIMEOUT = 5 -MAX_TIMEOUT = 625 -TIMEOUTMULTIPLE = 5 +TIMEOUT_MIN = 5 +TIMEOUT_MULTIPLE = 5 +MAX_RETRIES = 5 GET = "GET" @@ -81,20 +82,27 @@ def call_api2(method, target, endpoint, authstr, **kw): def call_api3(method, target, data, endpoint, authstr, **kw): req = mkrequest(method, target, data, endpoint, authstr, **kw) - trying = True - currentTimeout = MIN_TIMEOUT - while trying: + retries = 0 + currentTimeout = TIMEOUT_MIN + requestingStart = time.time() + payload = None + while payload == None: try: resp = urllib.request.urlopen(req, timeout=currentTimeout) + if retries > 0: + print(f"Succeeded for request {req.full_url} after {retries} retries.") payload = resp.read() - trying = False except urllib.error.URLError as exception: - if currentTimeout < MAX_TIMEOUT: - currentTimeout *= TIMEOUTMULTIPLE + if retries < MAX_RETRIES: + print(f"Error: {exception} for request {req.full_url}, sleeping for {currentTimeout} seconds and retrying.") + time.sleep(currentTimeout) + currentTimeout *= TIMEOUT_MULTIPLE + retries += 1 else: + requestingStop = time.time() sys.exit( - f"Exception raised after maximum number of retries and/or timeout {MAX_TIMEOUT} seconds reached. " - + f"Exception reason: {exception.reason}.\n Request: {req.full_url}" + f"Exception raised after maximum number of retries reached after {requestingStop - requestingStart} seconds. Retries: {retries}. " + + f"Exception reason: {exception}.\n Request: {req.full_url}" ) return json.loads(payload) if payload else None From 820d9781acd89cc1a41c66de35af674d42ede048 Mon Sep 17 00:00:00 2001 From: William Swanson Date: Mon, 10 Mar 2025 11:56:54 -0500 Subject: [PATCH 2/3] COmanage API Request library changes for PR --- comanage_utils.py | 40 ++++++++++++++++++++-------------------- exceptions.py | 11 +++++++++++ 2 files changed, 31 insertions(+), 20 deletions(-) create mode 100644 exceptions.py diff --git a/comanage_utils.py b/comanage_utils.py index f9c15ce..ffc3cd7 100644 --- a/comanage_utils.py +++ b/comanage_utils.py @@ -2,9 +2,9 @@ import os import re -import sys import json import time +import exceptions import urllib.error import urllib.request from ldap3 import Server, Connection, ALL, ALL_ATTRIBUTES, SAFE_SYNC @@ -27,9 +27,8 @@ TEST_UNIX_CLUSTER_ID = 10 TEST_LDAP_TARGET_ID = 9 - -TIMEOUT_MIN = 5 -TIMEOUT_MULTIPLE = 5 +# Value for the base of the exponential backoff +TIMEOUT_BASE = 5 MAX_RETRIES = 5 @@ -83,28 +82,29 @@ def call_api2(method, target, endpoint, authstr, **kw): def call_api3(method, target, data, endpoint, authstr, **kw): req = mkrequest(method, target, data, endpoint, authstr, **kw) retries = 0 - currentTimeout = TIMEOUT_MIN - requestingStart = time.time() + current_timeout = TIMEOUT_BASE + total_timeout = 0 payload = None - while payload == None: + while retries <= MAX_RETRIES: try: - resp = urllib.request.urlopen(req, timeout=currentTimeout) - if retries > 0: - print(f"Succeeded for request {req.full_url} after {retries} retries.") + resp = urllib.request.urlopen(req, timeout=current_timeout) payload = resp.read() - except urllib.error.URLError as exception: - if retries < MAX_RETRIES: - print(f"Error: {exception} for request {req.full_url}, sleeping for {currentTimeout} seconds and retrying.") - time.sleep(currentTimeout) - currentTimeout *= TIMEOUT_MULTIPLE - retries += 1 - else: - requestingStop = time.time() - sys.exit( - f"Exception raised after maximum number of retries reached after {requestingStop - requestingStart} seconds. Retries: {retries}. " + break + # exception catching, mainly for request timeouts and "Service Temporarily Unavailable" (Rate limiting). + except urllib.error.HTTPError as exception: + if retries >= MAX_RETRIES: + raise exceptions.URLRequestError( + "Exception raised after maximum number of retries reached after total backoff of " + + f"{total_timeout} seconds. Retries: {retries}. " + f"Exception reason: {exception}.\n Request: {req.full_url}" ) + print("waiting for seconds: " + str(current_timeout)) + time.sleep(current_timeout) + total_timeout += current_timeout + current_timeout *= TIMEOUT_BASE + retries += 1 + return json.loads(payload) if payload else None diff --git a/exceptions.py b/exceptions.py new file mode 100644 index 0000000..8d438c6 --- /dev/null +++ b/exceptions.py @@ -0,0 +1,11 @@ +""" Exceptions used in configuration script """ + + +class Error(Exception): + """Base exception class for all exceptions defined""" + pass + + +class URLRequestError(Error): + """Class for exceptions due to not being able to fulfill a URLRequest""" + pass \ No newline at end of file From de912b789eae8dc4d81edaa8b130a94e20634377 Mon Sep 17 00:00:00 2001 From: William Swanson Date: Tue, 11 Mar 2025 10:29:05 -0500 Subject: [PATCH 3/3] COmanage API Request library changes for PR 2 --- comanage_utils.py | 36 ++++++++++++++++++++++-------------- exceptions.py | 11 ----------- 2 files changed, 22 insertions(+), 25 deletions(-) delete mode 100644 exceptions.py diff --git a/comanage_utils.py b/comanage_utils.py index ffc3cd7..238b0ec 100644 --- a/comanage_utils.py +++ b/comanage_utils.py @@ -4,7 +4,6 @@ import re import json import time -import exceptions import urllib.error import urllib.request from ldap3 import Server, Connection, ALL, ALL_ATTRIBUTES, SAFE_SYNC @@ -29,7 +28,7 @@ # Value for the base of the exponential backoff TIMEOUT_BASE = 5 -MAX_RETRIES = 5 +MAX_ATTEMPTS = 5 GET = "GET" @@ -37,6 +36,16 @@ POST = "POST" DELETE = "DELETE" +#Exceptions +class Error(Exception): + """Base exception class for all exceptions defined""" + pass + + +class URLRequestError(Error): + """Class for exceptions due to not being able to fulfill a URLRequest""" + pass + def getpw(user, passfd, passfile): if ":" in user: @@ -81,29 +90,28 @@ def call_api2(method, target, endpoint, authstr, **kw): def call_api3(method, target, data, endpoint, authstr, **kw): req = mkrequest(method, target, data, endpoint, authstr, **kw) - retries = 0 + req_attempts = 0 current_timeout = TIMEOUT_BASE total_timeout = 0 payload = None - while retries <= MAX_RETRIES: + while req_attempts < MAX_ATTEMPTS: try: resp = urllib.request.urlopen(req, timeout=current_timeout) - payload = resp.read() - break - # exception catching, mainly for request timeouts and "Service Temporarily Unavailable" (Rate limiting). - except urllib.error.HTTPError as exception: - if retries >= MAX_RETRIES: - raise exceptions.URLRequestError( + # exception catching, mainly for request timeouts, "Service Temporarily Unavailable" (Rate limiting), and DNS failures. + except urllib.error.URLError as exception: + req_attempts += 1 + if req_attempts >= MAX_ATTEMPTS: + raise URLRequestError( "Exception raised after maximum number of retries reached after total backoff of " + - f"{total_timeout} seconds. Retries: {retries}. " + f"{total_timeout} seconds. Retries: {req_attempts}. " + f"Exception reason: {exception}.\n Request: {req.full_url}" ) - - print("waiting for seconds: " + str(current_timeout)) time.sleep(current_timeout) total_timeout += current_timeout current_timeout *= TIMEOUT_BASE - retries += 1 + else: + payload = resp.read() + break return json.loads(payload) if payload else None diff --git a/exceptions.py b/exceptions.py deleted file mode 100644 index 8d438c6..0000000 --- a/exceptions.py +++ /dev/null @@ -1,11 +0,0 @@ -""" Exceptions used in configuration script """ - - -class Error(Exception): - """Base exception class for all exceptions defined""" - pass - - -class URLRequestError(Error): - """Class for exceptions due to not being able to fulfill a URLRequest""" - pass \ No newline at end of file