CentOS/python-cicoclient

Properly handle when the API doesn't return any nodes

Closed this issue · 4 comments

When the API doesn't return any nodes, it returns "None" instead of a JSON dict with a hosts key.

Trace looks like this:

$ cico --debug node get
Starting new HTTP connection (1): admin.ci.centos.org
Resetting dropped connection: admin.ci.centos.org
None
'NoneType' object has no attribute '__getitem__'
Traceback (most recent call last):
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cliff/app.py", line 374, in run_subcommand
    result = cmd.run(parsed_args)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cliff/display.py", line 92, in run
    column_names, data = self.take_action(parsed_args)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cicoclient/utils.py", line 60, in wrapper
    return func(self, *args, **kwargs)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cicoclient/cli.py", line 103, in take_action
    count=parsed_args.count)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cicoclient/wrapper.py", line 131, in node_get
    for full_host in body['hosts']:
TypeError: 'NoneType' object has no attribute '__getitem__'
Traceback (most recent call last):
  File "/home/dmsimard/weirdo/.tox/py27/bin/cico", line 11, in <module>
    sys.exit(main())
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cicoclient/shell.py", line 71, in main
    return cicocli.run(argv)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cliff/app.py", line 255, in run
    result = self.run_subcommand(remainder)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cliff/app.py", line 374, in run_subcommand
    result = cmd.run(parsed_args)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cliff/display.py", line 92, in run
    column_names, data = self.take_action(parsed_args)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cicoclient/utils.py", line 60, in wrapper
    return func(self, *args, **kwargs)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cicoclient/cli.py", line 103, in take_action
    count=parsed_args.count)
  File "/home/dmsimard/weirdo/.tox/py27/lib/python2.7/site-packages/cicoclient/wrapper.py", line 131, in node_get
    for full_host in body['hosts']:
TypeError: 'NoneType' object has no attribute '__getitem__'

We can likely have the API return a valid json... would that help ? We might need to somehow communicate that change to everyone already handling this elsewhere.

I wonder if its easier to have the cico client check if the returned payload is json or not, before parsing it as such ?

Regular JSON might help if it followed a particular style, e.g. {"exception":{"message":"Out of nodes"}} as it could be used in other calls, but should still return a non-2xx HTTP status.

I'll think of something but if you have time to look into it feel free to open a PR! A quick hack would be to handle this here: https://github.com/dmsimard/python-cicoclient/blob/master/cicoclient/wrapper.py#L142 with a try/catch and return a helpful error (i.e, API did not return any nodes) but I would still raise an exception IMO.

my main hesitation to change anything on the duffy side of things is that it will break all existing setups. And I dont think we have a clear way to execute on this.

What I had planned longer term was to have a /v2/ in the url, and just push out the api improvements via that route, giving existing installs as much time as they want to migrate ( if they want to at all ). There is no clear ETA on that v2 though, look for that in the 12 to 14 week plan to hit stable