Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#31911 closed Bug (invalid)

Django-3.1: get_language() fails to return LANGUAGE_CODE during ./manage.py migrate

Reported by: George Tantiras Owned by: nobody
Component: Internationalization Version: 3.1
Severity: Normal Keywords:
Cc: Claude Paroz, Simon Charette Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When in the project/urls.py another urls.py is included, ./manage.py migrate forces get_language to return None instead of settings.LANGUAGE_CODE.

Only in Django-3.1. More specifically:

This is how get_language is used:

class PendingConsentForm(forms.ModelForm):
    class Meta:
        model = Term
        fields = (
            "summary_{0}".format(get_language()),
        )

Steps to reproduce:

  • Fresh Django-3.1 install
  • pip install --index-url https://test.pypi.org/simple/ django-fail-language
  • Add 'letsagree', in INSTALLED_APPS of project/settings.py
  • Include letsagree.urls in project/urls.py:
urlpatterns = [
   ...
    path('', include('letsagree.urls')),
   ...
]
  • ./manage.py migrate fails with: django.core.exceptions.FieldError: Unknown field(s) (summary_None) specified for Term.
  • ./manage.py makemigrations will correctly return the value of settings.LANGUAGE_CODE:

django.core.exceptions.FieldError: Unknown field(s) (summary_en-us) specified for Term

  • In a virtualenv with Django-3.0 ./manage.py migrate will behave as expected:

django.core.exceptions.FieldError: Unknown field(s) (summary_en-us) specified for Term:
Expected because en-us is the default for settings.LANGUAGE_CODE.

Change History (6)

comment:1 by Carlton Gibson, 4 years ago

Cc: Claude Paroz Simon Charette added

OK, this is a goody.

To be clear the issue is that get_language() returns None in the context of
a migrate command on 3.1, but settings.LANGUAGE_CODE (so en-us) on 3.0.

In the example project this manifests when the result of get_language() is interpolated into a field name on a ModelForm — either way there's an error, it's just what the missing field is called that's at stake.

So, on Django 3.0:

    from letsagree.views import PendingView
  File "/Users/carlton/Desktop/ticket_31911/letsagree/views.py", line 2, in <module>
    from letsagree.forms import TestView
  File "/Users/carlton/Desktop/ticket_31911/letsagree/forms.py", line 6, in <module>
    class PendingConsentForm(forms.ModelForm):
  File "/Users/carlton/Documents/Django-Stack/django/django/forms/models.py", line 267, in __new__
    raise FieldError(message)
django.core.exceptions.FieldError: Unknown field(s) (summary_en-us) specified for Term

vs Django 3.1:

    from letsagree.views import PendingView
  File "/Users/carlton/Desktop/ticket_31911/letsagree/views.py", line 2, in <module>
    from letsagree.forms import TestView
  File "/Users/carlton/Desktop/ticket_31911/letsagree/forms.py", line 6, in <module>
    class PendingConsentForm(forms.ModelForm):
  File "/Users/carlton/Documents/Django-Stack/django/django/forms/models.py", line 267, in __new__
    raise FieldError(message)
django.core.exceptions.FieldError: Unknown field(s) (summary_None) specified for Term

Other commands, e.g. check do not trigger this difference. That is because
migrate runs with the @no_translations decorator applied.

In this context, it looks like the 3.1 behaviour is correct:

get_language(): Returns the currently selected language code. Returns None if translations are temporarily deactivated (by deactivate_all() or when None is passed to override()).

Reading that, I'm expecting None here when @no_translations is in play.

The change in behaviour occurs in Simon's 0b83c8cc4db95812f1e15ca19d78614e94cf38dd.

From the change there to migrate.py, it looks like it must have something to do with:

This also removes unnecessary BaseCommand._run_checks()` hook.

(But I didn't yet experiment reverting that bit.)

@no_translations was introduced by Claude in d65b0f72de8d35617fe0554ddabc950c7f323eef (for Django 2.1).

Interestly, the behaviour before that commit (during migrate) was for
get_language() to return None. With d65b0f72 it changes to returning
LANGUAGE_CODE. Then with 0b83c8cc it went back to returning None.

So we need to decide what to say...

I'm inclined to think that using get_language() at import time (during start-up) shouldn't be supported, since we're not in an active language context, but happy to take guidance on that.

comment:2 by George Tantiras, 4 years ago

The example project resulted after stripping down django-letsagree which uses get_language() (through `django-translated-fields`) in the ModelForm to allow only certain fields to appear in the relevant admin page.

As a user who has set USE_I18N = True, I would expect of get_language() to at least return the default language, as per the "notion" I get from the docs of `LANGUAGE_CODE`:

It serves two purposes:
*If the locale middleware isn’t in use, it decides which translation is served to all users.
*If the locale middleware is active, it provides a fallback language in case the user’s preferred language can’t be determined or is not supported by the website. It also provides the fallback translation when a translation for a given literal doesn’t exist for the user’s preferred language.

"Notion": @no_translations ~= "the locale middleware isn’t in use"

Last edited 4 years ago by George Tantiras (previous) (diff)

comment:3 by Simon Charette, 4 years ago

Thanks for the detailed investigation Carlton.

Interestingly, the behaviour before that commit (during migrate) was for get_language() to return None. With d65b0f72de8d35617fe0554ddabc950c7f323eef it changes to returning LANGUAGE_CODE. Then with 0b83c8cc it went back to returning None.

I'm inclined to think that using get_language() at import time (during start-up) shouldn't be supported, since we're not in an active language context, but happy to take guidance on that.

I tend to agree here. If it always returned None before d65b0f72de8d35617fe0554ddabc950c7f323eef and wasn't tested to return LANGUAGE_CODE we could maybe accept a new feature request to make it default to LANGUAGE_CODE but we can hardly define it as a regression.

Version 0, edited 4 years ago by Simon Charette (next)

comment:4 by Claude Paroz, 4 years ago

I didn't dig deep, but calling get_language() at import time sounds wrong in the first place.

comment:5 by Carlton Gibson, 4 years ago

Resolution: invalid
Status: newclosed

OK, thanks Simon and Claude for the comments. I’m going to close on that basis as expected behaviour.

George, alas you’re caught in the unfortunate position of using a bug.

The example project resulted after stripping down ​django-letsagree which uses get_language() (through ​django-translated-fields) in the ModelForm to allow only certain fields to appear in the relevant admin page.

So, if it’s at import time, as per the example project (which triggers the error once the urls loading the views loading the form are included) then there’s only one possible value you’re looking for, which is settings.LANGUAGE_CODE — just use that. (If the form is defined dynamically, you should already be OK... — using modelform_factory() would be a way to defer the form creation though.)

comment:6 by George Tantiras, 4 years ago

:-D That's life! Django Rocks, nonetheless.

using modelform_factory() would be a way to defer the form creation though

Thank you, I have not thought of that, it is for sure that I shall give it a try.

Note: See TracTickets for help on using tickets.
Back to Top