I was testing out the updated version of the code (that includes the abstracted Configuration class), and I think it might be better if we remove BaseConfiguration from being the default configuration_type and instead require that it be passed in as an argument.
My problem with having it default to BaseConfiguration is that a user might be unaware that failing to pass in AtomicConfiguration would break things like functions that call aggregate_configuration_summaries. I ran into this problem and didn't catch it until much further down in my workflow pipeline -- not a big problem because I think it's safe to just re-assign client.configuration_type = AtomicConfiguration after the fact, but I think that forcing users to pass it in during instantiation would be better.
I was testing out the updated version of the code (that includes the abstracted Configuration class), and I think it might be better if we remove BaseConfiguration from being the default
configuration_typeand instead require that it be passed in as an argument.My problem with having it default to BaseConfiguration is that a user might be unaware that failing to pass in AtomicConfiguration would break things like functions that call
aggregate_configuration_summaries. I ran into this problem and didn't catch it until much further down in my workflow pipeline -- not a big problem because I think it's safe to just re-assignclient.configuration_type = AtomicConfigurationafter the fact, but I think that forcing users to pass it in during instantiation would be better.