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.
