django-extra-settings icon indicating copy to clipboard operation
django-extra-settings copied to clipboard

Handle initialization when models are not loaded

Open perepicornell opened this issue 1 year ago • 1 comments
trafficstars

Python version 3.12

Django version 5.1

Package version 0.12.0

Current behavior (bug description)

If you use a setting in a place that will be executed while the project is initialized, you'll get an AppRegistryNotReady error.

It might not be a bug but a design decision, if that's the case it might be necessary to mention in the documentation that settings cannot be accessed during initialization.

I believe that's what Martí faced in this issue: https://github.com/fabiocaccamo/django-extra-settings/issues/167

To reproduce, create this (or add to an existing model):

class TestModel(models.Model):
    test = models.BooleanField(
        "test",
        help_text=Setting.get("PROJECT_NAME"),
    )

(Another example would be using a setting in a property of a class view (that's included in some URL) will trigger the same)

Then initialize the project in an empty database and run python manage.py check.

If migrations are already executed and the extra-settings tables exist, the error will not raise, which is an added problem because it goes unnoticed until someone deploys a new instance of the project.

Expected behavior

Given that the get() method is no necessarily a database hit (it could take it from django settings or from the default value), I think it makes sense to make it possible to use it in those places.

Idea of a solution

The workaround that we're using (just trying it, still have to test it a bit more) is that we're overriding this method:

class Setting(BaseSetting):
    @staticmethod
    def _get_from_database(name):
        """
        This version catches the AppRegistryNotReady exception to make it possible
        for the extra-settings to load the Django settings or default value when you
        use it in a place that is loaded during the initialization, i.e., a
        `help_text` argument of a model's field.
        """
        try:
            setting_obj = Setting.objects.get(name=name)
            value = setting_obj.value
            set_cached_setting(name, value)
            return value
        except (AppRegistryNotReady, Setting.DoesNotExist):
            return None

If any of you think that this approach is a bad idea I'll really appreciate your thoughts.

Thanks a lot for maintaining this library!

perepicornell avatar Nov 18 '24 16:11 perepicornell

@perepicornell thank you very much for reporting how to reproduce the problem.

Yes, it's a design decision, I never though to use this app to access data from db at initialization level.

In case you want to provide a simple solution for lazily evaluating these calls, I would be open to review and merge it.

fabiocaccamo avatar Jan 11 '25 12:01 fabiocaccamo