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.

Sign up for GitHub

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

chore: address deprecations caused by updating packages #570

Merged
merged 11 commits into from
Feb 12, 2024
Merged

chore: address deprecations caused by updating packages #570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
abf43c9
chore: knock some sense into dependencies
NiceAesth Feb 9, 2024
5854bde
chore: lock black version
NiceAesth Feb 9, 2024
8a5de64
[CI] Generated new Pipfile.lock
Feb 9, 2024
e96da4b
chore: address deprecations in package updates
NiceAesth Feb 9, 2024
0a90aa9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 9, 2024
4808bb8
fix: remove missing call to `init_events`
NiceAesth Feb 9, 2024
591950c
Merge branch 'master' into deprecations
NiceAesth Feb 9, 2024
7393e2d
fix: type lifespan manager
NiceAesth Feb 9, 2024
afde0ab
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 9, 2024
c42bf81
fix: update pp value in test
NiceAesth Feb 9, 2024
3b9d354
Merge branch 'master' into deprecations
NiceAesth Feb 10, 2024
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
112 changes: 55 additions & 57 deletions app/api/init_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
import asyncio
import os
import pprint
from collections.abc import AsyncIterator
from contextlib import asynccontextmanager
from typing import Any
from typing import Never

import starlette.routing
from fastapi import FastAPI
Expand Down Expand Up @@ -64,6 +67,57 @@ def openapi(self) -> dict[str, Any]:
return self.openapi_schema


@asynccontextmanager
async def lifespan(asgi_app: BanchoAPI) -> AsyncIterator[Never]:
Copy link
Member

Choose a reason for hiding this comment

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

should this be AsyncIterator[None]?

Copy link
Member Author

Choose a reason for hiding this comment

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

i remember trying that and it failing mypy

app.state.loop = asyncio.get_running_loop()
if app.utils.is_running_as_admin():
log(
"Running the server with root privileges is not recommended.",
Ansi.LRED,
)

await app.state.services.database.connect()
await app.state.services.redis.initialize()

if app.state.services.datadog is not None:
app.state.services.datadog.start(
flush_in_thread=True,
flush_interval=15,
)
app.state.services.datadog.gauge("bancho.online_players", 0)

app.state.services.ip_resolver = app.state.services.IPResolver()

await app.state.services.run_sql_migrations()

async with app.state.services.database.connection() as db_conn:
await collections.initialize_ram_caches(db_conn)

await app.bg_loops.initialize_housekeeping_tasks()

log("Startup process complete.", Ansi.LGREEN)
log(
f"Listening @ {app.settings.APP_HOST}:{app.settings.APP_PORT}",
Ansi.LMAGENTA,
)

yield # type: ignore
Copy link
Member Author

@NiceAesth NiceAesth Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

mypy will scream about not returning anything with yield without this but the typedef expects Never

we could also yield a dictionary e.g.

    yield {
        "db": db,
        "redis": redis,
        "crypto_model": crypto_model,
        "osu_service": osu_service,
        "stats_service": stats_service,
    }

and then it would be available under request.state instead of request.app.state

Copy link
Member

Choose a reason for hiding this comment

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

just want to make sure, is this a 1:1 copy other than the yield?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep


# we want to attempt to gracefully finish any ongoing connections
# and shut down any of the housekeeping tasks running in the background.
await app.state.sessions.cancel_housekeeping_tasks()

# shutdown services

await app.state.services.http_client.aclose()
await app.state.services.database.disconnect()
await app.state.services.redis.aclose()

if app.state.services.datadog is not None:
app.state.services.datadog.stop()
app.state.services.datadog.flush()


