From d73cf76ad525abe63d6ed605093683eab015594c Mon Sep 17 00:00:00 2001 From: tada5hi Date: Thu, 13 Jun 2024 10:57:39 +0200 Subject: [PATCH 1/4] feat: adjusted interfaces and api methods for hub api changes --- project/hub.py | 58 +++++++++++++++++++++++---------- project/routers/final.py | 10 ++++-- project/routers/intermediate.py | 10 ++++-- tests/test_final.py | 4 +-- tests/test_hub.py | 20 ++++-------- 5 files changed, 62 insertions(+), 40 deletions(-) diff --git a/project/hub.py b/project/hub.py index 7c3e1b2..6c6d675 100644 --- a/project/hub.py +++ b/project/hub.py @@ -54,16 +54,23 @@ class BucketFile(BaseModel): created_at: datetime updated_at: datetime +class AnalysisBucket(BaseModel): + id: UUID + type: BucketType + external_id: UUID + analysis_id: UUID + created_at: datetime + updated_at: datetime -class AnalysisFile(BaseModel): +class AnalysisBucketFile(BaseModel): id: UUID name: Optional[str] - type: BucketType root: bool created_at: datetime updated_at: datetime - bucket_file_id: UUID - analysis_id: UUID + external_id: UUID + bucket_id: UUID + analysis_id: Optional[UUID] class ResourceListMeta(BaseModel): @@ -241,9 +248,9 @@ def get_bucket_list(self) -> ResourceList[Bucket]: r.raise_for_status() return ResourceList[Bucket](**r.json()) - def get_bucket_by_id_or_name(self, bucket_id_or_name: str | UUID) -> Bucket | None: + def get_bucket_by_id(self, bucket_id: str | UUID) -> Bucket | None: r = httpx.get( - urljoin(self.base_url, f"/storage/buckets/{bucket_id_or_name}"), + urljoin(self.base_url, f"/storage/buckets/{bucket_id}"), headers=self.auth_client.get_auth_bearer_header(), ) @@ -282,37 +289,52 @@ def upload_to_bucket( r.raise_for_status() return ResourceList[BucketFile](**r.json()) - def get_analysis_file_list(self) -> ResourceList[AnalysisFile]: + def get_analysis_bucket_file_list(self) -> ResourceList[AnalysisBucketFile]: r = httpx.get( - urljoin(self.base_url, "/analysis-files"), + urljoin(self.base_url, "/analysis-bucket-files"), headers=self.auth_client.get_auth_bearer_header(), ) r.raise_for_status() - return ResourceList[AnalysisFile](**r.json()) + return ResourceList[AnalysisBucketFile](**r.json()) + + def get_analysis_bucket( + self, + analysis_id: str | UUID, + type: BucketType + ) -> AnalysisBucket: + r = httpx.get( + urljoin(self.base_url, "/analysis-buckets?filter[analysis_id]=" + str(analysis_id) + "&filter[type]=" + str(type)), + headers=self.auth_client.get_auth_bearer_header(), + ) + + r.raise_for_status() + lst = ResourceList[AnalysisBucket](**r.json()) + + assert len(lst.data) == 1 + + return lst.data[0] def link_bucket_file_to_analysis( self, - analysis_id: str | UUID, - bucket_file_id: str | UUID, + bucket_id: str | UUID, + external_id: str | UUID, bucket_file_name: str, - bucket_type: BucketType, root=True, - ) -> AnalysisFile: + ) -> AnalysisBucketFile: r = httpx.post( - urljoin(self.base_url, "/analysis-files"), + urljoin(self.base_url, "/analysis-bucket-files"), headers=self.auth_client.get_auth_bearer_header(), json={ - "analysis_id": str(analysis_id), - "bucket_file_id": str(bucket_file_id), - "type": bucket_type, + "bucket_id": str(bucket_id), + "external_id": str(external_id), "name": bucket_file_name, "root": root, }, ) r.raise_for_status() - return AnalysisFile(**r.json()) + return AnalysisBucketFile(**r.json()) def stream_bucket_file(self, bucket_file_id: str | UUID, chunk_size=1024): with httpx.stream( diff --git a/project/routers/final.py b/project/routers/final.py index eeac652..2fc066e 100644 --- a/project/routers/final.py +++ b/project/routers/final.py @@ -14,7 +14,7 @@ get_client_id, get_api_client, ) -from project.hub import FlameHubClient, format_analysis_bucket_name +from project.hub import FlameHubClient router = APIRouter() logger = logging.getLogger(__name__) @@ -38,9 +38,13 @@ def __bg_upload_to_remote( try: # fetch from local minio minio_resp = minio.get_object(bucket_name, object_name) + + # fetch analysis bucket + analysis_bucket = api.get_analysis_bucket(client_id, "RESULT") + # upload to remote bucket_file_lst = api.upload_to_bucket( - format_analysis_bucket_name(client_id, "RESULT"), + analysis_bucket.external_id, object_name, io.BytesIO(minio_resp.data), minio_resp.headers.get("Content-Type", "application/octet-stream"), @@ -52,7 +56,7 @@ def __bg_upload_to_remote( bucket_file = bucket_file_lst.data[0] # link file to analysis api.link_bucket_file_to_analysis( - client_id, bucket_file.id, bucket_file.name, "RESULT" + client_id, bucket_file.id, bucket_file.name ) # remove from local minio minio.remove_object(bucket_name, object_name) diff --git a/project/routers/intermediate.py b/project/routers/intermediate.py index 00bb906..bca2e04 100644 --- a/project/routers/intermediate.py +++ b/project/routers/intermediate.py @@ -17,7 +17,7 @@ get_client_id, get_api_client, ) -from project.hub import FlameHubClient, format_analysis_bucket_name +from project.hub import FlameHubClient router = APIRouter() logger = logging.getLogger(__name__) @@ -46,8 +46,12 @@ def __bg_upload_to_remote( try: minio_resp = minio.get_object(bucket_name, object_name) + + # fetch analysis bucket + analysis_bucket = api.get_analysis_bucket(client_id, "TEMP") + bucket_file_lst = api.upload_to_bucket( - format_analysis_bucket_name(client_id, "TEMP"), + analysis_bucket.external_id, object_name, io.BytesIO(minio_resp.data), minio_resp.headers.get("Content-Type", "application/octet-stream"), @@ -56,7 +60,7 @@ def __bg_upload_to_remote( assert len(bucket_file_lst.data) == 1 bucket_file = bucket_file_lst.data[0] api.link_bucket_file_to_analysis( - client_id, bucket_file.id, bucket_file.name, "TEMP" + analysis_bucket.id, bucket_file.id, bucket_file.name ) object_id_to_hub_bucket_dict[object_id] = str(bucket_file.id) minio.remove_object(bucket_name, object_name) diff --git a/tests/test_final.py b/tests/test_final.py index 1d1b356..cf4bc27 100644 --- a/tests/test_final.py +++ b/tests/test_final.py @@ -9,7 +9,7 @@ def test_200_submit_to_upload(test_client, rng, api_client, analysis_id): - analysis_file_count_old = len(api_client.get_analysis_file_list().data) + analysis_file_count_old = len(api_client.get_analysis_bucket_file_list().data) blob = next_random_bytes(rng) r = test_client.put( @@ -21,7 +21,7 @@ def test_200_submit_to_upload(test_client, rng, api_client, analysis_id): assert r.status_code == status.HTTP_204_NO_CONTENT def __check_analysis_file_count_increases(): - analysis_file_count_new = len(api_client.get_analysis_file_list().data) + analysis_file_count_new = len(api_client.get_analysis_bucket_file_list().data) return analysis_file_count_new > analysis_file_count_old assert eventually(__check_analysis_file_count_increases) diff --git a/tests/test_hub.py b/tests/test_hub.py index d33160f..766e563 100644 --- a/tests/test_hub.py +++ b/tests/test_hub.py @@ -1,7 +1,6 @@ import pytest from project.hub import ( - format_analysis_bucket_name, BucketType, ) from tests.common.helpers import next_prefixed_name, eventually, next_random_bytes @@ -27,8 +26,8 @@ def result_bucket_name(analysis_id, api_client): # check that buckets are eventually created (happens asynchronously) def _check_buckets_exist(): for bucket_type in bucket_types: - bucket_name = format_analysis_bucket_name(analysis_id, bucket_type) - bucket = api_client.get_bucket_by_id_or_name(bucket_name) + analysis_bucket = api_client.get_analysis_bucket(analysis_id, bucket_type) + bucket = api_client.get_bucket_by_id(analysis_bucket.external_id) if bucket is None: return False @@ -37,15 +36,6 @@ def _check_buckets_exist(): assert eventually(_check_buckets_exist) - # check that buckets are listed correctly - bucket_list = api_client.get_bucket_list() - - for bucket_type in bucket_types: - bucket_name = format_analysis_bucket_name(analysis_id, bucket_type) - assert any([b.name == bucket_name for b in bucket_list.data]) - - yield format_analysis_bucket_name(analysis_id, "RESULT") - @pytest.fixture def uploaded_bucket_file(result_bucket_name, api_client, rng): @@ -73,16 +63,18 @@ def uploaded_bucket_file(result_bucket_name, api_client, rng): def test_link_bucket_file_to_analysis(uploaded_bucket_file, analysis_id, api_client): _, bucket_file = uploaded_bucket_file + analysis_bucket = api_client.get_analysis_bucket(analysis_id, "RESULT") + # check that the analysis file was created analysis_file = api_client.link_bucket_file_to_analysis( - analysis_id, bucket_file.id, bucket_file.name, bucket_type="RESULT" + analysis_bucket.external_id, bucket_file.id, bucket_file.name ) assert analysis_file.name == bucket_file.name assert analysis_file.bucket_file_id == bucket_file.id # check that it appears in the list - analysis_file_list = api_client.get_analysis_file_list() + analysis_file_list = api_client.get_analysis_bucket_file_list() assert any([af.id == analysis_file.id for af in analysis_file_list.data]) From 1c2e7bcb6c7897cd285a994f9832fcb935c01716 Mon Sep 17 00:00:00 2001 From: Maximilian Jugl Date: Fri, 14 Jun 2024 14:45:02 +0200 Subject: [PATCH 2/4] fix: adapt tests to new object model --- project/hub.py | 22 ++++++++++++++-------- tests/test_hub.py | 13 ++++++++----- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/project/hub.py b/project/hub.py index 6c6d675..41eeee6 100644 --- a/project/hub.py +++ b/project/hub.py @@ -54,6 +54,7 @@ class BucketFile(BaseModel): created_at: datetime updated_at: datetime + class AnalysisBucket(BaseModel): id: UUID type: BucketType @@ -62,6 +63,7 @@ class AnalysisBucket(BaseModel): created_at: datetime updated_at: datetime + class AnalysisBucketFile(BaseModel): id: UUID name: Optional[str] @@ -299,12 +301,16 @@ def get_analysis_bucket_file_list(self) -> ResourceList[AnalysisBucketFile]: return ResourceList[AnalysisBucketFile](**r.json()) def get_analysis_bucket( - self, - analysis_id: str | UUID, - type: BucketType + self, analysis_id: str | UUID, bucket_type: BucketType ) -> AnalysisBucket: r = httpx.get( - urljoin(self.base_url, "/analysis-buckets?filter[analysis_id]=" + str(analysis_id) + "&filter[type]=" + str(type)), + urljoin( + self.base_url, + "/analysis-buckets?filter[analysis_id]=" + + str(analysis_id) + + "&filter[type]=" + + str(bucket_type), + ), headers=self.auth_client.get_auth_bearer_header(), ) @@ -317,8 +323,8 @@ def get_analysis_bucket( def link_bucket_file_to_analysis( self, - bucket_id: str | UUID, - external_id: str | UUID, + analysis_bucket_id: str | UUID, + bucket_file_id: str | UUID, bucket_file_name: str, root=True, ) -> AnalysisBucketFile: @@ -326,8 +332,8 @@ def link_bucket_file_to_analysis( urljoin(self.base_url, "/analysis-bucket-files"), headers=self.auth_client.get_auth_bearer_header(), json={ - "bucket_id": str(bucket_id), - "external_id": str(external_id), + "bucket_id": str(analysis_bucket_id), + "external_id": str(bucket_file_id), "name": bucket_file_name, "root": root, }, diff --git a/tests/test_hub.py b/tests/test_hub.py index 766e563..3df399b 100644 --- a/tests/test_hub.py +++ b/tests/test_hub.py @@ -20,7 +20,7 @@ def test_auth_no_reissue(auth_client): @pytest.fixture -def result_bucket_name(analysis_id, api_client): +def result_bucket_id(analysis_id, api_client): bucket_types: tuple[BucketType, ...] = ("CODE", "TEMP", "RESULT") # check that buckets are eventually created (happens asynchronously) @@ -36,15 +36,18 @@ def _check_buckets_exist(): assert eventually(_check_buckets_exist) + # bucket id is referenced from analysis bucket by its external_id prop + yield api_client.get_analysis_bucket(analysis_id, "RESULT").external_id + @pytest.fixture -def uploaded_bucket_file(result_bucket_name, api_client, rng): +def uploaded_bucket_file(result_bucket_id, api_client, rng): file_name = next_prefixed_name() file_blob = next_random_bytes(rng) # check that bucket file is created bucket_file_created_list = api_client.upload_to_bucket( - result_bucket_name, file_name, file_blob + result_bucket_id, file_name, file_blob ) assert len(bucket_file_created_list.data) == 1 @@ -67,11 +70,11 @@ def test_link_bucket_file_to_analysis(uploaded_bucket_file, analysis_id, api_cli # check that the analysis file was created analysis_file = api_client.link_bucket_file_to_analysis( - analysis_bucket.external_id, bucket_file.id, bucket_file.name + analysis_bucket.id, bucket_file.id, bucket_file.name ) assert analysis_file.name == bucket_file.name - assert analysis_file.bucket_file_id == bucket_file.id + assert analysis_file.external_id == bucket_file.id # check that it appears in the list analysis_file_list = api_client.get_analysis_bucket_file_list() From 35fca4feb61aa3ea3648642d0a564cfd0a68514c Mon Sep 17 00:00:00 2001 From: Maximilian Jugl Date: Mon, 17 Jun 2024 13:29:25 +0200 Subject: [PATCH 3/4] fix: update link to analysis file in `/final` endpoint --- project/routers/final.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/project/routers/final.py b/project/routers/final.py index 2fc066e..ab957ab 100644 --- a/project/routers/final.py +++ b/project/routers/final.py @@ -52,11 +52,12 @@ def __bg_upload_to_remote( # check that only one file has been submitted assert len(bucket_file_lst.data) == 1 - # fetch file s.t. it can be linked + # fetch file s.t. it can be linked to result bucket bucket_file = bucket_file_lst.data[0] + analysis_bucket = api.get_analysis_bucket(client_id, "RESULT") # link file to analysis api.link_bucket_file_to_analysis( - client_id, bucket_file.id, bucket_file.name + analysis_bucket.id, bucket_file.id, bucket_file.name ) # remove from local minio minio.remove_object(bucket_name, object_name) From 62225c7caf9f721dbd46aad2ff5e5639a5b6b3dc Mon Sep 17 00:00:00 2001 From: Maximilian Jugl Date: Mon, 17 Jun 2024 13:31:41 +0200 Subject: [PATCH 4/4] feat: remove `format_analysis_bucket_name` in favor of new data model --- project/hub.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/project/hub.py b/project/hub.py index 41eeee6..b2b0cf0 100644 --- a/project/hub.py +++ b/project/hub.py @@ -88,12 +88,6 @@ def _now(): return int(time.time()) -def format_analysis_bucket_name( - analysis_id: str | UUID, bucket_type: BucketType -) -> str: - return f"analysis-{bucket_type.lower()}-files.{analysis_id}" - - class FlamePasswordAuthClient: def __init__( self,