diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index 514e31496..f86cb6786 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -76,3 +76,45 @@ 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 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/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' }} diff --git a/.github/workflows/deploy_mkdocs.yml b/.github/workflows/deploy_mkdocs.yml index b29f0732a..7132fdb6c 100644 --- a/.github/workflows/deploy_mkdocs.yml +++ b/.github/workflows/deploy_mkdocs.yml @@ -8,7 +8,6 @@ on: # Rebuild website when docs have changed or code has changed - "README.md" - "docs/**" - - "mkdocs.yml" - "**.py" workflow_dispatch: @@ -28,8 +27,8 @@ jobs: - name: Install dependencies run: | - pip install --upgrade pip - pip install \ + python -m pip install --upgrade pip + python -m pip install \ stac_fastapi/api[docs] \ stac_fastapi/types[docs] \ stac_fastapi/extensions[docs] \ @@ -37,7 +36,7 @@ jobs: - name: update API docs run: | pdocs as_markdown \ - --output_dir docs/api/ \ + --output_dir docs/src/api/ \ --exclude_source \ --overwrite \ stac_fastapi @@ -51,4 +50,4 @@ jobs: POSTGRES_HOST_WRITER: localhost - name: Deploy docs - run: mkdocs gh-deploy --force + run: mkdocs gh-deploy --force -f docs/mkdocs.yml diff --git a/CHANGES.md b/CHANGES.md index 7892e6efa..e4ca7c0f1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,6 +5,15 @@ ### Changed * Add `items` link to inferred link relations ([#634](https://github.com/stac-utils/stac-fastapi/issues/634)) +* 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 + +* 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 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cadb694e9..c89495c3f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,31 +5,31 @@ Issues and pull requests are more than welcome. **dev install** ```bash -$ git clone https://github.com/stac-utils/stac-fastapi.git -$ cd stac-fastapi -$ pip install -e stac_fastapi/api[dev] +git clone https://github.com/stac-utils/stac-fastapi.git +cd stac-fastapi +python -m pip install -e stac_fastapi/api[dev] ``` -**Python3.8 only** +**pre-commit** -This repo is set to use `pre-commit` to run *ruff*, *pydocstring*, *black* ("uncompromising Python code formatter") and mypy when committing new code. +This repo is set to use `pre-commit` to run *ruff*, *pydocstring* and mypy when committing new code. ```bash -$ pre-commit install +pre-commit install ``` ### Docs ```bash -$ git clone https://github.com/stac-utils/stac-fastapi.git -$ cd stac-fastapi -$ pip install -e stac_fastapi/api["docs"] +git clone https://github.com/stac-utils/stac-fastapi.git +cd stac-fastapi +python pip install -e stac_fastapi/api["docs"] ``` Hot-reloading docs: ```bash -$ mkdocs serve +$ mkdocs serve -f docs/mkdocs.yml ``` To manually deploy docs (note you should never need to do this because GitHub @@ -38,11 +38,11 @@ Actions deploys automatically for new commits.): ```bash Create API documentations $ pdocs as_markdown \ - --output_dir docs/api/ \ + --output_dir docs/src/api/ \ --exclude_source \ --overwrite \ stac_fastapi # deploy -$ mkdocs gh-deploy +$ mkdocs gh-deploy -f docs/mkdocs.yml ``` diff --git a/docs/contributing.md b/docs/contributing.md deleted file mode 120000 index 44fcc6343..000000000 --- a/docs/contributing.md +++ /dev/null @@ -1 +0,0 @@ -../CONTRIBUTING.md \ No newline at end of file diff --git a/docs/index.md b/docs/index.md deleted file mode 120000 index 32d46ee88..000000000 --- a/docs/index.md +++ /dev/null @@ -1 +0,0 @@ -../README.md \ No newline at end of file diff --git a/mkdocs.yml b/docs/mkdocs.yml similarity index 97% rename from mkdocs.yml rename to docs/mkdocs.yml index 60374dc3b..dff2035ca 100644 --- a/mkdocs.yml +++ b/docs/mkdocs.yml @@ -4,7 +4,10 @@ site_description: STAC FastAPI. # Repository repo_name: "stac-utils/stac-fastapi" repo_url: "https://github.com/stac-utils/stac-fastapi" -edit_uri: "blob/master/docs/src/" +edit_uri: "blob/main/docs/src/" + +docs_dir: 'src' +site_dir: 'build' # Social links extra: @@ -15,6 +18,7 @@ extra: # Layout nav: - Home: "index.md" + - Tips and Tricks: tips-and-tricks.md - API: - packages: api/stac_fastapi/index.md - stac_fastapi.api: @@ -70,9 +74,9 @@ nav: - search: api/stac_fastapi/types/search.md - stac: api/stac_fastapi/types/stac.md - version: api/stac_fastapi/types/version.md + - Performance Benchmarks: benchmarks.html - Development - Contributing: "contributing.md" - Release Notes: "release-notes.md" - - Tips and Tricks: tips-and-tricks.md plugins: - search diff --git a/docs/release-notes.md b/docs/release-notes.md deleted file mode 120000 index cf547089d..000000000 --- a/docs/release-notes.md +++ /dev/null @@ -1 +0,0 @@ -../CHANGES.md \ No newline at end of file diff --git a/docs/src/benchmarks.html b/docs/src/benchmarks.html new file mode 100644 index 000000000..f813e09b0 --- /dev/null +++ b/docs/src/benchmarks.html @@ -0,0 +1,292 @@ + + + + + + + Benchmarks + + + + +
+ + + + + + + diff --git a/docs/src/contributing.md b/docs/src/contributing.md new file mode 120000 index 000000000..f939e75f2 --- /dev/null +++ b/docs/src/contributing.md @@ -0,0 +1 @@ +../../CONTRIBUTING.md \ No newline at end of file diff --git a/docs/src/index.md b/docs/src/index.md new file mode 120000 index 000000000..fe8400541 --- /dev/null +++ b/docs/src/index.md @@ -0,0 +1 @@ +../../README.md \ No newline at end of file diff --git a/docs/src/release-notes.md b/docs/src/release-notes.md new file mode 120000 index 000000000..8980b4a7a --- /dev/null +++ b/docs/src/release-notes.md @@ -0,0 +1 @@ +../../CHANGES.md \ No newline at end of file diff --git a/docs/stylesheets/extra.css b/docs/src/stylesheets/extra.css similarity index 100% rename from docs/stylesheets/extra.css rename to docs/src/stylesheets/extra.css diff --git a/docs/tips-and-tricks.md b/docs/src/tips-and-tricks.md similarity index 100% rename from docs/tips-and-tricks.md rename to docs/src/tips-and-tricks.md 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/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..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 @@ -8,18 +10,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,12 +37,19 @@ async def run(*args, **kwargs): def create_async_endpoint( func: Callable, request_model: Union[Type[APIRequest], Type[BaseModel], Dict], - response_class: Type[Response] = JSONResponse, + 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) @@ -53,9 +60,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 +69,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 +78,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 new file mode 100644 index 000000000..ad73d2424 --- /dev/null +++ b/stac_fastapi/api/tests/benchmarks.py @@ -0,0 +1,190 @@ +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], + links=[ + {"href": "test", "rel": "root"}, + {"href": "test", "rel": "self"}, + {"href": "test", "rel": "parent"}, + ], + ) + + +@pytest.fixture(autouse=True) +def client_validation() -> TestClient: + 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: + 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, 1000]) +@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" + benchmark.name = ( + f"Items With Model validation ({limit})" + if validate + else f"Items Limit: ({limit})" + ) + benchmark.fullname = ( + f"Items With Model validation ({limit})" + if validate + else f"Items Limit: ({limit})" + ) + + 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" + benchmark.name = "Collection With Model validation" if validate else "Collection" + benchmark.fullname = ( + "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" + benchmark.name = "Collections With Model validation" if validate else "Collections" + benchmark.fullname = ( + "Collections With Model validation" if validate else "Collections" + ) + + response = benchmark(f) + assert response.status_code == 200