Re: [openstack-dev] [oslo] oslo.concurrency repo review
On Mon, Aug 11 2014, Joshua Harlow wrote: Sounds great, I've been wondering why https://github.com/stackforge/tooz/commit/f3e11e40f9871f8328 happened/merged (maybe it should be changed?). For the simple reason that there's people wanting to use a lock distributed against several processes without being distributed against several nodes. In that case, having a ZK or memcached backend is useless as IPC is good enough. -- Julien Danjou ;; Free Software hacker ;; http://julien.danjou.info signature.asc Description: PGP signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [oslo] oslo.concurrency repo review
Sure, thats great as to why a feature like it might exist (and I think such a feature is great to have, for cases when a bigger *distributed* system isn't desired). Just the one there taken from lockutils has some issues that IMHO I think tooz would be better to avoid for the time-being (until pylockfile is updated to have a more reliable implementation). Some of the current issues I can think of off the top of my head: 1. https://bugs.launchpad.net/oslo/+bug/1327946 (this means the usage in tooz will be similarily not resistant to program termination, which in a library like tooz seems more severe, since tooz has no way of knowing how it, as a library, will be used). With this bug future acquisition after *forceful* program termination will result in the acquire() method not working for the same IPClock name (ever). 2. The double API, tooz configures lockutils one way, someone else can go under tooz and use `set_defaults()` (or other ways that I'm not aware of that can be used to configure oslo.config) and expect that the settings they will have set will actually do something, when in fact they will not (or will they???). This seems like a bad point of confusion for an API to have, where some of its API is from methods/functions... and some is from oslo.config... 3. Bringing in oslo.config as a dependency (tooz isn't configured via oslo.config but now it has code that looks like it is configured this via it). What happens if the parts of tooz now are set by oslo.config and some parts aren't? This seems bad from a user experience (where the user is the library user) point of view and a testability point of view (and probably other points of view that I can't think of), when there are new options that can be set via a *secret* API that now affect how tooz works... 4. What happens with windows here (since tooz is a library it's hard to predict how it will be used, unless windows is not supported)? Windows will resort back to using a filelock, which will default to using whatever oslo.config file path was set for tooz, which again goes back to #2 and #3 and having 2 apis, one public and one *secret* that can affect how tooz operates... In this case it seems `default=os.environ.get(TOOZ_LOCK_PATH)` will be used, when that's not set tooz now blows up with a weird configuration error @ https://github.com/stackforge/tooz/blob/master/tooz/openstack/common/lockutils.py#L222 (this all seems bad for users of tooz)... What do u think about waiting until pylockfile is ready and avoiding 1-4 from above? At least if taskflow uses tooz I surely don't want taskflow to have to deal with #1-4 (which it will inherit from tooz if taskflow starts to use tooz by the very nature of taskflow using tooz as a library). Thoughts? -Josh On Tue, Aug 12, 2014 at 6:12 AM, Julien Danjou jul...@danjou.info wrote: On Mon, Aug 11 2014, Joshua Harlow wrote: Sounds great, I've been wondering why https://github.com/stackforge/tooz/commit/f3e11e40f9871f8328 happened/merged (maybe it should be changed?). For the simple reason that there's people wanting to use a lock distributed against several processes without being distributed against several nodes. In that case, having a ZK or memcached backend is useless as IPC is good enough. -- Julien Danjou ;; Free Software hacker ;; http://julien.danjou.info ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [oslo] oslo.concurrency repo review
On Tue, Aug 12 2014, Joshua Harlow wrote: […] What do u think about waiting until pylockfile is ready and avoiding 1-4 from above? At least if taskflow uses tooz I surely don't want taskflow to have to deal with #1-4 (which it will inherit from tooz if taskflow starts to use tooz by the very nature of taskflow using tooz as a library). I definitely agree with you! The thing is that I wanted to have this for Gnocchi and the patch https://review.openstack.org/#/c/110260/ which is going to be simplified by that. So I went ahead and implemented a first version of the IPC driver, which as you point out, is far from being perfect. Now, if you think – and you have good points – that the IPC driver in tooz could be better, I'm not going to disagree, and patches are welcome! :-) -- Julien Danjou ;; Free Software hacker ;; http://julien.danjou.info signature.asc Description: PGP signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [oslo] oslo.concurrency repo review
On Mon, Aug 11, 2014 at 5:44 AM, Joshua Harlow harlo...@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. -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [oslo] oslo.concurrency repo review
On 08/11/2014 01:02 PM, Yuriy Taraday wrote: On Mon, Aug 11, 2014 at 5:44 AM, Joshua Harlow harlo...@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. -Ben ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [oslo] oslo.concurrency repo review
On Mon, Aug 11, 2014 at 11:39 AM, Ben Nemec openst...@nemebean.com wrote: On 08/11/2014 01:02 PM, Yuriy Taraday wrote: On Mon, Aug 11, 2014 at 5:44 AM, Joshua Harlow harlo...@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. 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? 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@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [oslo] oslo.concurrency repo review
On Mon, Aug 11, 2014 at 11:02 AM, Yuriy Taraday yorik@gmail.com wrote: On Mon, Aug 11, 2014 at 5:44 AM, Joshua Harlow harlo...@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 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... 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@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [oslo] oslo.concurrency repo review
On Aug 11, 2014, at 3:26 PM, Joshua Harlow harlo...@outlook.com wrote: On Mon, Aug 11, 2014 at 11:02 AM, Yuriy Taraday yorik@gmail.com wrote: On Mon, Aug 11, 2014 at 5:44 AM, Joshua Harlow harlo...@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. 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. 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
Re: [openstack-dev] [oslo] oslo.concurrency repo review
On 08/11/2014 02:20 PM, Joshua Harlow wrote: On Mon, Aug 11, 2014 at 11:39 AM, Ben Nemec openst...@nemebean.com wrote: On 08/11/2014 01:02 PM, Yuriy Taraday wrote: On Mon, Aug 11, 2014 at 5:44 AM, Joshua Harlow harlo...@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
Re: [openstack-dev] [oslo] oslo.concurrency repo review
On Mon, Aug 11, 2014 at 12:47 PM, Doug Hellmann d...@doughellmann.com wrote: On Aug 11, 2014, at 3:26 PM, Joshua Harlow harlo...@outlook.com wrote: On Mon, Aug 11, 2014 at 11:02 AM, Yuriy Taraday yorik@gmail.com wrote: On Mon, Aug 11, 2014 at 5:44 AM, Joshua Harlow harlo...@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
Re: [openstack-dev] [oslo] oslo.concurrency repo review
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 On 08/07/2014 01:58 PM, Yuriy Taraday wrote: Hello, oslo cores. I've finished polishing up oslo.concurrency repo at [0] - please take a look at it. I used my new version of graduate.sh [1] to generate it, so history looks a bit different from what you might be used to. I've made as little changes as possible, so there're still some steps left that should be done after new repo is created: - fix PEP8 errors H405 and E126; - use strutils from oslo.utils; - remove eventlet dependency (along with random sleeps), but proper testing with eventlet should remain; - fix for bug [2] should be applied from [3] (although it needs some improvements); - oh, there's really no limit for this... I'll finalize and publish relevant change request to openstack-infra/config soon. Looking forward to any feedback! [0] https://github.com/YorikSar/oslo.concurrency [1] https://review.openstack.org/109779 [2] https://bugs.launchpad.net/oslo/+bug/1327946 [3] https://review.openstack.org/108954 ___ 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@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [oslo] oslo.concurrency repo review
Hello, oslo cores. I've finished polishing up oslo.concurrency repo at [0] - please take a look at it. I used my new version of graduate.sh [1] to generate it, so history looks a bit different from what you might be used to. I've made as little changes as possible, so there're still some steps left that should be done after new repo is created: - fix PEP8 errors H405 and E126; - use strutils from oslo.utils; - remove eventlet dependency (along with random sleeps), but proper testing with eventlet should remain; - fix for bug [2] should be applied from [3] (although it needs some improvements); - oh, there's really no limit for this... I'll finalize and publish relevant change request to openstack-infra/config soon. Looking forward to any feedback! [0] https://github.com/YorikSar/oslo.concurrency [1] https://review.openstack.org/109779 [2] https://bugs.launchpad.net/oslo/+bug/1327946 [3] https://review.openstack.org/108954 -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [oslo] oslo.concurrency repo review
On Thu, Aug 7, 2014 at 10:58 PM, Yuriy Taraday yorik@gmail.com wrote: Hello, oslo cores. I've finished polishing up oslo.concurrency repo at [0] - please take a look at it. I used my new version of graduate.sh [1] to generate it, so history looks a bit different from what you might be used to. I've made as little changes as possible, so there're still some steps left that should be done after new repo is created: - fix PEP8 errors H405 and E126; - use strutils from oslo.utils; - remove eventlet dependency (along with random sleeps), but proper testing with eventlet should remain; - fix for bug [2] should be applied from [3] (although it needs some improvements); - oh, there's really no limit for this... I'll finalize and publish relevant change request to openstack-infra/config soon. Here it is: https://review.openstack.org/112666 Looking forward to any feedback! [0] https://github.com/YorikSar/oslo.concurrency [1] https://review.openstack.org/109779 [2] https://bugs.launchpad.net/oslo/+bug/1327946 [3] https://review.openstack.org/108954 -- Kind regards, Yuriy. -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [oslo] oslo.concurrency repo review
LGTM. Plenty of things I could add to your list, but they're all post-import. :-) -Ben On 08/07/2014 01:58 PM, Yuriy Taraday wrote: Hello, oslo cores. I've finished polishing up oslo.concurrency repo at [0] - please take a look at it. I used my new version of graduate.sh [1] to generate it, so history looks a bit different from what you might be used to. I've made as little changes as possible, so there're still some steps left that should be done after new repo is created: - fix PEP8 errors H405 and E126; - use strutils from oslo.utils; - remove eventlet dependency (along with random sleeps), but proper testing with eventlet should remain; - fix for bug [2] should be applied from [3] (although it needs some improvements); - oh, there's really no limit for this... I'll finalize and publish relevant change request to openstack-infra/config soon. Looking forward to any feedback! [0] https://github.com/YorikSar/oslo.concurrency [1] https://review.openstack.org/109779 [2] https://bugs.launchpad.net/oslo/+bug/1327946 [3] https://review.openstack.org/108954 ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
