From ce5ca498dddb8a3bd26bd2ff48ec5ab4c7011eb8 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Thu, 2 Oct 2025 09:31:29 +0200 Subject: [PATCH 1/2] Partly revert "proper mockups for s3-related tests + retrying setup" This partly reverts commit b89339b6b967a55884d8af40ce9814c2269eabee. Signed-off-by: Tobias Wolf --- src/gardenlinux/constants.py | 5 - src/gardenlinux/s3/bucket.py | 5 +- tests/github/test_download_metadata_files.py | 103 ++++++++++--------- 3 files changed, 53 insertions(+), 60 deletions(-) diff --git a/src/gardenlinux/constants.py b/src/gardenlinux/constants.py index a904c3f8..4c52e137 100644 --- a/src/gardenlinux/constants.py +++ b/src/gardenlinux/constants.py @@ -161,8 +161,3 @@ # https://github.com/gardenlinux/gardenlinux/issues/3044 # Empty string is the 'legacy' variant with traditional root fs and still needed/supported IMAGE_VARIANTS = ["", "_usi", "_tpm2_trustedboot"] - -# configuration for https://github.com/groodt/retrying -RETRYING_MAX_ATTEMPTS = 5 -RETRYING_WAIT_EXPONENTIAL_MULTIPLIER = 1000 -RETRYING_WAIT_EXPONENTIAL_MAX = 16000 diff --git a/src/gardenlinux/s3/bucket.py b/src/gardenlinux/s3/bucket.py index de14fe65..3c596d74 100644 --- a/src/gardenlinux/s3/bucket.py +++ b/src/gardenlinux/s3/bucket.py @@ -14,7 +14,6 @@ import boto3 from retrying import retry -from ..constants import RETRYING_MAX_ATTEMPTS, RETRYING_WAIT_EXPONENTIAL_MAX, RETRYING_WAIT_EXPONENTIAL_MULTIPLIER from ..logger import LoggerSetup @@ -90,9 +89,7 @@ class tree for self). return getattr(self._bucket, name) - @retry(stop_max_attempt_number=RETRYING_MAX_ATTEMPTS, - wait_exponential_multiplier=RETRYING_WAIT_EXPONENTIAL_MULTIPLIER, - wait_exponential_max=RETRYING_WAIT_EXPONENTIAL_MAX) + @retry(stop_max_attempt_number=5, wait_exponential_multiplier=1000, wait_exponential_max=16000) def download_file(self, key, file_name, *args, **kwargs): """ boto3: Download an S3 object to a file. diff --git a/tests/github/test_download_metadata_files.py b/tests/github/test_download_metadata_files.py index 134401af..25dc2733 100644 --- a/tests/github/test_download_metadata_files.py +++ b/tests/github/test_download_metadata_files.py @@ -1,6 +1,6 @@ import pytest -from gardenlinux.constants import RETRYING_MAX_ATTEMPTS, S3_DOWNLOADS_DIR +from gardenlinux.constants import S3_DOWNLOADS_DIR from gardenlinux.features import CName from gardenlinux.github.release_notes.helpers import download_metadata_file from gardenlinux.s3 import S3Artifacts @@ -15,24 +15,34 @@ def test_download_metadata_file(downloads_dir, release_s3_bucket): - release_s3_bucket.upload_file(RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml", - f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}") + release_s3_bucket.upload_file( + RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml", + f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}", + ) s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME) s3_artifacts._bucket = release_s3_bucket - cname = CName("test-aws-gardener_prod", "amd64", "{0}-{1}".format(TEST_GARDENLINUX_RELEASE, TEST_GARDENLINUX_COMMIT_SHORT)) - download_metadata_file(s3_artifacts, - cname.cname, - TEST_GARDENLINUX_RELEASE, - TEST_GARDENLINUX_COMMIT_SHORT, - S3_DOWNLOADS_DIR) + cname = CName( + "test-aws-gardener_prod", + "amd64", + "{0}-{1}".format(TEST_GARDENLINUX_RELEASE, TEST_GARDENLINUX_COMMIT_SHORT), + ) + download_metadata_file( + s3_artifacts, + cname.cname, + TEST_GARDENLINUX_RELEASE, + TEST_GARDENLINUX_COMMIT_SHORT, + S3_DOWNLOADS_DIR, + ) assert (S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml").exists() def test_download_metadata_file_no_such_release(downloads_dir, release_s3_bucket): - release_s3_bucket.upload_file(RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml", - f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}") + release_s3_bucket.upload_file( + RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml", + f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}", + ) s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME) s3_artifacts._bucket = release_s3_bucket @@ -41,17 +51,19 @@ def test_download_metadata_file_no_such_release(downloads_dir, release_s3_bucket cname = CName("aws-gardener_prod", "amd64", "{0}-{1}".format(release, commit)) with pytest.raises(IndexError): - download_metadata_file(s3_artifacts, - cname.cname, - release, - commit, - S3_DOWNLOADS_DIR) - assert not (S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml").exists() + download_metadata_file( + s3_artifacts, cname.cname, release, commit, S3_DOWNLOADS_DIR + ) + assert not ( + S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml" + ).exists() def test_download_metadata_file_no_such_commit(downloads_dir, release_s3_bucket): - release_s3_bucket.upload_file(RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml", - f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}") + release_s3_bucket.upload_file( + RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml", + f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}", + ) s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME) s3_artifacts._bucket = release_s3_bucket @@ -61,17 +73,21 @@ def test_download_metadata_file_no_such_commit(downloads_dir, release_s3_bucket) cname = CName("test-aws-gardener_prod", "amd64", "{0}-{1}".format(release, commit)) with pytest.raises(IndexError): - download_metadata_file(s3_artifacts, - cname.cname, - release, - commit, - S3_DOWNLOADS_DIR) - assert not (S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml").exists() - - -def test_download_metadata_file_no_such_release_and_commit(downloads_dir, release_s3_bucket): - release_s3_bucket.upload_file(RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml", - f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}") + download_metadata_file( + s3_artifacts, cname.cname, release, commit, S3_DOWNLOADS_DIR + ) + assert not ( + S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml" + ).exists() + + +def test_download_metadata_file_no_such_release_and_commit( + downloads_dir, release_s3_bucket +): + release_s3_bucket.upload_file( + RELEASE_NOTES_S3_ARTIFACTS_DIR / "aws-gardener_prod-amd64.s3_metadata.yaml", + f"meta/singles/test-aws-gardener_prod-amd64-{TEST_GARDENLINUX_RELEASE}-{TEST_GARDENLINUX_COMMIT}", + ) s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME) s3_artifacts._bucket = release_s3_bucket @@ -81,24 +97,9 @@ def test_download_metadata_file_no_such_release_and_commit(downloads_dir, releas cname = CName("test-aws-gardener_prod", "amd64", "{0}-{1}".format(release, commit)) with pytest.raises(IndexError): - download_metadata_file(s3_artifacts, - cname.cname, - release, - commit, - S3_DOWNLOADS_DIR) - assert not (S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml").exists() - - -def test_download_metadata_uses_retrying_strategy(downloads_dir, blackhole_s3_bucket): - s3_artifacts = S3Artifacts(TEST_GARDENLINUX_RELEASE_BUCKET_NAME) - s3_artifacts._bucket._bucket = blackhole_s3_bucket - - cname = CName("test-aws-gardener_prod", "amd64", "{0}-{1}".format("foo", "bar")) - - with pytest.raises(IOError) as exn: - download_metadata_file(s3_artifacts, - cname.cname, - "foo", - "bar", - S3_DOWNLOADS_DIR) - assert str(exn.value) == f"Download attempt # {RETRYING_MAX_ATTEMPTS} failed" + download_metadata_file( + s3_artifacts, cname.cname, release, commit, S3_DOWNLOADS_DIR + ) + assert not ( + S3_DOWNLOADS_DIR / "test-aws-gardener_prod-amd64.s3_metadata.yaml" + ).exists() From 1efe2146a4e8a05b00164dfb4fdf7db5865149d4 Mon Sep 17 00:00:00 2001 From: Tobias Wolf Date: Thu, 2 Oct 2025 09:34:29 +0200 Subject: [PATCH 2/2] Partly revert "retrying for metadata artifacts download" This partly reverts commit 199e2cafbaa5e4587acaeb6ecbc8193462cf1a06. Signed-off-by: Tobias Wolf --- poetry.lock | 14 +------------- pyproject.toml | 1 - src/gardenlinux/s3/bucket.py | 2 -- 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/poetry.lock b/poetry.lock index 30eab668..4039ef29 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1373,18 +1373,6 @@ urllib3 = ">=1.25.10,<3.0" [package.extras] tests = ["coverage (>=6.0.0)", "flake8", "mypy", "pytest (>=7.0.0)", "pytest-asyncio", "pytest-cov", "pytest-httpserver", "tomli ; python_version < \"3.11\"", "tomli-w", "types-PyYAML", "types-requests"] -[[package]] -name = "retrying" -version = "1.4.2" -description = "Retrying" -optional = false -python-versions = ">=3.6" -groups = ["main"] -files = [ - {file = "retrying-1.4.2-py3-none-any.whl", hash = "sha256:bbc004aeb542a74f3569aeddf42a2516efefcdaff90df0eb38fbfbf19f179f59"}, - {file = "retrying-1.4.2.tar.gz", hash = "sha256:d102e75d53d8d30b88562d45361d6c6c934da06fab31bd81c0420acb97a8ba39"}, -] - [[package]] name = "rich" version = "14.1.0" @@ -1888,4 +1876,4 @@ test = ["pytest", "pytest-cov"] [metadata] lock-version = "2.1" python-versions = "^3.13" -content-hash = "121984ac6f15f17a01356441904a00f1f56aae5bff69cb466ca559273417bb7a" +content-hash = "7d3e290a730b99f059ded715059c76d7dd6581d67af90c9a77c994bb4ddbaf82" diff --git a/pyproject.toml b/pyproject.toml index 08b2c924..c2f71267 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,6 @@ pygit2 = "^1.18.2" pygments = "^2.19.2" PyYAML = "^6.0.2" gitpython = "^3.1.45" -retrying = "^1.4.2" [tool.poetry.group.dev.dependencies] bandit = "^1.8.6" diff --git a/src/gardenlinux/s3/bucket.py b/src/gardenlinux/s3/bucket.py index 3c596d74..403e8c95 100644 --- a/src/gardenlinux/s3/bucket.py +++ b/src/gardenlinux/s3/bucket.py @@ -12,7 +12,6 @@ from typing import Any, Optional import boto3 -from retrying import retry from ..logger import LoggerSetup @@ -89,7 +88,6 @@ class tree for self). return getattr(self._bucket, name) - @retry(stop_max_attempt_number=5, wait_exponential_multiplier=1000, wait_exponential_max=16000) def download_file(self, key, file_name, *args, **kwargs): """ boto3: Download an S3 object to a file.