From bb5390af40dc19b62c73ddb634dab2c1abb65ef6 Mon Sep 17 00:00:00 2001 From: dominik003 Date: Mon, 16 Sep 2024 13:36:36 +0200 Subject: [PATCH] feat: Apply PR comments --- .../projects/toolmodels/diagrams/models.py | 2 +- .../projects/toolmodels/diagrams/routes.py | 59 +++++-------------- .../projects/toolmodels/modelbadge/routes.py | 23 ++------ .../modelsources/git/github/handler.py | 8 +-- .../modelsources/git/gitlab/handler.py | 6 +- .../modelsources/git/handler/cache.py | 6 +- .../modelsources/git/handler/handler.py | 35 +++++++++-- backend/pyproject.toml | 2 +- .../model-diagram-code-block.component.ts | 4 +- .../model-diagram-code-block.stories.ts | 3 +- 10 files changed, 64 insertions(+), 84 deletions(-) diff --git a/backend/capellacollab/projects/toolmodels/diagrams/models.py b/backend/capellacollab/projects/toolmodels/diagrams/models.py index 835438cefc..d28a7172a1 100644 --- a/backend/capellacollab/projects/toolmodels/diagrams/models.py +++ b/backend/capellacollab/projects/toolmodels/diagrams/models.py @@ -18,7 +18,7 @@ class DiagramMetadata(core_pydantic.BaseModel): class DiagramCacheMetadata(core_pydantic.BaseModel): diagrams: list[DiagramMetadata] last_updated: datetime.datetime - job_id: str | int | None = None + job_id: str | None = None _validate_last_updated = pydantic.field_serializer("last_updated")( core_pydantic.datetime_serializer diff --git a/backend/capellacollab/projects/toolmodels/diagrams/routes.py b/backend/capellacollab/projects/toolmodels/diagrams/routes.py index 281a298b33..c6b4d41bf2 100644 --- a/backend/capellacollab/projects/toolmodels/diagrams/routes.py +++ b/backend/capellacollab/projects/toolmodels/diagrams/routes.py @@ -10,7 +10,6 @@ import fastapi import requests -from aiohttp import web import capellacollab.projects.toolmodels.modelsources.git.injectables as git_injectables from capellacollab.core import logging as log @@ -42,34 +41,21 @@ async def get_diagram_metadata( ), logger: logging.LoggerAdapter = fastapi.Depends(log.get_request_logger), ): - job_id = None try: - last_updated, diagram_metadata_entries = await handler.get_file( - trusted_file_path="diagram_cache/index.json", - revision=f"diagram-cache/{handler.revision}", + job_id, last_updated, diagram_metadata_entries = ( + await handler.get_file_or_artifact( + trusted_file_path="diagram_cache/index.json", + job_name="update_capella_diagram_cache", + file_revision=f"diagram-cache/{handler.revision}", + ) ) - except Exception: + except requests.HTTPError: logger.info( - "Failed fetching diagram metadata file for %s on revision %s.", + "Failed fetching diagram metadata file or artifact for %s", handler.path, - f"diagram-cache/{handler.revision}", exc_info=True, ) - try: - job_id, last_updated, diagram_metadata_entries = ( - await handler.get_artifact( - trusted_file_path="diagram_cache/index.json", - job_name="update_capella_diagram_cache", - ) - ) - except (web.HTTPError, requests.HTTPError): - logger.info( - "Failed fetching diagram metadata artifact for %s on revision %s", - handler.path, - handler.revision, - exc_info=True, - ) - raise exceptions.DiagramCacheNotConfiguredProperlyError() + raise exceptions.DiagramCacheNotConfiguredProperlyError() diagram_metadata_entries = json.loads(diagram_metadata_entries) return models.DiagramCacheMetadata( @@ -89,7 +75,7 @@ async def get_diagram_metadata( ) async def get_diagram( diagram_uuid_or_filename: str, - job_id: str | int | None = None, + job_id: str | None = None, handler: git_handler.GitHandler = fastapi.Depends( git_injectables.get_git_handler ), @@ -102,32 +88,17 @@ async def get_diagram( diagram_uuid = pathlib.PurePosixPath(diagram_uuid_or_filename).stem file_path = f"diagram_cache/{parse.quote(diagram_uuid, safe='')}.svg" - if not job_id: - try: - file = await handler.get_file( - trusted_file_path=file_path, - revision=f"diagram-cache/{handler.revision}", - ) - return responses.SVGResponse(content=file[1]) - except Exception: - logger.info( - "Failed fetching diagram file %s for %s on revision %s.", - diagram_uuid, - handler.path, - f"diagram-cache/{handler.revision}", - exc_info=True, - ) - try: - artifact = await handler.get_artifact( + file_or_artifact = await handler.get_file_or_artifact( trusted_file_path=file_path, job_name="update_capella_diagram_cache", job_id=job_id, + file_revision=f"diagram-cache/{handler.revision}", ) - return responses.SVGResponse(content=artifact[2]) - except (web.HTTPError, requests.HTTPError): + return responses.SVGResponse(content=file_or_artifact[2]) + except requests.HTTPError: logger.info( - "Failed fetching diagram artifact %s for %s on revision %s.", + "Failed fetching diagram file or artifact %s for %s.", diagram_uuid, handler.path, f"diagram-cache/{handler.revision}", diff --git a/backend/capellacollab/projects/toolmodels/modelbadge/routes.py b/backend/capellacollab/projects/toolmodels/modelbadge/routes.py index 942e995f43..734023778d 100644 --- a/backend/capellacollab/projects/toolmodels/modelbadge/routes.py +++ b/backend/capellacollab/projects/toolmodels/modelbadge/routes.py @@ -6,8 +6,6 @@ import logging import fastapi -import requests -from aiohttp import web import capellacollab.projects.toolmodels.modelsources.git.injectables as git_injectables from capellacollab.core import logging as log @@ -41,26 +39,13 @@ async def get_model_complexity_badge( logger: logging.LoggerAdapter = fastapi.Depends(log.get_request_logger), ): try: - file = await git_handler.get_file( - "model-complexity-badge.svg", git_handler.revision - ) - return responses.SVGResponse(content=file[1]) - except Exception: - logger.debug( - "Failed fetching model badge file for %s on revision %s.", - git_handler.path, - git_handler.revision, - exc_info=True, - ) - - try: - artifact = await git_handler.get_artifact( + file_or_artifact = await git_handler.get_file_or_artifact( "model-complexity-badge.svg", "generate-model-badge" ) - return responses.SVGResponse(content=artifact[2]) - except (web.HTTPError, requests.HTTPError): + return responses.SVGResponse(content=file_or_artifact[2]) + except Exception: logger.debug( - "Failed fetching model badge artifact for %s on revision %s.", + "Failed fetching model badge file or artifact for %s on revision %s.", git_handler.path, git_handler.revision, exc_info=True, diff --git a/backend/capellacollab/projects/toolmodels/modelsources/git/github/handler.py b/backend/capellacollab/projects/toolmodels/modelsources/git/github/handler.py index cc608eafdd..c9e68fd878 100644 --- a/backend/capellacollab/projects/toolmodels/modelsources/git/github/handler.py +++ b/backend/capellacollab/projects/toolmodels/modelsources/git/github/handler.py @@ -30,7 +30,7 @@ async def get_last_successful_job_run( created_at = datetime.datetime.fromisoformat( latest_job["created_at"] ) - return (latest_job["id"], created_at) + return (str(latest_job["id"]), created_at) raise git_exceptions.GitPipelineJobNotFoundError( job_name=job_name, revision=self.revision @@ -86,7 +86,7 @@ def get_last_pipeline_runs(self) -> t.Any: return response.json()["workflow_runs"] def get_artifact_from_job( - self, job_id: str | int, trusted_path_to_artifact: str + self, job_id: str, trusted_path_to_artifact: str ) -> bytes: artifact = self.__get_latest_artifact_metadata(job_id) artifact_id = artifact["id"] @@ -118,7 +118,7 @@ def get_last_updated_for_file( response.json()[0]["commit"]["author"]["date"] ) - def get_started_at_for_job(self, job_id: str | int) -> datetime.datetime: + def get_started_at_for_job(self, job_id: str) -> datetime.datetime: response = requests.get( f"{self.api_url}/repos/{self.repository_id}/actions/runs/{job_id}", headers=self.__get_headers(), @@ -153,7 +153,7 @@ def __get_latest_successful_job( job_name, matched_jobs[0]["conclusion"] ) - def __get_latest_artifact_metadata(self, job_id: str | int): + def __get_latest_artifact_metadata(self, job_id: str): response = requests.get( f"{self.api_url}/repos/{self.repository_id}/actions/runs/{job_id}/artifacts", headers=self.__get_headers(), diff --git a/backend/capellacollab/projects/toolmodels/modelsources/git/gitlab/handler.py b/backend/capellacollab/projects/toolmodels/modelsources/git/gitlab/handler.py index 698ea32631..709aab069a 100644 --- a/backend/capellacollab/projects/toolmodels/modelsources/git/gitlab/handler.py +++ b/backend/capellacollab/projects/toolmodels/modelsources/git/gitlab/handler.py @@ -47,7 +47,7 @@ async def get_last_successful_job_run( if job := await self.__get_job_id_for_job_name( pipeline_id, job_name ): - return job + return (str(job[0]), job[1]) raise git_exceptions.GitPipelineJobNotFoundError( job_name=job_name, revision=self.revision @@ -70,7 +70,7 @@ def get_last_updated_for_file( response.json()[0]["authored_date"] ) - def get_started_at_for_job(self, job_id: str | int) -> datetime.datetime: + def get_started_at_for_job(self, job_id: str) -> datetime.datetime: response = requests.get( f"{self.api_url}/projects/{self.repository_id}/jobs/{job_id}", headers={"PRIVATE-TOKEN": self.password}, @@ -117,7 +117,7 @@ async def __get_job_id_for_job_name( return None def get_artifact_from_job( - self, job_id: str | int, trusted_path_to_artifact: str + self, job_id: str, trusted_path_to_artifact: str ) -> bytes: response = requests.get( f"{self.api_url}/projects/{self.repository_id}/jobs/{job_id}/artifacts/{trusted_path_to_artifact}", diff --git a/backend/capellacollab/projects/toolmodels/modelsources/git/handler/cache.py b/backend/capellacollab/projects/toolmodels/modelsources/git/handler/cache.py index f783fa6108..0d958d06c7 100644 --- a/backend/capellacollab/projects/toolmodels/modelsources/git/handler/cache.py +++ b/backend/capellacollab/projects/toolmodels/modelsources/git/handler/cache.py @@ -26,7 +26,7 @@ def get_file_data( return None def get_artifact_data( - self, job_id: str | int, file_path: str + self, job_id: str, file_path: str ) -> tuple[datetime.datetime, bytes] | None: artifact_data = self._valkey.hmget( name=self._get_artifact_key(job_id, file_path), @@ -59,7 +59,7 @@ def put_file_data( def put_artifact_data( self, - job_id: str | int, + job_id: str, file_path: str, started_at: datetime.datetime, content: bytes, @@ -80,7 +80,7 @@ def clear(self) -> None: def _get_file_key(self, file_path: str, revision: str) -> str: return f"{self.git_model_id}:f:{self._escape_string(revision)}:{self._escape_string(file_path)}" - def _get_artifact_key(self, job_id: str | int, file_path: str) -> str: + def _get_artifact_key(self, job_id: str, file_path: str) -> str: return ( f"{self.git_model_id}:a:{job_id}:{self._escape_string(file_path)}" ) diff --git a/backend/capellacollab/projects/toolmodels/modelsources/git/handler/handler.py b/backend/capellacollab/projects/toolmodels/modelsources/git/handler/handler.py index 900d520c01..3bdb744357 100644 --- a/backend/capellacollab/projects/toolmodels/modelsources/git/handler/handler.py +++ b/backend/capellacollab/projects/toolmodels/modelsources/git/handler/handler.py @@ -6,6 +6,9 @@ import abc import datetime +import requests + +from .. import exceptions as git_exceptions from . import cache @@ -53,7 +56,7 @@ async def get_last_successful_job_run( @abc.abstractmethod def get_artifact_from_job( - self, job_id: str | int, trusted_path_to_artifact: str + self, job_id: str, trusted_path_to_artifact: str ) -> bytes: """ Retrieve an artifact from a specified job. @@ -103,7 +106,7 @@ def get_last_updated_for_file( """ @abc.abstractmethod - def get_started_at_for_job(self, job_id: str | int) -> datetime.datetime: + def get_started_at_for_job(self, job_id: str) -> datetime.datetime: """ Retrieve the start datetime for the specified job in the repository. @@ -115,8 +118,11 @@ def get_started_at_for_job(self, job_id: str | int) -> datetime.datetime: """ async def get_file( - self, trusted_file_path: str, revision: str + self, trusted_file_path: str, revision: str | None = None ) -> tuple[datetime.datetime, bytes]: + if not revision: + revision = self.revision + last_updated = self.get_last_updated_for_file( trusted_file_path, revision ) @@ -138,8 +144,8 @@ async def get_artifact( self, trusted_file_path: str, job_name: str, - job_id: str | int | None = None, - ) -> tuple[str | int, datetime.datetime, bytes]: + job_id: str | None = None, + ) -> tuple[str, datetime.datetime, bytes]: if not job_id: job_id, started_at = await self.get_last_successful_job_run( job_name @@ -158,3 +164,22 @@ async def get_artifact( ) return job_id, started_at, content + + async def get_file_or_artifact( + self, + trusted_file_path: str, + job_name: str, + job_id: str | None = None, + file_revision: str | None = None, + ) -> tuple[str | None, datetime.datetime, bytes]: + if not job_id: + try: + file = await self.get_file(trusted_file_path, file_revision) + return (None, file[0], file[1]) + except ( + requests.HTTPError, + git_exceptions.GitRepositoryFileNotFoundError, + ): + pass + + return await self.get_artifact(trusted_file_path, job_name, job_id) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 7b668868df..49cceb6f91 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -26,7 +26,7 @@ dependencies = [ "alembic==1.13.2", "appdirs", "cachetools", - "fastapi[all]>=0.112.4", + "fastapi>=0.112.4", "kubernetes", "psycopg2-binary>2.9.7", "pydantic>=2.0.0", diff --git a/frontend/src/app/projects/models/diagrams/model-diagram-dialog/model-diagram-code-block/model-diagram-code-block.component.ts b/frontend/src/app/projects/models/diagrams/model-diagram-dialog/model-diagram-code-block/model-diagram-code-block.component.ts index c1c69b7632..76cbf4adb7 100644 --- a/frontend/src/app/projects/models/diagrams/model-diagram-dialog/model-diagram-code-block/model-diagram-code-block.component.ts +++ b/frontend/src/app/projects/models/diagrams/model-diagram-dialog/model-diagram-code-block/model-diagram-code-block.component.ts @@ -23,7 +23,7 @@ import { MatTooltip } from '@angular/material/tooltip'; import hljs from 'highlight.js'; import { MetadataService } from 'src/app/general/metadata/metadata.service'; import { ToastService } from 'src/app/helpers/toast/toast.service'; -import { JobId, Metadata, Project, ToolModel } from 'src/app/openapi'; +import { Metadata, Project, ToolModel } from 'src/app/openapi'; import { getPrimaryGitModel } from 'src/app/projects/models/service/model.service'; import { UserWrapperService } from 'src/app/services/user/user.service'; import { TokenService } from 'src/app/users/basic-auth-service/basic-auth-token.service'; @@ -71,7 +71,7 @@ export class ModelDiagramCodeBlockComponent implements OnInit, AfterViewInit { project!: Project; @Input() - jobId: JobId | undefined; + jobId: string | undefined; @Input() expanded = false; diff --git a/frontend/src/app/projects/models/diagrams/model-diagram-dialog/model-diagram-code-block/model-diagram-code-block.stories.ts b/frontend/src/app/projects/models/diagrams/model-diagram-dialog/model-diagram-code-block/model-diagram-code-block.stories.ts index e5d81edd1f..b1b3945887 100644 --- a/frontend/src/app/projects/models/diagrams/model-diagram-dialog/model-diagram-code-block/model-diagram-code-block.stories.ts +++ b/frontend/src/app/projects/models/diagrams/model-diagram-dialog/model-diagram-code-block/model-diagram-code-block.stories.ts @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ import { Meta, StoryObj } from '@storybook/angular'; -import { JobId } from 'src/app/openapi'; import { mockModel } from 'src/storybook/model'; import { mockProject } from 'src/storybook/project'; import { ModelDiagramCodeBlockComponent } from './model-diagram-code-block.component'; @@ -38,6 +37,6 @@ export const CodeBlockWithJobID: Story = { model: mockModel, project: mockProject, expanded: true, - jobId: '1234' as JobId, + jobId: '1234', }, };