Skip to content

Commit

Permalink
Remove retries any request that are within the context of the web app…
Browse files Browse the repository at this point in the history
…lication

This is to ensure we do not hang web requests due to issues with third party APIs
  • Loading branch information
RishiDiwanTT committed Oct 26, 2023
1 parent 39abc33 commit 4a422f2
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
2 changes: 2 additions & 0 deletions api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from core.service.container import Services, container_instance
from core.util import LanguageCodes
from core.util.cache import CachedData
from core.util.http import HTTP
from scripts import InstanceInitializationScript


Expand Down Expand Up @@ -103,6 +104,7 @@ def initialize_database():


def initialize_application() -> PalaceFlask:
HTTP.set_quick_failure_settings()
with app.app_context(), flask_babel.force_locale("en"):
initialize_database()

Expand Down
19 changes: 17 additions & 2 deletions core/util/http.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import time
from json import JSONDecodeError
from typing import Any, Callable, Dict, List, Optional, Union
from urllib.parse import urlparse
Expand All @@ -12,6 +13,7 @@
import core
from core.exceptions import IntegrationException
from core.problem_details import INTEGRATION_ERROR
from core.util.log import LoggerMixin
from core.util.problem_detail import JSON_MEDIA_TYPE as PROBLEM_DETAIL_JSON_MEDIA_TYPE
from core.util.problem_detail import ProblemError

Expand Down Expand Up @@ -157,13 +159,20 @@ class RequestTimedOut(RequestNetworkException, requests.exceptions.Timeout):
internal_message = "Timeout accessing %s: %s"


class HTTP:
class HTTP(LoggerMixin):
"""A helper for the `requests` module."""

# In case an app version is not present, we can use this version as a fallback
# for all outgoing http requests without a custom user-agent
DEFAULT_USER_AGENT_VERSION = "1.x.x"

DEFAULT_REQUEST_RETRIES = 5

@classmethod
def set_quick_failure_settings(cls) -> None:
"""Ensure any outgoing requests aren't long-running"""
cls.DEFAULT_REQUEST_RETRIES = 0

@classmethod
def get_with_timeout(cls, url: str, *args, **kwargs) -> Response:
"""Make a GET request with timeout handling."""
Expand Down Expand Up @@ -221,7 +230,9 @@ def _request_with_timeout(
if not "timeout" in kwargs:
kwargs["timeout"] = 20

max_retry_count: int = int(kwargs.pop("max_retry_count", 5))
max_retry_count: int = int(
kwargs.pop("max_retry_count", cls.DEFAULT_REQUEST_RETRIES)
)
backoff_factor: float = float(kwargs.pop("backoff_factor", 1.0))

# Unicode data can't be sent over the wire. Convert it
Expand Down Expand Up @@ -258,6 +269,7 @@ def _request_with_timeout(
# arguments, it will still work.
args = args + (url,)

request_start_time = time.time()
if make_request_with == sessions.Session.request:
with sessions.Session() as session:
retry_strategy = Retry(
Expand All @@ -273,6 +285,9 @@ def _request_with_timeout(
response = session.request(*args, **kwargs)
else:
response = make_request_with(*args, **kwargs)
cls.logger().info(
f"Request time for {url} took {time.time() - request_start_time:.2f} seconds"
)

if verbose:
logging.info(
Expand Down
12 changes: 12 additions & 0 deletions tests/api/test_app.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from api.app import initialize_application
from core.util.http import HTTP
from tests.fixtures.database import DatabaseTransactionFixture


def test_initialize_application_http(db: DatabaseTransactionFixture):
# Use the db transaction fixture so that we don't use the production settings by mistake
assert HTTP.DEFAULT_REQUEST_RETRIES == 5
# Initialize the app, which will set the HTTP configuration
initialize_application()
# Now we have 0 retry logic
assert HTTP.DEFAULT_REQUEST_RETRIES == 0

0 comments on commit 4a422f2

Please sign in to comment.