From a46ce953cbe41d5b0aa1cf677d2117c3271e8606 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Mon, 30 Jun 2025 11:37:45 +0200 Subject: [PATCH 1/9] fix: switch to QgsBlockingNetworkRequest for network calls --- ORStools/common/client.py | 165 +++------ ORStools/common/networkaccessmanager.py | 427 ------------------------ 2 files changed, 52 insertions(+), 540 deletions(-) delete mode 100644 ORStools/common/networkaccessmanager.py diff --git a/ORStools/common/client.py b/ORStools/common/client.py index ac4db807..d29e9bd4 100644 --- a/ORStools/common/client.py +++ b/ORStools/common/client.py @@ -34,23 +34,27 @@ from typing import Union, Dict, List, Optional from urllib.parse import urlencode -from qgis.PyQt.QtCore import QObject, pyqtSignal +from qgis.PyQt.QtCore import QObject, pyqtSignal, QUrl +from qgis.PyQt.QtNetwork import QNetworkRequest from qgis.utils import iface -from qgis.core import Qgis +from qgis.core import ( + Qgis, + QgsSettings, + QgsBlockingNetworkRequest, + QgsNetworkAccessManager +) from requests.utils import unquote_unreserved from ORStools import __version__ -from ORStools.common import networkaccessmanager from ORStools.utils import exceptions, configmanager, logger -from qgis.core import QgsSettings - _USER_AGENT = f"ORSQGISClient@v{__version__}" - class Client(QObject): """Performs requests to the ORS API services.""" + overQueryLimit = pyqtSignal(int) + def __init__(self, provider: Optional[dict] = None, agent: Optional[str] = None) -> None: """ :param provider: A openrouteservice provider from config.yml @@ -65,13 +69,8 @@ def __init__(self, provider: Optional[dict] = None, agent: Optional[str] = None) self.key = provider["key"] self.base_url = provider["base_url"] self.ENV_VARS = provider.get("ENV_VARS") + self.timeout = provider.get("timeout") - # self.session = requests.Session() - retry_timeout = provider.get("timeout") - - self.nam = networkaccessmanager.NetworkAccessManager(debug=False, timeout=retry_timeout) - - self.retry_timeout = timedelta(seconds=retry_timeout) self.headers = { "User-Agent": _USER_AGENT, "Content-type": "application/json", @@ -88,13 +87,11 @@ def __init__(self, provider: Optional[dict] = None, agent: Optional[str] = None) self.url = None self.warnings = None - overQueryLimit = pyqtSignal() - def request( self, url: str, params: dict, - first_request_time: Optional[datetime.time] = None, + first_request_time: Optional[datetime] = None, retry_counter: int = 0, post_json: Optional[dict] = None, ): @@ -131,140 +128,82 @@ def request( first_request_time = datetime.now() elapsed = datetime.now() - first_request_time - if elapsed > self.retry_timeout: + if elapsed > timedelta(seconds=self.timeout): raise exceptions.Timeout() - if retry_counter > 0: - # 0.5 * (1.5 ^ i) is an increased sleep time of 1.5x per iteration, - # starting at 0.5s when retry_counter=1. The first retry will occur - # at 1, so subtract that first. - delay_seconds = 1.5 ** (retry_counter - 1) - - # Jitter this value by 50% and pause. - time.sleep(delay_seconds * (random.random() + 0.5)) - - authed_url = self._generate_auth_url( - url, - params, - ) + authed_url = self._generate_auth_url(url, params) self.url = self.base_url + authed_url - # Default to the client-level self.requests_kwargs, with method-level - # requests_kwargs arg overriding. - # final_requests_kwargs = self.requests_kwargs + request = QNetworkRequest(QUrl(self.url)) + for header, value in self.headers.items(): + request.setRawHeader(header.encode(), value.encode()) - # Determine GET/POST - # requests_method = self.session.get - requests_method = "GET" - body = None - if post_json is not None: - # requests_method = self.session.post - # final_requests_kwargs["json"] = post_json - body = post_json - requests_method = "POST" + blocking_request = QgsBlockingNetworkRequest() - logger.log(f"url: {self.url}\nParameters: {json.dumps(body, indent=2)}", 0) + logger.log(f"url: {self.url}\nParameters: {json.dumps(post_json, indent=2)}", 0) try: - response, content = self.nam.request( - self.url, method=requests_method, body=body, headers=self.headers, blocking=True - ) - except networkaccessmanager.RequestsExceptionTimeout: - raise exceptions.Timeout + if post_json is not None: + result = blocking_request.post(request, json.dumps(post_json).encode()) + else: + result = blocking_request.get(request) - except networkaccessmanager.RequestsException: - try: - self._check_status() + if result != QgsBlockingNetworkRequest.NoError: + self._check_status(blocking_request) + + reply = blocking_request.reply() + if not reply: + raise exceptions.GenericServerError("0", "No response received") + content = reply.content().data().decode('utf-8') + + except Exception as e: + try: + self._check_status(blocking_request) except exceptions.OverQueryLimit as e: - # Let the instances know something happened - # noinspection PyUnresolvedReferences + self.overQueryLimit.emit() logger.log(f"{e.__class__.__name__}: {str(e)}", 1) - - iface.messageBar().pushMessage( - "ORSTools", "Rate limit exceeded, retrying...", level=Qgis.Warning, duration=2 - ) - return self.request(url, params, first_request_time, retry_counter + 1, post_json) - except exceptions.ApiError as e: - logger.log( - f"Feature ID {post_json['id']} caused a {e.__class__.__name__}: {str(e)}", 2 - ) + if post_json: + logger.log( + f"Feature ID {post_json['id']} caused a {e.__class__.__name__}: {str(e)}", 2 + ) raise - raise - # Write env variables if successful if self.ENV_VARS: for env_var in self.ENV_VARS: - configmanager.write_env_var( - env_var, response.headers.get(self.ENV_VARS[env_var], "None") - ) + header_value = reply.rawHeader(self.ENV_VARS[env_var].encode()).data().decode() + configmanager.write_env_var(env_var, header_value) - # Reset to old value self.settings.setValue("qgis/networkAndProxy/userAgent", self.user_agent) - return json.loads(content.decode("utf-8")) - - def _check_status(self) -> None: - """ - Casts JSON response to dict - - :raises ORStools.utils.exceptions.OverQueryLimitError: when rate limit is exhausted, HTTP 429 - :raises ORStools.utils.exceptions.ApiError: when the backend API throws an error, HTTP 400 - :raises ORStools.utils.exceptions.InvalidKey: when API key is invalid (or quota is exceeded), HTTP 403 - :raises ORStools.utils.exceptions.GenericServerError: all other HTTP errors + return json.loads(content) - :returns: response body - :rtype: dict - """ + def _check_status(self, blocking_request: QgsBlockingNetworkRequest) -> None: + """Check response status and raise appropriate exceptions.""" + reply = blocking_request.reply() + if not reply: + raise Exception("No response received. Check provider settings and availability.") - status_code = self.nam.http_call_result.status_code - message = ( - self.nam.http_call_result.text - if self.nam.http_call_result.text != "" - else self.nam.http_call_result.reason - ) + status_code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) + message = reply.content().data().decode() if not status_code: - raise Exception( - f"{message}. Are your provider settings correct and the provider ready?" - ) + raise Exception(f"No status code received. Check provider settings.") - elif status_code == 403: + if status_code == 403: raise exceptions.InvalidKey(str(status_code), message) - elif status_code == 429: raise exceptions.OverQueryLimit(str(status_code), message) - # Internal error message for Bad Request elif 400 <= status_code < 500: raise exceptions.ApiError(str(status_code), message) - # Other HTTP errors have different formatting elif status_code != 200: raise exceptions.GenericServerError(str(status_code), message) def _generate_auth_url(self, path: str, params: Union[Dict, List]) -> str: - """Returns the path and query string portion of the request URL, first - adding any necessary parameters. - - :param path: The path portion of the URL. - :type path: string - - :param params: URL parameters. - :type params: dict or list of key/value tuples - - :returns: encoded URL - :rtype: string - """ - if isinstance(params, dict): params = sorted(dict(**params).items()) - - # Only auto-add API key when using ORS. If own instance, API key must - # be explicitly added to params - # if self.key: - # params.append(("api_key", self.key)) - - return path + "?" + unquote_unreserved(urlencode(params)) + return path + "?" + unquote_unreserved(urlencode(params)) \ No newline at end of file diff --git a/ORStools/common/networkaccessmanager.py b/ORStools/common/networkaccessmanager.py deleted file mode 100644 index 5641e6da..00000000 --- a/ORStools/common/networkaccessmanager.py +++ /dev/null @@ -1,427 +0,0 @@ -# -*- coding: utf-8 -*- -""" -*************************************************************************** - An httplib2 replacement that uses QgsNetworkAccessManager - - --------------------- - Date : August 2016 - Copyright : (C) 2016 Boundless, http://boundlessgeo.com - Email : apasotti at boundlessgeo dot com -*************************************************************************** -* * -* This program is free software; you can redistribute it and/or modify * -* it under the terms of the GNU General Public License as published by * -* the Free Software Foundation; either version 2 of the License, or * -* (at your option) any later version. * -* * -*************************************************************************** -""" - -from builtins import str -from builtins import object -import json - -__author__ = "Alessandro Pasotti" -__date__ = "August 2016" - -import re -import io -import urllib.parse - -from qgis.PyQt.QtCore import QUrl, QEventLoop - -from qgis.PyQt.QtNetwork import QNetworkRequest, QNetworkReply - -from qgis.core import QgsApplication, QgsNetworkAccessManager, QgsMessageLog - -# FIXME: ignored -DEFAULT_MAX_REDIRECTS = 4 - - -class RequestsException(Exception): - pass - - -class RequestsExceptionTimeout(RequestsException): - pass - - -class RequestsExceptionConnectionError(RequestsException): - pass - - -class RequestsExceptionUserAbort(RequestsException): - pass - - -class Map(dict): - """ - Example: - m = Map({'first_name': 'Eduardo'}, last_name='Pool', age=24, sports=['Soccer']) - """ - - def __init__(self, *args, **kwargs): - super(Map, self).__init__(*args, **kwargs) - for arg in args: - if isinstance(arg, dict): - for k, v in arg.items(): - self[k] = v - - if kwargs: - for k, v in kwargs.items(): - self[k] = v - - def __getattr__(self, attr): - return self.get(attr) - - def __setattr__(self, key, value): - self.__setitem__(key, value) - - def __setitem__(self, key, value): - super(Map, self).__setitem__(key, value) - self.__dict__.update({key: value}) - - def __delattr__(self, item): - self.__delitem__(item) - - def __delitem__(self, key): - super(Map, self).__delitem__(key) - del self.__dict__[key] - - -class Response(Map): - pass - - -class NetworkAccessManager(object): - """ - This class mimics httplib2 by using QgsNetworkAccessManager for all - network calls. - - The return value is a tuple of (response, content), the first being and - instance of the Response class, the second being a string that contains - the response entity body. - - Parameters - ---------- - debug : bool - verbose logging if True - exception_class : Exception - Custom exception class - - Usage 1 (blocking mode) - ----- - :: - nam = NetworkAccessManager(authcgf) - try: - (response, content) = nam.request('http://www.example.com') - except RequestsException as e: - # Handle exception - pass - - Usage 2 (Non blocking mode) - ------------------------- - :: - NOTE! if blocking mode returns immediately - it's up to the caller to manage listeners in case - of non blocking mode - - nam = NetworkAccessManager(authcgf) - try: - nam.request('http://www.example.com', blocking=False) - nam.reply.finished.connect(a_signal_listener) - except RequestsException as e: - # Handle exception - pass - - Get response using method: - nam.httpResult() that return a dictionary with keys: - 'status' - http code result come from reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) - 'status_code' - http code result come from reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) - 'status_message' - reply message string from reply.attribute(QNetworkRequest.HttpReasonPhraseAttribute) - 'content' - bytearray returned from reply - 'ok' - request success [True, False] - 'headers' - Dictionary containing the reply header - 'reason' - formatted message string with reply.errorString() - 'exception' - the exception returned during execution - """ - - def __init__( - self, - authid=None, - disable_ssl_certificate_validation=False, - exception_class=None, - debug=True, - timeout=60, - ) -> None: - self.disable_ssl_certificate_validation = disable_ssl_certificate_validation - self.authid = authid - self.reply = None - self.debug = debug - self.exception_class = exception_class - self.on_abort = False - self.blocking_mode = False - self.http_call_result = Response( - { - "status": 0, - "status_code": 0, - "status_message": "", - "content": "", - "ok": False, - "headers": {}, - "reason": "", - "exception": None, - } - ) - self.timeout = timeout - - def msg_log(self, msg: str) -> None: - if self.debug: - QgsMessageLog.logMessage(msg, "NetworkAccessManager") - - def httpResult(self) -> None: - return self.http_call_result - - def auth_manager(self) -> None: - return QgsApplication.authManager() - - def request( - self, url: str, method: str = "GET", body=None, headers=None, blocking: bool = True - ): - """ - Make a network request by calling QgsNetworkAccessManager. - redirections argument is ignored and is here only for httplib2 compatibility. - """ - self.msg_log(f"http_call request: {url}") - - self.blocking_mode = blocking - req = QNetworkRequest() - # Avoid double quoting form QUrl - url = urllib.parse.unquote(url) - req.setUrl(QUrl(url)) - if headers is not None: - # This fixes a weird error with compressed content not being correctly - # inflated. - # If you set the header on the QNetworkRequest you are basically telling - # QNetworkAccessManager "I know what I'm doing, please don't do any content - # encoding processing". - # See: https://bugs.webkit.org/show_bug.cgi?id=63696#c1 - try: - del headers["Accept-Encoding"] - except KeyError: - pass - for k, v in list(headers.items()): - self.msg_log("Setting header %s to %s" % (k, v)) - if k and v: - req.setRawHeader(k.encode(), v.encode()) - if self.authid: - self.msg_log(f"Update request w/ authid: {self.authid}") - self.auth_manager().updateNetworkRequest(req, self.authid) - if self.reply is not None and self.reply.isRunning(): - self.reply.close() - if method.lower() == "delete": - func = getattr(QgsNetworkAccessManager.instance(), "deleteResource") - else: - func = getattr(QgsNetworkAccessManager.instance(), method.lower()) - # Calling the server ... - # Let's log the whole call for debugging purposes: - self.msg_log("Sending %s request to %s" % (method.upper(), req.url().toString())) - self.on_abort = False - headers = {str(h): str(req.rawHeader(h)) for h in req.rawHeaderList()} - for k, v in list(headers.items()): - self.msg_log("%s: %s" % (k, v)) - if method.lower() in ["post", "put"]: - if isinstance(body, io.IOBase): - body = body.read() - if isinstance(body, str): - body = body.encode() - if isinstance(body, dict): - body = str(json.dumps(body)).encode(encoding="utf-8") - self.reply = func(req, body) - else: - self.reply = func(req) - if self.authid: - self.msg_log(f"Update reply w/ authid: {self.authid}") - self.auth_manager().updateNetworkReply(self.reply, self.authid) - - QgsNetworkAccessManager.instance().setTimeout(self.timeout * 1000) - - # necessary to trap local timeout managed by QgsNetworkAccessManager - # calling QgsNetworkAccessManager::abortRequest - QgsNetworkAccessManager.instance().requestTimedOut.connect(self.requestTimedOut) - - self.reply.sslErrors.connect(self.sslErrors) - self.reply.finished.connect(self.replyFinished) - self.reply.downloadProgress.connect(self.downloadProgress) - - # block if blocking mode otherwise return immediately - # it's up to the caller to manage listeners in case of no blocking mode - if not self.blocking_mode: - return None, None - - # Call and block - self.el = QEventLoop() - self.reply.finished.connect(self.el.quit) - - # Catch all exceptions (and clean up requests) - try: - self.el.exec(QEventLoop.ProcessEventsFlag.ExcludeUserInputEvents) - except Exception as e: - raise e - - if self.reply: - self.reply.finished.disconnect(self.el.quit) - - # emit exception in case of error - if not self.http_call_result.ok: - if self.http_call_result.exception and not self.exception_class: - raise self.http_call_result.exception - else: - raise self.exception_class(self.http_call_result.reason) - - return self.http_call_result, self.http_call_result.content - - def downloadProgress(self, bytesReceived, bytesTotal) -> None: - """Keep track of the download progress""" - # self.msg_log("downloadProgress %s of %s ..." % (bytesReceived, bytesTotal)) - pass - - # noinspection PyUnusedLocal - def requestTimedOut(self, reply) -> None: - """Trap the timeout. In Async mode requestTimedOut is called after replyFinished""" - # adapt http_call_result basing on receiving qgs timer timout signal - self.exception_class = RequestsExceptionTimeout - self.http_call_result.exception = RequestsExceptionTimeout("Timeout error") - - def replyFinished(self) -> None: - err = self.reply.error() - httpStatus = self.reply.attribute(QNetworkRequest.Attribute.HttpStatusCodeAttribute) - httpStatusMessage = self.reply.attribute( - QNetworkRequest.Attribute.HttpReasonPhraseAttribute - ) - self.http_call_result.status_code = httpStatus - self.http_call_result.status = httpStatus - self.http_call_result.status_message = httpStatusMessage - for k, v in self.reply.rawHeaderPairs(): - self.http_call_result.headers[str(k.data(), encoding="utf-8")] = str( - v.data(), encoding="utf-8" - ) - self.http_call_result.headers[str(k.data(), encoding="utf-8").lower()] = str( - v.data(), encoding="utf-8" - ) - - if err != QNetworkReply.NetworkError.NoError: - # handle error - # check if errorString is empty, if so, then set err string as - # reply dump - if re.match("(.)*server replied: $", self.reply.errorString()): - errString = self.reply.errorString() + self.http_call_result.content - else: - errString = self.reply.errorString() - # check if self.http_call_result.status_code is available (client abort - # does not produce http.status_code) - if self.http_call_result.status_code: - msg = f"Network error #{self.http_call_result.status_code}: {errString}" - else: - msg = f"Network error: {errString}" - - self.http_call_result.reason = msg - self.http_call_result.text = str(self.reply.readAll().data(), encoding="utf-8") - self.http_call_result.ok = False - self.msg_log(msg) - # set return exception - if err == QNetworkReply.NetworkError.TimeoutError: - self.http_call_result.exception = RequestsExceptionTimeout(msg) - - elif err == QNetworkReply.NetworkError.ConnectionRefusedError: - self.http_call_result.exception = RequestsExceptionConnectionError(msg) - - elif err == QNetworkReply.NetworkError.OperationCanceledError: - # request abort by calling NAM.abort() => cancelled by the user - if self.on_abort: - self.http_call_result.exception = RequestsExceptionUserAbort(msg) - else: - self.http_call_result.exception = RequestsException(msg) - - else: - self.http_call_result.exception = RequestsException(msg) - - # overload exception to the custom exception if available - if self.exception_class: - self.http_call_result.exception = self.exception_class(msg) - - else: - # Handle redirections - redirectionUrl = self.reply.attribute( - QNetworkRequest.Attribute.RedirectionTargetAttribute - ) - if redirectionUrl is not None and redirectionUrl != self.reply.url(): - if redirectionUrl.isRelative(): - redirectionUrl = self.reply.url().resolved(redirectionUrl) - - msg = f"Redirected from '{self.reply.url().toString()}' to '{redirectionUrl.toString()}'" - self.msg_log(msg) - - self.reply.deleteLater() - self.reply = None - self.request(redirectionUrl.toString()) - - # really end request - else: - msg = f"Network success #{self.reply.error()}" - self.http_call_result.reason = msg - self.msg_log(msg) - - ba = self.reply.readAll() - self.http_call_result.content = bytes(ba) - self.http_call_result.text = str(ba.data(), encoding="utf-8") - self.http_call_result.ok = True - - # Let's log the whole response for debugging purposes: - self.msg_log( - "Got response %s %s from %s" - % ( - self.http_call_result.status_code, - self.http_call_result.status_message, - self.reply.url().toString(), - ) - ) - for k, v in list(self.http_call_result.headers.items()): - self.msg_log("%s: %s" % (k, v)) - if len(self.http_call_result.content) < 1024: - self.msg_log("Payload :\n%s" % self.http_call_result.text) - else: - self.msg_log("Payload is > 1 KB ...") - - # clean reply - if self.reply is not None: - if self.reply.isRunning(): - self.reply.close() - self.msg_log("Deleting reply ...") - # Disconnect all slots - self.reply.sslErrors.disconnect(self.sslErrors) - self.reply.finished.disconnect(self.replyFinished) - self.reply.downloadProgress.disconnect(self.downloadProgress) - self.reply.deleteLater() - self.reply = None - else: - self.msg_log("Reply was already deleted ...") - - def sslErrors(self, ssl_errors) -> None: - """ - Handle SSL errors, logging them if debug is on and ignoring them - if disable_ssl_certificate_validation is set. - """ - if ssl_errors: - for v in ssl_errors: - self.msg_log("SSL Error: %s" % v.errorString()) - if self.disable_ssl_certificate_validation: - self.reply.ignoreSslErrors() - - def abort(self) -> None: - """ - Handle request to cancel HTTP call - """ - if self.reply and self.reply.isRunning(): - self.on_abort = True - self.reply.abort() From 6e38cabef76bb22387c32233ba83de1a5046d714 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Mon, 30 Jun 2025 13:33:22 +0200 Subject: [PATCH 2/9] fix: enhance error handling and retry logic for network requests --- ORStools/common/client.py | 68 +++++++++++++++++----- ORStools/proc/base_processing_algorithm.py | 4 +- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/ORStools/common/client.py b/ORStools/common/client.py index d29e9bd4..fe79f724 100644 --- a/ORStools/common/client.py +++ b/ORStools/common/client.py @@ -36,13 +36,7 @@ from qgis.PyQt.QtCore import QObject, pyqtSignal, QUrl from qgis.PyQt.QtNetwork import QNetworkRequest -from qgis.utils import iface -from qgis.core import ( - Qgis, - QgsSettings, - QgsBlockingNetworkRequest, - QgsNetworkAccessManager -) +from qgis.core import QgsSettings, QgsBlockingNetworkRequest from requests.utils import unquote_unreserved from ORStools import __version__ @@ -50,6 +44,7 @@ _USER_AGENT = f"ORSQGISClient@v{__version__}" + class Client(QObject): """Performs requests to the ORS API services.""" @@ -70,6 +65,7 @@ def __init__(self, provider: Optional[dict] = None, agent: Optional[str] = None) self.base_url = provider["base_url"] self.ENV_VARS = provider.get("ENV_VARS") self.timeout = provider.get("timeout") + self.retry_counter: int = 0 self.headers = { "User-Agent": _USER_AGENT, @@ -92,7 +88,6 @@ def request( url: str, params: dict, first_request_time: Optional[datetime] = None, - retry_counter: int = 0, post_json: Optional[dict] = None, ): """Performs HTTP GET/POST with credentials, returning the body as @@ -155,16 +150,20 @@ def request( if not reply: raise exceptions.GenericServerError("0", "No response received") - content = reply.content().data().decode('utf-8') + content = reply.content().data().decode("utf-8") - except Exception as e: + except Exception: try: self._check_status(blocking_request) - except exceptions.OverQueryLimit as e: - self.overQueryLimit.emit() + except exceptions.OverQueryLimit as e: logger.log(f"{e.__class__.__name__}: {str(e)}", 1) - return self.request(url, params, first_request_time, retry_counter + 1, post_json) + delay_seconds = self.get_delay_seconds(self.retry_counter) + self.overQueryLimit.emit(delay_seconds) + self.retry_counter += 1 + time.sleep(delay_seconds) + return self.request(url, params, first_request_time, post_json) + except exceptions.ApiError as e: if post_json: logger.log( @@ -173,15 +172,29 @@ def request( raise raise + # Write env variables if successful if self.ENV_VARS: for env_var in self.ENV_VARS: header_value = reply.rawHeader(self.ENV_VARS[env_var].encode()).data().decode() configmanager.write_env_var(env_var, header_value) + # Reset to old value self.settings.setValue("qgis/networkAndProxy/userAgent", self.user_agent) return json.loads(content) + def get_delay_seconds(self, retry_counter: int) -> int: + if retry_counter == 0: + delay_seconds = 61 # First retry after exactly 61 seconds + else: + # Exponential backoff starting from 60 seconds + base_delay = min(60 * (1.5 ** (retry_counter - 1)), 300) # Cap at 5 minutes + jitter = random.uniform(0.3, 0.6) + delay_seconds = base_delay * jitter + + logger.log(f"Retry Counter: {retry_counter}, Delay: {delay_seconds:.2f}s", 1) + return int(delay_seconds) + def _check_status(self, blocking_request: QgsBlockingNetworkRequest) -> None: """Check response status and raise appropriate exceptions.""" reply = blocking_request.reply() @@ -192,18 +205,41 @@ def _check_status(self, blocking_request: QgsBlockingNetworkRequest) -> None: message = reply.content().data().decode() if not status_code: - raise Exception(f"No status code received. Check provider settings.") + raise Exception( + f"{message}. Are your provider settings correct and the provider ready?" + ) - if status_code == 403: + elif status_code == 403: raise exceptions.InvalidKey(str(status_code), message) elif status_code == 429: raise exceptions.OverQueryLimit(str(status_code), message) + # Internal error message for Bad Request elif 400 <= status_code < 500: raise exceptions.ApiError(str(status_code), message) + # Other HTTP errors have different formatting elif status_code != 200: raise exceptions.GenericServerError(str(status_code), message) def _generate_auth_url(self, path: str, params: Union[Dict, List]) -> str: + """Returns the path and query string portion of the request URL, first + adding any necessary parameters. + + :param path: The path portion of the URL. + :type path: string + + :param params: URL parameters. + :type params: dict or list of key/value tuples + + :returns: encoded URL + :rtype: string + """ + if isinstance(params, dict): params = sorted(dict(**params).items()) - return path + "?" + unquote_unreserved(urlencode(params)) \ No newline at end of file + + # Only auto-add API key when using ORS. If own instance, API key must + # be explicitly added to params + # if self.key: + # params.append(("api_key", self.key)) + + return path + "?" + unquote_unreserved(urlencode(params)) diff --git a/ORStools/proc/base_processing_algorithm.py b/ORStools/proc/base_processing_algorithm.py index 3f5751d4..71b6fed1 100644 --- a/ORStools/proc/base_processing_algorithm.py +++ b/ORStools/proc/base_processing_algorithm.py @@ -203,7 +203,9 @@ def _get_ors_client_from_provider( ors_provider = providers[provider] ors_client = client.Client(ors_provider, agent) ors_client.overQueryLimit.connect( - lambda: feedback.reportError("OverQueryLimit: Retrying...") + lambda retry_time: feedback.reportError( + f"OverQueryLimit: Retrying in {retry_time} seconds." + ) ) return ors_client From 6f676bc6bd9b043eeafb02a4384ea6dbac03c75c Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Wed, 24 Sep 2025 10:15:22 +0200 Subject: [PATCH 3/9] docs: add to changelog --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 419a91eb..93ac5f01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,19 +40,19 @@ RELEASING: 14. Create new release in GitHub with tag version and release title of `vX.X.X` --> +<<<<<<< HEAD ## Unreleased ### Added - pre-commit configuration with Ruff linter for code quality enforcement +- Improve cursor behaviour during digitization ### Fixed - Delete annotations when plugin is uninstalled ([#346](https://github.com/GIScience/orstools-qgis-plugin/issues/346)) - Reset hand cursor after deactivating line tool ([#342](https://github.com/GIScience/orstools-qgis-plugin/issues/342)) - Set url slashes correctly with optimization requests in procs ([#347](https://github.com/GIScience/orstools-qgis-plugin/issues/347)) - Qt6 incompatibility with QVariant ([#355](https://github.com/GIScience/orstools-qgis-plugin/issues/355)) - -## Added -- Improve cursor behaviour during digitization +- Switch to QgsBlockingNetworkRequest ([#117](https://github.com/GIScience/orstools-qgis-plugin/issues/117)) ## [2.0.1] - 2025-06-01 ### Added From b54d065d68ec4522cb248b524539c69ba5ab192c Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Tue, 21 Oct 2025 12:53:36 +0200 Subject: [PATCH 4/9] test: add retry logic for client on OverQueryLimit --- tests/test_common.py | 56 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/tests/test_common.py b/tests/test_common.py index 2061f097..b161f5fe 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -12,6 +12,60 @@ def setUpClass(cls) -> None: if cls.api_key is None: raise ValueError("ORS_API_KEY environment variable is not set") + def test_client_retry_on_over_query_limit(self): + """Test that client retries on OverQueryLimit and eventually succeeds""" + provider = { + "ENV_VARS": None, + "base_url": "https://api.openrouteservice.org", + "key": self.api_key, + "name": "openrouteservice", + "timeout": 60, + } + + params = { + "preference": "fastest", + "geometry": "true", + "instructions": "false", + "elevation": True, + "id": 1, + "coordinates": [[8.684101, 50.131613], [8.68534, 50.131651]], + } + + agent = "QGIS_ORStools_testing" + profile = "driving-car" + clnt = client.Client(provider, agent) + + # Mock to fail twice, then succeed + call_count = [0] + def mock_request(post_json, blocking_request, request): + call_count[0] += 1 + if call_count[0] <= 2: + raise client.exceptions.OverQueryLimit("429", "Over query limit") + + # Return a mock successful response on third call + from unittest.mock import Mock + mock_reply = Mock() + mock_content = Mock() + mock_data = Mock() + mock_data.decode.return_value = '{"success": true}' + mock_content.data.return_value = mock_data + mock_reply.content.return_value = mock_content + return mock_reply + + clnt._request = mock_request + clnt.get_delay_seconds = lambda x: 0 # Speed up test by removing delays + + response = clnt.fetch_with_retry( + "/v2/directions/" + profile + "/geojson", + {}, + post_json=params, + max_retries=5 + ) + + self.assertEqual(call_count[0], 3) # Should have made 3 attempts + self.assertTrue(response) + self.assertEqual(response["success"], True) + def test_client_request_geometry(self): test_response = { "type": "FeatureCollection", @@ -94,7 +148,7 @@ def test_client_request_geometry(self): agent = "QGIS_ORStools_testing" profile = "driving-car" clnt = client.Client(provider, agent) - response = clnt.request("/v2/directions/" + profile + "/geojson", {}, post_json=params) + response = clnt.fetch_with_retry("/v2/directions/" + profile + "/geojson", {}, post_json=params) self.assertAlmostEqual( response["features"][0]["geometry"]["coordinates"][0][0], test_response["features"][0]["geometry"]["coordinates"][0][0], From b60986af8011e289b452b0cb1e523aad4ba1cf60 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Tue, 21 Oct 2025 12:54:58 +0200 Subject: [PATCH 5/9] refactor: use iteration instead of recursion for requests --- ORStools/common/client.py | 101 +++++++----------- ORStools/proc/directions_lines_proc.py | 4 +- ORStools/proc/directions_points_layer_proc.py | 4 +- .../proc/directions_points_layers_proc.py | 2 +- ORStools/proc/export_proc.py | 2 +- ORStools/proc/isochrones_layer_proc.py | 2 +- ORStools/proc/isochrones_point_proc.py | 2 +- ORStools/proc/matrix_proc.py | 2 +- ORStools/proc/snap_layer_proc.py | 2 +- ORStools/proc/snap_point_proc.py | 2 +- ORStools/utils/router.py | 4 +- 11 files changed, 49 insertions(+), 78 deletions(-) diff --git a/ORStools/common/client.py b/ORStools/common/client.py index fe79f724..3ec64784 100644 --- a/ORStools/common/client.py +++ b/ORStools/common/client.py @@ -35,7 +35,7 @@ from urllib.parse import urlencode from qgis.PyQt.QtCore import QObject, pyqtSignal, QUrl -from qgis.PyQt.QtNetwork import QNetworkRequest +from qgis.PyQt.QtNetwork import QNetworkRequest, QNetworkReply from qgis.core import QgsSettings, QgsBlockingNetworkRequest from requests.utils import unquote_unreserved @@ -65,7 +65,6 @@ def __init__(self, provider: Optional[dict] = None, agent: Optional[str] = None) self.base_url = provider["base_url"] self.ENV_VARS = provider.get("ENV_VARS") self.timeout = provider.get("timeout") - self.retry_counter: int = 0 self.headers = { "User-Agent": _USER_AGENT, @@ -83,53 +82,38 @@ def __init__(self, provider: Optional[dict] = None, agent: Optional[str] = None) self.url = None self.warnings = None - def request( + def _request( + self, + post_json: Optional[dict], + blocking_request: QgsBlockingNetworkRequest, + request: QNetworkRequest + ) -> str: + if post_json is not None: + result = blocking_request.post(request, json.dumps(post_json).encode()) + else: + result = blocking_request.get(request) + + if result != QgsBlockingNetworkRequest.NoError: + self._check_status(blocking_request.reply()) + return None + + return blocking_request.reply() + + def fetch_with_retry( self, url: str, params: dict, first_request_time: Optional[datetime] = None, post_json: Optional[dict] = None, + max_retries: int = 100, ): - """Performs HTTP GET/POST with credentials, returning the body as - JSON. - - :param url: URL extension for request. Should begin with a slash. - :type url: string - - :param params: HTTP GET parameters. - :type params: dict or list of key/value tuples - - :param first_request_time: The time of the first request (None if no - retries have occurred). - :type first_request_time: datetime.datetime - - :param retry_counter: Amount of retries with increasing timeframe before raising a timeout exception - :type retry_counter: int - - :param post_json: Parameters for POST endpoints - :type post_json: dict - - :param retry_counter: Duration the requests will be retried for before - raising a timeout exception. - :type retry_counter: int - - :raises ORStools.utils.exceptions.ApiError: when the API returns an error. - - :returns: openrouteservice response body - :rtype: dict - """ - - if not first_request_time: - first_request_time = datetime.now() - - elapsed = datetime.now() - first_request_time - if elapsed > timedelta(seconds=self.timeout): - raise exceptions.Timeout() + first_request_time = datetime.now() authed_url = self._generate_auth_url(url, params) self.url = self.base_url + authed_url request = QNetworkRequest(QUrl(self.url)) + for header, value in self.headers.items(): request.setRawHeader(header.encode(), value.encode()) @@ -137,44 +121,35 @@ def request( logger.log(f"url: {self.url}\nParameters: {json.dumps(post_json, indent=2)}", 0) - try: - if post_json is not None: - result = blocking_request.post(request, json.dumps(post_json).encode()) - else: - result = blocking_request.get(request) - - if result != QgsBlockingNetworkRequest.NoError: - self._check_status(blocking_request) - - reply = blocking_request.reply() - if not reply: - raise exceptions.GenericServerError("0", "No response received") + content = None - content = reply.content().data().decode("utf-8") - - except Exception: + for i in range(max_retries): try: - self._check_status(blocking_request) - + reply = self._request(post_json, blocking_request, request) + content = reply.content().data().decode() + break + except exceptions.OverQueryLimit as e: + if datetime.now() - first_request_time > timedelta(seconds=self.timeout): + raise exceptions.Timeout() + logger.log(f"{e.__class__.__name__}: {str(e)}", 1) - delay_seconds = self.get_delay_seconds(self.retry_counter) + + delay_seconds = self.get_delay_seconds(i) self.overQueryLimit.emit(delay_seconds) - self.retry_counter += 1 time.sleep(delay_seconds) - return self.request(url, params, first_request_time, post_json) - + except exceptions.ApiError as e: if post_json: logger.log( f"Feature ID {post_json['id']} caused a {e.__class__.__name__}: {str(e)}", 2 ) raise - raise # Write env variables if successful if self.ENV_VARS: for env_var in self.ENV_VARS: + #TODO header_value = reply.rawHeader(self.ENV_VARS[env_var].encode()).data().decode() configmanager.write_env_var(env_var, header_value) @@ -195,12 +170,8 @@ def get_delay_seconds(self, retry_counter: int) -> int: logger.log(f"Retry Counter: {retry_counter}, Delay: {delay_seconds:.2f}s", 1) return int(delay_seconds) - def _check_status(self, blocking_request: QgsBlockingNetworkRequest) -> None: + def _check_status(self, reply: QNetworkReply) -> None: """Check response status and raise appropriate exceptions.""" - reply = blocking_request.reply() - if not reply: - raise Exception("No response received. Check provider settings and availability.") - status_code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) message = reply.content().data().decode() diff --git a/ORStools/proc/directions_lines_proc.py b/ORStools/proc/directions_lines_proc.py index f81b5467..737349da 100644 --- a/ORStools/proc/directions_lines_proc.py +++ b/ORStools/proc/directions_lines_proc.py @@ -190,7 +190,7 @@ def processAlgorithm( endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])[ "optimization" ] - response = ors_client.request(f"/{endpoint}/", {}, post_json=params) + response = ors_client.fetch_with_retry(f"/{endpoint}/", {}, post_json=params) sink.addFeature( directions_core.get_output_features_optimization( @@ -228,7 +228,7 @@ def processAlgorithm( endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])[ "directions" ] - response = ors_client.request( + response = ors_client.fetch_with_retry( f"/v2/{endpoint}/{profile}/geojson", {}, post_json=params ) diff --git a/ORStools/proc/directions_points_layer_proc.py b/ORStools/proc/directions_points_layer_proc.py index 0f3310eb..7f18525e 100644 --- a/ORStools/proc/directions_points_layer_proc.py +++ b/ORStools/proc/directions_points_layer_proc.py @@ -231,7 +231,7 @@ def sort(f): endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])[ "optimization" ] - response = ors_client.request(f"/{endpoint}/", {}, post_json=params) + response = ors_client.fetch_with_retry(f"/{endpoint}/", {}, post_json=params) sink.addFeature( directions_core.get_output_features_optimization( @@ -269,7 +269,7 @@ def sort(f): endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])[ "directions" ] - response = ors_client.request( + response = ors_client.fetch_with_retry( f"/v2/{endpoint}/{profile}/geojson", {}, post_json=params ) diff --git a/ORStools/proc/directions_points_layers_proc.py b/ORStools/proc/directions_points_layers_proc.py index 024e05ef..f498ae10 100644 --- a/ORStools/proc/directions_points_layers_proc.py +++ b/ORStools/proc/directions_points_layers_proc.py @@ -242,7 +242,7 @@ def sort_end(f): endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])[ "directions" ] - response = ors_client.request( + response = ors_client.fetch_with_retry( f"/v2/{endpoint}/{profile}/geojson", {}, post_json=params ) except (exceptions.ApiError, exceptions.InvalidKey, exceptions.GenericServerError) as e: diff --git a/ORStools/proc/export_proc.py b/ORStools/proc/export_proc.py index 279d9f3a..1d98103c 100644 --- a/ORStools/proc/export_proc.py +++ b/ORStools/proc/export_proc.py @@ -113,7 +113,7 @@ def processAlgorithm( # Make request and catch ApiError try: endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])["export"] - response = ors_client.request(f"/v2/{endpoint}/{profile}", {}, post_json=params) + response = ors_client.fetch_with_retry(f"/v2/{endpoint}/{profile}", {}, post_json=params) nodes_dict = {item["nodeId"]: item["location"] for item in response["nodes"]} edges = response["edges"] for edge in edges: diff --git a/ORStools/proc/isochrones_layer_proc.py b/ORStools/proc/isochrones_layer_proc.py index 0a4b60e0..430b5403 100644 --- a/ORStools/proc/isochrones_layer_proc.py +++ b/ORStools/proc/isochrones_layer_proc.py @@ -200,7 +200,7 @@ def processAlgorithm( endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])[ "isochrones" ] - response = ors_client.request(f"/v2/{endpoint}/{profile}", {}, post_json=params) + response = ors_client.fetch_with_retry(f"/v2/{endpoint}/{profile}", {}, post_json=params) for isochrone in self.isochrones.get_features(response, params["id"]): sink.addFeature(isochrone) diff --git a/ORStools/proc/isochrones_point_proc.py b/ORStools/proc/isochrones_point_proc.py index b6b29c16..cd8127ad 100644 --- a/ORStools/proc/isochrones_point_proc.py +++ b/ORStools/proc/isochrones_point_proc.py @@ -157,7 +157,7 @@ def processAlgorithm( endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])[ "isochrones" ] - response = ors_client.request(f"/v2/{endpoint}/{profile}", {}, post_json=params) + response = ors_client.fetch_with_retry(f"/v2/{endpoint}/{profile}", {}, post_json=params) # Populate features from response for isochrone in self.isochrones.get_features(response, params["id"]): diff --git a/ORStools/proc/matrix_proc.py b/ORStools/proc/matrix_proc.py index 089347d3..c2f367f8 100644 --- a/ORStools/proc/matrix_proc.py +++ b/ORStools/proc/matrix_proc.py @@ -180,7 +180,7 @@ def processAlgorithm( # Make request and catch ApiError try: endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])["matrix"] - response = ors_client.request(f"/v2/{endpoint}/{profile}", {}, post_json=params) + response = ors_client.fetch_with_retry(f"/v2/{endpoint}/{profile}", {}, post_json=params) except (exceptions.ApiError, exceptions.InvalidKey, exceptions.GenericServerError) as e: msg = f"{e.__class__.__name__}: {str(e)}" diff --git a/ORStools/proc/snap_layer_proc.py b/ORStools/proc/snap_layer_proc.py index edd07be5..d3fbd64f 100644 --- a/ORStools/proc/snap_layer_proc.py +++ b/ORStools/proc/snap_layer_proc.py @@ -124,7 +124,7 @@ def processAlgorithm( # Make request and catch ApiError try: - response = ors_client.request("/v2/snap/" + profile, {}, post_json=params) + response = ors_client.fetch_with_retry("/v2/snap/" + profile, {}, post_json=params) point_features = get_snapped_point_features(response, sources_features, feedback) for feat in point_features: diff --git a/ORStools/proc/snap_point_proc.py b/ORStools/proc/snap_point_proc.py index 94fd2db1..6f077b58 100644 --- a/ORStools/proc/snap_point_proc.py +++ b/ORStools/proc/snap_point_proc.py @@ -109,7 +109,7 @@ def processAlgorithm( # Make request and catch ApiError try: - response = ors_client.request("/v2/snap/" + profile, {}, post_json=params) + response = ors_client.fetch_with_retry("/v2/snap/" + profile, {}, post_json=params) point_features = get_snapped_point_features(response, feedback=feedback) for feat in point_features: diff --git a/ORStools/utils/router.py b/ORStools/utils/router.py index bb320966..7496138d 100644 --- a/ORStools/utils/router.py +++ b/ORStools/utils/router.py @@ -60,7 +60,7 @@ def route_as_layer(dlg): """), ) return - response = clnt.request("/optimization", {}, post_json=params) + response = clnt.fetch_with_retry("/optimization", {}, post_json=params) feat = directions_core.get_output_features_optimization( response, params["vehicles"][0]["profile"] ) @@ -85,7 +85,7 @@ def route_as_layer(dlg): logger.log(msg, 0) dlg.debug_text.setText(msg) return - response = clnt.request("/v2/directions/" + profile + "/geojson", {}, post_json=params) + response = clnt.fetch_with_retry("/v2/directions/" + profile + "/geojson", {}, post_json=params) feat = directions_core.get_output_feature_directions( response, profile, params["preference"], directions.options ) From 485b0e100eb78bac102ebacfaa12eaed5b128162 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Tue, 21 Oct 2025 13:06:22 +0200 Subject: [PATCH 6/9] feat: use QTimer instead of time.sleep() --- ORStools/common/client.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ORStools/common/client.py b/ORStools/common/client.py index 3ec64784..63e1a09b 100644 --- a/ORStools/common/client.py +++ b/ORStools/common/client.py @@ -29,12 +29,11 @@ import json import random -import time from datetime import datetime, timedelta from typing import Union, Dict, List, Optional from urllib.parse import urlencode -from qgis.PyQt.QtCore import QObject, pyqtSignal, QUrl +from qgis.PyQt.QtCore import QObject, pyqtSignal, QUrl, QTimer, QEventLoop from qgis.PyQt.QtNetwork import QNetworkRequest, QNetworkReply from qgis.core import QgsSettings, QgsBlockingNetworkRequest from requests.utils import unquote_unreserved @@ -137,7 +136,10 @@ def fetch_with_retry( delay_seconds = self.get_delay_seconds(i) self.overQueryLimit.emit(delay_seconds) - time.sleep(delay_seconds) + + loop = QEventLoop() + QTimer.singleShot(delay_seconds * 1000, loop.quit) # milliseconds + loop.exec_() except exceptions.ApiError as e: if post_json: @@ -149,7 +151,6 @@ def fetch_with_retry( # Write env variables if successful if self.ENV_VARS: for env_var in self.ENV_VARS: - #TODO header_value = reply.rawHeader(self.ENV_VARS[env_var].encode()).data().decode() configmanager.write_env_var(env_var, header_value) From 803b28d5615c9918e2f2c3984fcf9c29bbdf58a3 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Tue, 21 Oct 2025 13:11:25 +0200 Subject: [PATCH 7/9] feat: add downloadProgress signal to Client --- ORStools/common/client.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ORStools/common/client.py b/ORStools/common/client.py index 63e1a09b..da3c5f45 100644 --- a/ORStools/common/client.py +++ b/ORStools/common/client.py @@ -48,6 +48,8 @@ class Client(QObject): """Performs requests to the ORS API services.""" overQueryLimit = pyqtSignal(int) + downloadProgress = pyqtSignal(int, int) + def __init__(self, provider: Optional[dict] = None, agent: Optional[str] = None) -> None: """ @@ -118,6 +120,10 @@ def fetch_with_retry( blocking_request = QgsBlockingNetworkRequest() + blocking_request.downloadProgress.connect( + lambda r, t: self.downloadProgress.emit(r, t) + ) + logger.log(f"url: {self.url}\nParameters: {json.dumps(post_json, indent=2)}", 0) content = None From 548f24007fb41e931ac74a960673278fdac883f0 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Tue, 21 Oct 2025 13:25:35 +0200 Subject: [PATCH 8/9] feat: download progress reporting --- ORStools/common/client.py | 8 +++--- ORStools/proc/base_processing_algorithm.py | 11 ++++++++ ORStools/proc/directions_lines_proc.py | 2 +- ORStools/proc/directions_points_layer_proc.py | 2 +- .../proc/directions_points_layers_proc.py | 2 +- ORStools/proc/export_proc.py | 6 +++-- ORStools/proc/isochrones_layer_proc.py | 6 +++-- ORStools/proc/isochrones_point_proc.py | 6 +++-- ORStools/proc/matrix_proc.py | 6 +++-- ORStools/proc/snap_layer_proc.py | 2 +- ORStools/proc/snap_point_proc.py | 2 +- ORStools/utils/router.py | 4 ++- tests/test_common.py | 25 ++++++++++--------- 13 files changed, 51 insertions(+), 31 deletions(-) diff --git a/ORStools/common/client.py b/ORStools/common/client.py index da3c5f45..34841332 100644 --- a/ORStools/common/client.py +++ b/ORStools/common/client.py @@ -48,7 +48,7 @@ class Client(QObject): """Performs requests to the ORS API services.""" overQueryLimit = pyqtSignal(int) - downloadProgress = pyqtSignal(int, int) + downloadProgress = pyqtSignal(int) def __init__(self, provider: Optional[dict] = None, agent: Optional[str] = None) -> None: @@ -120,9 +120,7 @@ def fetch_with_retry( blocking_request = QgsBlockingNetworkRequest() - blocking_request.downloadProgress.connect( - lambda r, t: self.downloadProgress.emit(r, t) - ) + blocking_request.downloadProgress.connect(lambda r, t: self.downloadProgress.emit(r/t)) logger.log(f"url: {self.url}\nParameters: {json.dumps(post_json, indent=2)}", 0) @@ -146,7 +144,7 @@ def fetch_with_retry( loop = QEventLoop() QTimer.singleShot(delay_seconds * 1000, loop.quit) # milliseconds loop.exec_() - + except exceptions.ApiError as e: if post_json: logger.log( diff --git a/ORStools/proc/base_processing_algorithm.py b/ORStools/proc/base_processing_algorithm.py index 71b6fed1..e9ff373f 100644 --- a/ORStools/proc/base_processing_algorithm.py +++ b/ORStools/proc/base_processing_algorithm.py @@ -232,6 +232,17 @@ def parseOptions(self, parameters: dict, context: QgsProcessingContext) -> dict: return options + def get_client( + self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback + ) -> client.Client: + """ + Returns a client instance for requests to the ors API + """ + ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client.downloadProgress.connect(feedback.setProgress) + + return ors_client + # noinspection PyUnusedLocal def initAlgorithm(self, configuration: Dict) -> None: """ diff --git a/ORStools/proc/directions_lines_proc.py b/ORStools/proc/directions_lines_proc.py index 737349da..094aa364 100644 --- a/ORStools/proc/directions_lines_proc.py +++ b/ORStools/proc/directions_lines_proc.py @@ -128,7 +128,7 @@ def __init__(self): def processAlgorithm( self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback ) -> Dict[str, str]: - ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client = self.get_client(parameters, context, feedback) profile = dict(enumerate(PROFILES))[parameters[self.IN_PROFILE]] diff --git a/ORStools/proc/directions_points_layer_proc.py b/ORStools/proc/directions_points_layer_proc.py index 7f18525e..20dfd73d 100644 --- a/ORStools/proc/directions_points_layer_proc.py +++ b/ORStools/proc/directions_points_layer_proc.py @@ -135,7 +135,7 @@ def __init__(self): def processAlgorithm( self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback ) -> Dict[str, str]: - ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client = self.get_client(parameters, context, feedback) profile = dict(enumerate(PROFILES))[parameters[self.IN_PROFILE]] diff --git a/ORStools/proc/directions_points_layers_proc.py b/ORStools/proc/directions_points_layers_proc.py index f498ae10..f0cc5615 100644 --- a/ORStools/proc/directions_points_layers_proc.py +++ b/ORStools/proc/directions_points_layers_proc.py @@ -149,7 +149,7 @@ def __init__(self): def processAlgorithm( self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback ) -> Dict[str, str]: - ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client = self.get_client(parameters, context, feedback) profile = dict(enumerate(PROFILES))[parameters[self.IN_PROFILE]] diff --git a/ORStools/proc/export_proc.py b/ORStools/proc/export_proc.py index 1d98103c..35a6d66a 100644 --- a/ORStools/proc/export_proc.py +++ b/ORStools/proc/export_proc.py @@ -77,7 +77,7 @@ def __init__(self): def processAlgorithm( self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback ) -> Dict[str, str]: - ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client = self.get_client(parameters, context, feedback) # Get profile value profile = dict(enumerate(PROFILES))[parameters[self.IN_PROFILE]] @@ -113,7 +113,9 @@ def processAlgorithm( # Make request and catch ApiError try: endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])["export"] - response = ors_client.fetch_with_retry(f"/v2/{endpoint}/{profile}", {}, post_json=params) + response = ors_client.fetch_with_retry( + f"/v2/{endpoint}/{profile}", {}, post_json=params + ) nodes_dict = {item["nodeId"]: item["location"] for item in response["nodes"]} edges = response["edges"] for edge in edges: diff --git a/ORStools/proc/isochrones_layer_proc.py b/ORStools/proc/isochrones_layer_proc.py index 430b5403..b23e7443 100644 --- a/ORStools/proc/isochrones_layer_proc.py +++ b/ORStools/proc/isochrones_layer_proc.py @@ -123,7 +123,7 @@ def __init__(self): def processAlgorithm( self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback ) -> Dict[str, str]: - ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client = self.get_client(parameters, context, feedback) profile = dict(enumerate(PROFILES))[parameters[self.IN_PROFILE]] dimension = dict(enumerate(DIMENSIONS))[parameters[self.IN_METRIC]] @@ -200,7 +200,9 @@ def processAlgorithm( endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])[ "isochrones" ] - response = ors_client.fetch_with_retry(f"/v2/{endpoint}/{profile}", {}, post_json=params) + response = ors_client.fetch_with_retry( + f"/v2/{endpoint}/{profile}", {}, post_json=params + ) for isochrone in self.isochrones.get_features(response, params["id"]): sink.addFeature(isochrone) diff --git a/ORStools/proc/isochrones_point_proc.py b/ORStools/proc/isochrones_point_proc.py index cd8127ad..aad94a99 100644 --- a/ORStools/proc/isochrones_point_proc.py +++ b/ORStools/proc/isochrones_point_proc.py @@ -109,7 +109,7 @@ def __init__(self): def processAlgorithm( self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback ) -> Dict[str, str]: - ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client = self.get_client(parameters, context, feedback) profile = dict(enumerate(PROFILES))[parameters[self.IN_PROFILE]] dimension = dict(enumerate(DIMENSIONS))[parameters[self.IN_METRIC]] @@ -157,7 +157,9 @@ def processAlgorithm( endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])[ "isochrones" ] - response = ors_client.fetch_with_retry(f"/v2/{endpoint}/{profile}", {}, post_json=params) + response = ors_client.fetch_with_retry( + f"/v2/{endpoint}/{profile}", {}, post_json=params + ) # Populate features from response for isochrone in self.isochrones.get_features(response, params["id"]): diff --git a/ORStools/proc/matrix_proc.py b/ORStools/proc/matrix_proc.py index c2f367f8..796296a5 100644 --- a/ORStools/proc/matrix_proc.py +++ b/ORStools/proc/matrix_proc.py @@ -93,7 +93,7 @@ def __init__(self): def processAlgorithm( self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback ) -> Dict[str, str]: - ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client = self.get_client(parameters, context, feedback) # Get profile value profile = dict(enumerate(PROFILES))[parameters[self.IN_PROFILE]] @@ -180,7 +180,9 @@ def processAlgorithm( # Make request and catch ApiError try: endpoint = self.get_endpoint_names_from_provider(parameters[self.IN_PROVIDER])["matrix"] - response = ors_client.fetch_with_retry(f"/v2/{endpoint}/{profile}", {}, post_json=params) + response = ors_client.fetch_with_retry( + f"/v2/{endpoint}/{profile}", {}, post_json=params + ) except (exceptions.ApiError, exceptions.InvalidKey, exceptions.GenericServerError) as e: msg = f"{e.__class__.__name__}: {str(e)}" diff --git a/ORStools/proc/snap_layer_proc.py b/ORStools/proc/snap_layer_proc.py index d3fbd64f..019377cd 100644 --- a/ORStools/proc/snap_layer_proc.py +++ b/ORStools/proc/snap_layer_proc.py @@ -76,7 +76,7 @@ def __init__(self) -> None: def processAlgorithm( self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback ) -> Dict[str, str]: - ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client = self.get_client(parameters, context, feedback) # Get profile value profile = dict(enumerate(PROFILES))[parameters[self.IN_PROFILE]] diff --git a/ORStools/proc/snap_point_proc.py b/ORStools/proc/snap_point_proc.py index 6f077b58..2e914c17 100644 --- a/ORStools/proc/snap_point_proc.py +++ b/ORStools/proc/snap_point_proc.py @@ -79,7 +79,7 @@ def __init__(self) -> None: def processAlgorithm( self, parameters: dict, context: QgsProcessingContext, feedback: QgsProcessingFeedback ) -> Dict[str, str]: - ors_client = self._get_ors_client_from_provider(parameters[self.IN_PROVIDER], feedback) + ors_client = self.get_client(parameters, context, feedback) # Get profile value profile = dict(enumerate(PROFILES))[parameters[self.IN_PROFILE]] diff --git a/ORStools/utils/router.py b/ORStools/utils/router.py index 7496138d..67e76d42 100644 --- a/ORStools/utils/router.py +++ b/ORStools/utils/router.py @@ -85,7 +85,9 @@ def route_as_layer(dlg): logger.log(msg, 0) dlg.debug_text.setText(msg) return - response = clnt.fetch_with_retry("/v2/directions/" + profile + "/geojson", {}, post_json=params) + response = clnt.fetch_with_retry( + "/v2/directions/" + profile + "/geojson", {}, post_json=params + ) feat = directions_core.get_output_feature_directions( response, profile, params["preference"], directions.options ) diff --git a/tests/test_common.py b/tests/test_common.py index b161f5fe..2558ea4a 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -21,7 +21,7 @@ def test_client_retry_on_over_query_limit(self): "name": "openrouteservice", "timeout": 60, } - + params = { "preference": "fastest", "geometry": "true", @@ -30,20 +30,22 @@ def test_client_retry_on_over_query_limit(self): "id": 1, "coordinates": [[8.684101, 50.131613], [8.68534, 50.131651]], } - + agent = "QGIS_ORStools_testing" profile = "driving-car" clnt = client.Client(provider, agent) - + # Mock to fail twice, then succeed call_count = [0] + def mock_request(post_json, blocking_request, request): call_count[0] += 1 if call_count[0] <= 2: raise client.exceptions.OverQueryLimit("429", "Over query limit") - + # Return a mock successful response on third call from unittest.mock import Mock + mock_reply = Mock() mock_content = Mock() mock_data = Mock() @@ -51,17 +53,14 @@ def mock_request(post_json, blocking_request, request): mock_content.data.return_value = mock_data mock_reply.content.return_value = mock_content return mock_reply - + clnt._request = mock_request clnt.get_delay_seconds = lambda x: 0 # Speed up test by removing delays - + response = clnt.fetch_with_retry( - "/v2/directions/" + profile + "/geojson", - {}, - post_json=params, - max_retries=5 + "/v2/directions/" + profile + "/geojson", {}, post_json=params, max_retries=5 ) - + self.assertEqual(call_count[0], 3) # Should have made 3 attempts self.assertTrue(response) self.assertEqual(response["success"], True) @@ -148,7 +147,9 @@ def test_client_request_geometry(self): agent = "QGIS_ORStools_testing" profile = "driving-car" clnt = client.Client(provider, agent) - response = clnt.fetch_with_retry("/v2/directions/" + profile + "/geojson", {}, post_json=params) + response = clnt.fetch_with_retry( + "/v2/directions/" + profile + "/geojson", {}, post_json=params + ) self.assertAlmostEqual( response["features"][0]["geometry"]["coordinates"][0][0], test_response["features"][0]["geometry"]["coordinates"][0][0], From ae53ec02aa0e699f63af96e4a083b0d92fcf8657 Mon Sep 17 00:00:00 2001 From: Till Frankenbach Date: Tue, 21 Oct 2025 13:30:36 +0200 Subject: [PATCH 9/9] docs: enhance Client class documentation --- ORStools/common/client.py | 115 ++++++++++++++++++++++++++++---------- 1 file changed, 85 insertions(+), 30 deletions(-) diff --git a/ORStools/common/client.py b/ORStools/common/client.py index 34841332..1d4e3063 100644 --- a/ORStools/common/client.py +++ b/ORStools/common/client.py @@ -45,20 +45,27 @@ class Client(QObject): - """Performs requests to the ORS API services.""" + """Performs requests to the ORS API services. + + This class handles HTTP communication with the openrouteservice API, + including authentication, retry logic for rate limiting, and progress + tracking. + + Signals: + overQueryLimit: Emitted when rate limit is hit, passes delay in seconds + downloadProgress: Emitted during download, passes progress ratio (0-1) + """ overQueryLimit = pyqtSignal(int) downloadProgress = pyqtSignal(int) - def __init__(self, provider: Optional[dict] = None, agent: Optional[str] = None) -> None: - """ - :param provider: A openrouteservice provider from config.yml - :type provider: dict + """Initialize the ORS API client. - :param retry_timeout: Timeout across multiple retryable requests, in - seconds. - :type retry_timeout: int + :param provider: Provider configuration containing base_url, key, timeout, and ENV_VARS + :type provider: dict + :param agent: User agent string for the HTTP requests + :type agent: str """ QObject.__init__(self) @@ -87,8 +94,20 @@ def _request( self, post_json: Optional[dict], blocking_request: QgsBlockingNetworkRequest, - request: QNetworkRequest + request: QNetworkRequest, ) -> str: + """Execute a blocking network request. + + :param post_json: JSON payload for POST requests, None for GET requests + :type post_json: dict or None + :param blocking_request: QGIS blocking network request handler + :type blocking_request: QgsBlockingNetworkRequest + :param request: Network request with URL and headers + :type request: QNetworkRequest + :return: Network reply content + :rtype: QgsNetworkReplyContent + :raises: Various ApiError exceptions based on HTTP status codes + """ if post_json is not None: result = blocking_request.post(request, json.dumps(post_json).encode()) else: @@ -108,19 +127,40 @@ def fetch_with_retry( post_json: Optional[dict] = None, max_retries: int = 100, ): + """Fetch data from the API with automatic retry on rate limit errors. + + Implements exponential backoff with jitter for retries. The first retry + occurs after exactly 61 seconds, subsequent retries use exponential + backoff capped at 5 minutes. + + :param url: API endpoint path (e.g., "/v2/directions/driving-car/geojson") + :type url: str + :param params: URL query parameters + :type params: dict + :param first_request_time: Timestamp of the first request attempt + :type first_request_time: datetime or None + :param post_json: JSON payload for POST requests + :type post_json: dict or None + :param max_retries: Maximum number of retry attempts + :type max_retries: int + :return: Parsed JSON response from the API + :rtype: dict + :raises exceptions.Timeout: When total retry time exceeds configured timeout + :raises exceptions.ApiError: On non-retryable API errors (4xx except 429) + """ first_request_time = datetime.now() authed_url = self._generate_auth_url(url, params) self.url = self.base_url + authed_url request = QNetworkRequest(QUrl(self.url)) - + for header, value in self.headers.items(): request.setRawHeader(header.encode(), value.encode()) blocking_request = QgsBlockingNetworkRequest() - blocking_request.downloadProgress.connect(lambda r, t: self.downloadProgress.emit(r/t)) + blocking_request.downloadProgress.connect(lambda r, t: self.downloadProgress.emit(r / t)) logger.log(f"url: {self.url}\nParameters: {json.dumps(post_json, indent=2)}", 0) @@ -131,7 +171,7 @@ def fetch_with_retry( reply = self._request(post_json, blocking_request, request) content = reply.content().data().decode() break - + except exceptions.OverQueryLimit as e: if datetime.now() - first_request_time > timedelta(seconds=self.timeout): raise exceptions.Timeout() @@ -140,9 +180,9 @@ def fetch_with_retry( delay_seconds = self.get_delay_seconds(i) self.overQueryLimit.emit(delay_seconds) - + loop = QEventLoop() - QTimer.singleShot(delay_seconds * 1000, loop.quit) # milliseconds + QTimer.singleShot(delay_seconds * 1000, loop.quit) loop.exec_() except exceptions.ApiError as e: @@ -164,11 +204,21 @@ def fetch_with_retry( return json.loads(content) def get_delay_seconds(self, retry_counter: int) -> int: + """Calculate delay before next retry attempt. + + First retry waits exactly 61 seconds. Subsequent retries use exponential + backoff with base delay of 60s * 1.5^(retry_counter-1), capped at 300s, + with random jitter between 30-60% of the calculated delay. + + :param retry_counter: Zero-based retry attempt number + :type retry_counter: int + :return: Delay in seconds before next retry + :rtype: int + """ if retry_counter == 0: - delay_seconds = 61 # First retry after exactly 61 seconds + delay_seconds = 61 else: - # Exponential backoff starting from 60 seconds - base_delay = min(60 * (1.5 ** (retry_counter - 1)), 300) # Cap at 5 minutes + base_delay = min(60 * (1.5 ** (retry_counter - 1)), 300) jitter = random.uniform(0.3, 0.6) delay_seconds = base_delay * jitter @@ -176,7 +226,16 @@ def get_delay_seconds(self, retry_counter: int) -> int: return int(delay_seconds) def _check_status(self, reply: QNetworkReply) -> None: - """Check response status and raise appropriate exceptions.""" + """Check HTTP response status and raise appropriate exceptions. + + :param reply: Network reply to check + :type reply: QNetworkReply + :raises exceptions.InvalidKey: On 403 Forbidden (invalid API key) + :raises exceptions.OverQueryLimit: On 429 Too Many Requests + :raises exceptions.ApiError: On 4xx client errors + :raises exceptions.GenericServerError: On 5xx server errors + :raises Exception: On network errors or invalid responses + """ status_code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) message = reply.content().data().decode() @@ -197,19 +256,15 @@ def _check_status(self, reply: QNetworkReply) -> None: raise exceptions.GenericServerError(str(status_code), message) def _generate_auth_url(self, path: str, params: Union[Dict, List]) -> str: - """Returns the path and query string portion of the request URL, first - adding any necessary parameters. - - :param path: The path portion of the URL. - :type path: string - - :param params: URL parameters. - :type params: dict or list of key/value tuples - - :returns: encoded URL - :rtype: string + """Generate authenticated URL with query parameters. + + :param path: API endpoint path + :type path: str + :param params: Query parameters as dict or list of tuples + :type params: dict or list + :return: URL path with encoded query string + :rtype: str """ - if isinstance(params, dict): params = sorted(dict(**params).items())