From 230071902179ca58027d23880f101a4aca32a424 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 23 May 2022 15:23:10 +0100 Subject: [PATCH 1/2] only try to create storage client if storage is enabled --- posthog/storage/object_storage.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/posthog/storage/object_storage.py b/posthog/storage/object_storage.py index d08de2269b0f1..e3d73b56d651c 100644 --- a/posthog/storage/object_storage.py +++ b/posthog/storage/object_storage.py @@ -7,6 +7,7 @@ from posthog.settings import ( OBJECT_STORAGE_ACCESS_KEY_ID, OBJECT_STORAGE_BUCKET, + OBJECT_STORAGE_ENABLED, OBJECT_STORAGE_ENDPOINT, OBJECT_STORAGE_SECRET_ACCESS_KEY, ) @@ -18,22 +19,24 @@ # noinspection PyMissingTypeHints def storage_client(): global s3_client - if not s3_client: + if OBJECT_STORAGE_ENABLED and not s3_client: s3_client = client( "s3", endpoint_url=OBJECT_STORAGE_ENDPOINT, aws_access_key_id=OBJECT_STORAGE_ACCESS_KEY_ID, aws_secret_access_key=OBJECT_STORAGE_SECRET_ACCESS_KEY, - config=Config(signature_version="s3v4"), + config=Config(signature_version="s3v4", connect_timeout=1, retries={"max_attempts": 1}), region_name="us-east-1", ) + return s3_client def health_check() -> bool: # noinspection PyBroadException try: - response = storage_client().head_bucket(Bucket=OBJECT_STORAGE_BUCKET) + client = storage_client() + response = client.head_bucket(Bucket=OBJECT_STORAGE_BUCKET) if client else False return bool(response) except Exception as e: logger.warn("object_storage.health_check_failed", error=e) From bce6205379c2f682eb73e842c19ac7a57a758ea0 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Mon, 23 May 2022 15:39:58 +0100 Subject: [PATCH 2/2] add a test to clarify --- posthog/storage/object_storage.py | 18 ++++++------------ posthog/storage/test/test_object_storage.py | 12 ++++++++++++ 2 files changed, 18 insertions(+), 12 deletions(-) create mode 100644 posthog/storage/test/test_object_storage.py diff --git a/posthog/storage/object_storage.py b/posthog/storage/object_storage.py index e3d73b56d651c..d5d9acdabf015 100644 --- a/posthog/storage/object_storage.py +++ b/posthog/storage/object_storage.py @@ -4,13 +4,7 @@ logger = structlog.get_logger(__name__) -from posthog.settings import ( - OBJECT_STORAGE_ACCESS_KEY_ID, - OBJECT_STORAGE_BUCKET, - OBJECT_STORAGE_ENABLED, - OBJECT_STORAGE_ENDPOINT, - OBJECT_STORAGE_SECRET_ACCESS_KEY, -) +from django.conf import settings s3_client = None @@ -19,12 +13,12 @@ # noinspection PyMissingTypeHints def storage_client(): global s3_client - if OBJECT_STORAGE_ENABLED and not s3_client: + if settings.OBJECT_STORAGE_ENABLED and not s3_client: s3_client = client( "s3", - endpoint_url=OBJECT_STORAGE_ENDPOINT, - aws_access_key_id=OBJECT_STORAGE_ACCESS_KEY_ID, - aws_secret_access_key=OBJECT_STORAGE_SECRET_ACCESS_KEY, + endpoint_url=settings.OBJECT_STORAGE_ENDPOINT, + aws_access_key_id=settings.OBJECT_STORAGE_ACCESS_KEY_ID, + aws_secret_access_key=settings.OBJECT_STORAGE_SECRET_ACCESS_KEY, config=Config(signature_version="s3v4", connect_timeout=1, retries={"max_attempts": 1}), region_name="us-east-1", ) @@ -36,7 +30,7 @@ def health_check() -> bool: # noinspection PyBroadException try: client = storage_client() - response = client.head_bucket(Bucket=OBJECT_STORAGE_BUCKET) if client else False + response = client.head_bucket(Bucket=settings.OBJECT_STORAGE_BUCKET) if client else False return bool(response) except Exception as e: logger.warn("object_storage.health_check_failed", error=e) diff --git a/posthog/storage/test/test_object_storage.py b/posthog/storage/test/test_object_storage.py new file mode 100644 index 0000000000000..32de1228097ab --- /dev/null +++ b/posthog/storage/test/test_object_storage.py @@ -0,0 +1,12 @@ +from unittest.mock import patch + +from posthog.storage.object_storage import health_check +from posthog.test.base import APIBaseTest + + +class TestStorage(APIBaseTest): + @patch("posthog.storage.object_storage.client") + def test_does_not_create_client_if_storage_is_disabled(self, patched_s3_client) -> None: + with self.settings(OBJECT_STORAGE_ENABLED=False): + self.assertFalse(health_check()) + patched_s3_client.assert_not_called()