Asciinema link: https://asciinema.org/a/131481

**New!**  wiki page for coordinating work: https://github.com/ansible/community/wiki/Testing:-boilerplate,-wildcard-imports,-and-get_exception

**Important Addendum**

* To find what files need modifying, please use this wiki page:
https://github.com/ansible/community/wiki/Testing:-boilerplate,-wildcard-imports,-and-get_exception
instead of the information in the asciinema video.
* There are now three tests for these cleanups rather than one.
   * boilerplate.sh
   * no-get-exception.sh
   * no-wildcard-import.sh


Hi, this is to a short asciicast to show how to help out with three cleanups
of the ansible code base

If you're going to work on this, please coordinate so that we don't step on
each others work.  Talk to abadger1999 on either irc.freenode.net or on the
slack channel for ansible (if you have access)

There are three cleanups that we're doing right now:

* Remove wild card imports (from ansible.modules_utils.basic import *)
* Removing get_exception in favor of python3/python-2.6+ syntax
  * For this cleanup, I also cleanup module.fail_json() calls since I have to
    touch the code around them anyways::

        try:
            do_something()
        except Exception:
            e = get_exception()
            module.fail_json(msg=str(e))

   Replace with::
        try:
            do_something()
        except Exception as e:
            module.fail_json(msg=to_native(e), exception=traceback.format_exc())

* Shorten GPL header and add boilerplate.  Replacing something like this::

    #!/usr/bin/python
    # -*- coding: utf-8 -*-

    # (c) 2014, Anders Ingemann <aim@secoya.dk>
    #
    # This file is part of Ansible
    #
    # Ansible is free software: you can redistribute it and/or modify
    # it under the terms of the GNU General Public License as published by
    # the Free Software Foundation, either version 3 of the License, or
    # (at your option) any later version.
    #
    # Ansible is distributed in the hope that it will be useful,
    # but WITHOUT ANY WARRANTY; without even the implied warranty of
    # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    # GNU General Public License for more details.
    #
    # You should have received a copy of the GNU General Public License
    # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
    #

    ANSIBLE_METADATA = {'metadata_version': '1.0',
                        'status': ['preview'],
                        'supported_by': 'community'}

  with something like this::

    #!/usr/bin/python
    # -*- coding: utf-8 -*-
    # (c) 2014, Anders Ingemann <aim@secoya.dk>
    # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)

    from __future__ import absolute_import, division, print_function
    __metaclass__ = type


    ANSIBLE_METADATA = {'metadata_version': '1.0',
                        'status': ['preview'],
                        'supported_by': 'community'}

== Step 1 Finding something to work on ==

* Look on the wiki https://github.com/ansible/community/wiki/Testing:-boilerplate,-wildcard-imports,-and-get_exception
* You can also coordinate with Toshio, abadger1999 on irc.freenode.net

## **EDIT** Removed some steps from the asciinema video that are no longer
## relevant.  Use the wiki page instead

* For this training I want to find a directory that has both get_exception porting and
  wildcard imports
* We'll choose monitoring
* Now we need to enter the monitoring directory and work on the code:
* Here are the two greps I use to discover just how much work there is in a directory:
  * Wildcard imports
    * grep -r 'import \*' .
  * get_exception removal
    * grep -r 'get_exception' .
* We'll choose to work on logicmonitor_facts.py since it needs both import * fixes and get_exception fixes

== Get helper script for removing wildcard imports ==

* We can run a script to help us take care of the wildcards.  It's not perfect but it cuts down significantly on the amount of work to do:
  * git clone https://github.com/abadger/dewildcard/
  * You'll also need pyflakes installed:
    * pip3 install --user --upgrade pyflakes
    * I installed the python3 version but for this script it shouldn't matter if you have py2 or py3 versions
  * And you'll also need the script somewhere in your PATH
    * cp dewildcard/dewildcard ~/bin/
  * For the script to work, you'll need to have the ansible library available to use:
    * source hacking/env-setup


== Fixing a module ==

* Okay, now we need to run the script on some of the files with problems.
  I am just going to specify one file but if you want, you can specify all of the files that you are going to work on.  dewildcard can handle multiple files
  
  * dewildcard logicmonitor_facts.py
  * git diff .
  
* As you can see, the script did a good job on the ansible.module_utils.urls
  import (only one thing left)
* ....But it did a mediocre job on the module_utils.basic import... We
  probably are not using all of those imports in this file.
* The imports also need some cleanup:
  * The script could not detect that there was a duplicate open_url import next.
  * It has these parenthesis even for something that has just one entry.
  * It didn't move the imports to the right location in the file
* Now for the manual cleanup!

=== Fixing boilerplate ===

* First thing when we open the file is to modify the boilerplate.
* Copy the header-boilerplate.txt file from this gist somewhere where you can
  copy and paste it easily into your editor.
  * https://gist.github.com/abadger/35f6918ac1ce48f97cc173c35a718d76

  * In vim, the way to do that is to highlight the text in visual mode (Use
    Capital V.  Then arrows to highlight the block of text and then: ("ly)
    * The (") selects a named register
    * The (l) is the register name I've chosen.
    * The (y) means to yank the highlighted text into the register.

* Go to the Copyright line.  We want to paste our new text just after that.
  * To paste from that register in vim, press ("lp)
    * (") Means select a named register
    * (l) is the register name
    * (p) means paste after the line that I'm on.

* I then delete the old license header.
  * In vim, I used (capital V) to enter Visual mode, used the arrows to
    highlight that whole block of text, and then (d) to delete the highlighted
    text.
* I leave two blank lines here (between __metaclass__ = type and ANSIBLE_METADATA) to keep pep8 happy
*  That's the boilerplate done.  Now for imports!

=== Fixing wildcard imports ===

*  Find the section that has the imports
   * In vim I used  (/import) to find the import lines.
* Notice that this module is a hybrid: some of the imports are just below the
  documentation where they are supposed to be but others are elsewhere.  We're
  going to be coming back to the "right place for imports" frequently so
  figure out how to save that location in your editor
  * In vim press (:ma a<ENTER>)  to drop a marker here
    * (:) Colon starts entering a command that has to be termineated with <ENTER>
    * (ma) says I'm going to name a marker at this line
    * (a) is the name I'm assigning to the marker
    * (<ENTER>) submits the command

* Then I go to the bottom of the file to see what imports are there
  * In vim: (:$<RETURN>)
    * (:) for another multi-character command
    * ($) means move to the last line of the file
    * (<ENTER) to submit the command

* There's imports down at the bottom so we're going to move them where they're
  supposed to be.
* Cut the imports from here and paste them in their proper location
  * In vim: Press (V) to enter Visual mode, select the whole block of text
    that has the imports using the arrow keys
  * (d) [for delete] to cut the text into a temporary register
  * Go back to the correct import section with ('a)
    * (') Means, move to a marker
    * (a) is the name of the marker I assigned earlier
  * Put the cursor in the right area and press (p) to paste the lines

* Next we're going to use pyflakes on the command line to figure out which of
  those imports are supposed to be there
* pyflakes logicmonitor_facts.py
* You can see in the output that there's quite a few things wrong.  You can
  fix everything if you know what you're doing but if not, just focus on the
  problems caused by our wildcard cleanup.  You can tell these by line number
  and by the import having ansible.module_utils in it.
* Most of these are going to be "imported but unused" because the dewildcard
  script failed to get rid of all the extra symbols that are in
  module_utils.basic.  Go through the logic_monitor_facts.py file and remove
  all of those.
* If there's other modules in here (json, types) or things that should be
  imported from a different module (to_native should come from
  ansible.module_utils._text, reduce should come from
  ansible.module_utils.six, etc), remove those as well and make sure they're
  imported elsewhere.
* Looks like json was already imported but not where it was supposed to
  be.  Also, the json import is using old code to handle python-2.4 and 2.5...
  We can get rid of all of that.
* Before we go on, get rid of the import of get_exception() as the third
  cleanup is to get rid of that anyway.
* Then go ahead and make the imports pretty and pep8 compliant
* Remember imports are supposed to be alphabetical and grouped by (stdlib,
  third party, ansible modules)
  * To combine lines in vim I'm using the join operation: (capital J).  Join
    combines the line you're on with the next one down with a space in the
    middle.  I'll be using that a lot when I modify get_exception as well)
* Back to the command window to rerun pyflakes
* Some more cleanups there.  We removed HAS_LIB_JSON when we got rid of the
  python-2.4 compat code.  So we can remove the section of code in main() as
  well.
* now, we're done with wildcards.  Let's do get_exception!

=== Removing get_exception ===

* Back to the command window to run pyflakes.  We can see that pyflakes
  detects the places that need changing (undefined name 'get_exception')
* However, it's easiest to just search for get_exception and keep fixing the locations 
  * Type (/get_exception) in vim to search
* first thing I do is move naming of the exception into the except and
  removing the get_exception() call:
  - except:
  -     e = get_exception()
  + except Exception as e:
* Next, you probably saw that in this example, the exception had a different
  name before?  So the next thing to do is fix this.
* The exception-formatting.txt from the gist
  https://gist.github.com/abadger/35f6918ac1ce48f97cc173c35a718d76 is a good
  saved string for that.  It's not perfect, though, you'll have to tweak it to fit in with all circumstances
* First paste it in and then edit the code to make use of it.
* It does the following:
  * Uses to_native instead of str() to turn the exception into a string.  This
    won't traceback in case there are non-ascii characters in the string
    representation ofthe exception.
  * Includes the exception argument so that the traceback is passed back to
    the controller.  In a few cases this isn't necessary but in most you'll be
    happy to have it to debug when something goes wrong.
* That looks like all of the get_exceptions in the file!  So are we done?  Not
  yet, first we have to check some things

== Checking our work ==

=== pyflakes ===

* Run pyflakes again to see if we've broken anything obvious with the new code
  (syntax errors, typoed variable names, and....)
* Ah ha!  We have used to_native and traceback but now we have to add those to
  the imports.  Navigate back to that section
  * ('a) if you marked it in vim
* The to_native-import.txt  from the gist is good for inserting the to_native import.
  Sometimes you'll have to work it in to an existing import of
  module_utils._text instead but it comes in handy most of the time.
* Aslo add import traceback in the stdlib portion of the imports
* I'm also going to remove the unused types import so pyflakes has no other
  issues to show.  Note that it's not always safe to remove imports.  Ask if
  you have questions.
* Run pyflakes again and it's clean!  Are we done now?

=== from __future__ import division ===

* Just a few more things to check.  Remember the from __future__ imports?
  absolute_import, division, print_function?
* We need to verify that those are safe to add to the code and fix fix any code
  that won't work.  (For module code, the only one that normally is a problem
  is division).
* First let's grep the code for potential division problems.
  * grep -r '\/' logicmonitor_facts.py
* Look for any places that are actually about division.  In this example, there
  aren't any so it's fine
* If there was some this is what we'd have to do:

  - retry_times = retry_seconds / 60
  + retry_times = retry_seconds // 60
    some_rest_api('verb', retries=retry_times)

* In python2 without from __future__ import division, if you have two ints and
  divide them you get another integer:  3/2 == 1.  In python3 or in python2
  with from __future__ import division, you get a float: 3/2 == 1.5.
* In some cases this is find but in some cases you need an int (for instance,
  here we're passing it to an external  REST API so we don't want to change it
  to a flost.
* Use "//" to perform integer division instead of float division.
* If in doubt, use the // operator.  the code's working on python2 right now,
  so we can just continue to do the same thing and it should still work.

=== from __future__ import print_function ===

* grep -r 'print' logicmonitor_facts.py
* You should see some output but nothing that's actually using the print
  keyword (no code like this::

    print "hello %s" % username

* Modules shouldn't be doing that anyway... If you see it, you probably want
  to remove that line or substitute a call to module.log("hello %s"
  % username) instead.

=== from __future__ import absolute_import ===

* This makes relative import syntax available on Python2.
* Modules can't make relative imports (we run them in a directory without
  other python modules) so you don't have to worry about this one.

* If you start working on module_utils, talk to me about what you might need
  to look for

=== Recording your changes ===

* the boilerplate test only limits on directories right now.  So you'll
  currently have to fix all the modules in a particular directory to need an
  update there.  Pretend like we just fixed all of monitoring.  This is what
  you do
* Once all of that's done, modify the test files to stop excluding those
  directories from being tested and remove the directory from the list to be
  processed and open a PR.

  * boilerplate.sh for from __future__ and one-line GPL changes
  * no-get-exception.sh for removal of get_exception()
  * no-wildcard-import.sh for removal of wildcard imports

* Contact me to merge it.

* All done!  thanks for watching, learning, and I look forward to PRs soon!
* I'll upload a better formatted copy of these notes to https://toshio.fedorapeople.org/ansible/cleanup-wildcard.txt
* And remember, I'm abadger1999 on slack and irc for any questions