AbstractBaseSession: Use model fields for subclassed cases
#2180
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
tony
force-pushed
the
django-abstract-base-session-fields
branch
from
43caa11 to
a085728
Compare
flaeppe
reviewed
May 24, 2024
tony
force-pushed
the
django-abstract-base-session-fields
branch
2 times, most recently
from
a2c020b to
ead98c8
Compare
intgr
reviewed
May 24, 2024
In situations where these fields are overridden in custom models, for instance extending 'session_key`'s `max_length`. Follow a similar style to `AuthBaseUser`. See also: - https://docs.djangoproject.com/en/5.0/topics/http/sessions/#extending-database-backed-session-engines - https://github.com/django/django/blob/5.0.6/django/contrib/sessions/base_session.py - https://github.com/typeddjango/django-stubs/blob/5.0.0/django-stubs/contrib/sessions/base_session.pyi#L13-L21
Co-authored-by: Marti Raudsepp <[email protected]>
tony
force-pushed
the
django-abstract-base-session-fields
branch
from
3dcaf71 to
6cbe46c
Compare
|
Yeah, do whatever is quickest. The commits will be squashed on merge anyway. |
tony
force-pushed
the
django-abstract-base-session-fields
branch
7 times, most recently
from
b44dcc4 to
da0fad5
Compare
tony
force-pushed
the
django-abstract-base-session-fields
branch
from
da0fad5 to
ae795ce
Compare
Maybe I spoke too soon. @intgr Is this solution fine with you? |
|
I'll merge this in the meantime. Lets circle back if we figure out that there's anything more that needs to be done. |
intgr
reviewed
May 27, 2024
| session_key = models.CharField(primary_key=True) | ||
| session_data = models.TextField() | ||
| expire_date = models.DateTimeField() | ||
| objects: ClassVar[BaseSessionManager[Self]] |
There was a problem hiding this comment.
Yep this is better because it's more precise, thanks.
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.
Changes
AbstractBaseSessionfieldsThese currently raise on mypy when overridden, such as by overriding
max_length, see example in #2179.In situations where these fields are overridden in custom models, for instance extending 'session_key
'smax_length. Follow a similar style toAbstractBaseUser` (django: 5.0.6, django-stubs: 5.0.0).See also:
Related issues
AbstractBaseSessionsubclassing: Fields not overridable #2179