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

Multiple override_settings with environ variable #93

Open
rastytheamateur opened this issue Sep 14, 2020 · 6 comments
Open

Multiple override_settings with environ variable #93

rastytheamateur opened this issue Sep 14, 2020 · 6 comments

Comments

@rastytheamateur
Copy link

The following code is not working properly. Exiting the second override loads the original environ variable value.

ATTR=terminal DJANGO_SETTINGS_MODULE=settings python test.py

class Settings(AppSettings):
    attr = StringSetting(default='Hello')

S = Settings()

if __name__ == '__main__':
    Settings.check()

    print(S.attr)  # output: terminal [correct]
    with override_settings(ATTR='Override1'):
        print(S.attr)  # output: Override1 [correct]
        with override_settings(ATTR='Override2'):
            print(S.attr)  # output: Override2 [correct]
        print(S.attr)  # output: terminal [incorrect]       <----
    print(S.attr)  # output: terminal [correct]
@pawamoy
Copy link
Owner

pawamoy commented Sep 14, 2020

Ouch, I don't think we had nested with statements in mind when implementing settings overrides and cache invalidation. Maybe there's a way to "unstack" the values when invalidating cache.

@rastytheamateur
Copy link
Author

I think this is not a serious bug (I believe nobody really uses environ variables with nested overrides). I will fix it later.

@pawamoy
Copy link
Owner

pawamoy commented Sep 14, 2020

It might not be possible, unless the override_settings manager does something special on exit that we could hook on to restore the previously stacked cache.

@pawamoy
Copy link
Owner

pawamoy commented Sep 14, 2020

Ah it's totally possible in fact, Django sends the signal twice with a boolean value in the enter argument 🙂

    def invalidate_cache(self, enter, **kwargs):
        """Invalidate cache. Run when receive ``setting_changed`` signal."""
        if enter:
            self._caches.append(copy.deepcopy(self._cache))  # maybe the copy is not necessary?
            self._cache = {}
        elif self._caches:
            self._cache = self._caches.pop()
        else:  # shouldn't happen, we always enter before exiting
            self._cache = {}

...or something similar 🙂

https://docs.djangoproject.com/en/3.1/ref/signals/#setting-changed

@ziima
Copy link
Collaborator

ziima commented Sep 15, 2020

The problem is not is cache, but in environment variables.

# ATTR=terminal
with override_settings(ATTR='Override1'):
    # Override shifts the environment: __DAP_ATTR=terminal, ATTR not defined
    print(S.attr)  # output: Override1 [correct]
    with override_settings(ATTR='Override2'):
        # There is no ATTR environment variable, so nothing to shift
        print(S.attr)  # output: Override2 [correct]
    # On exit, the override_settings finds __DAP_ATTR in environment and unshifts it: ATTR=terminal, __DAP_ATTR not defined
    # Since the cache is empty and the environment has priority, it's used and returned.
    print(S.attr)  # output: terminal [incorrect]       <----
print(S.attr)  # output: terminal [correct]

@stinovlas
Copy link
Collaborator

I do see one realistic example of nested overrides. If you decorate a test class and then want to change the value in one of its methods, that creates a nested override right there. It's probably not a top priority, but it would be nice to fix it eventually.

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

4 participants