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

'Settings' object has no attribute 'SELECT2_THEME' #203

Closed
ewjoachim opened this issue Feb 6, 2023 · 4 comments
Closed

'Settings' object has no attribute 'SELECT2_THEME' #203

ewjoachim opened this issue Feb 6, 2023 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@ewjoachim
Copy link

ewjoachim commented Feb 6, 2023

Describe the bug
django-select2 uses django-appconf, which has a problem sometimes appearing when using tests with @override_settings() that causes some tests not to be able to see the settings.

The fix is outlined here: consists of making sure that in Select2AppConfig.ready(), we import the conf module.

Exception & Traceback

Traceback (most recent call last):
  File "/home/joachim/other-src/select2_issue/bugshowcase/tests/tests.py", line 14, in test_2
    assert settings.SELECT2_THEME
  File "/home/joachim/.envs/select2_issue/lib/python3.10/site-packages/django/conf/__init__.py", line 83, in __getattr__
    val = getattr(self._wrapped, name)
AttributeError: 'Settings' object has no attribute 'SELECT2_THEME'

Code Snippet
https://github.com/ewjoachim/django_select2_showcase_203

To Reproduce
Steps to reproduce the behavior:

  1. Clone https://github.com/ewjoachim/django_select2_showcase_203.git
  2. Create a venv and activate it
  3. pip install -r requirements.txt
  4. python manage.py test bugshowcase/tests

The 1st test is written so that the 1st time django_select2's configuration is loaded is during a @override_settings context. Because of this, when the 1st test ends, the settings disappear. The 2nd test shows this by trying to read the settings.

The bug is especially annoying as is depends on test order, import order, and many other factors.

Adding an import to django_select2.conf within any AppConfig.ready() solves it.

Expected behavior
no error

@ewjoachim ewjoachim added the bug Something isn't working label Feb 6, 2023
@ewjoachim
Copy link
Author

ewjoachim commented Feb 6, 2023

note: a workaround for users waiting for this to be merged is to import django_select2.conf within any of their own AppConfig.ready() method.

@codingjoe
Copy link
Owner

Hi @ewjoachim,

Thank you for reaching out. I appreciate the effort you put into the bug report. I'd wish everyone would be so meticulous reporting bugs. That being said, I don't think I fully get the use case here: You are trying to override a setting, that isn't set in your project, right?

In that case, I feel the error is desirable. Our default values shouldn't have any side effects into your project. I am not a big fan of how django-appconf is implemented to begin with and have dropped it in multiple packages, but not this one.

Maybe I am missing something important thought. I'd be wonderful if you could further elaborate your use case.

Best!
Joe

@codingjoe codingjoe removed the bug Something isn't working label Feb 8, 2023
@ewjoachim
Copy link
Author

You are trying to override a setting, that isn't set in your project, right?

I think you're missing the point. The problem is not what I override (and it it may convince you, I encourage you to change the override to a setting that's set in the project, such as USE_I18N). The problem is that appconf initializes Select2's configuration when the override is active, and at the end of the tests when the override is disabled, it also removes the settings that appconf put. For example, the setting I use, SELECT2_THEME, supposedly has a default value.

Select2 itself requires this setting to be set e.g. here, and does not expect this setting to not exist. Either we define SELECT2_THEME in our own settings and we get whatever value we want, or we don't and appconf takes care of setting SELECT2_THEME = "default" so that the app works. The problem is that whatever appconf does, it does it the first time it's imported and if this imports happens to be during an override_settings context or decorator, then at the end of the context/decorator, the app is bound to crash, because it will expect SELECT2_THEME to be set and it isn't.

@ewjoachim
Copy link
Author

ewjoachim commented Feb 9, 2023

I've changed details on the reproduction repo. The tests now are:

class TestShowcase(TestCase):
    @override_settings(USE_I18N=False)
    @patch("bugshowcase.select2.get_widget", autospec=True)
    def test_1(self, s2):
        self.assertEqual(1 + 1, 2)

    def test_2(self):
        from bugshowcase.select2 import get_widget

        widget = get_widget()
        self.assertNotEqual(widget.render(name="foo", value="bar"), "")

The code under test is:

def get_widget():
    return forms.Select2Widget()

If you launch test 1 or 2 alone, they work. If you launch the 2 tests in the same process, test 2 fails because widget.render ultimately tries to access settings.SELECT2_DEFAULT which is not defined because of test 1

And just in case I need to spell it out, no the right fix is not to replace all settings.SELECT2_* with getattr(settings, "SELECT2_*", some_default). The right fix is to load your settings as soon as possible in django's AppConf.ready() to ensure important stuff doesn't rely on whatever happens as a side effect of the first time you import a part of the lib.

@codingjoe codingjoe added the bug Something isn't working label Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants