Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7k
URLField serializer is not playing nice with "built in" test server address http://testserver#9705
-
Initially encountered in our
with such minimal reproducer depicting the issue: fromrest_frameworkimportserializersclassMySerializer(serializers.Serializer):url=serializers.URLField()deftest_valid_url():serializer=MySerializer(data={'url':'http://testserver/path'})serializer.is_valid() running pytest on which would give you an ugly excepted demanding configuration since i18n would not yet be configured to give a proper localized error... but replacing that simple hostname
So
|
BetaWas this translation helpful?Give feedback.
All reactions
- if I got it right such validation stems from DJANGO itself, so should it be checked/fixed there?
It does. I was able to reproduce the problem with a simpler test case:
fromdjango.core.validatorsimportURLValidatordeftest_url_validator():validator=URLValidator()assertvalidator("http://testserver/path")isNone# fails
- any way to workaround?
Change the base URL to something with a TLD? I usually have a constant in my settings to be able to switch it between test/dev/staging/prod. Django has the standardMEDIA_URL
for media file, you can make up your own to fit your use case. Thistestserver
doesn't come from DRF either, it's vanilla Django too.
Not much specific t…
Replies: 2 comments 3 replies
-
you should have open a discussion first |
BetaWas this translation helpful?Give feedback.
All reactions
-
Converted to discussion |
BetaWas this translation helpful?Give feedback.
All reactions
👍 1
-
thanks for the quick handling! whatever makes us unstuck faster is great. |
BetaWas this translation helpful?Give feedback.
All reactions
-
It does. I was able to reproduce the problem with a simpler test case: fromdjango.core.validatorsimportURLValidatordeftest_url_validator():validator=URLValidator()assertvalidator("http://testserver/path")isNone# fails
Change the base URL to something with a TLD? I usually have a constant in my settings to be able to switch it between test/dev/staging/prod. Django has the standard Not much specific to DRF, basically, it's all Django. |
BetaWas this translation helpful?Give feedback.
All reactions
-
Thanks! Indeed works if we provide custom name in Here is a complete patch which I worked outcommit 3a348e80f0fb2c46da90556884fe688cbf618db9Author: Yaroslav O. Halchenko <debian@onerussian.com>Date: Thu May 22 09:52:33 2025 -0400 Provide custom name for django testserver instance The default name django uses is "testserver" and then URL becomes http://testserver, but DJANGO itself does not consider it to be a valid URL because it lacks TLD. So whenever we add "schema_url" pointing to our server (in tests just "testserver") DJANGO starts raising validation errors. See https://github.com/encode/django-rest-framework/discussions/9705 for more information/rationale.diff --git a/dandiapi/api/tests/test_info.py b/dandiapi/api/tests/test_info.pyindex 3ab461ba..22e849ec 100644--- a/dandiapi/api/tests/test_info.py+++ b/dandiapi/api/tests/test_info.py@@ -4,6 +4,7 @@ from django.conf import settings from django.urls import reverse from dandiapi import __version__+from dandiapi.conftest import TEST_SERVER_NAME def test_rest_info(api_client):@@ -11,7 +12,7 @@ def test_rest_info(api_client): assert resp.status_code == 200 # Get the expected schema URL- schema_url = f'http://testserver{reverse("schema-dandiset-latest")}'+ schema_url = f'http://{TEST_SERVER_NAME}{reverse("schema-dandiset-latest")}' assert resp.json() == { 'schema_version': settings.DANDI_SCHEMA_VERSION,diff --git a/dandiapi/conftest.py b/dandiapi/conftest.pyindex 332b8640..74a79bbe 100644--- a/dandiapi/conftest.py+++ b/dandiapi/conftest.py@@ -49,6 +49,9 @@ register(UploadFactory) register(ZarrArchiveFactory) register(EmbargoedZarrArchiveFactory, _name='embargoed_zarr_archive')+# Should have TLD since otherwise would invalidate the URL+TEST_SERVER_NAME = 'testserver.dandiarchive.org'+ # Register zarr file/directory factories @pytest.fixture@@ -77,14 +80,21 @@ def version(request): return request.param()+def _api_client() -> APIClient:+ """Get a new client for a fixture."""+ if TEST_SERVER_NAME not in settings.ALLOWED_HOSTS:+ settings.ALLOWED_HOSTS.append(TEST_SERVER_NAME)+ return APIClient(SERVER_NAME=TEST_SERVER_NAME)++ @pytest.fixture def api_client() -> APIClient:- return APIClient()+ return _api_client() @pytest.fixture def authenticated_api_client(user) -> APIClient:- client = APIClient()+ client = _api_client() client.force_authenticate(user=user) return client |
BetaWas this translation helpful?Give feedback.
All reactions
This discussion was converted from issue #9704 on May 21, 2025 07:26.