From 04bf2fc480749264cd849a8bedf4d9bcb79e6838 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Mon, 8 Apr 2024 12:49:22 +0200 Subject: [PATCH 1/7] add benchmark --- .github/workflows/cicd.yaml | 38 ++++++ stac_fastapi/api/setup.py | 3 + stac_fastapi/api/tests/benchmarks.py | 168 +++++++++++++++++++++++++++ 3 files changed, 209 insertions(+) create mode 100644 stac_fastapi/api/tests/benchmarks.py diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index 514e31496..8578d8370 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -76,3 +76,41 @@ jobs: - uses: actions/checkout@v4 - name: Test generating docs run: make docs + +benchmark: + needs: [test] + runs-on: ubuntu-20.04 + steps: + - name: Check out repository code + uses: actions/checkout@v4 + + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install types + run: | + python -m pip install ./stac_fastapi/types[dev] + + - name: Install core api + run: | + python -m pip install ./stac_fastapi/api[dev,benchmark] + + - name: Run Benchmark + run: python -m pytest stac_fastapi/tests/benchmarks.py --benchmark-only --benchmark-columns 'min, max, mean, median' --benchmark-json output.json + + # - name: Store and benchmark result + # uses: benchmark-action/github-action-benchmark@v1 + # with: + # name: STAC FastAPI Benchmarks + # tool: 'pytest' + # output-file-path: output.json + # alert-threshold: '130%' + # comment-on-alert: true + # fail-on-alert: false + # # GitHub API token to make a commit comment + # github-token: ${{ secrets.GITHUB_TOKEN }} + # gh-pages-branch: 'gh-benchmarks' + # # Make a commit only if main + # auto-push: ${{ github.ref == 'refs/heads/main' }} diff --git a/stac_fastapi/api/setup.py b/stac_fastapi/api/setup.py index 1e3b8002f..9dfa86ac9 100644 --- a/stac_fastapi/api/setup.py +++ b/stac_fastapi/api/setup.py @@ -23,6 +23,9 @@ "requests", "pystac[validation]==1.*", ], + "benchmark": [ + "pytest-benchmark", + ], "docs": ["mkdocs", "mkdocs-material", "pdocs"], } diff --git a/stac_fastapi/api/tests/benchmarks.py b/stac_fastapi/api/tests/benchmarks.py new file mode 100644 index 000000000..d672a62de --- /dev/null +++ b/stac_fastapi/api/tests/benchmarks.py @@ -0,0 +1,168 @@ +from datetime import datetime +from typing import List, Optional, Union + +import pytest +from stac_pydantic.api.utils import link_factory +from starlette.testclient import TestClient + +from stac_fastapi.api.app import StacApi +from stac_fastapi.types import stac as stac_types +from stac_fastapi.types.config import ApiSettings +from stac_fastapi.types.core import BaseCoreClient, BaseSearchPostRequest, NumType + +collection_links = link_factory.CollectionLinks("/", "test").create_links() +item_links = link_factory.ItemLinks("/", "test", "test").create_links() + + +collections = [ + stac_types.Collection( + id=f"test_collection_{n}", + title="Test Collection", + description="A test collection", + keywords=["test"], + license="proprietary", + extent={ + "spatial": {"bbox": [[-180, -90, 180, 90]]}, + "temporal": {"interval": [["2000-01-01T00:00:00Z", None]]}, + }, + links=collection_links.dict(exclude_none=True), + ) + for n in range(0, 10) +] + +items = [ + stac_types.Item( + id=f"test_item_{n}", + type="Feature", + geometry={"type": "Point", "coordinates": [0, 0]}, + bbox=[-180, -90, 180, 90], + properties={"datetime": "2000-01-01T00:00:00Z"}, + links=item_links.dict(exclude_none=True), + assets={}, + ) + for n in range(0, 1000) +] + + +class CoreClient(BaseCoreClient): + def post_search( + self, search_request: BaseSearchPostRequest, **kwargs + ) -> stac_types.ItemCollection: + raise NotImplementedError + + def get_search( + self, + collections: Optional[List[str]] = None, + ids: Optional[List[str]] = None, + bbox: Optional[List[NumType]] = None, + intersects: Optional[str] = None, + datetime: Optional[Union[str, datetime]] = None, + limit: Optional[int] = 10, + **kwargs, + ) -> stac_types.ItemCollection: + raise NotImplementedError + + def get_item(self, item_id: str, collection_id: str, **kwargs) -> stac_types.Item: + raise NotImplementedError + + def all_collections(self, **kwargs) -> stac_types.Collections: + return stac_types.Collections( + collections=[collections], + links=[ + {"href": "test", "rel": "root"}, + {"href": "test", "rel": "self"}, + {"href": "test", "rel": "parent"}, + ], + ) + + def get_collection(self, collection_id: str, **kwargs) -> stac_types.Collection: + return collections[0] + + def item_collection( + self, + collection_id: str, + bbox: Optional[List[Union[float, int]]] = None, + datetime: Optional[Union[str, datetime]] = None, + limit: int = 10, + token: str = None, + **kwargs, + ) -> stac_types.ItemCollection: + return stac_types.ItemCollection( + type="FeatureCollection", features=[items[0:limit]] + ) + + +@pytest.fixture(autouse=True) +def client_validation() -> TestClient: + app = StacApi( + settings=ApiSettings(enable_response_models=True), client=CoreClient() + ) + with TestClient(app.app) as client: + yield client + + +@pytest.fixture(autouse=True) +def client_no_validation() -> TestClient: + app = StacApi( + settings=ApiSettings(enable_response_models=False), client=CoreClient() + ) + with TestClient(app.app) as client: + yield client + + +@pytest.mark.parametrize("limit", [1, 10, 50, 100, 200, 250]) +@pytest.mark.parametrize("validate", [True, False]) +def test_benchmark_items( + benchmark, client_validation, client_no_validation, validate, limit +): + """Benchmark items endpoint.""" + params = {"limit": limit} + + def f(p): + if validate: + return client_validation.get("/collections/fake_collection/items", params=p) + else: + return client_no_validation.get( + "/collections/fake_collection/items", params=p + ) + + benchmark.group = "Items With Model validation" if validate else "Items" + + response = benchmark(f, params) + assert response.status_code == 200 + + +@pytest.mark.parametrize("validate", [True, False]) +def test_benchmark_collection( + benchmark, client_validation, client_no_validation, validate +): + """Benchmark items endpoint.""" + + def f(): + if validate: + return client_validation.get("/collections/fake_collection") + else: + return client_no_validation.get("/collections/fake_collection") + + benchmark.group = "Collection With Model validation" if validate else "Collection" + + response = benchmark(f) + assert response.status_code == 200 + + +@pytest.mark.parametrize("validate", [True, False]) +def test_benchmark_collections( + benchmark, client_validation, client_no_validation, validate +): + """Benchmark items endpoint.""" + + def f(): + if validate: + return client_validation.get("/collections") + else: + return client_no_validation.get("/collections") + + benchmark.group = "Collections With Model validation" if validate else "Collections" + + response = benchmark(f) + assert response.status_code == 200 From 9cad74c07f26ab32d8ede630ef8634284a7717c0 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Mon, 8 Apr 2024 12:51:01 +0200 Subject: [PATCH 2/7] fix workflow --- .github/workflows/cicd.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index 8578d8370..e6bb31d29 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -77,7 +77,7 @@ jobs: - name: Test generating docs run: make docs -benchmark: + benchmark: needs: [test] runs-on: ubuntu-20.04 steps: From f9c4402948492f61a9dc1af8d05fcccb83c563b9 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Mon, 8 Apr 2024 13:02:17 +0200 Subject: [PATCH 3/7] fix --- .github/workflows/cicd.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index e6bb31d29..998124185 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -93,12 +93,16 @@ jobs: run: | python -m pip install ./stac_fastapi/types[dev] + - name: Install extensions + run: | + python -m pip install ./stac_fastapi/extensions + - name: Install core api run: | python -m pip install ./stac_fastapi/api[dev,benchmark] - name: Run Benchmark - run: python -m pytest stac_fastapi/tests/benchmarks.py --benchmark-only --benchmark-columns 'min, max, mean, median' --benchmark-json output.json + run: python -m pytest stac_fastapi/api/tests/benchmarks.py --benchmark-only --benchmark-columns 'min, max, mean, median' --benchmark-json output.json # - name: Store and benchmark result # uses: benchmark-action/github-action-benchmark@v1 From 5c7d0772a23e89d02c5212577f1043f04d6e9629 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Mon, 8 Apr 2024 16:30:26 +0200 Subject: [PATCH 4/7] do not add Response output to endpoints --- stac_fastapi/api/stac_fastapi/api/app.py | 28 ++++++--------------- stac_fastapi/api/stac_fastapi/api/routes.py | 21 +++++----------- stac_fastapi/api/tests/benchmarks.py | 22 +++++++++------- 3 files changed, 27 insertions(+), 44 deletions(-) diff --git a/stac_fastapi/api/stac_fastapi/api/app.py b/stac_fastapi/api/stac_fastapi/api/app.py index 28fff912c..557896d8f 100644 --- a/stac_fastapi/api/stac_fastapi/api/app.py +++ b/stac_fastapi/api/stac_fastapi/api/app.py @@ -132,9 +132,7 @@ def register_landing_page(self): response_model_exclude_unset=False, response_model_exclude_none=True, methods=["GET"], - endpoint=create_async_endpoint( - self.client.landing_page, EmptyRequest, self.response_class - ), + endpoint=create_async_endpoint(self.client.landing_page, EmptyRequest), ) def register_conformance_classes(self): @@ -153,9 +151,7 @@ def register_conformance_classes(self): response_model_exclude_unset=True, response_model_exclude_none=True, methods=["GET"], - endpoint=create_async_endpoint( - self.client.conformance, EmptyRequest, self.response_class - ), + endpoint=create_async_endpoint(self.client.conformance, EmptyRequest), ) def register_get_item(self): @@ -172,9 +168,7 @@ def register_get_item(self): response_model_exclude_unset=True, response_model_exclude_none=True, methods=["GET"], - endpoint=create_async_endpoint( - self.client.get_item, ItemUri, GeoJSONResponse - ), + endpoint=create_async_endpoint(self.client.get_item, ItemUri), ) def register_post_search(self): @@ -195,7 +189,7 @@ def register_post_search(self): response_model_exclude_none=True, methods=["POST"], endpoint=create_async_endpoint( - self.client.post_search, self.search_post_request_model, GeoJSONResponse + self.client.post_search, self.search_post_request_model ), ) @@ -217,7 +211,7 @@ def register_get_search(self): response_model_exclude_none=True, methods=["GET"], endpoint=create_async_endpoint( - self.client.get_search, self.search_get_request_model, GeoJSONResponse + self.client.get_search, self.search_get_request_model ), ) @@ -237,9 +231,7 @@ def register_get_collections(self): response_model_exclude_unset=True, response_model_exclude_none=True, methods=["GET"], - endpoint=create_async_endpoint( - self.client.all_collections, EmptyRequest, self.response_class - ), + endpoint=create_async_endpoint(self.client.all_collections, EmptyRequest), ) def register_get_collection(self): @@ -256,9 +248,7 @@ def register_get_collection(self): response_model_exclude_unset=True, response_model_exclude_none=True, methods=["GET"], - endpoint=create_async_endpoint( - self.client.get_collection, CollectionUri, self.response_class - ), + endpoint=create_async_endpoint(self.client.get_collection, CollectionUri), ) def register_get_item_collection(self): @@ -287,9 +277,7 @@ def register_get_item_collection(self): response_model_exclude_unset=True, response_model_exclude_none=True, methods=["GET"], - endpoint=create_async_endpoint( - self.client.item_collection, request_model, GeoJSONResponse - ), + endpoint=create_async_endpoint(self.client.item_collection, request_model), ) def register_core(self): diff --git a/stac_fastapi/api/stac_fastapi/api/routes.py b/stac_fastapi/api/stac_fastapi/api/routes.py index f4eb759af..8c6d46b00 100644 --- a/stac_fastapi/api/stac_fastapi/api/routes.py +++ b/stac_fastapi/api/stac_fastapi/api/routes.py @@ -8,18 +8,16 @@ from pydantic import BaseModel from starlette.concurrency import run_in_threadpool from starlette.requests import Request -from starlette.responses import JSONResponse, Response +from starlette.responses import Response from starlette.routing import BaseRoute, Match from starlette.status import HTTP_204_NO_CONTENT from stac_fastapi.api.models import APIRequest -def _wrap_response(resp: Any, response_class: Type[Response]) -> Response: - if isinstance(resp, Response): +def _wrap_response(resp: Any) -> Any: + if resp is not None: return resp - elif resp is not None: - return response_class(resp) else: # None is returned as 204 No Content return Response(status_code=HTTP_204_NO_CONTENT) @@ -37,7 +35,6 @@ async def run(*args, **kwargs): def create_async_endpoint( func: Callable, request_model: Union[Type[APIRequest], Type[BaseModel], Dict], - response_class: Type[Response] = JSONResponse, ): """Wrap a function in a coroutine which may be used to create a FastAPI endpoint. @@ -53,9 +50,7 @@ async def _endpoint( request_data: request_model = Depends(), # type:ignore ): """Endpoint.""" - return _wrap_response( - await func(request=request, **request_data.kwargs()), response_class - ) + return _wrap_response(await func(request=request, **request_data.kwargs())) elif issubclass(request_model, BaseModel): @@ -64,9 +59,7 @@ async def _endpoint( request_data: request_model, # type:ignore ): """Endpoint.""" - return _wrap_response( - await func(request_data, request=request), response_class - ) + return _wrap_response(await func(request_data, request=request)) else: @@ -75,9 +68,7 @@ async def _endpoint( request_data: Dict[str, Any], # type:ignore ): """Endpoint.""" - return _wrap_response( - await func(request_data, request=request), response_class - ) + return _wrap_response(await func(request_data, request=request)) return _endpoint diff --git a/stac_fastapi/api/tests/benchmarks.py b/stac_fastapi/api/tests/benchmarks.py index d672a62de..3a194057d 100644 --- a/stac_fastapi/api/tests/benchmarks.py +++ b/stac_fastapi/api/tests/benchmarks.py @@ -67,7 +67,7 @@ def get_item(self, item_id: str, collection_id: str, **kwargs) -> stac_types.Ite def all_collections(self, **kwargs) -> stac_types.Collections: return stac_types.Collections( - collections=[collections], + collections=collections, links=[ {"href": "test", "rel": "root"}, {"href": "test", "rel": "self"}, @@ -88,29 +88,33 @@ def item_collection( **kwargs, ) -> stac_types.ItemCollection: return stac_types.ItemCollection( - type="FeatureCollection", features=[items[0:limit]] + type="FeatureCollection", + features=items[0:limit], + links=[ + {"href": "test", "rel": "root"}, + {"href": "test", "rel": "self"}, + {"href": "test", "rel": "parent"}, + ], ) @pytest.fixture(autouse=True) def client_validation() -> TestClient: - app = StacApi( - settings=ApiSettings(enable_response_models=True), client=CoreClient() - ) + settings = ApiSettings(enable_response_models=True) + app = StacApi(settings=settings, client=CoreClient()) with TestClient(app.app) as client: yield client @pytest.fixture(autouse=True) def client_no_validation() -> TestClient: - app = StacApi( - settings=ApiSettings(enable_response_models=False), client=CoreClient() - ) + settings = ApiSettings(enable_response_models=False) + app = StacApi(settings=settings, client=CoreClient()) with TestClient(app.app) as client: yield client -@pytest.mark.parametrize("limit", [1, 10, 50, 100, 200, 250]) +@pytest.mark.parametrize("limit", [1, 10, 50, 100, 200, 250, 1000]) @pytest.mark.parametrize("validate", [True, False]) def test_benchmark_items( benchmark, client_validation, client_no_validation, validate, limit From 7cc95dfd0e420718e944358e3136703416fb6ea8 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Mon, 8 Apr 2024 17:13:51 +0200 Subject: [PATCH 5/7] update changelog and add warning --- CHANGES.md | 12 ++++++++++++ stac_fastapi/api/stac_fastapi/api/routes.py | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 16bc9a809..3563a7cbc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,18 @@ ## [Unreleased] +### Changed + +* Make sure FastAPI use Pydantic validatio and serialization by no wrapping endpoint output with a Response object ([#650](https://github.com/stac-utils/stac-fastapi/pull/650)) + +### Removed + +* Deprecate `response_class` option in `stac_fastapi.api.routes.create_async_endpoint` method ([#650](https://github.com/stac-utils/stac-fastapi/pull/650)) + +### Added + +* Add benchmark in CI ([#650](https://github.com/stac-utils/stac-fastapi/pull/650)) + ## [2.4.9] - 2023-11-17 ### Added diff --git a/stac_fastapi/api/stac_fastapi/api/routes.py b/stac_fastapi/api/stac_fastapi/api/routes.py index 8c6d46b00..66b76d2d7 100644 --- a/stac_fastapi/api/stac_fastapi/api/routes.py +++ b/stac_fastapi/api/stac_fastapi/api/routes.py @@ -1,6 +1,8 @@ """Route factories.""" + import functools import inspect +import warnings from typing import Any, Callable, Dict, List, Optional, Type, TypedDict, Union from fastapi import Depends, params @@ -35,11 +37,19 @@ async def run(*args, **kwargs): def create_async_endpoint( func: Callable, request_model: Union[Type[APIRequest], Type[BaseModel], Dict], + response_class: Optional[Type[Response]] = None, ): """Wrap a function in a coroutine which may be used to create a FastAPI endpoint. Synchronous functions are executed asynchronously using a background thread. """ + + if response_class: + warnings.warns( + "`response_class` option is deprecated, please set the Response class directly in the endpoint.", # noqa: E501 + DeprecationWarning, + ) + if not inspect.iscoroutinefunction(func): func = sync_to_async(func) From b0e2d942261d0be203a1b1c8d0d02d5065901561 Mon Sep 17 00:00:00 2001 From: Vincent Sarago Date: Mon, 8 Apr 2024 17:21:41 +0200 Subject: [PATCH 6/7] Update CHANGES.md Co-authored-by: Pete Gadomski --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 3563a7cbc..bc6475fe1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,7 +4,7 @@ ### Changed -* Make sure FastAPI use Pydantic validatio and serialization by no wrapping endpoint output with a Response object ([#650](https://github.com/stac-utils/stac-fastapi/pull/650)) +* Make sure FastAPI uses Pydantic validation and serialization by not wrapping endpoint output with a Response object ([#650](https://github.com/stac-utils/stac-fastapi/pull/650)) ### Removed From 3dc8c762a9dd138d55a85196b2f770ecbac910ba Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Mon, 8 Apr 2024 17:54:39 +0200 Subject: [PATCH 7/7] store benchmark results --- .github/workflows/cicd.yaml | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index 998124185..f86cb6786 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -104,17 +104,17 @@ jobs: - name: Run Benchmark run: python -m pytest stac_fastapi/api/tests/benchmarks.py --benchmark-only --benchmark-columns 'min, max, mean, median' --benchmark-json output.json - # - name: Store and benchmark result - # uses: benchmark-action/github-action-benchmark@v1 - # with: - # name: STAC FastAPI Benchmarks - # tool: 'pytest' - # output-file-path: output.json - # alert-threshold: '130%' - # comment-on-alert: true - # fail-on-alert: false - # # GitHub API token to make a commit comment - # github-token: ${{ secrets.GITHUB_TOKEN }} - # gh-pages-branch: 'gh-benchmarks' - # # Make a commit only if main - # auto-push: ${{ github.ref == 'refs/heads/main' }} + - name: Store and benchmark result + uses: benchmark-action/github-action-benchmark@v1 + with: + name: STAC FastAPI Benchmarks + tool: 'pytest' + output-file-path: output.json + alert-threshold: '130%' + comment-on-alert: true + fail-on-alert: false + # GitHub API token to make a commit comment + github-token: ${{ secrets.GITHUB_TOKEN }} + gh-pages-branch: 'gh-benchmarks' + # Make a commit only if main + auto-push: ${{ github.ref == 'refs/heads/main' }}