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