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

Unwanted reset of parameter to default value #348

Open
lada-olc opened this issue Oct 23, 2019 · 10 comments
Open

Unwanted reset of parameter to default value #348

lada-olc opened this issue Oct 23, 2019 · 10 comments

Comments

@lada-olc
Copy link

Describe the problem

Time to time, I experience resetting of one parameter to the default value (False to True) and therefore enabling certain feature of our system.

Steps to reproduce

By logging to Sentry, I located the problem in the database backend of django-constance. See the comments in Config.getattr of base.py:

    def __getattr__(self, key):
        try:
            if not len(settings.CONFIG[key]) in (2, 3):
                raise AttributeError(key)
            default = settings.CONFIG[key][0]
        except KeyError:
            raise AttributeError(key)

        # here I get None even tough the parameter is in the database:
        result = self._backend.get(key)
        if result is None:
            result = default

            # so the default value is set:
            setattr(self, key, default)
            return result
        return result

How the DatabaseBackend.get method could return None:

    def get(self, key):
        key = self.add_prefix(key)
        if self._cache:
            value = self._cache.get(key)
            if value is None:
                self.autofill()
                value = self._cache.get(key)
        else:
            value = None
        if value is None:
            try:
                value = self._model._default_manager.get(key=key).value
            except (OperationalError, ProgrammingError, self._model.DoesNotExist):
                # HERE it is
                pass
            else:
                if self._cache:
                    self._cache.add(key, value)
        return value

I put logging command before the line with pass to get the specific Exception information next time, but still - is it correct to set the value to the default value when reading from database fails?

System configuration

  • Django version: 2.1.5
  • Python version: 3.7.1
@auuron
Copy link

auuron commented Dec 30, 2019

Are there any plans to fix that?

@rmaciejczyk
Copy link

I'm seeing pretty similar issue that some constance settings are sometimes reset to default values. What is weird I can see the correct values in the constance Django admin form but when I try to get them in the code they are default. I didn't debug it but it looks like this is the same issue.

@jack-monk
Copy link

I encountered same issue on our production stack, we sometimes get temporary connection issue between VMs where neither DB VM nor memcache VM is available. Problem is hard to catch, as exceptions are silenced and have big impact cause it permanently changes DB fields values to defaults which are development values. We are forced to use production values as defaults instead of relying on values from DB, making django-constance in this case useless :(
Are there plans to rework this logic?

@dabrowne
Copy link

FYI for anyone still dealing with this issue. We mitigated it by overriding the default DatabaseBackend. OperationalError and ProgrammingError now fail loudly rather than silent fallback to default value.

from constance.backends.database import DatabaseBackend as BaseDatabaseBackend


class DatabaseBackend(BaseDatabaseBackend):
    """
    Fix for https://github.com/jazzband/django-constance/issues/348
    Overrides the `get` method to remove silencing of database failures.
    Such errors would otherwise result in unwanted resetting of parameters
    to default values.
    """

    def get(self, key):
        key = self.add_prefix(key)
        if self._cache:
            value = self._cache.get(key)
            if value is None:
                self.autofill()
                value = self._cache.get(key)
        else:
            value = None
        if value is None:
            try:
                value = self._model._default_manager.get(key=key).value
            except self._model.DoesNotExist:  # Only catch DoesNotExist exceptions here
                pass
            else:
                if self._cache:
                    self._cache.add(key, value)
        return value

lada-olc added a commit to lada-olc/django-constance that referenced this issue Nov 1, 2020
@auuron
Copy link

auuron commented Feb 4, 2021

FYI for anyone still dealing with this issue. We mitigated it by overriding the default DatabaseBackend. OperationalError and ProgrammingError now fail loudly rather than silent fallback to default value.

from constance.backends.database import DatabaseBackend as BaseDatabaseBackend


class DatabaseBackend(BaseDatabaseBackend):
    """
    Fix for https://github.com/jazzband/django-constance/issues/348
    Overrides the `get` method to remove silencing of database failures.
    Such errors would otherwise result in unwanted resetting of parameters
    to default values.
    """

    def get(self, key):
        key = self.add_prefix(key)
        if self._cache:
            value = self._cache.get(key)
            if value is None:
                self.autofill()
                value = self._cache.get(key)
        else:
            value = None
        if value is None:
            try:
                value = self._model._default_manager.get(key=key).value
            except self._model.DoesNotExist:  # Only catch DoesNotExist exceptions here
                pass
            else:
                if self._cache:
                    self._cache.add(key, value)
        return value

Did you think about creating pull request?

@jabelone
Copy link

jabelone commented Sep 6, 2021

Hey there! I've noticed what appears to be the same issue affecting me. The strange thing is it's always the same constance setting, and it's not the first setting. If it were an intermittent db connection issue I'd expect it to be different each time, but maybe I'm just over thinking it.

I have a weekly cron scheduled to make a copy of the sqlite db and copy it to S3. Every 1-2 months this cron seems to trigger this bug and makes the constance setting reset to it's default value. At first I thought it was due to the server running, so I added a step to stop the docker container prior to copying the sqlite db but it's still happening.

I'll give the suggested fix above a try as it looks promising.

@olivierbicler
Copy link

Not quite sure it will help somebody, but I faced this issue, along #443 as well, at very specific time (=~ 400 threads calling 4 config keys each in less than 1 sec).
As far as I can see, missing keys were always those Constance failed to load.
I activated cache (django-redis, not tested memcached), problem's gone so far, and simultaneous connect to database drastically lowered.

@Mogost
Copy link
Member

Mogost commented Sep 10, 2024

Is this problem still relevant today?

@AnveshGoud
Copy link

I am facing same issue still, are there any plans to fix this issue?

@Mogost
Copy link
Member

Mogost commented Sep 19, 2024

@AnveshGoud could you please provide more details? Steps to reproduce, django/django-constance version etc.

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

9 participants