chore: address deprecations caused by updating packages #570
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
locking until further action due to psf/black#4175 most likely course of action is to get rid of reorder-python-imports
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
caused by updating akatsuki-pp-py version
NiceAesth
requested review from
cmyui,
skrungly,
tsunyoku and
7mochi
as code owners
NiceAesth
commented
Feb 9, 2024
| Ansi.LMAGENTA, | ||
| ) | ||
|
|
||
| yield # type: ignore |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
just want to make sure, is this a 1:1 copy other than the yield?
NiceAesth
commented
Feb 9, 2024
| @@ -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" | |||
There was a problem hiding this comment.
updated value due to changed pp calcs
There was a problem hiding this comment.
we should prolly use snapshot tests for tests like these
There was a problem hiding this comment.
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
NiceAesth
commented
Feb 9, 2024
| @@ -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() | |||
There was a problem hiding this comment.
close() is deprecated as of 5.0.1
1 task
cmyui
approved these changes
Feb 12, 2024
|
+1 👍🏻 |
cmyui
added
dependencies
cmyui
reviewed
Feb 16, 2024
| @@ -64,6 +67,57 @@ def openapi(self) -> dict[str, Any]: | |||
| return self.openapi_schema | |||
|
|
|||
|
|
|||
| @asynccontextmanager | |||
| async def lifespan(asgi_app: BanchoAPI) -> AsyncIterator[Never]: | |||
There was a problem hiding this comment.
should this be AsyncIterator[None]?
There was a problem hiding this comment.
i remember trying that and it failing mypy
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
RFC. This is the minimal required to get it into a working state. See: https://fastapi.tiangolo.com/advanced/events/#lifespan
Breaking changes spotted:
As a side effect, we now should technically support Python 3.12
Other things of note:
We may support Pytest 8 soon once pytest-asyncio v23.5 is out of alpha. pytest-dev/pytest-asyncio#737 (comment)done in #571Related Issues / Projects
Depends on #568
Checklist