[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