Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set root-path from uvicorn #221

Merged
merged 2 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ services:
- azurite
- redis
command: >
bash -c "pypgstac pgready && uvicorn pcstac.main:app --host 0.0.0.0 --port 8081 --reload --proxy-headers"
bash -c "pypgstac pgready && uvicorn pcstac.main:app --host 0.0.0.0 --port 8081 --reload --proxy-headers --root-path '/stac'"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally, this is the actual fix to the issue.


tiler:
image: pc-apis-tiler
Expand Down
6 changes: 5 additions & 1 deletion pcstac/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,8 @@ RUN --mount=type=cache,target=/root/.cache \
ENV APP_HOST=0.0.0.0
ENV APP_PORT=81

CMD uvicorn pcstac.main:app --host ${APP_HOST} --port ${APP_PORT} --log-level info
# This value should match that which is used as the root_path in FastAPI, which
# is typically set via the APP_ROOT_PATH environment variable.
ENV APP_ROOT_PATH=""

CMD uvicorn pcstac.main:app --host ${APP_HOST} --port ${APP_PORT} --root-path ${APP_ROOT_PATH} --log-level info
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, this fixes the issue in the deployed environment. APP_ROOT_PATH is already set in the Helm chart.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirming the behavior here: The "default" APP_ROOT_PATH is set to "", but if the Helm chart defines it it'll override that?

And is the default of "" is correct? Not "/"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, I didn't want to give it a default value. Locally and on PCT, it's /stac and in PC its /api/stac/v1. The value is always set by either docker-compose or helm. I believe you need to declare the ENV in order for it be available when the container is run.

3 changes: 1 addition & 2 deletions pcstac/Dockerfile.dev
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
FROM pc-apis-stac


RUN --mount=type=cache,target=/root/.cache \
--mount=type=bind,source=requirements-dev.txt,target=requirements-dev.txt \
pip install -r requirements-dev.txt

RUN --mount=type=cache,target=/root/.cache \
pip install -e ./pccommon[dev] -e ./pcstac
pip install --no-deps -e ./pccommon[dev] -e ./pcstac
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensures that pcstac dependencies aren't overwritten by pccommon

10 changes: 4 additions & 6 deletions pcstac/requirements-server.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
anyio==3.7.1
# via
# starlette
# watchgod
asgiref==3.8.1
# via uvicorn
# watchfiles
async-timeout==4.0.3
# via asyncpg
asyncpg==0.29.0
Expand Down Expand Up @@ -147,22 +145,22 @@ termcolor==2.4.0
# via fire
typing-extensions==4.10.0
# via
# asgiref
# fastapi
# psycopg
# psycopg-pool
# pydantic
# pygeoif
# starlette
# uvicorn
tzlocal==5.2
# via dateparser
uvicorn[standard]==0.17.6
uvicorn[standard]==0.30.1
# via pcstac (pcstac/setup.py)
uvloop==0.19.0
# via uvicorn
version-parser==1.0.1
# via pypgstac
watchgod==0.8.2
watchfiles==0.22.0
# via uvicorn
websockets==12.0
# via uvicorn
2 changes: 1 addition & 1 deletion pcstac/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

extra_reqs = {
"server": [
"uvicorn[standard]>=0.17.0,<0.18.0",
"uvicorn[standard]==0.30.1",
],
}

Expand Down
4 changes: 2 additions & 2 deletions pcstac/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def api_client(pqe_pg):
post_request_model=search_post_request_model,
),
extensions=EXTENSIONS,
app=FastAPI(default_response_class=ORJSONResponse),
app=FastAPI(root_path="/stac", default_response_class=ORJSONResponse),
search_get_request_model=search_get_request_model,
search_post_request_model=search_post_request_model,
)
Expand All @@ -102,7 +102,7 @@ async def app(api_client) -> AsyncGenerator[FastAPI, None]:
@pytest.fixture(scope="session")
async def app_client(app) -> AsyncGenerator[AsyncClient, None]:
async with AsyncClient(
app=app, base_url="http://test", headers={"X-Forwarded-For": "127.0.0.1"}
app=app, base_url="http://test/stac", headers={"X-Forwarded-For": "127.0.0.1"}
) as c:
yield c

Expand Down
4 changes: 3 additions & 1 deletion pcstac/tests/resources/test_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,17 +390,19 @@ async def test_pagination_item_collection(app_client):
idx = 0
item_ids = []
nextThing = None

while True:
idx += 1
page_data = page.json()
item_ids.append(page_data["features"][0]["id"])
print(idx, item_ids)
nextlink = [
link["href"] for link in page_data["links"] if link["rel"] == "next"
]
if len(nextlink) < 1:
break
nextThing = nextlink.pop()
assert nextThing.startswith("http://test/stac/collections")

page = await app_client.get(nextThing)
if idx >= 20:
assert False
Expand Down
6 changes: 3 additions & 3 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
black==22.3.0
black==24.4.2
flake8==3.8.4
isort==5.9.2
mypy==1.8.0
openapi-spec-validator==0.3.0
cachetools<=4.2.
pytest==7.*
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a dev dependency and conflicted with installed dependencies on server images.

pytest-asyncio==0.18.*
httpx>=0.22.0
json-schema<4.18.0 # https://github.com/stac-utils/pystac/issues/1186

pip-tools
# Mypy types
types-python-dateutil
types-cachetools
types-python-dateutil
types-redis
2 changes: 1 addition & 1 deletion scripts/generate-requirements
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ docker-compose \
-f docker-compose.dev.yml \
run --rm \
tiler-dev \
pip-compile ./pccommon/setup.py --extra server -o pccommon/requirements.txt $pip_compile_options
pip-compile ./pccommon/setup.py --extra server -o pccommon/requirements.txt $pip_compile_options
Loading