Skip to content

Commit

Permalink
Remove cloudstorages/id/content endpoint (#6946)
Browse files Browse the repository at this point in the history 
We discussed that support of the previous version of CS content endpoint
would be terminated in version 2.6.0
  • Loading branch information
@Marishka17
Marishka17 authored Nov 22, 2023
1 parent 410ea0b commit e530ebb
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 158 deletions.
4 changes: 4 additions & 0 deletions changelog.d/20231013_152455_maria_remove_get_cs_content_v1_endpoint.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Removed

- Support for V1 cloudstorages/id/content endpoint
(<https://github.com/opencv/cvat/pull/6946>)
52 changes: 0 additions & 52 deletions cvat/apps/engine/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
OpenApiParameter, OpenApiResponse, PolymorphicProxySerializer,
extend_schema_view, extend_schema
)
from drf_spectacular.plumbing import build_array_type, build_basic_type

from pathlib import Path
from rest_framework import mixins, serializers, status, viewsets
Expand Down Expand Up @@ -2483,57 +2482,6 @@ def create(self, request, *args, **kwargs):
response = HttpResponseBadRequest(str(ex))
return response

@extend_schema(summary='Method returns a manifest content',
parameters=[
OpenApiParameter('manifest_path', description='Path to the manifest file in a cloud storage',
location=OpenApiParameter.QUERY, type=OpenApiTypes.STR),
],
responses={
'200': OpenApiResponse(response=build_array_type(build_basic_type(OpenApiTypes.STR)), description='A manifest content'),
},
deprecated=True,
description="This method is deprecated and will be removed in version 2.6.0. "
"Please use the new version of API: /cloudstorages/id/content-v2/",
)
@action(detail=True, methods=['GET'], url_path='content')
def content(self, request, pk):
storage = None
try:
db_storage = self.get_object()
storage = db_storage_to_storage_instance(db_storage)
if not db_storage.manifests.count():
raise ValidationError('There is no manifest file')
manifest_path = request.query_params.get('manifest_path', db_storage.manifests.first().filename)
manifest_prefix = os.path.dirname(manifest_path)

full_manifest_path = os.path.join(db_storage.get_storage_dirname(), manifest_path)
if not os.path.exists(full_manifest_path) or \
datetime.fromtimestamp(os.path.getmtime(full_manifest_path), tz=timezone.utc) < storage.get_file_last_modified(manifest_path):
storage.download_file(manifest_path, full_manifest_path)
manifest = ImageManifestManager(full_manifest_path, db_storage.get_storage_dirname())
# need to update index
manifest.set_index()
manifest_files = [os.path.join(manifest_prefix, f) for f in manifest.data]
return Response(
data=manifest_files,
content_type='text/plain',
headers={'Deprecation': 'true'}
)

except CloudStorageModel.DoesNotExist:
message = f"Storage {pk} does not exist"
slogger.glob.error(message)
return HttpResponseNotFound(message)
except (ValidationError, PermissionDenied, NotFound) as ex:
msg = str(ex) if not isinstance(ex, ValidationError) else \
'\n'.join([str(d) for d in ex.detail])
slogger.cloud_storage[pk].info(msg)
return Response(data=msg, status=ex.status_code)
except Exception as ex:
slogger.glob.error(str(ex))
return Response("An internal error has occurred",
status=status.HTTP_500_INTERNAL_SERVER_ERROR)

@extend_schema(summary='Method returns the content of the cloud storage',
parameters=[
OpenApiParameter('manifest_path', description='Path to the manifest file in a cloud storage',
Expand Down
36 changes: 0 additions & 36 deletions cvat/schema.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -767,42 +767,6 @@ paths:
schema:
type: string
description: Cloud Storage actions (GET | PUT | DELETE)
/api/cloudstorages/{id}/content:
get:
operationId: cloudstorages_retrieve_content
description: 'This method is deprecated and will be removed in version 2.6.0.
Please use the new version of API: /cloudstorages/id/content-v2/'
summary: Method returns a manifest content
parameters:
- in: path
name: id
schema:
type: integer
description: A unique integer value identifying this cloud storage.
required: true
- in: query
name: manifest_path
schema:
type: string
description: Path to the manifest file in a cloud storage
tags:
- cloudstorages
security:
- sessionAuth: []
csrfAuth: []
tokenAuth: []
- signatureAuth: []
- basicAuth: []
deprecated: true
responses:
'200':
content:
application/vnd.cvat+json:
schema:
type: array
items:
type: string
description: A manifest content
/api/cloudstorages/{id}/content-v2:
get:
operationId: cloudstorages_retrieve_content_v2
Expand Down
79 changes: 27 additions & 52 deletions tests/python/rest_api/test_cloud_storages.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import io
import json
from enum import Enum
from functools import partial
from http import HTTPStatus
from typing import Any, Optional
Expand Down Expand Up @@ -449,55 +448,39 @@ def test_org_user_get_cloud_storage_preview(
class TestGetCloudStorageContent:
USER = "admin1"

class SUPPORTED_VERSIONS(str, Enum):
V1 = "v1"
V2 = "v2"

def _test_get_cloud_storage_content(
self,
cloud_storage_id: int,
version: SUPPORTED_VERSIONS = SUPPORTED_VERSIONS.V2,
manifest: Optional[str] = None,
**kwargs,
):
with make_api_client(self.USER) as api_client:
content_kwargs = {"manifest_path": manifest} if manifest else {}

if version == self.SUPPORTED_VERSIONS.V2:
for item in ["next_token", "prefix", "page_size"]:
if item_value := kwargs.get(item):
content_kwargs[item] = item_value
for item in ["next_token", "prefix", "page_size"]:
if item_value := kwargs.get(item):
content_kwargs[item] = item_value

methods = {
self.SUPPORTED_VERSIONS.V1: api_client.cloudstorages_api.retrieve_content,
self.SUPPORTED_VERSIONS.V2: api_client.cloudstorages_api.retrieve_content_v2,
}
(data, _) = methods[version](cloud_storage_id, **content_kwargs)
(data, _) = api_client.cloudstorages_api.retrieve_content_v2(
cloud_storage_id, **content_kwargs
)

return data

@pytest.mark.parametrize("cloud_storage_id", [2])
@pytest.mark.parametrize(
"version, manifest, prefix, default_bucket_prefix, page_size, expected_content",
"manifest, prefix, default_bucket_prefix, page_size, expected_content",
[
(
SUPPORTED_VERSIONS.V1, # [v1] list all bucket content
"sub/manifest.jsonl",
None,
None,
None,
["sub/image_case_65_1.png", "sub/image_case_65_2.png"],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list the top level of bucket with based on manifest
# [v2] list the top level of bucket with based on manifest
"sub/manifest.jsonl",
None,
None,
None,
[FileInfo(mime_type="DIR", name="sub", type="DIR")],
),
(
SUPPORTED_VERSIONS.V2, # [v2] search by some prefix in bucket content based on manifest
# [v2] search by some prefix in bucket content based on manifest
"sub/manifest.jsonl",
"sub/image_case_65_1",
None,
Expand All @@ -507,7 +490,7 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list the second layer (directory "sub") of bucket content based on manifest
# [v2] list the second layer (directory "sub") of bucket content based on manifest
"sub/manifest.jsonl",
"sub/",
None,
Expand All @@ -518,15 +501,15 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list the top layer of real bucket content
# [v2] list the top layer of real bucket content
None,
None,
None,
None,
[FileInfo(mime_type="DIR", name="sub", type="DIR")],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list the second layer (directory "sub") of real bucket content
# [v2] list the second layer (directory "sub") of real bucket content
None,
"sub/",
None,
Expand All @@ -537,7 +520,6 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2,
None,
"/sub/", # cover case: API is identical to share point API
None,
Expand All @@ -552,7 +534,7 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list bucket content based on manifest when default bucket prefix is set to directory
# [v2] list bucket content based on manifest when default bucket prefix is set to directory
"sub/manifest.jsonl",
None,
"sub/",
Expand All @@ -565,7 +547,6 @@ def _test_get_cloud_storage_content(
(
# [v2] list bucket content based on manifest when default bucket prefix
# is set to template from which the files should start
SUPPORTED_VERSIONS.V2,
"sub/manifest.jsonl",
None,
"sub/image_case_65_1",
Expand All @@ -575,7 +556,7 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list bucket content based on manifest when specified prefix is stricter than default bucket prefix
# [v2] list bucket content based on manifest when specified prefix is stricter than default bucket prefix
"sub/manifest.jsonl",
"sub/image_case_65_1",
"sub/image_case",
Expand All @@ -585,7 +566,7 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list bucket content based on manifest when default bucket prefix is stricter than specified prefix
# [v2] list bucket content based on manifest when default bucket prefix is stricter than specified prefix
"sub/manifest.jsonl",
"sub/image_case",
"sub/image_case_65_1",
Expand All @@ -595,15 +576,15 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list bucket content based on manifest when default bucket prefix and specified prefix have no intersection
# [v2] list bucket content based on manifest when default bucket prefix and specified prefix have no intersection
"sub/manifest.jsonl",
"sub/image_case_65_1",
"sub/image_case_65_2",
None,
[],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list bucket content based on manifest when default bucket prefix contains dirs and prefix starts with it
# [v2] list bucket content based on manifest when default bucket prefix contains dirs and prefix starts with it
"sub/manifest.jsonl",
"s",
"sub/",
Expand All @@ -613,7 +594,7 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list real bucket content when default bucket prefix is set to directory
# [v2] list real bucket content when default bucket prefix is set to directory
None,
None,
"sub/",
Expand All @@ -630,7 +611,6 @@ def _test_get_cloud_storage_content(
(
# [v2] list real bucket content when default bucket prefix
# is set to template from which the files should start
SUPPORTED_VERSIONS.V2,
None,
None,
"sub/demo",
Expand All @@ -640,7 +620,7 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list real bucket content when specified prefix is stricter than default bucket prefix
# [v2] list real bucket content when specified prefix is stricter than default bucket prefix
None,
"sub/image_case_65_1",
"sub/image_case",
Expand All @@ -650,7 +630,7 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list real bucket content when default bucket prefix is stricter than specified prefix
# [v2] list real bucket content when default bucket prefix is stricter than specified prefix
None,
"sub/image_case",
"sub/image_case_65_1",
Expand All @@ -660,15 +640,15 @@ def _test_get_cloud_storage_content(
],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list real bucket content when default bucket prefix and specified prefix have no intersection
# [v2] list real bucket content when default bucket prefix and specified prefix have no intersection
None,
"sub/image_case_65_1",
"sub/image_case_65_2",
None,
[],
),
(
SUPPORTED_VERSIONS.V2, # [v2] list real bucket content when default bucket prefix contains dirs and prefix starts with it
# [v2] list real bucket content when default bucket prefix contains dirs and prefix starts with it
None,
"s",
"sub/",
Expand All @@ -682,7 +662,6 @@ def _test_get_cloud_storage_content(
def test_get_cloud_storage_content(
self,
cloud_storage_id: int,
version: SUPPORTED_VERSIONS,
manifest: Optional[str],
prefix: Optional[str],
default_bucket_prefix: Optional[str],
Expand All @@ -703,30 +682,26 @@ def test_get_cloud_storage_content(
assert response.status == HTTPStatus.OK

result = self._test_get_cloud_storage_content(
cloud_storage_id, version, manifest, prefix=prefix, page_size=page_size
cloud_storage_id, manifest, prefix=prefix, page_size=page_size
)
if expected_content:
if version == self.SUPPORTED_VERSIONS.V1:
assert result == expected_content
else:
assert result["content"] == expected_content
assert result["content"] == expected_content
if page_size:
assert len(result["content"]) <= page_size

@pytest.mark.parametrize("cloud_storage_id, prefix, page_size", [(2, "sub/", 2)])
def test_iterate_over_cloud_storage_content(
self, cloud_storage_id: int, prefix: str, page_size: int
):
expected_content = self._test_get_cloud_storage_content(
cloud_storage_id, self.SUPPORTED_VERSIONS.V2, prefix=prefix
)["content"]
expected_content = self._test_get_cloud_storage_content(cloud_storage_id, prefix=prefix)[
"content"
]

current_content = []
next_token = None
while True:
result = self._test_get_cloud_storage_content(
cloud_storage_id,
self.SUPPORTED_VERSIONS.V2,
prefix=prefix,
page_size=page_size,
next_token=next_token,
Expand Down
Loading

0 comments on commit e530ebb

Please sign in to comment.