That's great! Thanks. On Fri, 1 Apr 2016 at 23:55 Toshio Kuratomi <[email protected]> wrote:
> Another update on ziploader: There's now a PR! > https://github.com/ansible/ansible/pull/15246 > > As explained in the PR, this should have feature parity with module > replacer (the current way that modules are constructed for shipment > over the wire). A few things are not 100% compatible but it should be > extremely rare that a module is actually using something that falls > into that category. I've tried to include how to port if the module > does fall into that. > > I'll be working on recursive imports and such next. I'm pretty sure > that those will also be ready to merge into the main repo before > feature freeze but wanted to make sure we have a checkpoint to revert > to if something unforeseen crops up. > > -Toshio > > On Tue, Mar 22, 2016 at 6:47 PM, Toshio Kuratomi <[email protected]> > wrote: > > 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 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/CAG9juEr%2BGToFt6CSL7PepKfM2gNiaRabzWmH_%2ByzgpcVX1qNdQ%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/CAObAkcbo1cgPKdsX198%3D0GnpK2CnEOXEAD3sUZODUXyQeCiaTw%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
