Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#30057 closed Bug (fixed)

diffsettings leaves out custom default settings

Reported by: orlnub123 Owned by: nobody
Component: Core (Management commands) Version: dev
Severity: Release blocker Keywords: diffsettings, settings
Cc: Hasan Ramezani Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you've manually configured the settings with default settings that differ from the default they won't show up if you run the diffsettings command. This is only observable on master as before 49b6793 the settings were forcefully setup, replacing the manually configured settings.

The reason this happens is because module_to_dict reads the settings from the __dict__. If you haven't manually configured your settings this is fine because the default wrapped settings object preassigns all settings to the __dict__. Since UserSettingsHolder, the class that wraps manually configured settings, relies on the __dir__ + __getattr__ pattern which allows the underlying default settings object to be dynamic it doesn't store settings from the default settings in its __dict__.

Change History (6)

comment:1 by Simon Charette, 5 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Simon Charette, 5 years ago

Cc: Hasan Ramezani added
Severity: NormalRelease blocker

Marking as master release blocker since it's a regression introduced by 49b679371fe9beddcd23a93b5fdbadea914f37f8 (#29236).

comment:3 by Tim Graham <timograham@…>, 5 years ago

Resolution: fixed
Status: newclosed

In 573f44d6:

Fixed #30057 -- Fixed diffsettings ignoring custom configured settings.

Regression in 49b679371fe9beddcd23a93b5fdbadea914f37f8.

comment:4 by orlnub123, 5 years ago

Resolution: fixed
Status: closednew

Reopening with a PR for some follow up.

Last edited 5 years ago by Tim Graham (previous) (diff)

comment:5 by Tim Graham, 5 years ago

Resolution: fixed
Status: newclosed

The PR seems to be about additional test coverage which can be added without an open ticket.

comment:6 by Tim Graham <timograham@…>, 5 years ago

In 2c0fda7f:

Refs #30057 -- Added more diffsettings tests.

The test in 573f44d62fe1e87e2c20a74eba5e20ca9ff0ed85 doesn't act as a
regression test.

Note: See TracTickets for help on using tickets.
Back to Top