Think you forgot to Reply-All ;-) Only spotted that now, while I was waiting for others to chime in and see different points of view.
On 22 November 2016 at 07:25, Wayne Warren <wa...@puppet.com> wrote: > > > On Tue, Nov 15, 2016 at 8:04 AM, Darragh Bailey <daragh.bai...@gmail.com> > wrote: > >> >> So on that patch: >> https://review.openstack.org/358019 Support explicit API and simple >> config creation >> >> Does it make sense? I'd like to hear other people's opinion on whether it >> makes sense to have an explicit API as well as a from_config(cfg) method >> when someone would create the config object and then pass it around, or is >> it more of a YAGNI? >> > > I think that if you don't have an explicit use case for it that you intend > to use personally it's probably a YAGNI. > > Personally, I prefer to pass configuration values around with and access > them through an instance of a single configuration class that > > * can be more easily instantiated with reasonable default settings > * we can reason about and write tests for configuration > * discourages proliferation of scattered configuration value munging > throughout the code base which is one problem that JJBConfig was intended > to address > > Also, should all classes be instantiable without a JJBConfig? Why? If not, > why not? I'm curious why the focus on the Builder object for this behavior, > but I'm also interested in the principles at work here so I can change my > way of thinking if it's flawed; sorry if I've missed an explanation in the > past. > Previous experience with libcurl, which also uses this idea of configuration all in one object and then passed around, resulted in me experiencing that fun behaviour of needing to refer back to the config object in order to determine how to control the behaviour of the other API's I was calling. It was exceedingly frustrating, though that might be just due to poor API documentation on the curl project's behalf. I'm not sure it'll discourage "proliferation of scattered configuration", but rather have methods that reach into the config object to get behavioural controlling settings that are not then clearly reflected in the arguments into the method/class. End up needing to define config options used internally as part of classes/methods docstrings in order to help convey how to use the API. Btw, the builder was just the easiest to look at first, I didn't see any point in going through the rest of the objects and making changes to have an expressive constructor which used params that controlled the current objects behaviour, if it was clear this doesn't make sense as an approach. Of course for the plugin settings, these will have to be passed through as a single config object to the dispatch methods, so perhaps unless we plan to look at that as well, maybe there is no need to worry about the other classes having separate params for each setting controlling instantiated objects behaviour. > Since it impacts the API that will be published, seems like it would be a >> good idea to get this ironed out before releasing? >> > > I'd prefer to choose one approach for instantiating objects now based on > its technical merits, make sure it's not incompatible with whatever > alternative we are considering, and save alternatives as additive features > that can be implemented sometime in the future when a need becomes apparent. > Agreed, it's probably only because there is no overloading of constructors available in most dynamic languages that it's needing a closer look because I'm not sure it's possible to add support for creation without a config object without another breaking API change. I think the only solution would be simple subclasses with different __init__ methods. Hence I'm willing to look at this pretty closely. > -- >> Darragh Bailey >> "Nothing is foolproof to a sufficiently talented fool" >> > > Suggested to Thanh, that it might be worth cutting a beta release (maybe tomorrow) and advertise it to allow people to start installing from pypi by using '--pre'. -- Darragh Bailey "Nothing is foolproof to a sufficiently talented fool"
_______________________________________________ OpenStack-Infra mailing list OpenStack-Infra@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra