#75 Stick to PEP8. Fix exceptions flow
Closed 6 years ago by sturivny. Opened 6 years ago by sturivny.
Unknown source local-inventory  into  master

@@ -3,42 +3,43 @@

  import argparse

  import json

  import os

- import shlex

  import sys

  

- def main(argv):

+ 

+ def main():

      parser = argparse.ArgumentParser(description="Inventory for local")

      parser.add_argument("--list", action="store_true", help="Verbose output")

      parser.add_argument('--host', help="Get host variables")

      opts = parser.parse_args()

  

-     try:

-         if opts.host:

-             data = host(opts.host)

-         else:

-             data = list()

-         sys.stdout.write(json.dumps(data, indent=4, separators=(',', ': ')))

-     except RuntimeError as ex:

-         sys.stderr.write("{0}: {1}\n".format(os.path.basename(sys.argv[0]), str(ex)))

-         return 1

- 

-     return 0

- 

- def list():

-     rpms = [ ]

-     hosts = [ ]

-     variables = { }

+     if opts.host:

+         arg_data = get_host_variables(opts.host)

+     else:

+         arg_data = get_list_variables()

+     sys.stdout.write(json.dumps(arg_data, indent=4, separators=(',', ': ')))

+ 

+ 

+ def get_list_variables():

+     hosts = []

+     variables = {}

      if os.environ.get("TEST_SUBJECTS", None) == "local":

-         vars = host("local")

-         if vars:

+         local_vars = get_host_variables("local")

+         if local_vars:

              hosts.append("local")

-             variables["local"] = vars

-     return { "subjects": { "hosts": hosts, "vars": { } }, "localhost": { "hosts": hosts, "vars": { } }, "_meta": { "hostvars": variables } }

+             variables["local"] = local_vars

+     return {"subjects": {"hosts": hosts, "vars": {}}, "localhost":

+             {"hosts": hosts, "vars": {}}, "_meta": {"hostvars": variables}}

+ 

+ 

+ def get_host_variables(working_host):

+     if working_host == "local":

+         return {"ansible_connection": "local"}

Let's write universal function. -> Define a default return value. At least describe what will be if working_host != "local" ?

  

- def host(host):

-     if host == "local":

-         return { "ansible_connection": "local" }

-     return None

  

  if __name__ == '__main__':

-     sys.exit(main(sys.argv))

+     try:

+         main()

+     except Exception as ex:

Catching object of base class Exception is a bad style. This a cause of many hidden nasty bugs.

+         sys.stderr.write("{}: {}\n".format(os.path.basename(sys.argv[0]),

+                                            ex.message))

+         sys.exit(1)

I cannot get idea behind "sys.exit(1)" ....
In case of exception python will do exactly the same: print info about exception + return with value != 0.

Cleanup code and test it

Catching object of base class Exception is a bad style. This a cause of many hidden nasty bugs.

Let's write universal function. -> Define a default return value. At least describe what will be if working_host != "local" ?

I cannot get idea behind "sys.exit(1)" ....
In case of exception python will do exactly the same: print info about exception + return with value != 0.

Pull-Request has been closed by sturivny

6 years ago
Metadata