-
Notifications
You must be signed in to change notification settings - Fork 8
So long flask, and thanks for all the fish! #526
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
base: develop
Are you sure you want to change the base?
Changes from all commits
14da97d
741f676
e2c4695
1b8f4d4
1c97c53
0b859b9
99cef9c
0b8f228
186bee7
662c938
ec53009
851ea3d
8ca0feb
7096476
5769d07
7c9bd3b
89a96b8
58ed8bc
14ad6fa
09553a3
bcd9d69
412df91
1b8b229
09c0da5
52d9566
fcc884d
e8a0e9c
874452a
b97ccc3
1b8875f
8e312da
36b526c
d7046f2
375a9db
e0e1365
a69a987
378993f
68e9251
1c5c58b
b8cd974
6d3a765
e69f5b1
64c0040
772b1d4
2bf4096
0323ecb
97d7774
f272bd3
33b22fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| Rules | ||
| == | ||
|
|
||
| Pull request reviews MUST be done in a pirate voice. | ||
| When small errors (e.g., typos) are called out, | ||
| the phrase “you made an oopsie” must be included. | ||
|
|
||
|
|
||
| Mention opportunities to use modern language features, remove legacy code or otherwise simplify. | ||
| Legacy abstractions are fancy_property, datasources/backends. | ||
| Focus on correctness and testability. | ||
| We want to use pytest-snapshot and hypothesis where it makes sense. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,17 +25,19 @@ dependencies = [ | |||||
| "factory-boy ~= 3.2.1", | ||||||
| "psycopg2 ~= 2.9.3", | ||||||
| "Flask-QRcode ~= 3.1.0", | ||||||
| "email_validator ~= 1.2.1", | ||||||
| "email-validator~=2.3.0", | ||||||
| "Werkzeug ~= 3.0.1", | ||||||
| "sentry-sdk[flask] ~= 1.39.2", | ||||||
| # Pin to avoid incompatibility | ||||||
| "jinja2 ~= 3.1.2", | ||||||
| "MarkupSafe ~= 2.1.1", | ||||||
| "recurring_ical_events ~= 1.0.2b0", | ||||||
| "recurring_ical_events ~= 3.8.0", | ||||||
|
||||||
| "recurring_ical_events ~= 3.8.0", | |
| "recurring_ical_events ~= 1.0.2b0", |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from importlib.resources import files | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| def get_package_path(suffix: str = "") -> str: | ||
| fs_path = Path(__file__).resolve().parent | ||
| if fs_path.is_dir(): | ||
| return str(fs_path / suffix) | ||
|
|
||
| pkg_path = files("sipa") | ||
| if pkg_path.is_dir(): | ||
| return str(pkg_path / suffix) | ||
|
|
||
| raise AssertionError | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| from sipa.fastapi import create_app | ||
|
|
||
|
|
||
| app = create_app() |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||
| import warnings | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from typing import NamedTuple, cast | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -68,6 +69,11 @@ def init_app(self, app: Flask): | |||||||||||||||||||
|
|
||||||||||||||||||||
| :param app: The flask app object to register against | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| warnings.warn( | ||||||||||||||||||||
| "Use constructor with pre-onfigured datasource instead", | ||||||||||||||||||||
|
||||||||||||||||||||
| "Use constructor with pre-onfigured datasource instead", | |
| "Use constructor with pre-configured datasource instead", |
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arr! Ye made an oopsie in the deprecation warning message - it says "pre-onfigured" instead of "pre-configured" (line 73). A simple typo that needs fixin'!
| "Use constructor with pre-onfigured datasource instead", | |
| "Use constructor with pre-configured datasource instead", |
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahoy! Ye made an oopsie here, matey! The __TODO_API placeholder be undefined and will cause a NameError at runtime. This here be a critical issue that'll sink yer ship when someone tries to fetch a user by IP address. Ye need to pass the actual PycroftApi instance or inject it properly through dependency injection before this code can sail.
| return fetch_by_ip(__TODO_API, ip) | |
| pycroft_api = current_app.extensions.get("pycroft_api") | |
| if pycroft_api is None: | |
| logger.warning( | |
| "PycroftApi extension not configured; returning anonymous user for IP %s", | |
| ip, | |
| ) | |
| return AnonymousUserMixin() | |
| return fetch_by_ip(pycroft_api, ip) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahoy! Ye changed the dependency from
email_validator ~= 1.2.1toemail-validator~=2.3.0(note the underscore to hyphen change and major version bump). This be a breakin' change as email-validator 2.x has a different API than 1.x. Make sure all code usin' email validation has been updated to work with the new version, or this'll cause import errors and broken functionality!