Re: [openstack-dev] [neutron] monkey patching strategy
Hi, The general discussion is going in https://review.openstack.org/#/c/148318/. My opinions on the following specific topics inline. 2015-02-18 0:04 GMT+09:00 Ihar Hrachyshka ihrac...@redhat.com: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, response was huge so far :) so to add more traction, I have a question for everyone. Let's assume we want to move entry points for all services and agents into neutron/cmd/... If so, - - Do we want all existing tools stored in the path to be monkey patched too? I start to think neutron/cmd/eventlet/* can be an option. The single place to apply monkey-patch is nice, but I am not sure it is good that all commands in neutron/cmd are monkey-patched. At now we only have small tools in cmd and they are not affected even if monkey-patched. If we have non-eventlet version of commands/agents, where should we place them? I believe that a good naming convention helps new/most developers understand the code base. I would say 'yes', to make sure we run our unit tests in the same environment as in real life; Regarding our unit tests, I am not sure what way is good. At now most codes are run with monkey-patched version of stdlib, so apply monkey-pathc in neutron/tests/__init__.py sounds good. However, what can we do if some non-eventlet modules are available? These modules should not be monkey-patched. - - Which parts of services we want to see there? Should they include any real main() or register_options() code, or should they be just a wrappers to call actual main() located somewhere in other parts of the tree? I lean toward leaving just a one liner main() under neutron/cmd/... that calls to 'real' main() located in a different place in the tree. My vote is for one-liner main(). More precisely, codes which are directly related to eventlet monkey-patch should be placed. It seems config options are not related to event monkey-patch directly. If we have non-eventlet version of commands/agents, real 'main' can stay in the same place and we can just remove the corresponding part from neutron/cmd/. Thanks, Akihiro Comments? /Ihar On 02/13/2015 04:37 PM, Ihar Hrachyshka wrote: On 02/13/2015 02:33 AM, Kevin Benton wrote: Why did the services fail with the stdlib patched? Are they incompatible with eventlet? It's not like *service entry points* are not ready for neutron.* to be monkey patched, but tools around it (flake8 that imports neutron.hacking.checks, setuptools that import hooks from neutron.hooks etc). It's also my belief that base library should not be monkey patched not to put additional assumptions onto consumers. (Though I believe that all the code in the tree should be monkey patched, including those agents that currently run without the library patched - for consistency and to reflect the same test environment for unit tests that will be patched from neutron/tests/__init__.py). /Ihar __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJU41iWAAoJEC5aWaUY1u57zBYIAIuobIYMZ1NJmm+7sV+NW6LS ZS4PNKlwcYRrdfArGliUq7GLVi/ZRNPNgilF9RIJXQAiOXEc6PmKqpKw1JnwkQ7v l3/NeciYmkMhSNRv1vIrOBHegAYx9Js6o2lOBCF7BFKIpu88OsC95oobcLGtcrYU BxoBUM7DYvHssDhRp3NujNbyMrRkg4roer7+4qGE3a449tv4xViTcoUWg5MoNalY vD1ld/Gg8LfKPt7v7FbF2YnHkMG+UJSk47rRd0yv9KGABS69TkNuvJXeJ14sgw0O YqIY3oMO0nza+T8tdQGTrYv9N4rWOMFsJMyrOLIvoUyq526QQZ/K7Hrijj1IQjE= =ZtVP -END PGP SIGNATURE- __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Akihiro Motoki amot...@gmail.com __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] monkey patching strategy
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/17/2015 04:19 PM, Salvatore Orlando wrote: My opinions inline. On 17 February 2015 at 16:04, Ihar Hrachyshka ihrac...@redhat.com mailto:ihrac...@redhat.com wrote: Hi, response was huge so far :) so to add more traction, I have a question for everyone. Let's assume we want to move entry points for all services and agents into neutron/cmd/... If so, I don't have anything again this assumption. Also it seems other projects are already doing it this way so there is no divergence issue here. - Do we want all existing tools stored in the path to be monkey patched too? I would say 'yes', to make sure we run our unit tests in the same environment as in real life; I say yes but mildly here. If you're referring to the tools used for running flake8 or unit tests in theory it should not really matter whether they're patched or not. However, I'm aware of unit tests which spawn eventlet threadpools, so it's definitely better to ensure all these tools are patched. No, I mean ovs_cleanup, sanity_check, usage_audit that are located in the neutron/cmd path but not patched. - Which parts of services we want to see there? Should they include any real main() or register_options() code, or should they be just a wrappers to call actual main() located somewhere in other parts of the tree? I lean toward leaving just a one liner main() under neutron/cmd/... that calls to 'real' main() located in a different place in the tree. My vote is for the one-liner. Comments? /Ihar On 02/13/2015 04:37 PM, Ihar Hrachyshka wrote: On 02/13/2015 02:33 AM, Kevin Benton wrote: Why did the services fail with the stdlib patched? Are they incompatible with eventlet? It's not like *service entry points* are not ready for neutron.* to be monkey patched, but tools around it (flake8 that imports neutron.hacking.checks, setuptools that import hooks from neutron.hooks etc). It's also my belief that base library should not be monkey patched not to put additional assumptions onto consumers. (Though I believe that all the code in the tree should be monkey patched, including those agents that currently run without the library patched - for consistency and to reflect the same test environment for unit tests that will be patched from neutron/tests/__init__.py). /Ihar __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJU429PAAoJEC5aWaUY1u57IkcH/1HCRHYzeXfWE3I3ZamAJufI /1/CPcrzW3w3pJebfSLXDNbdv9ziQXwZogcM7JcDMeeYfWA42OexhFQJqerkoJnr oL3eqUOgh19pgVUgAah1n7yQEHxyzbnVR0TcdVOvMlxno8I3hUXy78WvBWQPYIpr NRDSbT+SiQv/OP6/zTkKLkk2SA88lJlKQpGg5Q0iRQTnqiNtF3REBdUTM/32aJyh h9ZxmR8wyrXJv6oEUfGj210vJHUvmPHk3SsH2udQjCG0MdbIQTIZJwgzMVxD4aIs 3uQ9ONqn8Fd2LKzfawHVwT0azd6kCkQjEalZx9Tn/58NPuQ4WERhHcCzBT8pNyI= =DGy3 -END PGP SIGNATURE- __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] monkey patching strategy
My opinions inline. On 17 February 2015 at 16:04, Ihar Hrachyshka ihrac...@redhat.com wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, response was huge so far :) so to add more traction, I have a question for everyone. Let's assume we want to move entry points for all services and agents into neutron/cmd/... If so, I don't have anything again this assumption. Also it seems other projects are already doing it this way so there is no divergence issue here. - - Do we want all existing tools stored in the path to be monkey patched too? I would say 'yes', to make sure we run our unit tests in the same environment as in real life; I say yes but mildly here. If you're referring to the tools used for running flake8 or unit tests in theory it should not really matter whether they're patched or not. However, I'm aware of unit tests which spawn eventlet threadpools, so it's definitely better to ensure all these tools are patched. - - Which parts of services we want to see there? Should they include any real main() or register_options() code, or should they be just a wrappers to call actual main() located somewhere in other parts of the tree? I lean toward leaving just a one liner main() under neutron/cmd/... that calls to 'real' main() located in a different place in the tree. My vote is for the one-liner. Comments? /Ihar On 02/13/2015 04:37 PM, Ihar Hrachyshka wrote: On 02/13/2015 02:33 AM, Kevin Benton wrote: Why did the services fail with the stdlib patched? Are they incompatible with eventlet? It's not like *service entry points* are not ready for neutron.* to be monkey patched, but tools around it (flake8 that imports neutron.hacking.checks, setuptools that import hooks from neutron.hooks etc). It's also my belief that base library should not be monkey patched not to put additional assumptions onto consumers. (Though I believe that all the code in the tree should be monkey patched, including those agents that currently run without the library patched - for consistency and to reflect the same test environment for unit tests that will be patched from neutron/tests/__init__.py). /Ihar __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJU41iWAAoJEC5aWaUY1u57zBYIAIuobIYMZ1NJmm+7sV+NW6LS ZS4PNKlwcYRrdfArGliUq7GLVi/ZRNPNgilF9RIJXQAiOXEc6PmKqpKw1JnwkQ7v l3/NeciYmkMhSNRv1vIrOBHegAYx9Js6o2lOBCF7BFKIpu88OsC95oobcLGtcrYU BxoBUM7DYvHssDhRp3NujNbyMrRkg4roer7+4qGE3a449tv4xViTcoUWg5MoNalY vD1ld/Gg8LfKPt7v7FbF2YnHkMG+UJSk47rRd0yv9KGABS69TkNuvJXeJ14sgw0O YqIY3oMO0nza+T8tdQGTrYv9N4rWOMFsJMyrOLIvoUyq526QQZ/K7Hrijj1IQjE= =ZtVP -END PGP SIGNATURE- __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] monkey patching strategy
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, response was huge so far :) so to add more traction, I have a question for everyone. Let's assume we want to move entry points for all services and agents into neutron/cmd/... If so, - - Do we want all existing tools stored in the path to be monkey patched too? I would say 'yes', to make sure we run our unit tests in the same environment as in real life; - - Which parts of services we want to see there? Should they include any real main() or register_options() code, or should they be just a wrappers to call actual main() located somewhere in other parts of the tree? I lean toward leaving just a one liner main() under neutron/cmd/... that calls to 'real' main() located in a different place in the tree. Comments? /Ihar On 02/13/2015 04:37 PM, Ihar Hrachyshka wrote: On 02/13/2015 02:33 AM, Kevin Benton wrote: Why did the services fail with the stdlib patched? Are they incompatible with eventlet? It's not like *service entry points* are not ready for neutron.* to be monkey patched, but tools around it (flake8 that imports neutron.hacking.checks, setuptools that import hooks from neutron.hooks etc). It's also my belief that base library should not be monkey patched not to put additional assumptions onto consumers. (Though I believe that all the code in the tree should be monkey patched, including those agents that currently run without the library patched - for consistency and to reflect the same test environment for unit tests that will be patched from neutron/tests/__init__.py). /Ihar __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJU41iWAAoJEC5aWaUY1u57zBYIAIuobIYMZ1NJmm+7sV+NW6LS ZS4PNKlwcYRrdfArGliUq7GLVi/ZRNPNgilF9RIJXQAiOXEc6PmKqpKw1JnwkQ7v l3/NeciYmkMhSNRv1vIrOBHegAYx9Js6o2lOBCF7BFKIpu88OsC95oobcLGtcrYU BxoBUM7DYvHssDhRp3NujNbyMrRkg4roer7+4qGE3a449tv4xViTcoUWg5MoNalY vD1ld/Gg8LfKPt7v7FbF2YnHkMG+UJSk47rRd0yv9KGABS69TkNuvJXeJ14sgw0O YqIY3oMO0nza+T8tdQGTrYv9N4rWOMFsJMyrOLIvoUyq526QQZ/K7Hrijj1IQjE= =ZtVP -END PGP SIGNATURE- __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] monkey patching strategy
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/13/2015 02:33 AM, Kevin Benton wrote: Why did the services fail with the stdlib patched? Are they incompatible with eventlet? It's not like *service entry points* are not ready for neutron.* to be monkey patched, but tools around it (flake8 that imports neutron.hacking.checks, setuptools that import hooks from neutron.hooks etc). It's also my belief that base library should not be monkey patched not to put additional assumptions onto consumers. (Though I believe that all the code in the tree should be monkey patched, including those agents that currently run without the library patched - for consistency and to reflect the same test environment for unit tests that will be patched from neutron/tests/__init__.py). /Ihar -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJU3hpVAAoJEC5aWaUY1u57nBUIANIjk5j31l2E9+HNvUhiMulP mIa7zB3PNO+XsFHq9DWWUHzKoY5GVYF9DdLN56wbiLCCD7aoR3XZ/euGRm9NnJzf Akb6+qsq/1+qge4zG0C33aBc/lted+1RdVU7aDk8pUpUbWAEW833EqwolXdtg0RF aljsg5U3759MPpZpRV8o/GzHScmTvWenn6hLzXQ2frGFHoR4OpPkEctME1LAmNxs GqfKeK5ST0wVsCimTFMM/BV7zQTQSeiMNYQesnqTh4V3mjaS3UiHJfCQF9KtiJJG AaMEW3wyBAO3ew373oXUMTd4k0HhO34RuK4eznYdq7BFQO6KDjFSC1HtvFJaPCk= =KckY -END PGP SIGNATURE- __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [neutron] monkey patching strategy
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi all, there were some moves recently to make monkey patching strategy sane in neutron. This was triggered by some bugs found when interacting with external oslo libraries [1], and a cross project spec to make eventlet usage sane throughout the project [2]. Specifically, instead of monkey patching stdlib in each of services and agents (and forgetting to do so for some of them [3]), we should monkey patch it as part of a common import (ideally, it would be any neutron.* import). Initially, we've tried to patch it inside neutron/__init__.py [4], but it didn't place nice with some advanced services importing from neutron while not expecting stdlib to be patched, and so was reverted. So an alternative that I currently look into is the Nova way. Specifically, moving all main() functions for all agents and services into neutron/cmd/... and monkey patching stdlib thru neutron/cmd/__init__.py. I've sent a series of patches to do just that [5]. It was rightfully blocked by Mark to seek for broader agreement. I encourage community to say your word on the direction. [1]: https://bugs.launchpad.net/oslo.concurrency/+bug/1418541 [2]: https://review.openstack.org/154642 [3]: http://git.openstack.org/cgit/openstack/neutron/tree/neutron/plugins/mlnx/agent/eswitch_neutron_agent.py [4]: https://review.openstack.org/153699 [5]: https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bug/1418541,n,z Cheers, /Ihar -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJU3P4QAAoJEC5aWaUY1u57A/cH/AuKbkewZy5Z0Hus2m4bClGp 4DJ37ygcY9HwGmJTLpvUyfRcDWnaO9S+6sj28Ebv49MN1w9qJ4MuQmaYA1xsFERb aR6uKgnkiIT0FS8CVjbClEC7gN5elHCe2dcB8cakrk7uUsTJ2LP5A6rdNQqly/uN 2hkdfa1WBcAYMX6raFWD8DJ49R1MhbPr09YXXU9ApoflMY6ZywvNBzwIZEw5gqPO Vpjb9DwevaFZ9kqzjHTrXk47CqOSYS7ZXQjS1bOGUOJFOBqNRLzl2qPX7wkBiA2N 12U4Qe3/3MvWwBig0O+mY2RwN2OtnxhK8X5tP6kbrybyOKLGUe4ZgIlvfQHI33Q= =8pX5 -END PGP SIGNATURE- __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] monkey patching strategy
Why did the services fail with the stdlib patched? Are they incompatible with eventlet? On Thu, Feb 12, 2015 at 11:25 AM, Ihar Hrachyshka ihrac...@redhat.com wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi all, there were some moves recently to make monkey patching strategy sane in neutron. This was triggered by some bugs found when interacting with external oslo libraries [1], and a cross project spec to make eventlet usage sane throughout the project [2]. Specifically, instead of monkey patching stdlib in each of services and agents (and forgetting to do so for some of them [3]), we should monkey patch it as part of a common import (ideally, it would be any neutron.* import). Initially, we've tried to patch it inside neutron/__init__.py [4], but it didn't place nice with some advanced services importing from neutron while not expecting stdlib to be patched, and so was reverted. So an alternative that I currently look into is the Nova way. Specifically, moving all main() functions for all agents and services into neutron/cmd/... and monkey patching stdlib thru neutron/cmd/__init__.py. I've sent a series of patches to do just that [5]. It was rightfully blocked by Mark to seek for broader agreement. I encourage community to say your word on the direction. [1]: https://bugs.launchpad.net/oslo.concurrency/+bug/1418541 [2]: https://review.openstack.org/154642 [3]: http://git.openstack.org/cgit/openstack/neutron/tree/neutron/plugins/mlnx/agent/eswitch_neutron_agent.py [4]: https://review.openstack.org/153699 [5]: https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bug/1418541,n,z Cheers, /Ihar -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJU3P4QAAoJEC5aWaUY1u57A/cH/AuKbkewZy5Z0Hus2m4bClGp 4DJ37ygcY9HwGmJTLpvUyfRcDWnaO9S+6sj28Ebv49MN1w9qJ4MuQmaYA1xsFERb aR6uKgnkiIT0FS8CVjbClEC7gN5elHCe2dcB8cakrk7uUsTJ2LP5A6rdNQqly/uN 2hkdfa1WBcAYMX6raFWD8DJ49R1MhbPr09YXXU9ApoflMY6ZywvNBzwIZEw5gqPO Vpjb9DwevaFZ9kqzjHTrXk47CqOSYS7ZXQjS1bOGUOJFOBqNRLzl2qPX7wkBiA2N 12U4Qe3/3MvWwBig0O+mY2RwN2OtnxhK8X5tP6kbrybyOKLGUe4ZgIlvfQHI33Q= =8pX5 -END PGP SIGNATURE- __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Kevin Benton __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
