Before submitting PRs to add new behaviors, I wanted to wrap my head around the current codebase. I started with some basic contributor-friendly changes:
add documentation
clarify variable names (e.g. local instead of l)
add test cases
I had to do a lot of mocking to get my first wave of test cases working. To reduce the complexity of test cases, I needed to rearrange code to better encapsulate behaviors. This PR is the product of that effort and includes changes like (edit: renamed State classes to clearly indicate storage backend):
To ensure consistent behavior through their lifecycle, all classes are now configured on __init__ (should avoid issues like #15 for programmatic users)
To support custom diffing classes/behaviors (e.g. issues #11, #13, and #16), DiffResolver (formerly FlatDictDiffer) is now injected into the ParameterStore (formely RemoteState) constructor.
To configure DiffResolver without coupling, I created a configure() classmethod that uses functools.partial()` to pre-populate constructor kwargs.
All flattening behavior is encapsulated in DiffResolver. This simplifies the interface on ParameterStore and YAMLFile (formely LocalState) which only accept nested dicts (though RemoteState uses some flattened endpoints).
Some of the changes (e.g. more git-like method names on DiffResolver) could be reverted, but I changed the signature of most/all methods so it wouldn't really help backwards compatibility.
This PR should be 100% backwards compatible for CLI users. Obviously, the interfaces for most classes/methods have changed so it will be a breaking change (i.e. major release) for programmatic users.
I know big rewrites aren't normally welcomed from a new contributor, but I hope you'll forgive the overhaul since it should simplify/enable additional improvements. I'm going to submit additional PRs, all based on this foundation. If you object to this direction, I can certainly try to rewrite more to your liking.
Before submitting PRs to add new behaviors, I wanted to wrap my head around the current codebase. I started with some basic contributor-friendly changes:
localinstead ofl)I had to do a lot of mocking to get my first wave of test cases working. To reduce the complexity of test cases, I needed to rearrange code to better encapsulate behaviors. This PR is the product of that effort and includes changes like (edit: renamed
Stateclasses to clearly indicate storage backend):__init__(should avoid issues like #15 for programmatic users)DiffResolver(formerlyFlatDictDiffer) is now injected into theParameterStore(formelyRemoteState) constructor.DiffResolverwithout coupling, I created aconfigure()classmethod that uses functools.partial()` to pre-populate constructor kwargs.DiffResolver. This simplifies the interface onParameterStoreandYAMLFile(formelyLocalState) which only accept nested dicts (thoughRemoteStateuses some flattened endpoints).DiffResolver) could be reverted, but I changed the signature of most/all methods so it wouldn't really help backwards compatibility.This PR should be 100% backwards compatible for CLI users. Obviously, the interfaces for most classes/methods have changed so it will be a breaking change (i.e. major release) for programmatic users.
I know big rewrites aren't normally welcomed from a new contributor, but I hope you'll forgive the overhaul since it should simplify/enable additional improvements. I'm going to submit additional PRs, all based on this foundation. If you object to this direction, I can certainly try to rewrite more to your liking.