From fd1d6c41d1b6c9d7a64fa14dfa3781b816e07502 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Fri, 23 Sep 2016 22:31:28 +0100
Subject: [PATCH 1/7] Extracted the server side of the configurables.

---
 remoteappmanager/docker/configurables.py      | 68 +++++++++++++++++++
 remoteappmanager/docker/image.py              |  7 +-
 remoteappmanager/restresources/application.py |  3 +-
 remoteappmanager/restresources/container.py   | 50 ++++++++++++--
 .../restresources/tests/test_application.py   | 27 +++++---
 5 files changed, 137 insertions(+), 18 deletions(-)
 create mode 100644 remoteappmanager/docker/configurables.py

diff --git a/remoteappmanager/docker/configurables.py b/remoteappmanager/docker/configurables.py
new file mode 100644
index 000000000..106564880
--- /dev/null
+++ b/remoteappmanager/docker/configurables.py
@@ -0,0 +1,68 @@
+import abc
+# Contains a controlled dictionary of information
+# that can be configured by the user at startup time.
+
+
+class Configurable(metaclass=abc.ABCMeta):
+    #: unique, controlled tag string that identifies the configurable.
+    tag = None
+
+    @abc.abstractclassmethod
+    def supported_by(cls, image):
+        """Checks if the passed image supports the configurable.
+        Returns True if so, False otherwise"""
+        pass
+
+    @abc.abstractmethod
+    def config_dict_to_env(self, config_dict):
+        """
+        Extracts the relevant data from a dictionary.
+        Returns a dictionary with the environment to transmit to the image
+        to set this particular configurable.
+
+        Raises if the config_dict is not in the expected format.
+
+        IMPORTANT: the received dictionary is likely coming from
+        hostile environment.
+        """
+
+
+class Resolution(Configurable):
+    """Support for images that allow resolution change of the VNC server."""
+
+    tag = "resolution"
+
+    @classmethod
+    def supported_by(cls, image):
+        return all(x in image.env
+                   for x in ["X11_WIDTH", "X11_HEIGHT", "X11_DEPTH"])
+
+    def config_dict_to_env(self, config_dict):
+        """
+        the config dict must contain the following (example)
+
+        {
+            "resolution" : "1024x768"
+        }
+
+        """
+        resolution = config_dict["resolution"]
+        w, h = [int(value) for value in resolution.split("x")]
+        if w < 0 or h < 0:
+            raise ValueError("invalid width or height")
+
+        return {
+            "X11_WIDTH": str(w),
+            "X11_HEIGHT": str(h),
+            "X11_DEPTH": 16
+        }
+
+
+def for_image(image):
+    """Returns the configurables that are available for a specific
+    image."""
+    available = [Resolution]
+
+    return [conf_class
+            for conf_class in available
+            if conf_class.supported_by(image)]
diff --git a/remoteappmanager/docker/image.py b/remoteappmanager/docker/image.py
index 8e6d64557..a3d965011 100644
--- a/remoteappmanager/docker/image.py
+++ b/remoteappmanager/docker/image.py
@@ -1,7 +1,8 @@
 import string
-from traitlets import Unicode, HasTraits, Dict
+from traitlets import Unicode, HasTraits, Dict, List
 
 from remoteappmanager.docker.docker_labels import SIMPHONY_NS
+from remoteappmanager.docker import configurables
 
 # Characters that are allowed in the environment variables.
 _ALLOWED_ENVCHARS = set(string.ascii_lowercase + string.digits + "-")
@@ -37,6 +38,9 @@ class Image(HasTraits):
     # Only keys are used at the moment.
     env = Dict()
 
