Add new config types, improve existing types #2010

djmattyg007 · 2021-08-14T05:43:45Z

Added new Float type. It's very similar to the existing Integer type,
but accepts floating point numbers, and doesn't allow you to set
choices.
Added new Pair type, for when you want to accept a pair of values in a
single config setting. Pairs default to a pair of strings, but you can
set either subtype to whatever type you want (including nested pairs).
Added transformer property to String type. This allows you to apply an
arbitrary transformation to individual config settings that could
utimately be anything at all.
If a transformation is applied to a String config value, it will now
return a subclass of the built-in str type. This subclass stores the
original value as well as the transformed value, so that it can be
re-serialised correctly.
The "Expected Path" type originally created for the Path type is (for
now) just a subclass of the "Transformed Value" class, due to the
duplicate implementation. In future, this should be updated to return
pathlib Path objects, not strings.
Made Secret type pass through the transformer property correctly to
its String parent class.
Lists can now return a frozenset instead of a tuple, configured by the
"unique" property.
Lists now accept an arbitrary subtype, similar to the Pair type. This
can be any type at all, including Pairs.
Resolved a couple of todo comments regarding missing tests for the
List type.
The Path type will now correctly serialise None to an empty string.

djmattyg007 · 2021-08-14T05:45:49Z

This is based on ideas originally described in #1965 and #1966. Follow-up PRs to actually update the file and http extensions will be created once this PR is merged, to avoid this PR becoming any bigger than it already is.

djmattyg007 · 2021-08-14T10:43:53Z

Here are commits for the HTTP and File extensions, demonstrating how the changes in this commit would be used internally.

f10c93d
f61587c

jodal

Wow, that was quite a lot to review, but it looks solid. 🙌

This leaves me wondering if there isn't some other format (like TOML) or library (like Pydantic) we could use instead of rolling our own config subsystem. I realize that would be a breaking change and not the most productive thing to spend our spare time on, so I'll settle with a solid test suite :-)

mopidy/config/types.py

tests/config/test_types.py

djmattyg007 · 2022-01-27T11:50:16Z

This leaves me wondering if there isn't some other format (like TOML) or library (like Pydantic) we could use instead of rolling our own config subsystem. I realize that would be a breaking change and not the most productive thing to spend our spare time on, so I'll settle with a solid test suite :-)

The thought had definitely occurred to me too, but yeah, it's obviously a significant amount of work and BC breakage.

For the record, I'd be strongly opposed to implementing Pydantic. I'm not a fan of how it requires your domain models to inherit from their base classes, and all the potential problems that can bring.

While I'm not wedded to it, I've had a lot of success with marshmallow for serialisation+deserialisation, and plain old dataclasses from the stdlib for the domain models. In fact, I've even considered whether it would be worth swapping out the existing models implementation for this paradigm. In python 3.10, dataclasses support automatically generating __slots__, so you can get the benefit of better memory usage (as well as the long-standing built-in support for immutability) while still avoiding boilerplate.

jodal · 2022-01-30T10:49:31Z

I've done the rounds through attrs, dataclasses, and Pydantic, and have been quite happy with all of them. I'd use dataclasses for any library that doesn't strictly need more. dataclasses with slots and immutability sounds like a good fit for Mopidy's models, but that still leaves the validation part to us.

kingosticks · 2022-01-30T10:55:04Z

I had a random look at some projects in this space yesterday but never used any of them (Mopidy is still my only Python project!). Maybe we should start a new issue/discussion about this topic. Currently I'm not convinced any switch is worth it, we'd always end up having to write extra validators anyway.

jodal · 2022-01-30T11:26:06Z

That discussion is clearly too long-term to block this PR. Do you want to give this a review before we merge, @kingosticks ?

kingosticks · 2022-01-30T22:00:10Z

mopidy/config/types.py

+            subtype.deserialize(v.strip()) for v in values if v.strip()
+        )
+        if self._unique:
+            values = frozenset(values_iter)


This may return a different order compared to the value. I think that would be unexpected.

How much of a concern is it likely to be? The behaviour is opt-in, so I guess we can always adjust the behaviour later based on feedback if absolutely necessary.

If it was documented it'd be better. I just don't like the surprise.

I've added a comment 👍

kingosticks · 2022-01-30T23:23:11Z

If you rebase you'll pick up the flake8 fix

