Update on ziploader: After going down a rabbit hole of related bugs, I've had some time to work on jimi-c's initial branch. The branch I have now[1]_ is passing unittests and integration tests (at least through mysql... I have an issue on my system that's preventing mysql from passing and I've been too busy coding to address that).
.. [1]_: https://github.com/ansible/ansible/compare/devel...abadger:ziploader?expand=1 There's further feature work to do on this so don't take it as finalized. Feel free to complain about things that don't work ;-) The very best way to reach me is to find me on irc.freenode.net abadger1999 in #ansible-devel. Things that are working now: * Modules tested via the integration test suite seem to be working. Other modules (including user's custom modules) are probably working but I did have to make some changes to the module_utils code that some of those rely on so there could be problems. Please let me know. * Non-wildcard imports are currently working although no official modules have been ported over yet. I'd like to do that so that we can start checking the modules for things like undefined variables but there's some objections to doing that for 2.1.0. We'll have t ohave a discussion and make a decision once the ziploading code is basically feature complete. * Tracebacks from modules should take a step forward with this code. Before, by convention, modules had their module_utils imports at the bottom of the file. This allowed line numbers in tracebacks to match with the line numbers inside of the module file but tracebacks generated from module_utils code would have line numbers that didn't equate to anything on the controller (only in the generated module). The new ziploader code preserves the separation of files so the line numbers should be correct. Next things on my list: * Ability to specify the compression method for ziploader modules. python's zipfile module supports no compression and zip-compatible deflate when python is compiled against the zlib library is [Update: Done] * "recursive imports" -- this is to allow imports from module_utils to trigger importing other module_utils code. Currently this is working for current ansible modules in a very hacky manner. the current modules import all of the module_utils code that they use so ziploader knows to include both modules' code. What I want to achieve is module_utils code that doesn't depend on anything special in the module to trigger including its module_utils dependencies. * Other valid python imports. Current code looks for "from ansible.module_utils.foo import bar" in order to trigger inclusion of a module. I want to also include things like import ansible.module_utils.foo, from ansible.module_utils.foo.bar import baz, from ansible.module_utils import foo, bar and maybe even from ansible.module_utils import foo ; from ansible.module_utils import bar. I'm not yet sure that I'll be able to do this (The big question is how big a speed hit we take from all of this when combined with recursive imports) but it would be nice to support the full range of things that python understands as an import with the ziploader code. -Toshio On Wed, Mar 9, 2016 at 12:56 PM, David Barroso <[email protected]> wrote: > Ok, let me know if I can help testing or providing feedback. I am really > interesting on this as I have a lot of modules that I wrote with duplicated > code. Being able to share code amongst them is going to be a huge win. > > On Wed, 9 Mar 2016 at 17:36 Toshio Kuratomi <[email protected]> wrote: >> >> We're hoping to have 2.1 out in late April or May (3-4 months from >> 2.0's release). >> >> I hope to land code in a publically available branch by next week. >> There are some things about it after that I think we (core committers) >> will need to discuss and come to a consensus on before it gets merged >> into devel. >> >> -Toshio >> >> On Wed, Mar 9, 2016 at 12:53 AM, David Barroso <[email protected]> >> wrote: >> > Hello Toshio, >> > thanks for the detailed explanation. Is there any ETA for the ziploader? >> > >> > Thanks! >> > David >> > >> > On Mon, 7 Mar 2016 at 18:54 Toshio Kuratomi <[email protected]> >> > wrote: >> >> >> >> On Mon, Mar 7, 2016 at 12:02 AM, <[email protected]> wrote: >> >> >> >> >> 2nd it is not a real import and is already confusing >> >> > >> >> > I know it's not a real import but ansible made it look like a real >> >> > import so >> >> > coding styles should apply. >> >> > >> >> >> >> Like you I really hate wildcard imports. They make static analysis of >> >> the code harder and pollute the namespace. However... >> >> >> >> >> Your change makes it even more misleading as it implies a restricted >> >> >> import which is not true >> >> > >> >> > My change only makes a completely arbitrary line that we are forced >> >> > to >> >> > add >> >> > to comply to coding standards. The misleading part comes from making >> >> > a >> >> > line >> >> > that looks like an import to do something that is definitively not an >> >> > import >> >> > : ) (I wonder how many people contributing to ansible know that line >> >> > is >> >> > actually not an import). >> >> >> >> Brian's point is that the way module replacer works, the effect of a >> >> real "import *" is closer to what actually happens: all of the names >> >> defined in the other module are then available in the module and they >> >> overwrite other globals that were defined earlier in the module. >> >> Contrariwise doing an import AnsibleModule is farther from the truth >> >> as it makes things like the overwriting of global names less visible >> >> to someone who doesn't know about this being, essentially, a >> >> preprocessor directive instead of an actual import. >> >> >> >> So it's bad that ansible reuses a "from ansible.module_utils.<NAME> >> >> import *" line to mean "Insert the contents of <NAME>.py here" but it >> >> would be worse if ansible >> >> *also* reused "from ansible.module_utils.<NAME> import <FOO>" to mean >> >> the same thing. This is a case of doing one wrong thing being less >> >> bad than doing two wrong things. >> >> >> >> == Ways forward == >> >> >> >> Although we won't take your PR, there are two things that could make >> >> the problem you're describing better. First, I'm working on ziploader >> >> for 2.1. That will allow people to write modules that use actual >> >> python imports instead of these preprocessor directives. When I >> >> spec'd it out, I anticipated a lot of work because some things in >> >> module_utils and in actual modules assume that everything is in one >> >> file and therefore some things are going to be global. Those things >> >> won't work under ziploader and will either need special handling or >> >> porting in order to use ziploader versions of the module_utils >> >> modules. However, jimi-c wrote a proof-of-concept that will at least >> >> load module_utils/basic.py This week I'm going to be looking at >> >> whether that generically works around my worries about backwards >> >> compatibility or if it's just that basic.py is cleaner in this respect >> >> than other module snippets. then working on getting a POC merged into >> >> a branch of ansible/ansible/. My goal is to have at minimum, the >> >> ziploader framework in 2.1.0. If possible, to also have module_utils >> >> adapted to work with ziploader (as noted, this may require porting and >> >> changing the APIs of things in module_utils somewhat. If so, I'll >> >> probably have to create a new namespace for these module_utils to live >> >> within). >> >> >> >> Second, no one inside of ansible is working on this but bcoca and I >> >> have both been amenable to the idea that "from >> >> ansible.module_utils.<NAME> import *" is bad syntax. So we'd be >> >> willing to accept a pull request that added an alternative syntax. >> >> ansible already uses b"#<<INCLUDE_ANSIBLE_MODULE_COMMON>>" as an older >> >> way to include basic.py. We'd probably want to build on this syntax. >> >> Maybe something like b"#<<INCLUDE_ANSIBLE_MODULE basic.py>>" (where >> >> basic.py can be substituted with any python filename inside of the >> >> ansible/module_utils directory.) We aren't all thinking along the >> >> same lines (yet) about whether we'd want to use that alternate syntax >> >> inside of the modules we ship in core and extras or just allow it for >> >> third-party custom modules. I think we'll be better able to make a >> >> decision about that once we have ziploader in place and see whether >> >> we'll have many modules that have to use module_replacer to include >> >> snippets from module_utils or if almost everything works with >> >> ziploader. >> >> >> >> ziploader links: >> >> * jimi-c's proof of concept: >> >> >> >> >> >> https://github.com/ansible/ansible/compare/devel...jimi-c:module_utils_commmon_loading >> >> * Some sample code from me, mostly contained in jimi-c's POC: >> >> https://gist.github.com/abadger/d3592c1c9ef37ca54db0 >> >> * An earlier mailing list post I made on the subject: >> >> >> >> >> >> https://groups.google.com/forum/#!search/d3592c1c9ef37ca54db0/ansible-devel/Tv_9GXnDJRI/XitYR7erCwAJ >> >> >> >> -Toshio >> >> >> >> -- >> >> You received this message because you are subscribed to a topic in the >> >> Google Groups "Ansible Project" group. >> >> To unsubscribe from this topic, visit >> >> >> >> https://groups.google.com/d/topic/ansible-project/0bwHEFfKOro/unsubscribe. >> >> To unsubscribe from this group and all its topics, send an email to >> >> [email protected]. >> >> To post to this group, send email to [email protected]. >> >> To view this discussion on the web visit >> >> >> >> https://groups.google.com/d/msgid/ansible-project/CAG9juEpn%3DdYfHwoGxRnHU2zA3KD33iibeCtRmBvshxhJKcDUgw%40mail.gmail.com. >> >> For more options, visit https://groups.google.com/d/optout. >> > >> > -- >> > You received this message because you are subscribed to the Google >> > Groups >> > "Ansible Project" group. >> > To unsubscribe from this group and stop receiving emails from it, send >> > an >> > email to [email protected]. >> > To post to this group, send email to [email protected]. >> > To view this discussion on the web visit >> > >> > https://groups.google.com/d/msgid/ansible-project/CAObAkcY4Lf8b%2BF4ogQPS7OcOHKKm3hMS9f6pYvfqKMZTuFuNXQ%40mail.gmail.com. >> > >> > For more options, visit https://groups.google.com/d/optout. >> >> -- >> You received this message because you are subscribed to a topic in the >> Google Groups "Ansible Project" group. >> To unsubscribe from this topic, visit >> https://groups.google.com/d/topic/ansible-project/0bwHEFfKOro/unsubscribe. >> To unsubscribe from this group and all its topics, send an email to >> [email protected]. >> To post to this group, send email to [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/ansible-project/CAG9juEqef8xTVxzRntegt6HRdz1R%2Bwb-1edD4i0atA%3DUe%2BROzQ%40mail.gmail.com. >> For more options, visit https://groups.google.com/d/optout. > > -- > You received this message because you are subscribed to the Google Groups > "Ansible Project" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/ansible-project/CAObAkcZxw-nT%2BMAtySeDK31jTB1VJjh4ZCCbKbaD3isxXFQuDw%40mail.gmail.com. > > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "Ansible Project" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/ansible-project/CAG9juEo6KC0ER_0msL1eWLyqcaX2Fc9XH25%3Dni0fu-rRa8koug%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
