From 9e4d0b7e55e7a41eab3efb669662fafd8ee97ca0 Mon Sep 17 00:00:00 2001 From: Omar Selo Date: Thu, 9 Jan 2025 12:47:44 +0000 Subject: [PATCH 1/5] Convert snap promoter to class and split loop in promoter controller --- backend/test_observer/promotion/promoter.py | 132 +++++++++++--------- 1 file changed, 72 insertions(+), 60 deletions(-) diff --git a/backend/test_observer/promotion/promoter.py b/backend/test_observer/promotion/promoter.py index 06b14c4d..f956080d 100644 --- a/backend/test_observer/promotion/promoter.py +++ b/backend/test_observer/promotion/promoter.py @@ -77,76 +77,88 @@ def promoter_controller(session: Session) -> tuple[dict, dict]: :return: tuple of dicts, the first the processed cards and the status of execution the second only for the processed cards with the corresponding error message """ - family_mapping = { - FamilyName.snap: run_snap_promoter, - FamilyName.deb: run_deb_promoter, - } processed_artefacts_status = {} processed_artefacts_error_messages = {} - for family, promoter_function in family_mapping.items(): - artefacts = get_artefacts_by_family(session, family) - for artefact in artefacts: - artefact_key = f"{family} - {artefact.name} - {artefact.version}" - try: - processed_artefacts_status[artefact_key] = True - promoter_function(session, artefact) - except Exception as exc: - processed_artefacts_status[artefact_key] = False - processed_artefacts_error_messages[artefact_key] = str(exc) - logger.warning("WARNING: %s", str(exc), exc_info=True) + + snaps = get_artefacts_by_family(session, FamilyName.snap) + for snap in snaps: + snap_key = f"snap - {snap.name} - {snap.version}" + try: + processed_artefacts_status[snap_key] = True + SnapPromoter(session, snap).execute() + except Exception as exc: + processed_artefacts_status[snap_key] = False + processed_artefacts_error_messages[snap_key] = str(exc) + logger.warning("WARNING: %s", str(exc), exc_info=True) + + debs = get_artefacts_by_family(session, FamilyName.deb) + for deb in debs: + deb_key = f"deb - {deb.name} - {deb.version}" + try: + processed_artefacts_status[deb_key] = True + run_deb_promoter(session, deb) + except Exception as exc: + processed_artefacts_status[deb_key] = False + processed_artefacts_error_messages[deb_key] = str(exc) + logger.warning("WARNING: %s", str(exc), exc_info=True) + return processed_artefacts_status, processed_artefacts_error_messages -def run_snap_promoter(session: Session, artefact: Artefact) -> None: - """ - Check snap artefacts state and move/archive them if necessary +class SnapPromoter: + def __init__(self, db_session: Session, an_artefact: Artefact): + assert an_artefact.family == FamilyName.snap + self._snap = an_artefact + self._db_session = db_session - :session: DB connection session - :artefact_build: an ArtefactBuild object - """ - store = artefact.store - assert store is not None, f"Store is not set for the artefact {artefact.id}" - - latest_builds = session.scalars( - queries.latest_artefact_builds.where(ArtefactBuild.artefact_id == artefact.id) - ) - - for build in latest_builds: - arch = build.architecture - channel_map = get_channel_map_from_snapcraft( - arch=arch, - snapstore=store, - snap_name=artefact.name, + def execute(self): + store = self._snap.store + assert store is not None, f"Store is not set for the artefact {self._snap.id}" + + latest_builds = self._db_session.scalars( + queries.latest_artefact_builds.where( + ArtefactBuild.artefact_id == self._snap.id + ) ) - track = artefact.track - for channel_info in channel_map: - if not ( - channel_info.channel.track == track - and channel_info.channel.architecture == arch - ): - continue - - risk = channel_info.channel.risk - try: - version = channel_info.version - revision = channel_info.revision - except KeyError as exc: - logger.warning( - "No key '%s' is found. Continue processing...", - str(exc), - ) - continue + for build in latest_builds: + arch = build.architecture + channel_map = get_channel_map_from_snapcraft( + arch=arch, + snapstore=store, + snap_name=self._snap.name, + ) + track = self._snap.track - if ( - risk != artefact.stage - and version == artefact.version - and revision == build.revision - ): - logger.info("Move artefact '%s' to the '%s' stage", artefact, risk) + for channel_info in channel_map: + if not ( + channel_info.channel.track == track + and channel_info.channel.architecture == arch + ): + continue - artefact.stage = StageName(risk) - session.commit() + risk = channel_info.channel.risk + try: + version = channel_info.version + revision = channel_info.revision + except KeyError as exc: + logger.warning( + "No key '%s' is found. Continue processing...", + str(exc), + ) + continue + + if ( + risk != self._snap.stage + and version == self._snap.version + and revision == build.revision + ): + logger.info( + "Move artefact '%s' to the '%s' stage", self._snap, risk + ) + + self._snap.stage = StageName(risk) + self._db_session.commit() def run_deb_promoter(session: Session, artefact: Artefact) -> None: From fc0635e1173e38dc9c871bc58cd31c643527a371 Mon Sep 17 00:00:00 2001 From: Omar Selo Date: Thu, 9 Jan 2025 13:53:57 +0000 Subject: [PATCH 2/5] Fix bug promoting snap that's in two risks --- .../test_observer/data_access/models_enums.py | 16 +++++ .../test_observer/external_apis/snapcraft.py | 6 +- .../external_apis/snapcraft_models.py | 8 ++- backend/test_observer/promotion/promoter.py | 67 +++++-------------- backend/tests/promotion/test_promoter.py | 41 ++++++++++++ 5 files changed, 84 insertions(+), 54 deletions(-) diff --git a/backend/test_observer/data_access/models_enums.py b/backend/test_observer/data_access/models_enums.py index 010b9c56..04d8fdc1 100644 --- a/backend/test_observer/data_access/models_enums.py +++ b/backend/test_observer/data_access/models_enums.py @@ -36,6 +36,22 @@ class StageName(str, Enum): candidate = "candidate" stable = "stable" + def _compare(self, other: str) -> int: + stages = list(StageName.__members__.values()) + return stages.index(self) - stages.index(StageName(other)) + + def __lt__(self, other: str) -> bool: + return self._compare(other) < 0 + + def __le__(self, other: str) -> bool: + return self._compare(other) <= 0 + + def __gt__(self, other: str) -> bool: + return self._compare(other) > 1 + + def __ge__(self, other: str) -> bool: + return self._compare(other) >= 0 + class TestExecutionStatus(str, Enum): __test__ = False diff --git a/backend/test_observer/external_apis/snapcraft.py b/backend/test_observer/external_apis/snapcraft.py index 9e436afb..8c409377 100644 --- a/backend/test_observer/external_apis/snapcraft.py +++ b/backend/test_observer/external_apis/snapcraft.py @@ -2,23 +2,21 @@ import requests -from .snapcraft_models import SnapInfo, rename_keys +from .snapcraft_models import ChannelMap, SnapInfo, rename_keys logger = logging.getLogger("test-observer-backend") -def get_channel_map_from_snapcraft(arch: str, snapstore: str, snap_name: str): +def get_channel_map_from_snapcraft(snapstore: str, snap_name: str) -> list[ChannelMap]: """ Get channel_map from snapcraft.io - :arch: architecture :snapstore: Snapstore name :snap_name: snap name :return: channgel map as python dict (JSON format) """ headers = { "Snap-Device-Series": "16", - "Snap-Device-Architecture": arch, "Snap-Device-Store": snapstore, } req = requests.get( diff --git a/backend/test_observer/external_apis/snapcraft_models.py b/backend/test_observer/external_apis/snapcraft_models.py index 938fff7d..55107f67 100644 --- a/backend/test_observer/external_apis/snapcraft_models.py +++ b/backend/test_observer/external_apis/snapcraft_models.py @@ -19,12 +19,18 @@ """Mappings for json objects from snapcraft""" +from typing import Literal + from pydantic import BaseModel +from test_observer.data_access.models_enums import StageName + class Channel(BaseModel): architecture: str - risk: str + risk: Literal[StageName.edge] | Literal[StageName.beta] | Literal[ + StageName.candidate + ] | Literal[StageName.stable] track: str diff --git a/backend/test_observer/promotion/promoter.py b/backend/test_observer/promotion/promoter.py index f956080d..80051cd8 100644 --- a/backend/test_observer/promotion/promoter.py +++ b/backend/test_observer/promotion/promoter.py @@ -106,59 +106,28 @@ def promoter_controller(session: Session) -> tuple[dict, dict]: class SnapPromoter: - def __init__(self, db_session: Session, an_artefact: Artefact): - assert an_artefact.family == FamilyName.snap - self._snap = an_artefact + def __init__(self, db_session: Session, snap: Artefact): + assert snap.family == FamilyName.snap + assert snap.store, f"Store is not set for the snap artefact {snap.id}" + self._snap = snap self._db_session = db_session def execute(self): - store = self._snap.store - assert store is not None, f"Store is not set for the artefact {self._snap.id}" - - latest_builds = self._db_session.scalars( - queries.latest_artefact_builds.where( - ArtefactBuild.artefact_id == self._snap.id - ) + all_channel_maps = get_channel_map_from_snapcraft( + snapstore=self._snap.store, + snap_name=self._snap.name, ) - - for build in latest_builds: - arch = build.architecture - channel_map = get_channel_map_from_snapcraft( - arch=arch, - snapstore=store, - snap_name=self._snap.name, - ) - track = self._snap.track - - for channel_info in channel_map: - if not ( - channel_info.channel.track == track - and channel_info.channel.architecture == arch - ): - continue - - risk = channel_info.channel.risk - try: - version = channel_info.version - revision = channel_info.revision - except KeyError as exc: - logger.warning( - "No key '%s' is found. Continue processing...", - str(exc), - ) - continue - - if ( - risk != self._snap.stage - and version == self._snap.version - and revision == build.revision - ): - logger.info( - "Move artefact '%s' to the '%s' stage", self._snap, risk - ) - - self._snap.stage = StageName(risk) - self._db_session.commit() + self._snap.stage = max( + ( + cm.channel.risk + for cm in all_channel_maps + if cm.channel.track == self._snap.track + and cm.channel.architecture in self._snap.architectures + and cm.version == self._snap.version + ), + default=self._snap.stage, + ) + self._db_session.commit() def run_deb_promoter(session: Session, artefact: Artefact) -> None: diff --git a/backend/tests/promotion/test_promoter.py b/backend/tests/promotion/test_promoter.py index 2147260a..8db705a9 100644 --- a/backend/tests/promotion/test_promoter.py +++ b/backend/tests/promotion/test_promoter.py @@ -189,3 +189,44 @@ def test_promote_snap_from_beta_to_stable( promote_artefacts(db_session) assert artefact.stage == StageName.stable + + +def test_snap_that_is_in_two_stages( + db_session: Session, + requests_mock: Mocker, + generator: DataGenerator, +): + artefact = generator.gen_artefact(StageName.edge, store="ubuntu") + build = generator.gen_artefact_build(artefact, revision=1) + + requests_mock.get( + f"https://api.snapcraft.io/v2/snaps/info/{artefact.name}", + json={ + "channel-map": [ + { + "channel": { + "architecture": build.architecture, + "risk": "beta", + "track": artefact.track, + }, + "revision": build.revision, + "type": "app", + "version": artefact.version, + }, + { + "channel": { + "architecture": build.architecture, + "risk": "edge", + "track": artefact.track, + }, + "revision": build.revision, + "type": "app", + "version": artefact.version, + }, + ] + }, + ) + + promote_artefacts(db_session) + + assert artefact.stage == StageName.beta From 9cd51eef0a86222b711d727545982d2c45fe08b5 Mon Sep 17 00:00:00 2001 From: Omar Selo Date: Thu, 9 Jan 2025 13:59:42 +0000 Subject: [PATCH 3/5] Merge loops back again --- backend/test_observer/promotion/promoter.py | 38 ++++++++------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/backend/test_observer/promotion/promoter.py b/backend/test_observer/promotion/promoter.py index 80051cd8..33c72b1a 100644 --- a/backend/test_observer/promotion/promoter.py +++ b/backend/test_observer/promotion/promoter.py @@ -23,8 +23,7 @@ from sqlalchemy.orm import Session -from test_observer.data_access import queries -from test_observer.data_access.models import Artefact, ArtefactBuild +from test_observer.data_access.models import Artefact from test_observer.data_access.models_enums import FamilyName, StageName from test_observer.data_access.repository import get_artefacts_by_family from test_observer.external_apis.archive import ArchiveManager @@ -80,27 +79,20 @@ def promoter_controller(session: Session) -> tuple[dict, dict]: processed_artefacts_status = {} processed_artefacts_error_messages = {} - snaps = get_artefacts_by_family(session, FamilyName.snap) - for snap in snaps: - snap_key = f"snap - {snap.name} - {snap.version}" - try: - processed_artefacts_status[snap_key] = True - SnapPromoter(session, snap).execute() - except Exception as exc: - processed_artefacts_status[snap_key] = False - processed_artefacts_error_messages[snap_key] = str(exc) - logger.warning("WARNING: %s", str(exc), exc_info=True) - - debs = get_artefacts_by_family(session, FamilyName.deb) - for deb in debs: - deb_key = f"deb - {deb.name} - {deb.version}" - try: - processed_artefacts_status[deb_key] = True - run_deb_promoter(session, deb) - except Exception as exc: - processed_artefacts_status[deb_key] = False - processed_artefacts_error_messages[deb_key] = str(exc) - logger.warning("WARNING: %s", str(exc), exc_info=True) + for family in (FamilyName.snap, FamilyName.deb): + artefacts = get_artefacts_by_family(session, family) + for artefact in artefacts: + artefact_key = f"{family} - {artefact.name} - {artefact.version}" + try: + processed_artefacts_status[artefact_key] = True + if family == FamilyName.snap: + SnapPromoter(session, artefact).execute() + elif family == FamilyName.deb: + run_deb_promoter(session, artefact) + except Exception as exc: + processed_artefacts_status[artefact_key] = False + processed_artefacts_error_messages[artefact_key] = str(exc) + logger.warning("WARNING: %s", str(exc), exc_info=True) return processed_artefacts_status, processed_artefacts_error_messages From 36b8f66f78fe161d801c41ba40de4bf4200b0693 Mon Sep 17 00:00:00 2001 From: Omar Selo Date: Thu, 9 Jan 2025 14:13:20 +0000 Subject: [PATCH 4/5] Revert class to function --- backend/test_observer/promotion/promoter.py | 53 ++++++++++----------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/backend/test_observer/promotion/promoter.py b/backend/test_observer/promotion/promoter.py index 33c72b1a..d056a502 100644 --- a/backend/test_observer/promotion/promoter.py +++ b/backend/test_observer/promotion/promoter.py @@ -76,50 +76,45 @@ def promoter_controller(session: Session) -> tuple[dict, dict]: :return: tuple of dicts, the first the processed cards and the status of execution the second only for the processed cards with the corresponding error message """ + family_mapping = { + FamilyName.snap: run_snap_promoter, + FamilyName.deb: run_deb_promoter, + } processed_artefacts_status = {} processed_artefacts_error_messages = {} - - for family in (FamilyName.snap, FamilyName.deb): + for family, promoter_function in family_mapping.items(): artefacts = get_artefacts_by_family(session, family) for artefact in artefacts: artefact_key = f"{family} - {artefact.name} - {artefact.version}" try: processed_artefacts_status[artefact_key] = True - if family == FamilyName.snap: - SnapPromoter(session, artefact).execute() - elif family == FamilyName.deb: - run_deb_promoter(session, artefact) + promoter_function(session, artefact) except Exception as exc: processed_artefacts_status[artefact_key] = False processed_artefacts_error_messages[artefact_key] = str(exc) logger.warning("WARNING: %s", str(exc), exc_info=True) - return processed_artefacts_status, processed_artefacts_error_messages -class SnapPromoter: - def __init__(self, db_session: Session, snap: Artefact): - assert snap.family == FamilyName.snap - assert snap.store, f"Store is not set for the snap artefact {snap.id}" - self._snap = snap - self._db_session = db_session +def run_snap_promoter(db_session: Session, snap: Artefact): + assert snap.family == FamilyName.snap + assert snap.store, f"Store is not set for the snap artefact {snap.id}" - def execute(self): - all_channel_maps = get_channel_map_from_snapcraft( - snapstore=self._snap.store, - snap_name=self._snap.name, - ) - self._snap.stage = max( - ( - cm.channel.risk - for cm in all_channel_maps - if cm.channel.track == self._snap.track - and cm.channel.architecture in self._snap.architectures - and cm.version == self._snap.version - ), - default=self._snap.stage, - ) - self._db_session.commit() + all_channel_maps = get_channel_map_from_snapcraft( + snapstore=snap.store, + snap_name=snap.name, + ) + snap.stage = max( + ( + cm.channel.risk + for cm in all_channel_maps + if cm.channel.track == snap.track + and cm.channel.architecture in snap.architectures + and cm.version == snap.version + ), + default=snap.stage, + ) + db_session.commit() def run_deb_promoter(session: Session, artefact: Artefact) -> None: From 46e6791c15704a40a1c83ad62008d2ac2fe0075a Mon Sep 17 00:00:00 2001 From: Omar Selo Date: Thu, 9 Jan 2025 14:28:41 +0000 Subject: [PATCH 5/5] Merge Snap Stage Literals type hint --- backend/test_observer/external_apis/snapcraft_models.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/backend/test_observer/external_apis/snapcraft_models.py b/backend/test_observer/external_apis/snapcraft_models.py index 55107f67..fec1b356 100644 --- a/backend/test_observer/external_apis/snapcraft_models.py +++ b/backend/test_observer/external_apis/snapcraft_models.py @@ -28,9 +28,7 @@ class Channel(BaseModel): architecture: str - risk: Literal[StageName.edge] | Literal[StageName.beta] | Literal[ - StageName.candidate - ] | Literal[StageName.stable] + risk: Literal[StageName.edge, StageName.beta, StageName.candidate, StageName.stable] track: str