Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merging settings makes it almost impossible to fully override settings #2284

Closed
alexdutton opened this issue Jul 5, 2021 · 3 comments · Fixed by #2294
Closed

Merging settings makes it almost impossible to fully override settings #2284

alexdutton opened this issue Jul 5, 2021 · 3 comments · Fixed by #2294
Assignees
Labels
bug Something's not working

Comments

@alexdutton
Copy link
Contributor

Describe the bug

In load_janeway_settings the settings referred to by DJANGO_SETTINGS_MODULE are merged into the base core.janeway_global_settings.

The method for merging involves appending to any existing sequence settings, and applying new keys to existing dictionaries. This means it's almost impossible to completely override sequence- or dict-based settings, and this behaviour is also potentially counter-intuitive.

For example, my attempt to insert a new middleware class into the existing Janeway default middleware list results in the default middleware being included twice:

from core import janeway_global_settings

MIDDLEWARE_CLASSES = list(janeway_global_settings.MIDDLEWARE_CLASSES)
MIDDLEWARE_CLASSES.insert(
    MIDDLEWARE_CLASSES.index("django.middleware.security.SecurityMiddleware") + 1,
    "whitenoise.middleware.WhiteNoiseMiddleware",
)

Janeway version

This is on master, currently 7739efb.

Expected behavior

One of three things:

  • A setting in DJANGO_SETTINGS_MODULE completely overrides the corresponding setting in global_janeway_settings, but the current custom configure_settings() behaviour continues, or
  • The current configure_settings() approach is removed, and from core.global_janeway_settings import * can be used in DJANGO_SETTINGS_MODULE to the same effect, making things more explicit, with no backwards-compatibility, or
  • load_janeway_settings could detect whether global_janeway_settings is *-imported into DJANGO_SETTINGS_MODULE — as the new preferred approach — and disable the settings-merging behavour. This would provide backwards-compatibility for the current behaviour.
@alexdutton alexdutton added the bug Something's not working label Jul 5, 2021
@mauromsl
Copy link
Member

mauromsl commented Jul 8, 2021

I think the best approach is a combination of A and B, by moving the list of settings to be merged from the base installation to the actual settings file.. let me draft a PR

@ajrbyers ajrbyers added this to the v1.4.1 milestone Oct 26, 2021
@mauromsl mauromsl self-assigned this Nov 11, 2021
@ajrbyers ajrbyers added this to v1.4.1 Nov 18, 2021
@ajrbyers ajrbyers modified the milestones: v1.4.1, 1.4.2 Nov 18, 2021
@ajrbyers ajrbyers removed this from v1.4.1 Nov 18, 2021
@ajrbyers ajrbyers added this to v1.4.2 Nov 18, 2021
@joemull joemull moved this to Todo in v1.4.2 Feb 3, 2022
@ajrbyers ajrbyers added this to v1.4.3 Jun 9, 2022
@ajrbyers ajrbyers moved this to Todo in v1.4.3 Jun 9, 2022
@ajrbyers ajrbyers removed this from v1.4.2 Jun 9, 2022
@mauromsl mauromsl moved this from Todo to PR Submitted in v1.4.3 Sep 7, 2022
@ajrbyers ajrbyers removed this from the 1.4.2 milestone Sep 21, 2022
@ajrbyers ajrbyers removed this from v1.4.3 Sep 21, 2022
@ajrbyers ajrbyers moved this to Todo in Old v1.6 Sep 21, 2022
@joemull joemull removed this from Old v1.6 Sep 22, 2022
@joemull joemull moved this to Todo in v1.5 Torres Sep 22, 2022
@ajrbyers ajrbyers removed this from v1.5 Torres Mar 27, 2023
@ajrbyers
Copy link
Member

I've re-assigned this to v1.5.1

@ajrbyers ajrbyers moved this to Todo in v1.5.1 Archer May 9, 2023
@ajrbyers ajrbyers added this to Old v1.6 Jun 8, 2023
@ajrbyers ajrbyers removed this from v1.5.1 Archer Jun 8, 2023
@github-project-automation github-project-automation bot moved this to Todo in Old v1.6 Jun 8, 2023
@mauromsl mauromsl removed this from Old v1.6 Feb 7, 2024
@mauromsl mauromsl added this to v1.5.2 Feb 7, 2024
@mauromsl mauromsl moved this to In Progress in v1.5.2 Feb 7, 2024
mauromsl added a commit that referenced this issue Mar 12, 2024
Signed-off-by: Mauro MSL <mauro.msl@outlook.com>
@mauromsl
Copy link
Member

Closed by #2294

@github-project-automation github-project-automation bot moved this from In Progress to Done in v1.5.2 Mar 12, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something's not working
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants