-
Notifications
You must be signed in to change notification settings - Fork 165
Upgrade to django 6.0 #2588
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: main
Are you sure you want to change the base?
Upgrade to django 6.0 #2588
Conversation
| "Django~=6.0.0", | ||
| "mozilla-django-oidc~=4.0.1", | ||
| "openpyxl~=3.1.5", | ||
| "psycopg~=3.2.3", |
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.
This was actually required. model-bakery and selenium I updated only because I thought it would help, but it didn't in the end, but I thought I'd leave it in, I believe python 3.14 would require the new selenium anyways.
| parser.add_argument("wheels", type=Path, nargs="*") | ||
| try: | ||
| args = parser.parse_args(argv) | ||
| except SystemExit: |
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.
I wasn't exactly sure about this. exit_on_error=False seemed to suggest that a system exit was unwanted, but the test test_usage suggested the exit behavior was wanted (only with exit_on_error the usage message is printed). Python 3.13 fixed a bug where exit_on_error=False wasn't respected, so now the test failed. I opted for using the test as expected behavior. @niklasmohrin?
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.
Hm, I don't fully remember why I did it like this, I think it was because I didn't want to catch SystemExit in the test. Nowadays I don't really care
# Conflicts: # uv.lock
niklasmohrin
left a comment
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.
Agree on waiting for 6.0.1
| EmailT = TypeVar("EmailT", str, None) | ||
| def clean_email(email: str | None) -> str | None: | ||
| if email is None: | ||
| return None | ||
|
|
||
|
|
||
| def clean_email(email: EmailT) -> EmailT: |
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.
The previous bound actually was tighter: EmailT is constrained to be either exactly str or exactly None, and the same type is returned; with the new signature we also permit str -> None and None -> str
I think we use the str->str guarantee in some auth code
| parser.add_argument("wheels", type=Path, nargs="*") | ||
| try: | ||
| args = parser.parse_args(argv) | ||
| except SystemExit: |
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.
Hm, I don't fully remember why I did it like this, I think it was because I didn't want to catch SystemExit in the test. Nowadays I don't really care
richardebeling
left a comment
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.
lgtm when the 6.0.1 fix is in and the open discussions are resolved. Thanks!
| [dependency-groups] | ||
| dev = [ | ||
| "coverage[toml]~=7.10.2", | ||
| "coverage~=7.10.2", |
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.
Out of curiosity: How did you notice this, did some tooling tell you?
| M = TypeVar("M", bound=Model) | ||
| T = TypeVar("T") | ||
| CellValue = str | int | float | None | ||
| CV = TypeVar("CV", bound=CellValue) |
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.
any reason why we're keeping T and CV? T seems to be unused and the usages of CV can also be written using the new syntax
|
Very nice to have this! 👍 |
fixes #2537
Includes an update to python 3.13. Does not update to python 3.14, since that threw errors during installing. Something about setuptools in cffi (from mozilla-django-oidc -> josepy -> cryptography -> cffi), hopefully is solved with the ext mozilla-django-oidc release.
Includes a workaround for https://code.djangoproject.com/ticket/36800. It has been labeled as release blocker, so it should be in django 6.0.1, I'd suggest to wait for that.
I looked into the template partials, even gathered a statistic about our template includes, but even for files that were included multiple times but only by a single other file, I thought having it in a separate file was actually nicer.