- Added new Float type. It's very similar to the existing Integer type, but accepts floating point numbers, and doesn't allow you to set choices. - Added new Pair type, for when you want to accept a pair of values in a single config setting. Pairs default to a pair of strings, but you can set either subtype to whatever type you want (including nested pairs). - Added transformer property to String type. This allows you to apply an arbitrary transformation to individual config settings that could utimately be anything at all. - If a transformation is applied to a String config value, it will now return a subclass of the built-in str type. This subclass stores the original value as well as the transformed value, so that it can be re-serialised correctly. - The "Expected Path" type originally created for the Path type is (for now) just a subclass of the "Transformed Value" class, due to the duplicate implementation. In future, this should be updated to return pathlib Path objects, not strings. - Made Secret type pass through the transformer property correctly to its String parent class. - Lists can now return a frozenset instead of a tuple, configured by the "unique" property. - Lists now accept an arbitrary subtype, similar to the Pair type. This can be any type at all, including Pairs. - Resolved a couple of todo comments regarding missing tests for the List type. - The Path type will now correctly serialise None to an empty string.

djmattyg007 · 2022-01-31T03:59:38Z

I've rebased the branch and added a changelog entry 👍

…ntries

djmattyg007 force-pushed the config_handling_improvements branch 2 times, most recently from 63f6da2 to 007a582 Compare August 14, 2021 06:16

djmattyg007 mentioned this pull request Nov 20, 2021

Add exponential scale option to softwaremixer #2025

Open

djmattyg007 mentioned this pull request Jan 26, 2022

Fix missing CORS headers on RPC response. #2029

Merged

djmattyg007 requested review from jodal and kingosticks January 26, 2022 04:06

kingosticks added A-config Area: Config management C-enhancement Category: A PR with an enhancement or an issue with an enhancement proposal labels Jan 26, 2022

jodal approved these changes Jan 26, 2022

View reviewed changes

mopidy/config/types.py Outdated Show resolved Hide resolved

mopidy/config/types.py Outdated Show resolved Hide resolved

mopidy/config/types.py Show resolved Hide resolved

tests/config/test_types.py Outdated Show resolved Hide resolved

djmattyg007 force-pushed the config_handling_improvements branch 4 times, most recently from 9f93b26 to feb53cd Compare January 27, 2022 11:23

kingosticks approved these changes Jan 30, 2022

View reviewed changes

djmattyg007 force-pushed the config_handling_improvements branch from feb53cd to ec3ab45 Compare January 31, 2022 02:29

djmattyg007 force-pushed the config_handling_improvements branch from ec3ab45 to 6f55166 Compare January 31, 2022 03:59

Add note about underlying data structure when enforcing unique list e…

6a7e097

…ntries

kingosticks merged commit 3ecbc98 into develop Jan 31, 2022

kingosticks deleted the config_handling_improvements branch January 31, 2022 23:32

kingosticks assigned djmattyg007 Jan 31, 2022

kingosticks added this to the Next feature release milestone Jan 31, 2022

jodal modified the milestones: v3.4.0, v3.3.0 Oct 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new config types, improve existing types #2010

Add new config types, improve existing types #2010

djmattyg007 commented Aug 14, 2021 •

edited

djmattyg007 commented Aug 14, 2021

djmattyg007 commented Aug 14, 2021

jodal left a comment

djmattyg007 commented Jan 27, 2022

jodal commented Jan 30, 2022

kingosticks commented Jan 30, 2022

jodal commented Jan 30, 2022

kingosticks Jan 30, 2022

djmattyg007 Jan 31, 2022

djmattyg007 Jan 31, 2022

kingosticks Jan 31, 2022

djmattyg007 Jan 31, 2022

kingosticks commented Jan 30, 2022

djmattyg007 commented Jan 31, 2022

Add new config types, improve existing types #2010

Add new config types, improve existing types #2010

Conversation

djmattyg007 commented Aug 14, 2021 • edited

djmattyg007 commented Aug 14, 2021

djmattyg007 commented Aug 14, 2021

jodal left a comment

Choose a reason for hiding this comment

djmattyg007 commented Jan 27, 2022

jodal commented Jan 30, 2022

kingosticks commented Jan 30, 2022

jodal commented Jan 30, 2022

kingosticks Jan 30, 2022

Choose a reason for hiding this comment

djmattyg007 Jan 31, 2022

Choose a reason for hiding this comment

djmattyg007 Jan 31, 2022

Choose a reason for hiding this comment

kingosticks Jan 31, 2022

Choose a reason for hiding this comment

djmattyg007 Jan 31, 2022

Choose a reason for hiding this comment

kingosticks commented Jan 30, 2022

djmattyg007 commented Jan 31, 2022

djmattyg007 commented Aug 14, 2021 •

edited