From e2a2caebce82d629795207555afc944707a4a430 Mon Sep 17 00:00:00 2001 From: daveoconnor Date: Fri, 10 Jan 2025 13:19:11 -0800 Subject: [PATCH] Improved error processing (#1590) --- conftest.py | 15 +++++++++++++++ core/boostrenderer.py | 9 +++++++++ core/githubhelper.py | 15 +++++++++++---- env.template | 2 +- justfile | 3 +++ libraries/tasks.py | 5 ++--- versions/releases.py | 19 ++++++++++--------- versions/tasks.py | 12 +++++++----- 8 files changed, 58 insertions(+), 22 deletions(-) diff --git a/conftest.py b/conftest.py index 03313239..9cbbc462 100644 --- a/conftest.py +++ b/conftest.py @@ -1,3 +1,5 @@ +import os + import pytest import tempfile from PIL import Image @@ -41,3 +43,16 @@ def pytest_collection_modifyitems(config, items): for item in items: if "asciidoctor" in item.keywords: item.add_marker(skip_asciidoctor) + + +@pytest.fixture(scope="session", autouse=True) +def ensure_github_token_env_variable(): + # I wanted to use pytest_env but skip_if_set=true only applies if the env var + # is not set at all, not if the env var is empty, so this is needed anyway. + VAR_NAME = "GITHUB_TOKEN" # Replace with your actual variable name + VAR_DEFAULT_VALUE = "top-secret" + current_value = os.getenv(VAR_NAME) + + if not current_value: + os.environ[VAR_NAME] = VAR_DEFAULT_VALUE + print(f"Env variable '{VAR_NAME}' not set. Forced to {VAR_DEFAULT_VALUE=}.") diff --git a/core/boostrenderer.py b/core/boostrenderer.py index 227559bc..b8bfb0cf 100644 --- a/core/boostrenderer.py +++ b/core/boostrenderer.py @@ -160,6 +160,15 @@ def get_s3_client(): ) +def does_s3_key_exist(client, bucket_name, s3_key): + try: + client.head_object(Bucket=bucket_name, Key=s3_key.lstrip("/")) + return True + except ClientError: + logger.debug(f"{s3_key} does not exist in {bucket_name}") + return False + + def get_s3_keys(content_path, config_filename=None): """ Get the S3 key for a given content path diff --git a/core/githubhelper.py b/core/githubhelper.py index 85a13635..55b90aba 100644 --- a/core/githubhelper.py +++ b/core/githubhelper.py @@ -62,6 +62,8 @@ def __init__( "cmake", "more", ] + if not self.token: + raise ValueError("No GitHub token provided or set in environment.") def initialize_api(self) -> GhApi: """ @@ -311,9 +313,7 @@ def get_libraries_json(self, repo_slug: str, tag: str = "master"): # This usually happens because the library does not have a `meta/libraries.json` # in the requested tag. More likely to happen with older versions of libraries. except requests.exceptions.HTTPError: - self.logger.exception( - "get_library_metadata_failed", repo=repo_slug, url=url - ) + self.logger.warning(f"get_library_metadata_failed {repo_slug=}, {url=}") return None else: return response.json() @@ -357,7 +357,14 @@ def get_ref(self, repo_slug: str = None, ref: str = None) -> dict: repo_slug = self.repo_slug if not ref: ref = self.ref - return self.api.git.get_ref(owner=self.owner, repo=repo_slug, ref=ref) + try: + ref_response = self.api.git.get_ref( + owner=self.owner, repo=repo_slug, ref=ref + ) + except OSError as e: + logger.warning("get_ref_failed", repo=repo_slug, ref=ref, exc_msg=str(e)) + raise ValueError(f"Could not get ref for {repo_slug} and {ref}") + return ref_response def get_repo(self, repo_slug: str = None) -> dict: """ diff --git a/env.template b/env.template index 90565258..b66072ce 100644 --- a/env.template +++ b/env.template @@ -14,7 +14,7 @@ DJANGO_DEBUG=1 # Don't use this secret key in production obviously SECRET_KEY="top-secret" -GITHUB_TOKEN="top-secret" +GITHUB_TOKEN= # AWS_ACCESS_KEY_ID="changeme" # AWS_SECRET_ACCESS_KEY="changeme" diff --git a/justfile b/justfile index 24afaa18..4275800f 100644 --- a/justfile +++ b/justfile @@ -126,3 +126,6 @@ alias shell := console @pip-compile-upgrade: ## Upgrade existing Python dependencies to their latest versions just pip-compile --upgrade + +@manage args: + docker compose run --rm web python manage.py {{ args }} diff --git a/libraries/tasks.py b/libraries/tasks.py index 784e7b33..26469c3c 100644 --- a/libraries/tasks.py +++ b/libraries/tasks.py @@ -74,9 +74,8 @@ def get_and_store_library_version_documentation_urls_for_version(version_pk): library_version.save() except LibraryVersion.DoesNotExist: logger.info( - "get_library_version_documentation_urls_version_does_not_exist", - library_name=library_name, - version_slug=version.slug, + f"get_library_version_documentation_urls_version_does_not_exist" + f"{library_name=} {version.slug=}", ) continue except LibraryVersion.MultipleObjectsReturned: diff --git a/versions/releases.py b/versions/releases.py index 887b83f2..90419598 100644 --- a/versions/releases.py +++ b/versions/releases.py @@ -9,7 +9,7 @@ from django.conf import settings from core.asciidoc import convert_adoc_to_html -from core.boostrenderer import get_file_data, get_s3_client +from core.boostrenderer import get_file_data, get_s3_client, does_s3_key_exist from core.htmlhelper import modernize_release_notes from core.models import RenderedContent @@ -188,15 +188,16 @@ def get_release_notes_for_version_s3(version_pk): bucket_name = settings.STATIC_CONTENT_BUCKET_NAME primary_key = f"release-notes/master/{filename}.adoc" - response = get_file_data(s3_client, bucket_name, primary_key) - if not response: - # Some beta release notes end in _x.html instead of _0.html; try that. - fallback_filename = filename.rsplit("_", 1)[0] + "_x" - fallback_key = f"release-notes/master/{fallback_filename}.adoc" + fallback_key = f"release-notes/master/{filename.rsplit('_', 1)[0] + '_x'}.adoc" + + response = None + if does_s3_key_exist(s3_client, bucket_name, primary_key): + response = get_file_data(s3_client, bucket_name, primary_key) + elif does_s3_key_exist(s3_client, bucket_name, fallback_key): response = get_file_data(s3_client, bucket_name, fallback_key) - if response: - return response["content"].decode() - return "" + else: + logger.info(f"no release notes found for {filename=}") + return response["content"].decode() if response else "" def get_release_notes_for_version_github(version_pk): diff --git a/versions/tasks.py b/versions/tasks.py index 337d4877..19f8eb40 100644 --- a/versions/tasks.py +++ b/versions/tasks.py @@ -75,7 +75,7 @@ def import_versions( def import_release_notes(): """Imports release notes from the existing rendered release notes in the repository.""" - for version in Version.objects.active(): + for version in Version.objects.exclude(name__in=["master", "develop"]).active(): store_release_notes_task.delay(str(version.pk)) store_release_notes_in_progress_task.delay() @@ -262,10 +262,12 @@ def import_library_versions(version_name, token=None, version_type="tag"): # Get the gitmodules file for the version, which contains library data # The master and develop branches are not tags, so we retrieve their data # from the heads/ namespace instead of tags/ - if version_type == "tag": - ref = client.get_ref(ref=f"tags/{version_name}") - else: - ref = client.get_ref(ref=f"heads/{version_name}") + ref_s = f"tags/{version_name}" if version_type == "tag" else f"heads/{version_name}" + try: + ref = client.get_ref(ref=ref_s) + except ValueError: + logger.info(f"import_library_versions_invalid_ref {ref_s=}") + return raw_gitmodules = client.get_gitmodules(ref=ref) if not raw_gitmodules: