[openstack-dev] [oslo] oslo.concurrency repo review
Joshua Harlow
harlowja at outlook.com
Mon Aug 11 20:10:07 UTC 2014
On Mon, Aug 11, 2014 at 12:47 PM, Doug Hellmann <doug at doughellmann.com>
wrote:
>
> On Aug 11, 2014, at 3:26 PM, Joshua Harlow <harlowja at outlook.com>
> wrote:
>
>>
>>
>> On Mon, Aug 11, 2014 at 11:02 AM, Yuriy Taraday
>> <yorik.sar at gmail.com> 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).
>>
>> Agreed, it just seems confusing (and bad) to have parts of the API
>> come in from `CONF.lock_path` (or other `CONF.*` options) and other
>> parts of the API come in via function parameters. This just makes
>> understanding the API and knowing how to interact with it that much
>> harder (after all what is the right way of using XYZ feature when it
>> can be changed via a out-of-band *hidden* API call via configuration
>> adjustments under the covers?)... This makes it really hard to use
>> oslo.concurrency in taskflow (and likely other libraries that would
>> like to consume oslo.concurrency, seeing that it will be on pypi, I
>> would
>
> The libraries placed in the oslo namespace are very much NOT meant to
> be used by anything other than OpenStack. They are intended to be the
> glue layer between OpenStack and some other implementation libraries.
>
> oslo.concurrency wraps pylockfile and the plan is to move the actual
> lock code into pylockfile without the oslo.config dependency. That
> will make pylockfile reusable by taskflow and tooz, and the locking
> stuff in oslo.concurrency a smaller API with consistent configuration
> for use by applications.
Sounds great, I've been wondering why
https://github.com/stackforge/tooz/commit/f3e11e40f9871f8328
happened/merged (maybe it should be changed?). I see that
https://review.openstack.org/#/c/102202/ merged so that's good news and
hopefully makes the underlying lockutils functionality more useful to
outside of openstack users in the near-term future (which includes
taskflow, being that it is being used in & outside openstack by various
entities).
>
>
>> expect this number to grow...) since taskflow would really
>> appreciate and desire to have stable APIs that don't change by some
>> configuration that can be set by some party via some out-of-band
>> method (for example some other library or program calling
>> `set_defaults()`). This kind of way of using an API (half of the
>> settings from config, half of the settings from the functions
>> API...) may be ok for applications but it's not IMHO ok for
>> libraries (or clients) that want to use oslo.concurrency.
>> Hopefully it can be fixed some that it works via both ways? Oslo.db
>> I believe made this work better by allowing for configuration to
>> come in via a configuration object that can be provided by the user
>> of oslo.db, this makes the API that oslo.db exposes strongly tied to
>> the attributes & documentation of that object. I still don't think
>> thats perfect either since its likely that the documentation for
>> what that objects attributes should be is not as update to date or
>> easy to change as updating function/method documentation…
>
> That technique of having the configuration object passed to the oslo
> library will be repeated in the other new libraries we are creating
> if they already depend on configuration settings of some sort. The
> configuration options are not part of the public API of the library,
> so they and their definitions will be hidden from the caller, but the
> library has to be given a configuration object in order to load the
> settings for itself.
Great, thanks for confirming (that seems to be the most sane solution I
guess thats possible with these libraries at the current time).
>
>
>>
>>> 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.
>>
>> Sounds like tech debt to me, it always requires work to make
>> something better. Are we the type of community that will avoid
>> changing things (for the better) because we fear introducing new
>> bugs that may be found along the way? I for one hope that we are not
>> that type of community (that type of community will die due to its
>> own *fake* fears...).
>>
>>> --
>>> Kind regards, Yuriy.
>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
> _______________________________________________
> 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