[openstack-dev] [oslo] oslo.concurrency repo review
Ben Nemec
openstack at nemebean.com
Mon Aug 11 20:05:13 UTC 2014
On 08/11/2014 02:20 PM, Joshua Harlow wrote:
>
>
> On Mon, Aug 11, 2014 at 11:39 AM, Ben Nemec <openstack at nemebean.com>
> wrote:
>> On 08/11/2014 01:02 PM, Yuriy Taraday wrote:
>>> On Mon, Aug 11, 2014 at 5:44 AM, Joshua Harlow
>>> <harlowja at outlook.com> wrote:
>>>
>>>> One question from me:
>>>>
>>>> Will there be later fixes to remove oslo.config dependency/usage
>>>> from
>>>> oslo.concurrency?
>>>>
>>>> I still don't understand how oslo.concurrency can be used as a
>>>> library
>>>> with the configuration being set in a static manner via
>>>> oslo.config (let's
>>>> use the example of `lock_path` @ https://github.com/YorikSar/
>>>> oslo.concurrency/blob/master/oslo/concurrency/lockutils.py#L41).
>>>> For
>>>> example:
>>>>
>>>> Library X inside application Z uses lockutils (via the nice
>>>> oslo.concurrency library) and sets the configuration `lock_path`
>>>> to its
>>>> desired settings, then library Y (also a user of oslo.concurrency)
>>>> inside
>>>> same application Z sets the configuration for `lock_path` to its
>>>> desired
>>>> settings. Now both have some unknown set of configuration they
>>>> have set and
>>>> when library X (or Y) continues to use lockutils they will be
>>>> using some
>>>> mix of configuration (likely some mish mash of settings set by X
>>>> and Y);
>>>> perhaps to a `lock_path` that neither actually wants to be able to
>>>> write
>>>> to...
>>>>
>>>> This doesn't seem like it will end well; and will just cause
>>>> headaches
>>>> during debug sessions, testing, integration and more...
>>>>
>>>> The same question can be asked about the `set_defaults()`
>>>> function, how is
>>>> library Y or X expected to use this (are they?)??
>>>>
>>>> I hope one of the later changes is to remove/fix this??
>>>>
>>>> Thoughts?
>>>>
>>>> -Josh
>>>
>>>
>>> I'd be happy to remove lock_path config variable altogether. It's
>>> basically
>>> never used. There are two basic branches in code wrt lock_path:
>>> - when you provide lock_path argument to lock (and derivative
>>> functions),
>>> file-based lock is used and CONF.lock_path is ignored;
>>> - when you don't provide lock_path in arguments, semaphore-based
>>> lock is
>>> used and CONF.lock_path is just a prefix for its name (before
>>> hashing).
>>>
>>> I wonder if users even set lock_path in their configs as it has
>>> almost no
>>> effect. So I'm all for removing it, but...
>>> From what I understand, every major change in lockutils drags along
>>> a lot
>>> of headache for everybody (and risk of bugs that would be
>>> discovered very
>>> late). So is such change really worth it? And if so, it will
>>> require very
>>> thorough research of lockutils usage patterns.
>>
>> Two things lock_path has to stay for: Windows and consumers who
>> require
>> file-based locking semantics. Neither of those use cases are trivial
>> to
>> remove, so IMHO it would not be appropriate to do it as part of the
>> graduation. If we were going to alter the API that much it needed to
>> happen in incubator.
>>
>>
>> As far as lock_path mismatches, that shouldn't be a problem unless a
>> consumer is doing something very unwise. Oslo libs get their
>> configuration from the application using them, so unless the
>> application
>> passes two separate conf objects to library X and Y they're both going
>> to get consistent settings. If someone _is_ doing that, then I think
>> it's their responsibility to make sure the options in both config
>> files
>> are compatible with each other.
>
> Why would it be assumed they would pass the same settings (how is that
> even possible to know ahead of time? especially if library X pulls in a
> new library ZZ that requires a new configuration setting). For example,
> one directory for `lock_path` may be reasonable for tooz and another
> may be reasonable for taskflow (completly depends on there intended
> usage), it would not likely desireable to have them go to the same
> location.
The only reason I can see that you would want to do this is to avoid
lock name collisions, but I think I'd rather namespace lib locks than
require users to manage multiple lock paths. Even the one lock path has
been a hassle.
> Forcing application Z to know the inner workings of library X
> and library Y (or future unknown library ZZ) is just pushing the
> problem onto the library user, which seems inappropriate and breaks the
> whole point of having abstractions & APIs in the first place... This
> IMHO is part of the problem with having statically set *action at a
> distance* type of configuration, the libraries themselves are not in
> control of their own configuration, which breaks abstractions & APIs
> left and right. If some application Y can go under a library and pull
> the rug out from under it, how is that a reasonable thing to expect the
> library to be able to predict & handle?
The application doesn't have to, and shouldn't, know about lib options.
The libraries have a list_opts method that returns the list of options
the lib needs, which is sufficient for the config generator to do the
right thing (a bit of an oversimplification maybe, but the point is
there's an API for handling the options so the app doesn't need to).
Besides that the application doesn't need to care what opts the lib uses
- all the application is responsible for is passing its conf object to
the lib, which will then read the options it cares about. Not only
that, but we've recently added a config filter so that the application
can't even see the options registered by the lib.
For at least some of the libs (like oslo.db) there _is_ a
non-oslo.config interface where the application does need to know what
options the lib cares about, but there's really no way around that. The
options have to get set somehow. Using oslo.config makes it more
transparent, but requires using oslo.config.
>
> This kind of requirement has always made me wonder how other libraries
> (like tooz, or taskflow) actually interact with any of the oslo.*
> libraries in any predicatable way (since those library could be
> interacting with oslo.* libraries that have configuration that can be
> switched out from underneath them, making those libraries have *secret*
> APIs that appear and disappear depending on what used oslo.* library
> was newly added as a dependency and what newly added configuration that
> library sucked in/exposed...).
>
> -Josh
>
>>
>>
>> -Ben
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
More information about the OpenStack-dev
mailing list