+    # A list of configurables that the image supports.
+    configurables = List()
+
     @classmethod
     def from_docker_dict(cls, docker_dict):
         """Converts the dict response from the dockerpy library into an
@@ -77,4 +81,5 @@ def from_docker_dict(cls, docker_dict):
                 env = env.upper().replace("-", "_")
                 self.env[env] = None
 
+        self.configurables = configurables.for_image(self)
         return self
diff --git a/remoteappmanager/restresources/application.py b/remoteappmanager/restresources/application.py
index d5a38a980..7720c6444 100644
--- a/remoteappmanager/restresources/application.py
+++ b/remoteappmanager/restresources/application.py
@@ -44,7 +44,8 @@ def retrieve(self, identifier):
                     "volume_source": policy.volume_source,
                     "volume_target": policy.volume_target,
                     "volume_mode": policy.volume_mode,
-                }
+                },
+                "configurables": [conf.tag for conf in image.configurables]
             },
             "mapping_id": identifier,
         }
diff --git a/remoteappmanager/restresources/container.py b/remoteappmanager/restresources/container.py
index 4cceb7f62..b99569a2a 100644
--- a/remoteappmanager/restresources/container.py
+++ b/remoteappmanager/restresources/container.py
@@ -25,8 +25,10 @@ def create(self, representation):
         except KeyError:
             raise exceptions.BadRequest(message="missing mapping_id")
 
+        webapp = self.application
         account = self.current_user.account
-        all_apps = self.application.db.get_apps_for_user(account)
+        all_apps = webapp.db.get_apps_for_user(account)
+        container_manager = webapp.container_manager
 
         choice = [(m_id, app, policy)
                   for m_id, app, policy in all_apps
@@ -39,12 +41,25 @@ def create(self, representation):
 
         _, app, policy = choice[0]
 
+        image = yield container_manager.image(app.image)
+        if image is None:
+            raise exceptions.BadRequest(message="unrecognized image")
+
+        try:
+            environment = self._environment_from_configurables(image,
+                                                               representation)
+        except Exception:
+            raise exceptions.BadRequest(message="invalid configurables")
+
+        # Everything is fine. Start and wait for the container to come online.
         try:
             container = yield self._start_container(
                 self.current_user.name,
                 app,
                 policy,
-                mapping_id)
+                mapping_id,
+                environment
+                )
         except Exception as e:
             raise exceptions.Unable(message=str(e))
 
@@ -175,7 +190,12 @@ def _remove_container_noexcept(self, container):
                     container.docker_id))
 
     @gen.coroutine
-    def _start_container(self, user_name, app, policy, mapping_id):
+    def _start_container(self,
+                         user_name,
+                         app,
+                         policy,
+                         mapping_id,
+                         environment):
         """Start the container. This method is a helper method that
         works with low level data and helps in issuing the request to the
         data container.
@@ -191,6 +211,9 @@ def _start_container(self, user_name, app, policy, mapping_id):
         policy : ABCApplicationPolicy
             The startup policy for the application
 
+        environment: Dict
+            A dictionary of envvars to pass to the container.
+
         Returns
         -------
         remoteappmanager.docker.container.Container
@@ -220,8 +243,12 @@ def _start_container(self, user_name, app, policy, mapping_id):
                                       'mode': volume_mode}
 
         try:
