Skip to content
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 1.8 get_language returns None #90

Closed
CptLemming opened this issue Jun 5, 2015 · 15 comments
Closed

Django 1.8 get_language returns None #90

CptLemming opened this issue Jun 5, 2015 · 15 comments
Assignees

Comments

@CptLemming
Copy link

Django 1.8 changed the default behaviour of 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()).

Changed in Django 1.8:
Before Django 1.8, get_language() always returned LANGUAGE_CODE when translations were deactivated.

Now, when fetching objects without specifying the language language_code is no longer populated:

print Colour.objects.language('en-gb').all()[0].language_code
>>> 'en-gb'
print Colour.objects.all()[0].language_code
>>> None

And I cannot create objects on the shell:

c = Colour.objects.create(name='Black', slug='black')
>>> IntegrityError: (1048, "Column 'language_code' cannot be null")
@vdboor
Copy link
Contributor

vdboor commented Jun 18, 2015

Yikes! That clearly has some implications. I wonder whether we should take care of this though - after all because Django wants to be explicit about setting a language first before doing I18N calls. I think bailing out with an error would be better then trying do insert LANGUAGE_CODE as fallback.

Note you can use the following to create objects on the shell:

c = Colour.objects.language('en-gb').create(name='Black', slug='black')

(I've seen people writing such code expecting it to work, so this API was added).

@AdrianLC
Copy link

Hello,

This is causing a crash for us when using safe_translation_getter('field', any_language=True).

     58         return "({}) {}".format(
     59             self.pk,
---> 60             self.safe_translation_getter('name', any_language=True),
     61         )
     62 

/home/alopez/.virtualenvs/project/lib/python3.4/site-packages/parler/models.py in safe_translation_getter(self, field, default, language_code, any_language)
    702             # which also attempts the fallback language if configured to do so.
    703             try:
--> 704                 return getattr(self, field)
    705             except TranslationDoesNotExist:
    706                 pass

/home/alopez/.virtualenvs/project/lib/python3.4/site-packages/parler/fields.py in __get__(self, instance, instance_type)
     88         meta = self.field.meta
     89         try:
---> 90             translation = instance._get_translated_model(use_fallback=True, meta=meta)
     91         except meta.model.DoesNotExist as e:
     92             if self.field.any_language:

/home/alopez/.virtualenvs/project/lib/python3.4/site-packages/parler/models.py in _get_translated_model(self, language_code, use_fallback, auto_create, meta)
    470         # 4. Fallback?
    471         fallback_msg = None
--> 472         lang_dict = get_language_settings(language_code)
    473 
    474         if language_code not in local_cache:

/home/alopez/.virtualenvs/project/lib/python3.4/site-packages/parler/utils/i18n.py in get_language_settings(language_code, site_id)
     72     # to have their own variation of the settings with this method functionality included.
     73     from parler import appsettings
---> 74     return appsettings.PARLER_LANGUAGES.get_language(language_code, site_id)
     75 
     76 

/home/alopez/.virtualenvs/project/lib/python3.4/site-packages/parler/utils/conf.py in get_language(self, language_code, site_id)
    112         # no language match, search for variant: fr-ca falls back to fr
    113         for lang_dict in self.get(site_id, ()):
--> 114             if lang_dict['code'].split('-')[0] == language_code.split('-')[0]:
    115                 return lang_dict
    116 

AttributeError: 'NoneType' object has no attribute 'split'

The instance in question has a single translation but translations are not active (in the shell in this case), the field does not have any_language=True and both LANGUAGE_CODE and PARLER_DEFAULT_LANGUAGE_CODE are set to 'en'.

The error happens in the call to _get_translated_model through TranslatedFieldDescriptor. At some point calls get_language_settings(None) because self._current_language is None. So it doesn't reach the condition with any_language.

@AdrianLC
Copy link

I would also like to add that I'm doing extensive use of parler from outside the request-response cycle (i.e. for automatic translations in background -- celery), so I'd have to hack parler quite hard if _current_language is left at None :(

I suggest we write a wrapper like people at modeltranslations has done: https://github.com/deschler/django-modeltranslation/blob/master/modeltranslation/utils.py#L13 . This way it could even be configurable with a new settings like PARLER_OFFLINE_TRANSLATIONS if we still want to match the new Django behaviour.

@vstoykov
Copy link
Contributor

vstoykov commented Apr 1, 2016

For cases where parler is used outside of request-response cycle (management commands, celery tasks) did you guys @AdrianLC, @CptLemming are activating translation system with translation.activate?

If you are not activating a translation then this is intended behavior from Django 1.8, and you need to activate some language (and probably you need to correct your code to activate the right language based on other criteria).

If this is not the case then probably something in django-parler need to be fixed.

@vdboor
Copy link
Contributor

vdboor commented Apr 7, 2016

For the next release, I've added a warning in 8bcc2ae + 0bffe14 that reminds you to use translation.activate(). Right now that is the best solution, as I really can't tell what else breaks when we support "magical" default languages; since you end up with code that partially uses i18n, and partially not. I'm favoring explicitness here t

@vdboor vdboor self-assigned this Apr 7, 2016
@vstoykov
Copy link
Contributor

@vdboor Now when I'm looking more deeply into this I think that one thing can be changed in django-parler. Even if translations are not activated when user is using safe_translation_getter with any_language=True to have a fallback.

I think that this is the reason for any_language in the first place. You want to show something. Now I need to use:

def __str__(self):
    return self.safe_translation_getter('title', any_language=True,
                                        language_code=get_language() or settings.LANGUAGE_CODE)

Which is kind of overhead for every model's __str__ method.

If you use instance.title let's raise an error, but when you are using instance.safe_translation_getter('title', any_language=True) you are explicit enough that you want fallback in any case.

What you think about that?

@vdboor
Copy link
Contributor

vdboor commented May 4, 2016

With respect to any_language, you have a good point that it could prefer the LANGUAGE_CODE. It does require some changes deep in the code to fix that.

I wonder, you mentioned that you need to use language_code=get_language() or settings.LANGUAGE_CODE, but shouldn't that be fixed by using translations.activate(settings.LANGUAGE_CODE)?

I still fear we open a can of worms if we support something in this direction. All other holes also need to be closed then.

@yakky
Copy link
Member

yakky commented May 4, 2016

I guess that supporting this transparently it's a bit of a nightmare, and probably it will mean going too far
I think that one of the best point of parler is that it uses very little magic (comparing to hvad) and does very little "transparently"
This helps a lot in being simpler to use and it poses far less restrictions than other "magical" solutions.

@vstoykov
Copy link
Contributor

vstoykov commented May 4, 2016

@vdboor the reason that I use language_code=get_language() or settings.LANGUAGE_CODE was because I wanted repr() of the object to work nevertheless of if translations are activated or not.

It's kind of annoying every time when I open the shell and to import django.utils.translation and to active it then. Especially when I'm using explicitly self.safe_translation_getter('title', any_language=True) all over the place instead of simply self.title

@vdboor @yakky
I understand now that this can lead to other unpredictable situations if some code deep in the core need to be changed and this is not good. Also I understand that too much magic also can lead to unpredictable situations.
I'm just wondering in the current situation what is the usage of safe_translation_getter with any_language=True without passing a language_code and what people will expected from it to do based on the documentation?

@vstoykov
Copy link
Contributor

vstoykov commented May 5, 2016

Now when I'm playing with django's translation system I can't see a situation when error will be raised when translations are not activated. It just returns the hardcoded text in code without checking any *.mo file.

The reason for that I think is mostly because of migration framework to not create migrations all the time based on translations in verbose_name or help_text, or some situations where when trying to use translation system in management command can cause a error or something before something else to be initialized correctly.

In django-parler situation is different because translations are stored in database and there is no hardcoded fallback but still raising an exception in every situation is probably too overkill.

As I said in my previous comment I understand that too many magic is bad thing. But now I think that if we came up with well defined behavior which is reasonable will be better.

For initial issue reported by @CptLemming there is nothing that can be done in django-parler. For creating objects you need to be explicit when there is no active language.

For what @AdrianLC reported as a comment in the issue I think that is more legit to be changed something in django-parler.

@AdrianLC
Copy link

AdrianLC commented May 5, 2016

I don't think parler should crash like this, sending a null to the database, even when wrongly creating an instance without activating the translation machinery. IMO there should be some kind of fail-safe assertion within parler that would report the cause of error, without actually sending the query.

As for the any_language=True, I could understand the decision if avoiding it requires too much work to the internals. It will still be really annoying for me though, I will basically have to change most of my safe_translation_getter calls to:

if not get_language():
    with translations.override(settings.LANGUAGE_CODE):
         return self.safe_translation_getter('field', any_language=True)
return self.safe_translation_getter('field', any_language=True)

Your suggestion, @vdboor (translations.activate(settings.LANGUAGE_CODE) would not achieve the same as it switches to default language at all cases and we might still need to respect the language detected by the middleware, specially in methods such as __str__ or __repr__.

I understand the change of behavior in Django's part, as one might not need i18n and so the site can drop the weight of the localization middleware. But why would someone have parler installed, without wanting the translations activated, at all times (i.e. whenever calling something in parler). I still think that a wrapper would make more sense, even if it means that parler doesn't support deactivating translations at all.

def get_language():
    lang = _get_language()  # original get_language
    if lang is None:  # Django >= 1.8
        return settings.LANGUAGE_CODE
    return lang

@hrayr-artunyan
Copy link

Has this issue been resolved? I'm running into the same problem.

@vdboor
Copy link
Contributor

vdboor commented Aug 23, 2016

For now, you're required to manually activate the translations:

from django.utils import translation
translation.activate(settings.LANGUAGE_CODE)

we're a bit cautious to allow working with translations without having an language activated, it brings parler into untested territory.

@mhemeryck
Copy link
Contributor

mhemeryck commented Oct 6, 2016

Stumbled upon this issue while using django-parler in our own code base. I did make this PR such that we can have the pre-Django 1.8 behavior back with a global setting optional setting. The default behavior would be to use Django's get_language behavior.

Would this be something that can be merged in? If not, what would be the alternative?

@mhemeryck
Copy link
Contributor

Thanks for merging this in. Follow-up question: will there also be a (patch-level) version bump for this change (e.g. v1.6.6)? What is the typical release schedule / process you adhere to?

Having a separate tag for this fix makes it easier for us to pull in the django-parler dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants