5
\$\begingroup\$

The following piece of code is imported in another file to get the status of the docker containers. I am printing the required information in a proper tabular form. I am using next() for finding out the only key available in the dictionary. This particular key will change hence used next() to find out the key. But the next() raises an exception when it reaches to the end. Currently I a handling that using PASS.

My question is "Is there a better to way to handle the exception caused by next () : StopIteration" ?

import requests
from prettytable import PrettyTable 

def container_status(status):

    url = None
    if status == "all": url = "http://127.0.0.1:6000/containers/json?all=1"
    elif status == "running" : url = "http://127.0.0.1:6000/containers/json?all"
    else: raise ValueError("status should be either 'all' or 'running'")

    return requests.get(url)

def active_containers(status):

    response = container_status(status)
    table = PrettyTable(["Container Name", "Container ID", "Status", "IP ADDR"])
    for i in response.json():
        try:
            table.add_row([i["Names"][0].encode('utf-8').replace('/', ''),
                          i['Id'].encode('utf-8')[:12],
                          i["State"],
                          i["NetworkSettings"]["Networks"][next(iter(i["NetworkSettings"]["Networks"]))]["IPAddress"]])

        except StopIteration:
            pass
    print(table)

What are the other possibilities where I can improve the code?

\$\endgroup\$
5
\$\begingroup\$

Dictionaries, as any iterable in Python can be unpacked. It means that, if you know the length of said iterable, you can use as many variables to hold each item of the iterable independently.

For instance:

>>> l = [1, 2, 3]
>>> a, b, c = l
>>> a
1
>>> b
2
>>> c
3
>>> d = {"one": 1, "two": 2}
>>> a, b = d
>>> a
"two"
>>> b
"one"

(order may vary for the dictionary)

If the number of elements in the iterable and the number of variables mismatch, you get a ValueError:

>>> d = {"one": 1}
>>> a, b = d
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: not enough values to unpack (expected 2, got 1)

So, if you know that a dictionary has a single key, you can

network_settings, = i["NetworkSettings"]["Networks"]

and get it that way. If it appears that the dictionary may not contain that key, you have no other choice but to use a try: ... except ValueError: pass.

However, you only care about the values of this dictionary. So I wouldn't use this directly on the dictionary but on its .values() (.itervalues() in Python 2). So the loop would look like:

for container in response.json():
    network_settings, = container['NetworkSettings']['Networks'].values()
    table.add_row(
            container['Names'][0].encode('utf-8').replace('/', ''),
            container['Id'].encode('utf-8')[:12],
            container['State'],
            network_settings['IPAddress'])

or

for container in response.json():
    try:
        network_settings, = container['NetworkSettings']['Networks'].values()
    except ValueError:
        continue
    table.add_row(
            container['Names'][0].encode('utf-8').replace('/', ''),
            container['Id'].encode('utf-8')[:12],
            container['State'],
            network_settings['IPAddress'])

In case of possible missing keys.


Now, for the rest of the code. I wouldn't use any of the prettytable stuff within active_containers. It may be interesting, for reusability, to only extract relevant informations in this function, and let the caller format it.

I would also change the meaning of the container_status function a bit. Looking at the URLs, it seems to me that the status parameter is only there to filter out inactive containers. So it should be more obvious using a binary choice:

def container_status(include_stopped=False):
    url = 'http://127.0.0.1:6000/containers/json?all'
    if include_stopped:
        url += '=1'
    return requests.get(url)

You get to choose the default based on your most common needs.

Full code would look like:

import requests
from prettytable import PrettyTable 


def container_status(include_stopped=False):
    url = 'http://127.0.0.1:6000/containers/json?all'
    if include_stopped:
        url += '=1'
    return requests.get(url)


def active_containers():
    for container in container_status().json():
        network_settings, = container['NetworkSettings']['Networks'].values()
        yield (
            container['Names'][0].encode('utf-8').replace('/', ''),
            container['Id'].encode('utf-8')[:12],
            container['State'],
            network_settings['IPAddress'])


def main():
    table = PrettyTable(['Container Name', 'Container ID', 'Status', 'IP ADDR'])
    for container_infos in active_containers():
        table.add_row(*container_infos)
    print(table)


if __name__ == '__main__':
    main()

(Just assuming that active_containers here means you only want running ones, main and active_containers may need an include_stopped parameter otherwise.)

\$\endgroup\$
  • \$\begingroup\$ Thanks for your detailed explanation and the way you changed the functions \$\endgroup\$ – Here_2_learn May 17 '17 at 5:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.