# Ansible forces me break PEP8 when writing modules **URL:** \ **Category:** Ansible Project\ **Created:** [March 4, 2016, 4:38pm UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776 "2016-03-04T16:38:16Z")\ **Posts on this page:** 11\ **Page:** 1
**Author:** ![dbarrosop](https://avatars.discourse-cdn.com/v4/letter/d/7ab992/32.png) [@dbarrosop](https://forum.ansible.com/u/dbarrosop)\ **Post date:** [March 4, 2016, 4:38pm UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/1 "2016-03-04T16:38:16Z")
Hello, when writing ansible modules, ansible requires you to add the following line: from ansible.module\_utils. import \* which breaks PEP8. The solution is rather simple and is proposed in the following PR: [https://github.com/ansible/ansible/pull/14805](https://github.com/ansible/ansible/pull/14805) The patch allows you to import either from ansible.module\_utils. import \* or from ansible.module\_utils. import AnsibleModule without breaking anything as you can see in the tests: [https://travis-ci.org/ansible/ansible/builds/113691610](https://travis-ci.org/ansible/ansible/builds/113691610) I understand that line is not a real import and you replace it with ansible code so you can ship a single file to the remote host (which the sanity of the idea could be argued but it’s not the point of this thread). However, you made it look like a real import that breaks PEP8 so I don’t understand why my PR was rejected without further discussion. Specially when the patch is so simple and transparent and allows people to write PEP8 compliant code. Thanks for your time. David Barroso ---
**Author:** ![Brian\_Coca](https://avatars.discourse-cdn.com/v4/letter/b/6de8d8/32.png) [@Brian\_Coca](https://forum.ansible.com/u/Brian_Coca)\ **Post date:** [March 4, 2016, 5:30pm UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/2 "2016-03-04T17:30:51Z")
First, its not a pep8 violation, having the import at the bottom is but that can be considered again as ‘not a violation’ as it improves redability when errors happen (line number line up). 2nd it is not a real import and is already confusing, I personally much preferred the \<\< MODULES\_BASIC \>\> way as it was clear that this is NOT a python import. Your change makes it even more misleading as it implies a restricted import which is not true. ---
**Author:** ![alexey.vazhnov](https://avatars.discourse-cdn.com/v4/letter/a/b487fb/32.png) [@alexey.vazhnov](https://forum.ansible.com/u/alexey.vazhnov)\ **Post date:** [March 5, 2016, 6:04am UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/3 "2016-03-05T06:04:38Z")
And this is bad. This can alienate new developers. I like Python because it have formatting standards. When I open Ansible source files in my VIM with installed vim-syntastic, I see too much bad formatting. And I begin formatting fixing at first, instead of writing/fixing code. Please, let us possibility to fix formatting issues. ---
**Author:** ![dbarrosop](https://avatars.discourse-cdn.com/v4/letter/d/7ab992/32.png) [@dbarrosop](https://forum.ansible.com/u/dbarrosop)\ **Post date:** [March 7, 2016, 8:02am UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/4 "2016-03-07T08:02:13Z")
> having the import at the bottom is but that can be considered again as ‘not a violation’ That’s not the issue I am trying to solve with that PR. > 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. > 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). > Last, but not least, we don’t enforce PEP8 nor are particularly keen on following it blindly as many others do. You can follow any coding style that you prefer. It’s your decision. I decided to write PEP8 compliant code and you are forcing me to write code that is not compliant by forcing me to write a wildcard import. ---
**Author:** ![Toshio\_Kuratomi](https://avatars.discourse-cdn.com/v4/letter/t/41988e/32.png) [@Toshio\_Kuratomi](https://forum.ansible.com/u/Toshio_Kuratomi)\ **Post date:** [March 7, 2016, 5:54pm UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/5 "2016-03-07T17:54:46Z")
> > 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.\ import \*" line to mean "Insert the contents of \.py here" but it would be worse if ansible \*also\* reused "from ansible.module\_utils.\ import \" 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.\ import \*" is bad syntax. So we'd be willing to accept a pull request that added an alternative syntax. ansible already uses b"#\<\\>" as an older way to include basic.py. We'd probably want to build on this syntax. Maybe something like b"#\<\\>" (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](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](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](https://groups.google.com/forum/#!search/d3592c1c9ef37ca54db0/ansible-devel/Tv_9GXnDJRI/XitYR7erCwAJ) -Toshio ---
**Author:** ![dbarrosop](https://avatars.discourse-cdn.com/v4/letter/d/7ab992/32.png) [@dbarrosop](https://forum.ansible.com/u/dbarrosop)\ **Post date:** [March 9, 2016, 8:53am UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/6 "2016-03-09T08:53:16Z")
Hello Toshio, thanks for the detailed explanation. Is there any ETA for the ziploader? Thanks! David ---
**Author:** ![Toshio\_Kuratomi](https://avatars.discourse-cdn.com/v4/letter/t/41988e/32.png) [@Toshio\_Kuratomi](https://forum.ansible.com/u/Toshio_Kuratomi)\ **Post date:** [March 9, 2016, 4:36pm UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/7 "2016-03-09T16:36:09Z")
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 ---
**Author:** ![dbarrosop](https://avatars.discourse-cdn.com/v4/letter/d/7ab992/32.png) [@dbarrosop](https://forum.ansible.com/u/dbarrosop)\ **Post date:** [March 9, 2016, 8:56pm UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/8 "2016-03-09T20:56:18Z")
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. ---
**Author:** ![Toshio\_Kuratomi](https://avatars.discourse-cdn.com/v4/letter/t/41988e/32.png) [@Toshio\_Kuratomi](https://forum.ansible.com/u/Toshio_Kuratomi)\ **Post date:** [March 23, 2016, 1:47am UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/9 "2016-03-23T01:47:35Z")
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](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 ---
**Author:** ![Toshio\_Kuratomi](https://avatars.discourse-cdn.com/v4/letter/t/41988e/32.png) [@Toshio\_Kuratomi](https://forum.ansible.com/u/Toshio_Kuratomi)\ **Post date:** [April 1, 2016, 9:55pm UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/10 "2016-04-01T21:55:18Z")
Another update on ziploader: There's now a PR! [https://github.com/ansible/ansible/pull/15246](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 ---
**Author:** ![dbarrosop](https://avatars.discourse-cdn.com/v4/letter/d/7ab992/32.png) [@dbarrosop](https://forum.ansible.com/u/dbarrosop)\ **Post date:** [April 3, 2016, 6:08pm UTC](https://forum.ansible.com/t/ansible-forces-me-break-pep8-when-writing-modules/20776/11 "2016-04-03T18:08:43Z")
That’s great! Thanks.