From 94d24a51b76f87a9c97ea0661f061bbb6757362d Mon Sep 17 00:00:00 2001 From: "John M. Kuchta" Date: Wed, 9 Apr 2025 20:46:43 -0700 Subject: [PATCH 1/4] Extractathon! * Extracting parts of rest_session.py into common.py * Creating input validation for caller and be_geo_id arguments * Improving help text on config.py --- meraki/aio/rest_session.py | 10 +- meraki/common.py | 63 ++++++++++++ meraki/config.py | 6 +- meraki/exception_handler.py | 0 meraki/exceptions.py | 16 +++- meraki/response_handler.py | 8 ++ meraki/rest_session.py | 185 ++++++++++++++++-------------------- 7 files changed, 172 insertions(+), 116 deletions(-) create mode 100644 meraki/exception_handler.py create mode 100644 meraki/response_handler.py diff --git a/meraki/aio/rest_session.py b/meraki/aio/rest_session.py index 1eae47b2..770bf3e6 100644 --- a/meraki/aio/rest_session.py +++ b/meraki/aio/rest_session.py @@ -5,7 +5,7 @@ import sys import time import urllib.parse -from datetime import datetime +from datetime import datetime, timezone import aiohttp @@ -66,18 +66,14 @@ def __init__( check_python_version() # Check base URL - if "v0" in self._base_url: - sys.exit(f'This library does not support dashboard API v0 ({self._base_url} was configured as the base' - f' URL). API v0 has been end of life since 2020 August 5.') - elif self._base_url[-1] == "/": - self._base_url = self._base_url[:-1] + reject_v0_base_url(self) # Update the headers for the session self._headers = { "Authorization": "Bearer " + self._api_key, "Content-Type": "application/json", "User-Agent": f"python-meraki/aio-{self._version} " - + user_agent_extended(self._be_geo_id, self._caller), + + validate_user_agent(self._be_geo_id, self._caller), } if self._certificate_path: self._sslcontext = ssl.create_default_context() diff --git a/meraki/common.py b/meraki/common.py index b34ba5a8..495e6c02 100644 --- a/meraki/common.py +++ b/meraki/common.py @@ -1,5 +1,8 @@ import platform from meraki.exceptions import * +import re +import sys +import urllib.parse def check_python_version(): @@ -20,3 +23,63 @@ def check_python_version(): ) raise PythonVersionError(message) + + +def validate_user_agent(be_geo_id, caller): + # Generate extended portion of the User Agent + # Validate that it follows the expected format + user_agent = dict() + + allowed_format_in_regex = r'^[A-Za-z0-9]+(?:/[0-9A-Za-z]+(?:\.[0-9A-Za-z]+)*(-[a-z]+)?)? [A-Za-z-0-9]+$' + + if caller and re.match(allowed_format_in_regex, caller): + user_agent["caller"] = caller + elif be_geo_id and re.match(allowed_format_in_regex, be_geo_id): + user_agent["caller"] = be_geo_id + else: + if caller: + message = "Please follow the user agent format prescribed in our User Agents guide, available here:" + doc_link = "https://developer.cisco.com/meraki/api-v1/user-agents-overview/" + raise SessionInputError("MERAKI_PTYHON_SDK_CALLER", caller, message, doc_link) + elif be_geo_id: + message = "Use of be_geo_id is deprecated. Please use the argument MERAKI_PTYHON_SDK_CALLER instead." + doc_link = "https://developer.cisco.com/meraki/api-v1/user-agents-overview/" + raise SessionInputError("BE_GEO_ID", caller, message, doc_link) + else: + user_agent["caller"] = "unidentified" + + caller_string = f'Caller/({user_agent["caller"]})' + + return caller_string + + +def reject_v0_base_url(self): + if 'v0' in self._base_url: + sys.exit(f'This library does not support dashboard API v0 ({self._base_url} was configured as the base' + f' URL). API v0 has been end of life since 2020 August 5.') + elif self._base_url[-1] == '/': + self._base_url = self._base_url[:-1] + + +def iterator_for_get_pages_bool(self): + return self._use_iterator_for_get_pages + + +def use_iterator_for_get_pages_setter(self, value): + if value: + self.get_pages = self._get_pages_iterator + else: + self.get_pages = self._get_pages_legacy + + self._use_iterator_for_get_pages = value + + +def validate_base_url(self, url): + allowed_domains = ['meraki.com', 'meraki.ca', 'meraki.cn', 'meraki.in', 'gov-meraki.com'] + parsed_url = urllib.parse.urlparse(url) + if any(domain in parsed_url.netloc for domain in allowed_domains): + abs_url = url + else: + abs_url = self._base_url + url + return abs_url + diff --git a/meraki/config.py b/meraki/config.py index 03d63be6..bed11ed1 100644 --- a/meraki/config.py +++ b/meraki/config.py @@ -82,13 +82,15 @@ # 1. Application name precedes vendor name in all cases. # 2. If your application or vendor name normally contains spaces or special casing, you should omit them in favor of # normal CamelCasing here. -# 3. The optional slash and version number are optional. Leave both out if you like. +# 3. The slash and version number are optional. Leave both out if you like. # 4. The slash is a forward slash, '/' -- not a backslash. -# 5. Don't use the 'Meraki' name in your application name here. Maybe in general? I'm a config file, not a lawyer. +# 5. Don't use the 'Meraki' or 'Cisco' names in your application name here. Maybe in general? I'm a config file, not a +# lawyer. # Example 1: if your application is named 'Mambo', version number is 5.0, and your vendor/company name is Vega, then # you would use, at minimum: 'Mambo Vega'. Optionally: 'Mambo/5.0 Vega'. # Example 2: if your application is named 'Sunshine Rainbows', and company name is 'hunter2 for Life', and if you # don't want to report version number, then you would use, at minimum: 'SunshineRainbows hunter2ForLife' # The choice is yours as long as you follow the format. You should **not** include other information in this string. # If you are an official ecosystem partner, this is required. +# For more guidance, please refer to https://developer.cisco.com/meraki/api-v1/user-agents-overview/ MERAKI_PYTHON_SDK_CALLER = "" diff --git a/meraki/exception_handler.py b/meraki/exception_handler.py new file mode 100644 index 00000000..e69de29b diff --git a/meraki/exceptions.py b/meraki/exceptions.py index 0130f3cd..acfd236b 100644 --- a/meraki/exceptions.py +++ b/meraki/exceptions.py @@ -57,7 +57,7 @@ def __init__(self, metadata, response): except ValueError: self.message = self.response.content[:100].decode("UTF-8").strip() if ( - type(self.message) == str + isinstance(self.message, str) and self.status == 404 and self.reason == "Not Found" ): @@ -85,7 +85,7 @@ def __init__(self, metadata, response, message): response.reason if response is not None and response.reason else None ) self.message = message - if type(self.message) == str: + if isinstance(self.message, str): self.message = self.message.strip() if self.status == 404 and self.reason == "Not Found": self.message += ( @@ -107,3 +107,15 @@ def __init__(self, message): self.message = message super().__init__(self.message) + + +class SessionInputError(Exception): + """Exception raised for unsupported session inputs.""" + + def __init__(self, argument, value, message, doc_link): + self.argument = argument + self.value = value + self.message = message + self.doc_link = doc_link + + super().__init__(f'{self.message} {self.doc_link}') \ No newline at end of file diff --git a/meraki/response_handler.py b/meraki/response_handler.py new file mode 100644 index 00000000..4bf79b33 --- /dev/null +++ b/meraki/response_handler.py @@ -0,0 +1,8 @@ +def handle_3xx(self, response): + abs_url = response.headers['Location'] + substring = 'meraki.com/api/v' + if substring not in abs_url: + substring = 'meraki.cn/api/v' + self._base_url = abs_url[:abs_url.find(substring) + len(substring) + 1] + return abs_url + diff --git a/meraki/rest_session.py b/meraki/rest_session.py index 697ff4e6..a2e40ed7 100644 --- a/meraki/rest_session.py +++ b/meraki/rest_session.py @@ -1,33 +1,17 @@ -import json import random -import sys -import time import urllib.parse -from datetime import datetime +from datetime import datetime, timezone +import json +import time import requests from meraki.__init__ import __version__ from meraki.common import * +from meraki.response_handler import * from meraki.config import * -def user_agent_extended(be_geo_id, caller): - # Generate extended portion of the User-Agent - user_agent = dict() - - if caller: - user_agent["caller"] = caller - elif be_geo_id: - user_agent["caller"] = be_geo_id - else: - user_agent["caller"] = "unidentified" - - caller_string = f'Caller/({user_agent["caller"]})' - - return caller_string - - # Main module interface class RestSession(object): def __init__( @@ -75,20 +59,17 @@ def __init__( self._req_session = requests.session() self._req_session.encoding = 'utf-8' + # Check Python version check_python_version() # Check base URL - if 'v0' in self._base_url: - sys.exit(f'This library does not support dashboard API v0 ({self._base_url} was configured as the base' - f' URL). API v0 has been end of life since 2020 August 5.') - elif self._base_url[-1] == '/': - self._base_url = self._base_url[:-1] + reject_v0_base_url(self) # Update the headers for the session self._req_session.headers = { 'Authorization': 'Bearer ' + self._api_key, 'Content-Type': 'application/json', - 'User-Agent': f'python-meraki/{self._version} ' + user_agent_extended(self._be_geo_id, self._caller), + 'User-Agent': f'python-meraki/{self._version} ' + validate_user_agent(self._be_geo_id, self._caller), } # Log API calls @@ -104,16 +85,11 @@ def __init__( @property def use_iterator_for_get_pages(self): - return self._use_iterator_for_get_pages + return iterator_for_get_pages_bool(self) @use_iterator_for_get_pages.setter def use_iterator_for_get_pages(self, value): - if value: - self.get_pages = self._get_pages_iterator - else: - self.get_pages = self._get_pages_legacy - - self._use_iterator_for_get_pages = value + use_iterator_for_get_pages_setter(self, value) def request(self, metadata, method, url, **kwargs): # Metadata on endpoint @@ -121,20 +97,10 @@ def request(self, metadata, method, url, **kwargs): operation = metadata['operation'] # Update request kwargs with session defaults - if self._certificate_path: - kwargs.setdefault('verify', self._certificate_path) - if self._requests_proxy: - kwargs.setdefault('proxies', {'https': self._requests_proxy}) - kwargs.setdefault('timeout', self._single_request_timeout) + self.prepare_request(kwargs) # Ensure proper base URL - allowed_domains = ['meraki.com', 'meraki.ca', 'meraki.cn', 'meraki.in', 'gov-meraki.com'] - parsed_url = urllib.parse.urlparse(url) - - if any(domain in parsed_url.netloc for domain in allowed_domains): - abs_url = url - else: - abs_url = self._base_url + url + abs_url = validate_base_url(self, url) # Set maximum number of retries retries = self._maximum_retries @@ -173,66 +139,66 @@ def request(self, metadata, method, url, **kwargs): else: continue - # Handle 3XX redirects automatically - if str(status)[0] == '3': - abs_url = response.headers['Location'] - substring = 'meraki.com/api/v' - if substring not in abs_url: - substring = 'meraki.cn/api/v' - self._base_url = abs_url[:abs_url.find(substring) + len(substring) + 1] - - # 2XX success - elif response.ok: - if 'page' in metadata: - counter = metadata['page'] - if self._logger: - self._logger.info(f'{tag}, {operation}; page {counter} - {status} {reason}') - else: - if self._logger: - self._logger.info(f'{tag}, {operation} - {status} {reason}') - # For non-empty response to GET, ensure valid JSON - try: - if method == 'GET' and response.content.strip(): - response.json() - return response - except json.decoder.JSONDecodeError as e: + match status: + # Handle 3xx redirects automatically + case status if 300 <= status < 400: + abs_url = handle_3xx(self, response) + # Handle 2xx success + case status if 200 <= status < 300: + if 'page' in metadata: + counter = metadata['page'] + if self._logger: + self._logger.info(f'{tag}, {operation}; page {counter} - {status} {reason}') + else: + if self._logger: + self._logger.info(f'{tag}, {operation} - {status} {reason}') + # For non-empty response to GET, ensure valid JSON + try: + if method == 'GET' and response.content.strip(): + response.json() + return response + except json.decoder.JSONDecodeError as e: + if self._logger: + self._logger.warning(f'{tag}, {operation} - {e}, retrying in 1 second') + time.sleep(1) + retries -= 1 + if retries == 0: + raise APIError(metadata, response) + else: + continue + # Handle rate limiting + case 429: + # Retry if 429 retries are enabled and there are retries left + if self._wait_on_rate_limit and retries > 0: + if 'Retry-After' in response.headers: + wait = int(response.headers['Retry-After']) + else: + wait = random.randint(1, self._nginx_429_retry_wait_time) + if self._logger: + self._logger.warning(f'{tag}, {operation} - {status} {reason}, retrying in {wait} seconds') + time.sleep(wait) + retries -= 1 + # We're either out of retries or the client told us not to retry + else: + raise APIError(metadata, response) + # Handle 5xx errors + case status if 500 <= status: if self._logger: - self._logger.warning(f'{tag}, {operation} - {e}, retrying in 1 second') + self._logger.warning(f'{tag}, {operation} - {status} {reason}, retrying in 1 second') time.sleep(1) retries -= 1 if retries == 0: raise APIError(metadata, response) - else: - continue - - # Rate limit 429 errors - elif status == 429: - # Retry if 429 retries are enabled and there are retries left - if self._wait_on_rate_limit and retries > 0: - if 'Retry-After' in response.headers: - wait = int(response.headers['Retry-After']) - else: - wait = random.randint(1, self._nginx_429_retry_wait_time) - if self._logger: - self._logger.warning(f'{tag}, {operation} - {status} {reason}, retrying in {wait} seconds') - time.sleep(wait) - retries -= 1 - # We're either out of retries or the client told us not to retry - else: - raise APIError(metadata, response) + # Handle other 4xx errors + case status if status != 429 and 400 <= status < 500: + retries = self.handle_4xx_errors(metadata, operation, reason, response, retries, status, tag) - # 5XX errors - elif status >= 500: - if self._logger: - self._logger.warning(f'{tag}, {operation} - {status} {reason}, retrying in 1 second') - time.sleep(1) - retries -= 1 - if retries == 0: - raise APIError(metadata, response) - - # 4XX errors - else: - retries = self.handle_4xx_errors(metadata, operation, reason, response, retries, status, tag) + def prepare_request(self, kwargs): + if self._certificate_path: + kwargs.setdefault('verify', self._certificate_path) + if self._requests_proxy: + kwargs.setdefault('proxies', {'https': self._requests_proxy}) + kwargs.setdefault('timeout', self._single_request_timeout) def handle_4xx_errors(self, metadata, operation, reason, response, retries, status, tag): try: @@ -313,10 +279,14 @@ def _get_pages_iterator( direction="next", event_log_end_time=None, ): - if type(total_pages) == str and total_pages.lower() == "all": + if isinstance(total_pages, str) and total_pages.lower() == "all": total_pages = -1 - elif type(total_pages) == str and total_pages.isnumeric(): + elif isinstance(total_pages, str) and total_pages.isnumeric(): total_pages = int(total_pages) + elif not isinstance(total_pages, int): + raise SessionInputError("total_pages", total_pages, "total_pages must be either an" + " integer or 'all' as a string (remember to add the" + " quotation marks).", None) metadata["page"] = 1 response = self.request(metadata, 'GET', url, params=params) @@ -333,8 +303,8 @@ def _get_pages_iterator( starting_after = urllib.parse.unquote( str(links["next"]["url"]).split("startingAfter=")[1] ) - delta = datetime.utcnow() - datetime.fromisoformat( - starting_after[:-1] + delta = datetime.now(timezone.utc) - datetime.fromisoformat( + starting_after ) # Break out of loop if startingAfter returned from next link is within 5 minutes of current time if delta.total_seconds() < 300: @@ -384,10 +354,15 @@ def _get_pages_iterator( response = self.request(metadata, 'GET', nextlink) def _get_pages_legacy(self, metadata, url, params=None, total_pages=-1, direction='next', event_log_end_time=None): - if isinstance(total_pages, str) and total_pages.lower() == 'all': + if isinstance(total_pages, str) and total_pages.lower() == "all": total_pages = -1 elif isinstance(total_pages, str) and total_pages.isnumeric(): total_pages = int(total_pages) + elif not isinstance(total_pages, int): + raise SessionInputError("total_pages", total_pages, "total_pages must be either an" + " integer or 'all' as a string (remember to add the" + " quotation marks).", None) + metadata['page'] = 1 response = self.request(metadata, 'GET', url, params=params) @@ -413,7 +388,7 @@ def _get_pages_legacy(self, metadata, url, params=None, total_pages=-1, directio # Prevent getNetworkEvents from infinite loop as time goes forward if metadata['operation'] == 'getNetworkEvents': starting_after = urllib.parse.unquote(links['next']['url'].split('startingAfter=')[1]) - delta = datetime.utcnow() - datetime.fromisoformat(starting_after[:-1]) + delta = datetime.now(timezone.utc) - datetime.fromisoformat(starting_after) # Break out of loop if startingAfter returned from next link is within 5 minutes of current time if delta.total_seconds() < 300: break From f2c76c2fc9ee2167a7365b5fabcef301161cc529 Mon Sep 17 00:00:00 2001 From: "John M. Kuchta" Date: Wed, 9 Apr 2025 21:32:16 -0700 Subject: [PATCH 2/4] Update pyproject.toml Tested and verified working on pytest 8.3.5. Bumping version requirement. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d05ddee6..d6816640 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ dependencies = [ "requests (>=2.32.2,<3.0.0)", "aiohttp (>=3.9.4,<4.0.0)", "jinja2 (==3.1.6)", - "pytest (>=7.2.1,<8.0.0)", + "pytest (>=8.3.5,<9.0.0)", "setuptools (>=70.0.0,<71.0.0)" ] From b484233a2b681d3a98293cf78c61ac31a7d01c5c Mon Sep 17 00:00:00 2001 From: "John M. Kuchta" Date: Mon, 21 Apr 2025 14:44:27 -0700 Subject: [PATCH 3/4] Fix missing return statement --- meraki/rest_session.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/meraki/rest_session.py b/meraki/rest_session.py index a2e40ed7..d4713c03 100644 --- a/meraki/rest_session.py +++ b/meraki/rest_session.py @@ -193,6 +193,8 @@ def request(self, metadata, method, url, **kwargs): case status if status != 429 and 400 <= status < 500: retries = self.handle_4xx_errors(metadata, operation, reason, response, retries, status, tag) + return response + def prepare_request(self, kwargs): if self._certificate_path: kwargs.setdefault('verify', self._certificate_path) From c5abc368517b64de05574d7b92edabe687b2691d Mon Sep 17 00:00:00 2001 From: "John M. Kuchta" Date: Mon, 21 Apr 2025 15:42:57 -0700 Subject: [PATCH 4/4] Clean up rest_session.py extractions --- meraki/rest_session.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/meraki/rest_session.py b/meraki/rest_session.py index c0adfa66..157b4cb0 100644 --- a/meraki/rest_session.py +++ b/meraki/rest_session.py @@ -146,7 +146,7 @@ def __init__( self._req_session = requests.session() self._req_session.encoding = 'utf-8' - # Check Python version + # Check the Python version check_python_version() # Check base URL @@ -289,8 +289,6 @@ def prepare_request(self, kwargs): kwargs.setdefault('proxies', {'https': self._requests_proxy}) kwargs.setdefault('timeout', self._single_request_timeout) - return response - def handle_4xx_errors(self, metadata, operation, reason, response, retries, status, tag): try: @@ -304,7 +302,7 @@ def handle_4xx_errors(self, metadata, operation, reason, response, retries, stat network_delete_concurrency_error_text = 'concurrent' action_batch_concurrency_error_text = 'executing batches' - # First we check for network deletion concurrency errors + # First, we check for network deletion concurrency errors if operation == 'deleteNetwork' and response.status_code == 400: # message['errors'][0] is the first error, and it contains helpful text # here we use it to confirm that the 400 error is related to concurrent requests @@ -317,7 +315,7 @@ def handle_4xx_errors(self, metadata, operation, reason, response, retries, stat if retries == 0: raise APIError(metadata, response) - # Next we check for action batch concurrency errors + # Next, we check for action batch concurrency errors # message['errors'][0] is the first error, and it contains helpful text # here we use it to confirm that the 400 error is related to concurrent requests elif (message_is_dict and 'errors' in message.keys() and action_batch_concurrency_error_text @@ -401,7 +399,7 @@ def _get_pages_iterator( # Break out of loop if startingAfter returned from next link is within 5 minutes of current time if delta.total_seconds() < 300: break - # Or if next page is past the specified window's end time + # Or if the next page is past the specified window's end time elif event_log_end_time and starting_after > event_log_end_time: break @@ -425,7 +423,7 @@ def _get_pages_iterator( response.close() return_items = [] - # just prepare the list + # Just prepare the list if isinstance(results, list): return_items = results elif isinstance(results, dict) and "items" in results: