Fix issues with tox targets lint & docs (juju#664)

* `lint` target: this ignores the vendored in
  `charmtools/diff_match_patch.py` file which uses completely
  non-standard indenting (2 spaces). It also fixes various issues around
  bare exceptions, indendation and equality tests on types.

* `docs` target: this fixes the custom
  /docs/_extensions/automembersummary.py extension to be able to use a
  more recent version of sphinx. This fixes a long-standing broken
  sphinx docs generation issue.

charmtools/build/__init__.py

@@ -1,5 +1,6 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
+# flake8: noqa
 from __future__ import absolute_import
 
 import charmtools.build.tactics

charmtools/build/builder.py

@@ -124,8 +124,6 @@ def fetch(self):
             # using the revision as the branch so that things do at least
             # build.
             self.branch = self.revision
-            # self.branch = fetcher.get_branch_for_revision(self.directory,
-                                                          # self.revision)
 
         if not self.directory.exists():
             raise BuildError(

charmtools/build/tactics.py

@@ -1132,7 +1132,8 @@ def _add(self, wheelhouse, *reqs):
                               if self.binary_build_from_source else tuple(),
                               '-w', temp_dir, *reqs)
                 else:
-                    self._pip('download', *_no_binary_opts, '-d', temp_dir, *reqs)
+                    self._pip(
+                        'download', *_no_binary_opts, '-d', temp_dir, *reqs)
             except BuildError:
                 log.info('Build failed. If you are building on Focal and have '
                          'Jinja2 or MarkupSafe as part of your dependencies, '

charmtools/bundles.py

@@ -85,7 +85,9 @@ def is_bundle(self):
     def is_v4(self, data=None):
         if data is None:
             data = self.bundle_file()
-        v4_keys = {'applications', 'services', 'relations', 'machines', 'series'}
+        v4_keys = {
+            'applications', 'services', 'relations', 'machines', 'series'
+        }
         bundle_keys = set(data.keys())
         return bool(v4_keys & bundle_keys)
 

charmtools/charms.py

@@ -129,7 +129,7 @@ def check_relation_hooks(self, relations, subordinate, hooks_path):
         for r in relations.items():
             if r[0].startswith('juju-'):
                 self.info('juju-* is a reserved relation name')
-            if type(r[1]) != dict:
+            if not isinstance(r[1], dict):
                 self.err("relation %s is not a map" % (r[0]))
             else:
                 if 'scope' in r[1]:
@@ -379,14 +379,14 @@ def proof(self):
                             with open(readme_path) as r:
                                 readme_content = r.read()
                                 lc = 0
-                                for l in bad_lines:
-                                    if not len(l):
+                                for bl in bad_lines:
+                                    if not len(bl):
                                         continue
                                     lc += 1
-                                    if l in readme_content:
+                                    if bl in readme_content:
                                         err_msg = ('%s includes boilerplate: '
                                                    '%s')
-                                        lint.warn(err_msg % (readme, l))
+                                        lint.warn(err_msg % (readme, bl))
                 except IOError as e:
                     lint.warn(
                         "Error while opening %s (%s)" %
@@ -395,7 +395,7 @@ def proof(self):
                 lint.warn("no README file")
 
             subordinate = charm.get('subordinate', False)
-            if type(subordinate) != bool:
+            if type(subordinate) is not bool:
                 lint.err("subordinate must be a boolean value")
 
             # All charms should provide at least one thing
@@ -977,9 +977,11 @@ def validate_functions(functions, function_hooks, linter):
             continue
         h = os.path.join(function_hooks, k)
         if not os.path.isfile(h):
-            linter.warn('functions.{0}: functions/{0} does not exist'.format(k))
+            linter.warn('functions.{0}: functions/{0} does not exist'
+                        .format(k))
         elif not os.access(h, os.X_OK):
-            linter.err('functions.{0}: functions/{0} is not executable'.format(k))
+            linter.err('functions.{0}: functions/{0} is not executable'
+                       .format(k))
 
 
 def validate_maintainer(charm, linter):
@@ -1027,12 +1029,12 @@ def validate_categories_and_tags(charm, linter):
 
     if 'tags' in charm:
         tags = charm['tags']
-        if type(tags) != list or tags == []:
+        if not isinstance(tags, list) or tags == []:
             linter.warn('Metadata field "tags" must be a non-empty list')
 
     if 'categories' in charm:
         categories = charm['categories']
-        if type(categories) != list or categories == []:
+        if not isinstance(categories, list) or categories == []:
             # The category names are not validated because they may
             # change.
             linter.warn(

charmtools/generators/utils.py

@@ -49,7 +49,7 @@ def apt_fill(package):
         v['summary'] = p.summary
         v['description'] = textwrap.fill(p.description, width=72,
                                          subsequent_indent='  ')
-    except:
+    except Exception:
         log.info(
             "No %s in apt cache; creating an empty charm instead.", package)
         v['summary'] = '<Fill in summary here>'
@@ -69,7 +69,7 @@ def portable_get_maintainer():
 
             if not len(name):
                 name = pwd.getpwuid(os.getuid())[0]
-        except:
+        except Exception:
             name = 'Your Name'
 
     if not len(name):

charmtools/proof.py

@@ -53,7 +53,7 @@ def proof(path, is_bundle, debug):
         except Exception:
             try:
                 c = Bundle(path, debug)
-            except Exception as e:
+            except Exception:
                 return ["FATAL: No bundle.yaml (Bundle) or metadata.yaml "
                         "(Charm) found, cannot proof"], 200
     else:

charmtools/test.py

@@ -112,7 +112,7 @@ def run(self):
             try:
                 t = Orchestra(self, test)
                 t.perform()
-            except:
+            except Exception:
                 self.fails += 1
                 if self.args.set_e:
                     self.log.info('Breaking here as requested by --set-e')
@@ -127,7 +127,7 @@ def run(self):
                 time.sleep(2)
                 try:
                     self.destroy(self.juju_env)
-                except:
+                except Exception:
                     continue
 
         return self.errors, self.fails, self.passes
@@ -163,7 +163,7 @@ def get_environment(self, juju_env):
         except IOError:
             raise  # Do something more clever here?
 
-        if not juju_env in env_yaml['environments']:
+        if juju_env not in env_yaml['environments']:
             raise KeyError('%s does not exist in %s/environments.yaml' %
                            (juju_env, juju_home))
 
@@ -193,7 +193,7 @@ def bootstrap(self, juju_env, wait_for=400):
         except TimeoutError:
             try:
                 self.destroy(self.juju_env)
-            except:
+            except Exception:
                 pass
 
             raise BootstrapUnreliable('Bootstrap timeout after %ss' % wait_for)
@@ -229,7 +229,7 @@ def status(self, juju_env):
         self.log.debug('Running the following: %s' % ' '.join(cmd))
         try:
             output = subprocess.check_output(cmd, env=self.env)
-        except:
+        except Exception:
             self.log.debug('Status command failed, returning nothing')
             return None
 
@@ -327,7 +327,7 @@ def archive_logs(self):
 
         try:
             self.rsync(0, logs[0], os.path.join(logdir, 'bootstrap', ''))
-        except:
+        except Exception:
             self.log.warn('Failed to fetch logs for bootstrap node')
 
         for service in services:
@@ -337,7 +337,7 @@ def archive_logs(self):
                     try:
                         self.rsync(machine, log, os.path.join(logdir, service,
                                                               ''))
-                    except:
+                    except Exception:
                         self.log.warn('Failed to grab logs for %s' % unit)
 
     def print_status(self, exit_code):
@@ -360,7 +360,7 @@ def determine_status(self, exit_code):
             else:
                 return timeout_status
 
-        if not exit_code in TEST_RESERVED_EXITS.keys():
+        if exit_code not in TEST_RESERVED_EXITS.keys():
             return 'fail'
 
         if exit_code == 100 and self.conductor.args.fail_on_skip:
@@ -447,15 +447,15 @@ def get_juju_version():
     try:
         version = subprocess.check_output(cmd)
         version = version.split('-')[0]
-    except:
+    except Exception:
         cmd[1] = '--version'
         version = subprocess.check_output(cmd)
         version = version.split()[1]
 
     for i, ver in enumerate(version.split('.')):
         try:
             setattr(jv, jv.mapping[i], int(ver))
-        except:
+        except Exception:
             break  # List out of range? Versions not semantic? Not my problem.
 
     return jv
@@ -516,7 +516,7 @@ def filter(self, substrates):
         creation.
         """
         if isinstance(substrates, str):
-            substrates = [s.strip() for s in re.split('[,\s]', substrates)]
+            substrates = [s.strip() for s in re.split(r'[,\s]', substrates)]
 
         # Apply the rules to the list of substrates returning anything that
         # matches
@@ -646,7 +646,6 @@ def setup_parser():
     parser.add_argument('-e', '--environment', metavar='JUJU_ENV',
                         default=os.environ.get('JUJU_ENV'),
                         dest='juju_env',
-                        #required=True,
                         help="juju environment to operate in")
     parser.add_argument('--upload-tools', default=False, action="store_true",
                         help="upload juju tools (available for juju > 1.x)")
@@ -665,6 +664,7 @@ def setup_parser():
 def status(self, message, *args, **kws):
     self._log(TEST_RESULT_LEVELV_NUM, message, args, **kws)
 
+
 logging.addLevelName(TEST_RESULT_LEVELV_NUM, "RESULT")
 logging.Logger.status = status
 
@@ -711,8 +711,6 @@ def main():
     except Exception as e:
         logger.critical(str(e))
         sys.exit(1)
-    except:
-        raise
 
     logger.info('Results: %s passed, %s failed, %s errored' %
                 (passes, failures, errors))

charmtools/utils.py

@@ -645,7 +645,7 @@ def host_env():
     for key in ('PREFIX', 'PYTHONHOME', 'PYTHONPATH',
                 'GIT_TEMPLATE_DIR', 'GIT_EXEC_PATH'):
         if key in env:
-            del(env[key])
+            del env[key]
     env['PATH'] = ':'.join([
         element
         for element in env['PATH'].split(':')

docs/_extensions/automembersummary.py

@@ -44,7 +44,8 @@ def get_items(self, names):
 
         def _get_items(name):
             _items = super(AutoMemberSummary, self).get_items([shorten + name])
-            if self.result.data and ".. deprecated::" in self.result.data[0]:
+            if (self.bridge.result.data and
+                    ".. deprecated::" in self.bridge.result.data[0]):
                 # don't show deprecated classes / functions in summary
                 return
             for dn, sig, summary, rn in _items:

docs/conf.py

@@ -62,7 +62,7 @@
 #
 # This is also used if you do content translation via gettext catalogs.
 # Usually you set "language" from the command line for these cases.
-language = None
+language = "en"
 
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.
@@ -90,7 +90,7 @@
 # Add any paths that contain custom static files (such as style sheets) here,
 # relative to this directory. They are copied after the builtin static files,
 # so a file named "default.css" will overwrite the builtin "default.css".
-html_static_path = ['_static']
+# html_static_path = ['_static']
 
 # Custom sidebar templates, must be a dictionary that maps document names
 # to template names.

setup.cfg

@@ -5,4 +5,8 @@ detailed-errors=1
 #pdb-failures=1
 logging-level=INFO
 
+[flake8]
+exclude =
+    charmtools/diff_match_patch.py
+    charmtools/templates