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

Settings file to declare what settings should be merged #2294

Merged
merged 5 commits into from
Mar 12, 2024
Merged

Settings file to declare what settings should be merged #2294

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d75aae5
Settings file to declare what settings to merge
mauromsl Jul 8, 2021
908c485
Update jenkins settings
mauromsl Jul 9, 2021
21c5c14
#2284: Preserve default mergeable settings
mauromsl Mar 12, 2024
157fe9e
Merge branch 'b_1_5_2' into 2284-bugfix
mauromsl Mar 12, 2024
56b5707
#2284: Undo whitespace change from rebase
mauromsl Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/core/dev_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
'hijack',
'compat',
]
# Combine our apps and middleware classes with the defaults
MERGEABLE_SETTINGS = {"INSTALLED_APPS", "MIDDLEWARE_CLASSES"}


def show_toolbar(request):
Expand Down
5 changes: 3 additions & 2 deletions src/utils/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

LOCK = threading.Lock()

MERGEABLE_SETTINGS = {"INSTALLED_APPS", "MIDDLEWARE_CLASSES"}
MERGEABLE_SETTINGS = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look to be used anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try to be extra defensive when removing public constants like this in case a plugin out there might be using them, so we keep them for at least one release cycle.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense :-). For full backwards-compatibility then, should this be unchanged, and be the default if it's not provided in custom_settings?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, though as {} it's now a dictionary and not a set as before :-)


def load_janeway_settings():

Expand All @@ -43,8 +43,9 @@ def load_janeway_settings():
"Loading the following custom settings: %s" %(
custom_settings.keys(),
))
mergeable_settings = custom_settings.get("MERGEABLE_SETTINGS", {})
for k, v in custom_settings.items():
if k in MERGEABLE_SETTINGS:
if k in mergeable_settings:
janeway_settings[k] = merge_settings(janeway_settings[k], v)
else:
janeway_settings[k] = v
Expand Down