From beac4f656df0539c13b14f3d675e9dd5fae4abf5 Mon Sep 17 00:00:00 2001 From: Roman Romanov Date: Fri, 24 Jan 2025 14:19:03 +0200 Subject: [PATCH 1/3] Allow etag inside of files API --- aidial_client/__init__.py | 4 ++ aidial_client/_exception.py | 18 ++++++ aidial_client/_http_client/_async.py | 4 +- aidial_client/_http_client/_sync.py | 6 +- aidial_client/resources/files.py | 96 +++++++++++++++++++++++++--- aidial_client/types/metadata.py | 1 + tests/integration/fixtures.py | 18 ++++++ 7 files changed, 134 insertions(+), 13 deletions(-) diff --git a/aidial_client/__init__.py b/aidial_client/__init__.py index 08e34d4..a61d282 100644 --- a/aidial_client/__init__.py +++ b/aidial_client/__init__.py @@ -3,9 +3,11 @@ from aidial_client._client_pool import AsyncDialClientPool, DialClientPool from aidial_client._exception import ( DialException, + EtagMismatchError, InvalidDialURLError, InvalidRequestError, ParsingDataError, + ResourceNotFoundError, ) __all__ = [ @@ -21,4 +23,6 @@ "InvalidDialURLError", "InvalidRequestError", "ParsingDataError", + "EtagMismatchError", + "ResourceNotFoundError", ] diff --git a/aidial_client/_exception.py b/aidial_client/_exception.py index a7e041e..aed54ff 100644 --- a/aidial_client/_exception.py +++ b/aidial_client/_exception.py @@ -74,3 +74,21 @@ def __init__(self, message: str, **kwargs) -> None: status_code=HTTPStatus.UNPROCESSABLE_ENTITY, **kwargs, ) + + +class EtagMismatchError(DialException): + def __init__(self, message: str, **kwargs) -> None: + super().__init__( + message=message, + status_code=HTTPStatus.PRECONDITION_FAILED, + **kwargs, + ) + + +class ResourceNotFoundError(DialException): + def __init__(self, message: str, **kwargs) -> None: + super().__init__( + message=message, + status_code=HTTPStatus.NOT_FOUND, + **kwargs, + ) diff --git a/aidial_client/_http_client/_async.py b/aidial_client/_http_client/_async.py index 266b096..690169e 100644 --- a/aidial_client/_http_client/_async.py +++ b/aidial_client/_http_client/_async.py @@ -48,7 +48,7 @@ async def request( options: FinalRequestOptions, cast_to: Type[ResponseT], remaining_retries: Optional[int] = None, - _on_http_error: Optional[ + on_http_error: Optional[ Callable[[httpx.HTTPStatusError], Optional[DialException]] ] = None, ) -> ResponseT: @@ -100,7 +100,7 @@ async def request( remaining_retries=retries, ) # Try to get custom error from response status_code/code/message - custom_error = _on_http_error(err) if _on_http_error else None + custom_error = on_http_error(err) if on_http_error else None # or fallback to default processing raised_error = custom_error or self._make_dial_error_from_response( err.response diff --git a/aidial_client/_http_client/_sync.py b/aidial_client/_http_client/_sync.py index 382b9a5..2441dee 100644 --- a/aidial_client/_http_client/_sync.py +++ b/aidial_client/_http_client/_sync.py @@ -43,11 +43,11 @@ def auth_headers(self) -> Dict[str, str]: def request( self, + *, cast_to: Type[ResponseT], options: FinalRequestOptions, remaining_retries: Optional[int] = None, - *, - error_processor: Optional[ + on_http_error: Optional[ Callable[[httpx.HTTPStatusError], Optional[DialException]] ] = None, ) -> ResponseT: @@ -100,7 +100,7 @@ def request( remaining_retries=retries, ) # Try to get custom error from response status_code/code/message - custom_error = error_processor(err) if error_processor else None + custom_error = on_http_error(err) if on_http_error else None # or fallback to default processing raised_error = custom_error or self._make_dial_error_from_response( err.response diff --git a/aidial_client/resources/files.py b/aidial_client/resources/files.py index e489291..73fa1f7 100644 --- a/aidial_client/resources/files.py +++ b/aidial_client/resources/files.py @@ -1,16 +1,22 @@ from pathlib import PurePosixPath -from typing import Union +from typing import Literal, Optional, Union from urllib.parse import urljoin import httpx from aidial_client._constants import API_PREFIX -from aidial_client._exception import InvalidDialURLError +from aidial_client._exception import ( + DialException, + EtagMismatchError, + InvalidDialURLError, + ResourceNotFoundError, +) from aidial_client._internal_types._generic import NoneType from aidial_client._internal_types._http_request import ( FileTypes, FinalRequestOptions, ) +from aidial_client._utils._dict import remove_none from aidial_client.helpers.storage_resource import DialStorageResourceMixin from aidial_client.resources.base import AsyncResource, Resource from aidial_client.resources.metadata import AsyncMetadata, Metadata @@ -18,12 +24,30 @@ from aidial_client.types.metadata import FileMetadata +def _files_error_processor( + http_status_error: httpx.HTTPStatusError, +) -> Optional[DialException]: + if http_status_error.response.status_code == 412: + return EtagMismatchError( + message=http_status_error.response.text, + ) + elif http_status_error.response.status_code == 404: + return ResourceNotFoundError( + message=http_status_error.response.text, + ) + return None + + class Files(Resource, DialStorageResourceMixin): metadata: Metadata resource_type: str = "files" def upload( - self, url: Union[str, PurePosixPath], file: FileTypes + self, + url: Union[str, PurePosixPath], + file: FileTypes, + etag_if_match: Optional[str] = None, + etag_if_none_match: Optional[Literal["*"]] = None, ) -> FileMetadata: return self.http_client.request( cast_to=FileMetadata, @@ -31,10 +55,21 @@ def upload( method="PUT", url=urljoin(API_PREFIX, self.get_api_path(str(url))), files={"file": file}, + headers=remove_none( + { + "If-Match": etag_if_match, + "If-None-Match": etag_if_none_match, + } + ), ), + on_http_error=_files_error_processor, ) - def download(self, url: Union[str, PurePosixPath]) -> FileDownloadResponse: + def download( + self, + url: Union[str, PurePosixPath], + etag_if_match: Optional[str] = None, + ) -> FileDownloadResponse: storage_resource = self.get_storage_resource(str(url)) if storage_resource.filename is None: raise InvalidDialURLError("URL points to a directory, not a file") @@ -43,19 +78,35 @@ def download(self, url: Union[str, PurePosixPath]) -> FileDownloadResponse: options=FinalRequestOptions( method="GET", url=urljoin(API_PREFIX, storage_resource.api_path), + headers=remove_none( + { + "If-Match": etag_if_match, + } + ), ), + on_http_error=_files_error_processor, ) return FileDownloadResponse( response=response, filename=storage_resource.filename ) - def delete(self, url: Union[str, PurePosixPath]) -> None: + def delete( + self, + url: Union[str, PurePosixPath], + etag_if_match: Optional[str] = None, + ) -> None: return self.http_client.request( cast_to=NoneType, options=FinalRequestOptions( method="DELETE", url=urljoin(API_PREFIX, self.get_api_path(str(url))), + headers=remove_none( + { + "If-Match": etag_if_match, + } + ), ), + on_http_error=_files_error_processor, ) def get_metadata(self, url: Union[str, PurePosixPath]) -> FileMetadata: @@ -70,7 +121,11 @@ class AsyncFiles(AsyncResource, DialStorageResourceMixin): resource_type: str = "files" async def upload( - self, url: Union[str, PurePosixPath], file: FileTypes + self, + url: Union[str, PurePosixPath], + file: FileTypes, + etag_if_match: Optional[str] = None, + etag_if_none_match: Optional[Literal["*"]] = None, ) -> FileMetadata: return await self.http_client.request( @@ -79,11 +134,20 @@ async def upload( method="PUT", url=urljoin(API_PREFIX, self.get_api_path(str(url))), files={"file": file}, + headers=remove_none( + { + "If-Match": etag_if_match, + "If-None-Match": etag_if_none_match, + } + ), ), + on_http_error=_files_error_processor, ) async def download( - self, url: Union[str, PurePosixPath] + self, + url: Union[str, PurePosixPath], + etag_if_match: Optional[str] = None, ) -> FileDownloadResponse: storage_resource = self.get_storage_resource(str(url)) if storage_resource.filename is None: @@ -93,19 +157,35 @@ async def download( options=FinalRequestOptions( method="GET", url=urljoin(API_PREFIX, storage_resource.api_path), + headers=remove_none( + { + "If-Match": etag_if_match, + } + ), ), + on_http_error=_files_error_processor, ) return FileDownloadResponse( response=response, filename=storage_resource.filename ) - async def delete(self, url: Union[str, PurePosixPath]) -> None: + async def delete( + self, + url: Union[str, PurePosixPath], + etag_if_match: Optional[str] = None, + ) -> None: return await self.http_client.request( cast_to=NoneType, options=FinalRequestOptions( method="DELETE", url=urljoin(API_PREFIX, self.get_api_path(str(url))), + headers=remove_none( + { + "If-Match": etag_if_match, + } + ), ), + on_http_error=_files_error_processor, ) async def get_metadata( diff --git a/aidial_client/types/metadata.py b/aidial_client/types/metadata.py index b414af5..7662c20 100644 --- a/aidial_client/types/metadata.py +++ b/aidial_client/types/metadata.py @@ -38,6 +38,7 @@ class FileMetadata(BaseMetadata): content_length: Optional[int] = None content_type: Optional[str] = None items: Optional[List[FileItem]] = None + etag: Optional[str] = None class ConversationItem(BaseMetadata): diff --git a/tests/integration/fixtures.py b/tests/integration/fixtures.py index 1789cbe..7927dd6 100644 --- a/tests/integration/fixtures.py +++ b/tests/integration/fixtures.py @@ -1,8 +1,10 @@ import os +import uuid import pytest from aidial_client import AsyncDial, Dial +from aidial_client._exception import ResourceNotFoundError @pytest.fixture @@ -36,3 +38,19 @@ def test_deployment(sync_client: Dial) -> str: deployment = next((d for d in deployments if d.id.startswith("gpt-"))) assert deployment return deployment.id + + +@pytest.fixture +def absent_test_file(sync_client): + + def _save_delete_file(p): + try: + sync_client.files.delete(p) + except ResourceNotFoundError: + pass + + unique_name = f"test-file-{uuid.uuid4()}.txt" + full_path = sync_client.my_files_home() / unique_name + _save_delete_file(full_path) + yield full_path + _save_delete_file(full_path) From ee093759150184133ac106c5da0cd5c806d25932 Mon Sep 17 00:00:00 2001 From: Roman Romanov Date: Fri, 24 Jan 2025 14:19:16 +0200 Subject: [PATCH 2/3] Cover etag with integration tests --- tests/integration/test_async_files.py | 96 +++++++++++++++++++++++++++ tests/integration/test_sync_files.py | 87 ++++++++++++++++++++++++ 2 files changed, 183 insertions(+) diff --git a/tests/integration/test_async_files.py b/tests/integration/test_async_files.py index eaa4190..c5e62d3 100644 --- a/tests/integration/test_async_files.py +++ b/tests/integration/test_async_files.py @@ -3,6 +3,7 @@ import pytest from aidial_client import AsyncDial, DialException +from aidial_client._exception import EtagMismatchError from aidial_client.types.metadata import FileMetadata from tests.integration.fixtures import * # type: ignore # noqa @@ -61,3 +62,98 @@ async def test_delete(async_client: AsyncDial): await async_client.files.download( url=await async_client.my_files_home() / file_path ) + + +@pytest.mark.asyncio +async def test_etag_in_response(async_client, absent_test_file): + upload_response = await async_client.files.upload( + url=absent_test_file, file=b"test 1" + ) + assert upload_response.etag is not None + first_etag = upload_response.etag + + upload_response = await async_client.files.upload( + url=absent_test_file, file=b"test 2" + ) + assert upload_response.etag is not None + second_etag = upload_response.etag + + assert first_etag != second_etag + + +@pytest.mark.asyncio +async def test_upload_with_etag_if_match(async_client, absent_test_file): + upload_response = await async_client.files.upload( + url=absent_test_file, file=b"test 1" + ) + first_etag = upload_response.etag + + upload_response = await async_client.files.upload( + url=absent_test_file, + file=b"test 2", + etag_if_match=first_etag, + ) + assert upload_response.etag != first_etag + + with pytest.raises(EtagMismatchError): + await async_client.files.upload( + url=absent_test_file, + file=b"test 3", + etag_if_match="invalid_etag", + ) + + +@pytest.mark.asyncio +async def test_upload_with_etag_if_none_match(async_client, absent_test_file): + await async_client.files.upload( + url=absent_test_file, + file=b"test 1", + etag_if_none_match="*", + ) + + with pytest.raises(EtagMismatchError): + await async_client.files.upload( + url=absent_test_file, + file=b"test 2", + etag_if_none_match="*", + ) + + +@pytest.mark.asyncio +async def test_delete_with_etag(async_client, absent_test_file): + upload_response = await async_client.files.upload( + url=absent_test_file, file=b"test 1" + ) + etag = upload_response.etag + + with pytest.raises(EtagMismatchError): + await async_client.files.delete( + url=absent_test_file, + etag_if_match="invalid_etag", + ) + + await async_client.files.delete( + url=absent_test_file, + etag_if_match=etag, + ) + + +@pytest.mark.asyncio +async def test_download_with_etag(async_client, absent_test_file): + upload_response = await async_client.files.upload( + url=absent_test_file, + file=b"test 1", + ) + etag = upload_response.etag + + download_result = await async_client.files.download( + url=absent_test_file, + etag_if_match=etag, + ) + assert await download_result.aget_content() == b"test 1" + + with pytest.raises(EtagMismatchError): + await async_client.files.download( + url=absent_test_file, + etag_if_match="invalid_etag", + ) diff --git a/tests/integration/test_sync_files.py b/tests/integration/test_sync_files.py index c8f6e88..4a54490 100644 --- a/tests/integration/test_sync_files.py +++ b/tests/integration/test_sync_files.py @@ -3,6 +3,7 @@ import pytest from aidial_client import Dial, DialException +from aidial_client._exception import EtagMismatchError from aidial_client.types.metadata import FileMetadata from tests.integration.fixtures import * # type: ignore # noqa @@ -56,3 +57,89 @@ def test_delete(sync_client): # Try to access content with pytest.raises(DialException): sync_client.files.download(url=sync_client.my_files_home() / file_path) + + +def test_etag_in_response(sync_client, absent_test_file): + upload_response = sync_client.files.upload( + url=absent_test_file, file=b"test 1" + ) + assert upload_response.etag is not None + first_etag = upload_response.etag + + upload_response = sync_client.files.upload( + url=absent_test_file, file=b"test 2" + ) + assert upload_response.etag is not None + second_etag = upload_response.etag + + assert first_etag != second_etag + + +def test_upload_with_etag_if_match(sync_client, absent_test_file): + upload_response = sync_client.files.upload( + url=absent_test_file, file=b"test 1" + ) + first_etag = upload_response.etag + + upload_response = sync_client.files.upload( + url=absent_test_file, + file=b"test 2", + etag_if_match=first_etag, + ) + assert upload_response.etag != first_etag + + with pytest.raises(EtagMismatchError): + sync_client.files.upload( + url=absent_test_file, + file=b"test 3", + etag_if_match="invalid_etag", + ) + + +def test_upload_with_etag_if_none_match(sync_client, absent_test_file): + sync_client.files.upload( + url=absent_test_file, file=b"test 1", etag_if_none_match="*" + ) + + with pytest.raises(EtagMismatchError): + sync_client.files.upload( + url=absent_test_file, file=b"test 2", etag_if_none_match="*" + ) + + +def test_delete_with_etag(sync_client, absent_test_file): + upload_response = sync_client.files.upload( + url=absent_test_file, file=b"test 1" + ) + etag = upload_response.etag + + with pytest.raises(EtagMismatchError): + sync_client.files.delete( + url=absent_test_file, + etag_if_match="invalid_etag", + ) + + sync_client.files.delete( + url=absent_test_file, + etag_if_match=etag, + ) + + +def test_download_with_etag(sync_client, absent_test_file): + upload_response = sync_client.files.upload( + url=absent_test_file, + file=b"test 1", + ) + etag = upload_response.etag + + download_result = sync_client.files.download( + url=absent_test_file, + etag_if_match=etag, + ) + assert download_result.get_content() == b"test 1" + + with pytest.raises(EtagMismatchError): + sync_client.files.download( + url=absent_test_file, + etag_if_match="invalid_etag", + ) From ac52ae80094c22482cd40578c4390061169ff162 Mon Sep 17 00:00:00 2001 From: Roman Romanov Date: Fri, 24 Jan 2025 14:19:35 +0200 Subject: [PATCH 3/3] Fix: allow extra object type in deployment response --- aidial_client/types/deployment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aidial_client/types/deployment.py b/aidial_client/types/deployment.py index b625a76..85acc97 100644 --- a/aidial_client/types/deployment.py +++ b/aidial_client/types/deployment.py @@ -11,7 +11,7 @@ class Deployment(ExtraAllowModel): id: str model: str owner: str - object: Literal["deployment"] + object: Literal["deployment", "model"] status: Literal["succeeded"] created_at: int updated_at: int