def init_exception_handlers(asgi_app: BanchoAPI) -> None:
@asgi_app.exception_handler(RequestValidationError)
async def handle_validation_error(
Expand Down Expand Up @@ -110,61 +164,6 @@ async def http_middleware(
raise exc


def init_events(asgi_app: BanchoAPI) -> None:
"""Initialize our app's event handlers."""

@asgi_app.on_event("startup")
async def on_startup() -> None:
app.state.loop = asyncio.get_running_loop()

if app.utils.is_running_as_admin():
log(
"Running the server with root privileges is not recommended.",
Ansi.LRED,
)

await app.state.services.database.connect()
await app.state.services.redis.initialize()

if app.state.services.datadog is not None:
app.state.services.datadog.start(
flush_in_thread=True,
flush_interval=15,
)
app.state.services.datadog.gauge("bancho.online_players", 0)

app.state.services.ip_resolver = app.state.services.IPResolver()

await app.state.services.run_sql_migrations()

async with app.state.services.database.connection() as db_conn:
await collections.initialize_ram_caches(db_conn)

await app.bg_loops.initialize_housekeeping_tasks()

log("Startup process complete.", Ansi.LGREEN)
log(
f"Listening @ {app.settings.APP_HOST}:{app.settings.APP_PORT}",
Ansi.LMAGENTA,
)

@asgi_app.on_event("shutdown")
async def on_shutdown() -> None:
# we want to attempt to gracefully finish any ongoing connections
# and shut down any of the housekeeping tasks running in the background.
await app.state.sessions.cancel_housekeeping_tasks()

# shutdown services

await app.state.services.http_client.aclose()
await app.state.services.database.disconnect()
await app.state.services.redis.close()

if app.state.services.datadog is not None:
app.state.services.datadog.stop()
app.state.services.datadog.flush()


def init_routes(asgi_app: BanchoAPI) -> None:
"""Initialize our app's route endpoints."""
for domain in ("ppy.sh", app.settings.DOMAIN):
Expand All @@ -180,11 +179,10 @@ def init_routes(asgi_app: BanchoAPI) -> None:

def init_api() -> BanchoAPI:
"""Create & initialize our app."""
asgi_app = BanchoAPI()
asgi_app = BanchoAPI(lifespan=lifespan)

init_middlewares(asgi_app)
init_exception_handlers(asgi_app)
init_events(asgi_app)
init_routes(asgi_app)

return asgi_app
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/domains/osu_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,5 +239,5 @@ async def test_score_submission(
assert response.status_code == status.HTTP_200_OK
assert (
response.read()
== b"beatmapId:315|beatmapSetId:141|beatmapPlaycount:1|beatmapPasscount:1|approvedDate:2014-05-18 15:41:48|\n|chartId:beatmap|chartUrl:https://osu.cmyui.xyz/s/141|chartName:Beatmap Ranking|rankBefore:|rankAfter:1|rankedScoreBefore:|rankedScoreAfter:26810|totalScoreBefore:|totalScoreAfter:26810|maxComboBefore:|maxComboAfter:52|accuracyBefore:|accuracyAfter:81.94|ppBefore:|ppAfter:9.56523|onlineScoreId:1|\n|chartId:overall|chartUrl:https://cmyui.xyz/u/3|chartName:Overall Ranking|rankBefore:|rankAfter:1|rankedScoreBefore:|rankedScoreAfter:26810|totalScoreBefore:|totalScoreAfter:26810|maxComboBefore:|maxComboAfter:52|accuracyBefore:|accuracyAfter:81.94|ppBefore:|ppAfter:10|achievements-new:osu-skill-pass-4+Insanity Approaches+You're not twitching, you're just ready./all-intro-hidden+Blindsight+I can see just perfectly"
== b"beatmapId:315|beatmapSetId:141|beatmapPlaycount:1|beatmapPasscount:1|approvedDate:2014-05-18 15:41:48|\n|chartId:beatmap|chartUrl:https://osu.cmyui.xyz/s/141|chartName:Beatmap Ranking|rankBefore:|rankAfter:1|rankedScoreBefore:|rankedScoreAfter:26810|totalScoreBefore:|totalScoreAfter:26810|maxComboBefore:|maxComboAfter:52|accuracyBefore:|accuracyAfter:81.94|ppBefore:|ppAfter:10.04103|onlineScoreId:1|\n|chartId:overall|chartUrl:https://cmyui.xyz/u/3|chartName:Overall Ranking|rankBefore:|rankAfter:1|rankedScoreBefore:|rankedScoreAfter:26810|totalScoreBefore:|totalScoreAfter:26810|maxComboBefore:|maxComboAfter:52|accuracyBefore:|accuracyAfter:81.94|ppBefore:|ppAfter:10|achievements-new:osu-skill-pass-4+Insanity Approaches+You're not twitching, you're just ready./all-intro-hidden+Blindsight+I can see just perfectly"
Copy link
Member Author

Choose a reason for hiding this comment

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

updated value due to changed pp calcs

Copy link
Member

@cmyui cmyui Feb 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

we should prolly use snapshot tests for tests like these

Copy link
Member

@cmyui cmyui Feb 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

also the test suite runs on the docker-compose db rn which is super weird - we should be spinning up a temp db & running migrations every time we run tests

)
2 changes: 1 addition & 1 deletion tools/recalc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ async def main(argv: Sequence[str] | None = None) -> int:

await app.state.services.http_client.aclose()
await db.disconnect()
await redis.close()
await redis.aclose()
Copy link
Member Author

Choose a reason for hiding this comment

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

close() is deprecated as of 5.0.1


return 0

Expand Down
Loading