-
Notifications
You must be signed in to change notification settings - Fork 13
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
Django 5 support. #537
Merged
Merged
Django 5 support. #537
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
Note: This relies on django-rest-framework 3.14.0 and not 3.15.1 (which has formal Django 5 support).. This is due to changes in 3.15.* with regards to unique_together in models. For us, that hits Ipaddress when creating a host, leading to, where we get: ```python ERROR django.request:log.py:241 Internal Server Error: /api/v1/hosts/ Traceback (most recent call last): File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner response = get_response(request) ^^^^^^^^^^^^^^^^^^^^^ File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/views/decorators/csrf.py", line 65, in _view_wrapper return view_func(request, *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view return self.dispatch(request, *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/views.py", line 509, in dispatch response = self.handle_exception(exc) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/views.py", line 469, in handle_exception self.raise_uncaught_exception(exc) File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception raise exc File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/views.py", line 506, in dispatch response = handler(request, *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../projects/uio/mreg/mreg/api/v1/views.py", line 354, in post if ipserializer.is_valid(): ^^^^^^^^^^^^^^^^^^^^^^^ File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/serializers.py", line 223, in is_valid self._validated_data = self.run_validation(self.initial_data) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/serializers.py", line 444, in run_validation self.run_validators(value) File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/serializers.py", line 477, in run_validators super().run_validators(to_validate) File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/fields.py", line 553, in run_validators validator(value, self) File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/validators.py", line 169, in __call__ checked_values = [ ^ File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/validators.py", line 172, in <listcomp> if field in self.fields and value != getattr(serializer.instance, field) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 264, in __get__ raise self.RelatedObjectDoesNotExist( mreg.models.host.Ipaddress.host.RelatedObjectDoesNotExist: Ipaddress has no host. ``` I'll create an upstream ticket for this issue.
- Needed for docker unit tests.
pederhan
approved these changes
May 28, 2024
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
pederhan
requested changes
May 29, 2024
- Also check against dict, not QueryDict to retain JSON support! - Oh, and write a better docstring representing reality.
pederhan
approved these changes
May 29, 2024
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.
Note: This relies on django-rest-framework 3.14.0 and not 3.15.1 (which has formal Django 5 support)... Iin 3.15.* there were changes with regards to
unique_together
in models. For us, that hits Ipaddress when creating a host, where we get:The issue comes from this bit of code:
mreg/mreg/api/v1/views.py
Lines 345 to 351 in fa6ca20
unique_together
constraint inIpaddress
:mreg/mreg/models/host.py
Lines 25 to 36 in fa6ca20
Reported as part of encode/django-rest-framework#9358.