-            f = manager.start_container(user_name, image_name,
-                                        mapping_id, volumes)
+            f = manager.start_container(user_name,
+                                        image_name,
+                                        mapping_id,
+                                        volumes,
+                                        environment
+                                        )
             container = yield gen.with_timeout(
                 timedelta(
                     seconds=self.application.file_config.network_timeout
@@ -245,6 +272,19 @@ def _start_container(self, user_name, app, policy, mapping_id):
 
         return container
 
+    def _environment_from_configurables(self, image, representation):
+        """Helper routine: extracts the configurables from the
+        image, matches them to the appropriate configurables
+        data in the representation, and returns the resulting environment
+        """
+        env = {}
+
+        for img_conf in image.configurables:
+            config_dict = representation["configurables"][img_conf.tag]
+            env.update(img_conf.config_dict_to_env(config_dict))
+
+        return env
+
     @gen.coroutine
     def _wait_for_container_ready(self, container):
         """ Wait until the container is ready to be connected
diff --git a/remoteappmanager/restresources/tests/test_application.py b/remoteappmanager/restresources/tests/test_application.py
index ecd4eef80..c7c87fba6 100644
--- a/remoteappmanager/restresources/tests/test_application.py
+++ b/remoteappmanager/restresources/tests/test_application.py
@@ -86,16 +86,19 @@ def test_retrieve(self):
         self.assertEqual(res.code, httpstatus.OK)
         self.assertEqual(escape.json_decode(res.body),
                          {'container': None,
-                          'image': {'description': '',
-                                    'icon_128': '',
-                                    'name': 'boo',
-                                    'ui_name': 'foo_ui',
-                                    'policy': {
-                                        "allow_home": True,
-                                        "volume_mode": 'ro',
-                                        "volume_source": "foo",
-                                        "volume_target": "bar",
-                                    }},
+                          'image': {
+                              'description': '',
+                              'icon_128': '',
+                              'name': 'boo',
+                              'ui_name': 'foo_ui',
+                              'policy': {
+                                    "allow_home": True,
+                                    "volume_mode": 'ro',
+                                    "volume_source": "foo",
+                                    "volume_target": "bar",
+                              },
+                              'configurables': []
+                          },
                           'mapping_id': 'one'})
 
         self._app.container_manager.containers_from_mapping_id = \
@@ -121,7 +124,9 @@ def test_retrieve(self):
                                         "volume_mode": 'ro',
                                         "volume_source": "foo",
                                         "volume_target": "bar",
-                                    }},
+                                    },
+                                    'configurables': [],
+                                    },
                           'mapping_id': 'one'})
 
         res = self.fetch("/api/v1/applications/three/")

From 5ef6d2304105e2c86d8ade06199fe67c54449ebd Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Fri, 23 Sep 2016 23:49:45 +0100
Subject: [PATCH 2/7] Added tests

---
 remoteappmanager/docker/configurables.py      | 10 ++---
 remoteappmanager/docker/image.py              |  6 ++-
 .../docker/tests/test_configurables.py        | 38 +++++++++++++++++++
 remoteappmanager/docker/tests/test_image.py   |  6 ++-
 .../tests/mocking/virtual/docker_client.py    |  4 +-
 5 files changed, 56 insertions(+), 8 deletions(-)
 create mode 100644 remoteappmanager/docker/tests/test_configurables.py

diff --git a/remoteappmanager/docker/configurables.py b/remoteappmanager/docker/configurables.py
index 106564880..086c5b9b9 100644
--- a/remoteappmanager/docker/configurables.py
+++ b/remoteappmanager/docker/configurables.py
@@ -11,10 +11,9 @@ class Configurable(metaclass=abc.ABCMeta):
     def supported_by(cls, image):
         """Checks if the passed image supports the configurable.
         Returns True if so, False otherwise"""
-        pass
 
-    @abc.abstractmethod
-    def config_dict_to_env(self, config_dict):
+    @abc.abstractclassmethod
+    def config_dict_to_env(cls, config_dict):
         """
         Extracts the relevant data from a dictionary.
         Returns a dictionary with the environment to transmit to the image
@@ -37,7 +36,8 @@ def supported_by(cls, image):
         return all(x in image.env
                    for x in ["X11_WIDTH", "X11_HEIGHT", "X11_DEPTH"])
 
-    def config_dict_to_env(self, config_dict):
+    @classmethod
+    def config_dict_to_env(cls, config_dict):
         """
         the config dict must contain the following (example)
 
@@ -54,7 +54,7 @@ def config_dict_to_env(self, config_dict):
         return {
             "X11_WIDTH": str(w),
             "X11_HEIGHT": str(h),
-            "X11_DEPTH": 16
+            "X11_DEPTH": "16"
         }
 
 
diff --git a/remoteappmanager/docker/image.py b/remoteappmanager/docker/image.py
index a3d965011..bfe140362 100644
--- a/remoteappmanager/docker/image.py
+++ b/remoteappmanager/docker/image.py
@@ -79,7 +79,11 @@ def from_docker_dict(cls, docker_dict):
                     continue
 
                 env = env.upper().replace("-", "_")
-                self.env[env] = None
+                # Docker does not allow unexistent values in
+                # labels, but we should not rely on them anyway,
+                # as only presence of the env key has a clear
+                # meaning, hence we force the value to empty.
+                self.env[env] = ""
 
         self.configurables = configurables.for_image(self)
         return self
diff --git a/remoteappmanager/docker/tests/test_configurables.py b/remoteappmanager/docker/tests/test_configurables.py
new file mode 100644
index 000000000..99d87ed8d
--- /dev/null
+++ b/remoteappmanager/docker/tests/test_configurables.py
@@ -0,0 +1,38 @@
+import unittest
+from remoteappmanager.docker.configurables import Resolution, for_image
+from remoteappmanager.docker.image import Image
+from remoteappmanager.tests.mocking.virtual.docker_client import \
+    create_docker_client
+
+
+class TestConfigurables(unittest.TestCase):
+    def setUp(self):
+        docker_client = create_docker_client()
+        image_dict = docker_client.inspect_image('image_id2')
+        self.image_2 = Image.from_docker_dict(image_dict)
+
+        image_dict = docker_client.inspect_image('image_id1')
+        self.image_1 = Image.from_docker_dict(image_dict)
+
+    def test_resolution(self):
+        self.assertTrue(Resolution.supported_by(self.image_1))
+        self.assertFalse(Resolution.supported_by(self.image_2))
+
+    def test_for_image(self):
+        self.assertEqual(for_image(self.image_1), [Resolution])
+        self.assertEqual(for_image(self.image_2), [])
+
+    def test_config_dict_to_env(self):
+        self.assertEqual(
+            Resolution.config_dict_to_env({"resolution": "1024x768" }),
+            {"X11_WIDTH": "1024",
+             "X11_HEIGHT": "768",
+             "X11_DEPTH": "16"
+             }
+        )
+
+        with self.assertRaises(KeyError):
+            Resolution.config_dict_to_env({})
+
+        with self.assertRaises(ValueError):
+            Resolution.config_dict_to_env({"resolution": "1024"})
diff --git a/remoteappmanager/docker/tests/test_image.py b/remoteappmanager/docker/tests/test_image.py
index 129d33853..506010c3a 100644
--- a/remoteappmanager/docker/tests/test_image.py
+++ b/remoteappmanager/docker/tests/test_image.py
@@ -39,7 +39,11 @@ def test_from_docker_dict_inspect_image(self):
             image.ui_name,
             image_dict['Config']["Labels"][SIMPHONY_NS.ui_name])
         self.assertEqual(image.type, 'vncapp')
-        self.assertEqual(image.env, {"X11_WIDTH": None})
+        self.assertEqual(image.env, {
+            "X11_WIDTH": "",
+            "X11_DEPTH": "",
+            "X11_HEIGHT": "",
+        })
 
     def test_missing_image_type(self):
         docker_client = create_docker_client()
diff --git a/remoteappmanager/tests/mocking/virtual/docker_client.py b/remoteappmanager/tests/mocking/virtual/docker_client.py
index 6a8a339f4..f4dc39ad8 100644
--- a/remoteappmanager/tests/mocking/virtual/docker_client.py
+++ b/remoteappmanager/tests/mocking/virtual/docker_client.py
@@ -36,7 +36,9 @@ def get_fake_image_labels(num=2):
             {'eu.simphony-project.docker.description': 'Ubuntu machine with mayavi preinstalled',  # noqa
              'eu.simphony-project.docker.ui_name': 'Mayavi 4.4.4',
              'eu.simphony-project.docker.type': 'vncapp',
-             'eu.simphony-project.docker.env.x11-width': None,
+             'eu.simphony-project.docker.env.x11-width': '',
+             'eu.simphony-project.docker.env.x11-height': '',
+             'eu.simphony-project.docker.env.x11-depth': '',
              },
             {'eu.simphony-project.docker.description': 'A vanilla Ubuntu installation'},  # noqa
         )

From 5a091f195d1f8d5f29c4ca71a7ccc7b3bb69d6b1 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Fri, 23 Sep 2016 23:58:13 +0100
Subject: [PATCH 3/7] flake

---
 remoteappmanager/docker/tests/test_configurables.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/remoteappmanager/docker/tests/test_configurables.py b/remoteappmanager/docker/tests/test_configurables.py
index 99d87ed8d..d5d5c33f4 100644
--- a/remoteappmanager/docker/tests/test_configurables.py
+++ b/remoteappmanager/docker/tests/test_configurables.py
@@ -24,7 +24,7 @@ def test_for_image(self):
 
     def test_config_dict_to_env(self):
         self.assertEqual(
-            Resolution.config_dict_to_env({"resolution": "1024x768" }),
+            Resolution.config_dict_to_env({"resolution": "1024x768"}),
             {"X11_WIDTH": "1024",
              "X11_HEIGHT": "768",
              "X11_DEPTH": "16"

From 508f0b26af96b987c20ecf10b8cc402defdf761e Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Sat, 24 Sep 2016 19:21:36 +0100
Subject: [PATCH 4/7] Fixed tests

---
 .../restresources/tests/test_container.py     | 37 ++++++++++++++++---
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/remoteappmanager/restresources/tests/test_container.py b/remoteappmanager/restresources/tests/test_container.py
index 0659e584d..8728cd347 100644
--- a/remoteappmanager/restresources/tests/test_container.py
+++ b/remoteappmanager/restresources/tests/test_container.py
@@ -75,7 +75,12 @@ def test_create(self):
                     "Cookie": "jupyter-hub-token-username=foo"
                 },
                 body=escape.json_encode(dict(
-                    mapping_id="mapping_id"
+                    mapping_id="mapping_id",
+                    configurables={
+                        "resolution": {
+                            "resolution": "1024x768"
+                        }
+                    }
                 )))
 
             self.assertEqual(res.code, httpstatus.CREATED)
@@ -101,7 +106,12 @@ def test_create_fails(self):
                     "Cookie": "jupyter-hub-token-username=foo"
                 },
                 body=escape.json_encode(dict(
-                    mapping_id="mapping_id"
+                    mapping_id="mapping_id",
+                    configurables={
+                        "resolution": {
+                            "resolution": "1024x768"
+                        }
+                    }
                 )))
 
             self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
@@ -130,7 +140,12 @@ def test_create_fails_for_reverse_proxy_failure(self):
                     "Cookie": "jupyter-hub-token-username=foo"
                 },
                 body=escape.json_encode(dict(
-                    mapping_id="mapping_id"
+                    mapping_id="mapping_id",
+                    configurables={
+                        "resolution": {
+                            "resolution": "1024x768"
+                        }
+                    }
                 )))
 
             self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
@@ -159,7 +174,12 @@ def test_create_fails_for_start_container_failure(self):
                     "Cookie": "jupyter-hub-token-username=foo"
                 },
                 body=escape.json_encode(dict(
-                    mapping_id="mapping_id"
+                    mapping_id="mapping_id",
+                    configurables={
+                        "resolution": {
+                            "resolution": "1024x768"
+                        }
+                    }
                 )))
 
             self.assertEqual(res.code, httpstatus.INTERNAL_SERVER_ERROR)
@@ -256,7 +276,14 @@ def test_post_start(self):
                        headers={
                                 "Cookie": "jupyter-hub-token-username=foo"
                        },
-                       body=escape.json_encode({"mapping_id": "mapping_id"}))
+                       body=escape.json_encode({
+                           "mapping_id": "mapping_id",
+                           "configurables": {
+                                "resolution": {
+                                    "resolution": "1024x768"
+                                }
+                           }
+                       }))
 
             self.assertTrue(self._app.reverse_proxy.register.called)
 

From e7ba69591c8dd434b4ffab83fbbbcb6a9d75dcf5 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Sat, 24 Sep 2016 20:41:34 +0100
Subject: [PATCH 5/7] Added a few tests + cosmetic fixes

---
 remoteappmanager/docker/configurables.py        |  4 ++--
 .../docker/tests/test_configurables.py          |  6 ++++++
 .../restresources/tests/test_container.py       | 17 +++++++++++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/remoteappmanager/docker/configurables.py b/remoteappmanager/docker/configurables.py
index 086c5b9b9..d1132e4bc 100644
--- a/remoteappmanager/docker/configurables.py
+++ b/remoteappmanager/docker/configurables.py
@@ -17,7 +17,7 @@ def config_dict_to_env(cls, config_dict):
         """
         Extracts the relevant data from a dictionary.
         Returns a dictionary with the environment to transmit to the image
-        to set this particular configurable.
+        to set this particular configurable. Values must be strings.
 
         Raises if the config_dict is not in the expected format.
 
@@ -48,7 +48,7 @@ def config_dict_to_env(cls, config_dict):
         """
         resolution = config_dict["resolution"]
         w, h = [int(value) for value in resolution.split("x")]
-        if w < 0 or h < 0:
+        if w <= 0 or h <= 0:
             raise ValueError("invalid width or height")
 
         return {
diff --git a/remoteappmanager/docker/tests/test_configurables.py b/remoteappmanager/docker/tests/test_configurables.py
index d5d5c33f4..4442e24b4 100644
--- a/remoteappmanager/docker/tests/test_configurables.py
+++ b/remoteappmanager/docker/tests/test_configurables.py
@@ -36,3 +36,9 @@ def test_config_dict_to_env(self):
 
         with self.assertRaises(ValueError):
             Resolution.config_dict_to_env({"resolution": "1024"})
+
+        with self.assertRaises(ValueError):
+            Resolution.config_dict_to_env({"resolution": "-10x-10"})
+
+        with self.assertRaises(ValueError):
+            Resolution.config_dict_to_env({"resolution": "0x1024"})
diff --git a/remoteappmanager/restresources/tests/test_container.py b/remoteappmanager/restresources/tests/test_container.py
index 8728cd347..56c44dc64 100644
--- a/remoteappmanager/restresources/tests/test_container.py
+++ b/remoteappmanager/restresources/tests/test_container.py
@@ -187,6 +187,23 @@ def test_create_fails_for_start_container_failure(self):
                 "type": "Unable",
                 "message": "Boom!"})
 
+    def test_create_fails_for_incorrect_configurable(self):
+        res = self.fetch(
+            "/user/username/api/v1/containers/",
+            method="POST",
+            headers={
+                "Cookie": "jupyter-hub-token-username=foo"
+            },
+            body=escape.json_encode(dict(
+                mapping_id="mapping_id",
+                configurables={
+                    "resolution": {
+                    }
+                }
+            )))
+
+        self.assertEqual(res.code, httpstatus.BAD_REQUEST)
+
     def test_create_fails_for_missing_mapping_id(self):
         res = self.fetch(
             "/user/username/api/v1/containers/",

From 3f96e22f93b671346bc751a2e1a6f4b91ba6e8cb Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Sat, 24 Sep 2016 22:33:55 +0100
Subject: [PATCH 6/7] Added some docs

---
 remoteappmanager/docker/configurables.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/remoteappmanager/docker/configurables.py b/remoteappmanager/docker/configurables.py
index d1132e4bc..d90f23b7b 100644
--- a/remoteappmanager/docker/configurables.py
+++ b/remoteappmanager/docker/configurables.py
@@ -1,9 +1,12 @@
 import abc
-# Contains a controlled dictionary of information
-# that can be configured by the user at startup time.
 
 
 class Configurable(metaclass=abc.ABCMeta):
+    """Base class for Configurables.
+    They describe startup configuration entry points for images.
+    Injection of the configurables data is done through a defined
+    set of environment variables that the image accepts.
+    """
     #: unique, controlled tag string that identifies the configurable.
     tag = None
 
@@ -45,6 +48,8 @@ def config_dict_to_env(cls, config_dict):
             "resolution" : "1024x768"
         }
 
+        Observe that the key used has nothing to do with the tag.
+        They are only accidentally the same.
         """
         resolution = config_dict["resolution"]
         w, h = [int(value) for value in resolution.split("x")]
@@ -61,6 +66,7 @@ def config_dict_to_env(cls, config_dict):
 def for_image(image):
     """Returns the configurables that are available for a specific
     image."""
+    # We lack automatic registration. Add here new configurables
     available = [Resolution]
 
     return [conf_class

From eb73193fc8cba8ccf35917a4e10e78eecbdd2516 Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Sat, 24 Sep 2016 23:07:33 +0100
Subject: [PATCH 7/7] Even more docs

---
 remoteappmanager/docker/configurables.py | 49 ++++++++++++++++++++----
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/remoteappmanager/docker/configurables.py b/remoteappmanager/docker/configurables.py
index d90f23b7b..3259b106a 100644
--- a/remoteappmanager/docker/configurables.py
+++ b/remoteappmanager/docker/configurables.py
@@ -7,13 +7,24 @@ class Configurable(metaclass=abc.ABCMeta):
     Injection of the configurables data is done through a defined
     set of environment variables that the image accepts.
     """
-    #: unique, controlled tag string that identifies the configurable.
+    #: Unique, controlled tag string that identifies the configurable.
     tag = None
 
     @abc.abstractclassmethod
     def supported_by(cls, image):
         """Checks if the passed image supports the configurable.
-        Returns True if so, False otherwise"""
+        Returns True if so, False otherwise.
+
+        Parameters
+        ----------
+        image: remoteappmanager.docker.Image
+            The image to check if it supports this configurable
+
+        Returns
+        -------
+        bool: True if the image supports the configurable.
+
+        """
 
     @abc.abstractclassmethod
     def config_dict_to_env(cls, config_dict):
@@ -22,10 +33,23 @@ def config_dict_to_env(cls, config_dict):
         Returns a dictionary with the environment to transmit to the image
         to set this particular configurable. Values must be strings.
 
-        Raises if the config_dict is not in the expected format.
+        Raises various exceptions if the config_dict is not in the expected
+        format.
 
         IMPORTANT: the received dictionary is likely coming from
-        hostile environment.
+        hostile environment. Validate the contents strictly.
+
+        Parameters
+        ----------
+        config_dict: Dict
+            A dictionary containing data that the Configurable class
+            understands.
+
+        Returns
+        -------
+        Dict(Str, Str):
+            A dictionary of environment variables to pass to the
+            image at startup.
         """
 
 
@@ -41,8 +65,7 @@ def supported_by(cls, image):
 
     @classmethod
     def config_dict_to_env(cls, config_dict):
-        """
-        the config dict must contain the following (example)
+        """the config dict must contain the following (example)
 
         {
             "resolution" : "1024x768"
@@ -65,7 +88,19 @@ def config_dict_to_env(cls, config_dict):
 
 def for_image(image):
     """Returns the configurables that are available for a specific
-    image."""
+    image.
+
+    Parameters
+    ----------
+    image: remoteappmanager.docker.Image
+        The image to check.
+
+    Returns
+    -------
+    List
+        A list of Configurable classes supported by the given image.
+
+    """
     # We lack automatic registration. Add here new configurables
     available = [Resolution]