[OpenStack-Infra] JJB V2.0 planning

Darragh Bailey daragh.bailey at gmail.com
Thu Dec 1 14:21:21 UTC 2016


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 <wayne at puppet.com> wrote:

>
>
> On Tue, Nov 15, 2016 at 8:04 AM, Darragh Bailey <daragh.bailey at 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"
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-infra/attachments/20161201/5d7c67b5/attachment.html>


More information about the OpenStack-Infra mailing list