Merge lp:~frankban/charms/precise/juju-gui/guiserver-bundles-base-helpers into lp:~juju-gui/charms/precise/juju-gui/trunk

[Francesco Banconi]

GUI server: base deployer helpers.

This branch includes some helper functions
and objects that are required to implement
the Deployer and DeployMiddleware classes.

To avoid this branch to include too many
changes, the base classes above will be
implemented in a separate branch.

Tests: `make unittest` from the branch root.

https://codereview.appspot.com/13153043/

[Francesco Banconi (frankban)]

Reviewers: mp+181269_code.launchpad.net,

Message:
Please take a look.

Description:
GUI server: base deployer helpers.

This branch includes some helper functions
and objects that are required to implement
the Deployer and DeployMiddleware classes.

To avoid this branch to include too many
changes, the base classes above will be
implemented in a separate branch.

Tests: `make unittest` from the branch root.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/guiserver-bundles-base-helpers/+merge/181269

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/13153043/

Affected files:
   A [revision details]
   M revision
   M server/guiserver/bundles/__init__.py
   M server/guiserver/bundles/utils.py
   M server/guiserver/tests/bundles/test_utils.py
   M server/guiserver/tests/test_utils.py
   M server/guiserver/utils.py

[Brad Crittenden (bac)]

LGTM, thanks.

https://codereview.appspot.com/13153043/diff/1/server/guiserver/bundles/utils.py
File server/guiserver/bundles/utils.py (right):

https://codereview.appspot.com/13153043/diff/1/server/guiserver/bundles/utils.py#newcode41
server/guiserver/bundles/utils.py:41: - Time: the time in seconds since
the epoch as an int.
Would be nice to document the optional fields.

https://codereview.appspot.com/13153043/diff/1/server/guiserver/tests/bundles/test_utils.py
File server/guiserver/tests/bundles/test_utils.py (right):

https://codereview.appspot.com/13153043/diff/1/server/guiserver/tests/bundles/test_utils.py#newcode143
server/guiserver/tests/bundles/test_utils.py:143: def
test_notify_position(self):
A test exercising showing the STARTED state would be nice.

https://codereview.appspot.com/13153043/

[lp:~frankban/charms/precise/juju-gui/guiserver-bundles-base-helpers]

118. By Francesco Banconi on 2013-08-21

Changes as per review.

[Francesco Banconi (frankban)]

*** Submitted:

GUI server: base deployer helpers.

This branch includes some helper functions
and objects that are required to implement
the Deployer and DeployMiddleware classes.

To avoid this branch to include too many
changes, the base classes above will be
implemented in a separate branch.

Tests: `make unittest` from the branch root.

R=benji, bac
CC=
https://codereview.appspot.com/13153043

https://codereview.appspot.com/13153043/diff/1/server/guiserver/bundles/utils.py
File server/guiserver/bundles/utils.py (right):

https://codereview.appspot.com/13153043/diff/1/server/guiserver/bundles/utils.py#newcode41
server/guiserver/bundles/utils.py:41: - Time: the time in seconds since
the epoch as an int.
On 2013/08/21 14:19:18, bac wrote:
> Would be nice to document the optional fields.

Done.

https://codereview.appspot.com/13153043/diff/1/server/guiserver/bundles/utils.py#newcode88
server/guiserver/bundles/utils.py:88: """Add a change to the deployment
watcher notifying a new position."""
On 2013/08/21 13:18:10, benji wrote:
> I think I see that "position" is the position of the deployment in the
queue but
> I don't quite see how bool(position) == False implies that the
deployment has
> been started. Maybe some more text in a comment the docstring would
help.

Done.

https://codereview.appspot.com/13153043/diff/1/server/guiserver/tests/bundles/test_utils.py
File server/guiserver/tests/bundles/test_utils.py (right):

https://codereview.appspot.com/13153043/diff/1/server/guiserver/tests/bundles/test_utils.py#newcode143
server/guiserver/tests/bundles/test_utils.py:143: def
test_notify_position(self):
On 2013/08/21 14:19:18, bac wrote:
> A test exercising showing the STARTED state would be nice.

You are right! Missed it, thank you.

https://codereview.appspot.com/13153043/

[Francesco Banconi (frankban)]

Thanks for the reviews!

https://codereview.appspot.com/13153043/

=== modified file 'revision'
--- revision	2013-08-20 08:02:05 +0000
+++ revision	2013-08-21 16:12:55 +0000
@@ -1,1 +1,1 @@
-72
+73

=== modified file 'server/guiserver/bundles/__init__.py'
--- server/guiserver/bundles/__init__.py	2013-08-20 08:00:13 +0000
+++ server/guiserver/bundles/__init__.py	2013-08-21 16:12:55 +0000
@@ -178,9 +178,12 @@
         'RequestId': 3,
         'Response': {
             'Changes': [
-                {'DeploymentId': 42, 'Status': 'scheduled', 'Queue': 2},
-                {'DeploymentId': 42, 'Status': 'scheduled', 'Queue': 1},
-                {'DeploymentId': 42, 'Status': 'started', 'Queue': 0},
+                {'DeploymentId': 42, 'Status': 'scheduled', 'Time': 1377080066,
+                 'Queue': 2},
+                {'DeploymentId': 42, 'Status': 'scheduled', 'Time': 1377080062,
+                 'Queue': 1},
+                {'DeploymentId': 42, 'Status': 'started', 'Time': 1377080000,
+                 'Queue': 0},
             ],
         },
     }
@@ -192,6 +195,9 @@
 
 The Status can be one of the following: 'scheduled', 'started' and 'completed'.
 
+The Time field indicates the number of seconds since the epoch at the time of
+the change.
+
 The Next request can be performed as many times as required by the API clients
 after receiving a response from a previous one. However, if the Status of the
 last deployment change is 'completed', no further changes will be notified, and
@@ -204,6 +210,7 @@
                 {
                   'DeploymentId': 42,
                   'Status': 'completed',
+                  'Time': 1377080000,
                   'Error': 'this field is only present if an error occurred',
                 },
             ],
@@ -238,10 +245,14 @@
         'RequestId': 5,
         'Response': {
             'LastChanges': [
-                {'DeploymentId': 42, 'Status': 'completed', 'Error': 'error'},
-                {'DeploymentId': 43, 'Status': 'completed'},
-                {'DeploymentId': 44, 'Status': 'started', 'Queue': 0},
-                {'DeploymentId': 45, 'Status': 'scheduled', 'Queue': 1},
+                {'DeploymentId': 42, 'Status': 'completed', 'Time': 1377080001,
+                 'Error': 'error'},
+                {'DeploymentId': 43, 'Status': 'completed',
+                 'Time': 1377080002},
+                {'DeploymentId': 44, 'Status': 'started', 'Time': 1377080003,
+                 'Queue': 0},
+                {'DeploymentId': 45, 'Status': 'scheduled', 'Time': 1377080004,
+                 'Queue': 1},
             ],
         },
     }

=== modified file 'server/guiserver/bundles/utils.py'
--- server/guiserver/bundles/utils.py	2013-08-20 14:05:39 +0000
+++ server/guiserver/bundles/utils.py	2013-08-21 16:12:55 +0000
@@ -14,13 +14,97 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Bundle deployment utility functions."""
+"""Bundle deployment utility functions and objects."""
 
 from functools import wraps
+import itertools
 import logging
+import time
 
 from tornado import gen
 
+from guiserver.watchers import AsyncWatcher
+
+
+# Change statuses.
+SCHEDULED = 'scheduled'
+STARTED = 'started'
+COMPLETED = 'completed'
+
+
+def create_change(deployment_id, status, queue=None, error=None):
+    """Return a dict representing a deployment change.
+
+    The resulting dict contains at least the following fields:
+      - DeploymentId: the deployment identifier;
+      - Status: the deployment's current status;
+      - Time: the time in seconds since the epoch as an int.
+
+    These optional fields can also be present:
+      - Queue: the deployment position in the queue at the time of this change;
+      - Error: a message describing an error occurred during the deployment.
+    """
+    result = {
+        'DeploymentId': deployment_id,
+        'Status': status,
+        'Time': int(time.time()),
+    }
+    if queue is not None:
+        result['Queue'] = queue
+    if error is not None:
+        result['Error'] = error
+    return result
+
+
+class Observer(object):
+    """Handle multiple deployment watchers."""
+
+    def __init__(self):
+        # Map deployment identifiers to watchers.
+        self.deployments = {}
+        # Map watcher identifiers to deployment identifiers.
+        self.watchers = {}
+        # This counter is used to generate deployment identifiers.
+        self._deployment_counter = itertools.count()
+        # This counter is used to generate watcher identifiers.
+        self._watcher_counter = itertools.count()
+
+    def add_deployment(self):
+        """Start observing a deployment.
+
+        Generate a deployment id and add it to self.deployments.
+        Return the generated deployment id.
+        """
+        deployment_id = self._deployment_counter.next()
+        self.deployments[deployment_id] = AsyncWatcher()
+        return deployment_id
+
+    def add_watcher(self, deployment_id):
+        """Return a new watcher id for the given deployment id.
+
+        Also add the generated watcher id to self.watchers.
+        """
+        watcher_id = self._watcher_counter.next()
+        self.watchers[watcher_id] = deployment_id
+        return watcher_id
+
+    def notify_position(self, deployment_id, position):
+        """Add a change to the deployment watcher notifying a new position.
+
+        If the position in the queue is 0, it means the deployment is started
+        or about to start. Therefore set its status to STARTED.
+        """
+        watcher = self.deployments[deployment_id]
+        status = SCHEDULED if position else STARTED
+        change = create_change(deployment_id, status, queue=position)
+        watcher.put(change)
+
+    def notify_completed(self, deployment_id, error=None):
+        """Add a change to the deployment watcher notifying it is completed."""
+        watcher = self.deployments[deployment_id]
+        change = create_change(deployment_id, COMPLETED, error=error)
+        watcher.close(change)
+
 
 def require_authenticated_user(view):
     """Require the user to be authenticated when executing the decorated view.

=== modified file 'server/guiserver/tests/bundles/test_utils.py'
--- server/guiserver/tests/bundles/test_utils.py	2013-08-20 14:05:39 +0000
+++ server/guiserver/tests/bundles/test_utils.py	2013-08-21 16:12:55 +0000
@@ -14,10 +14,11 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Tests for the deployment utility functions."""
+"""Tests for the deployment utility functions and objects."""
 
 import unittest
 
+import mock
 from tornado import gen
 from tornado.testing import(
     AsyncTestCase,
@@ -26,10 +27,178 @@
     LogTrapTestCase,
 )
 
+from guiserver import watchers
 from guiserver.bundles import utils
 from guiserver.tests import helpers
 
 
[e-mail]('time.time', mock.Mock(return_value=1234))
+class TestCreateChange(unittest.TestCase):
+
+    def test_status(self):
+        # The change includes the deployment status.
+        expected = {'DeploymentId': 0, 'Status': utils.STARTED, 'Time': 1234}
+        obtained = utils.create_change(0, utils.STARTED)
+        self.assertEqual(expected, obtained)
+
+    def test_queue(self):
+        # The change includes the deployment queue length.
+        expected = {
+            'DeploymentId': 1,
+            'Status': utils.SCHEDULED,
+            'Time': 1234,
+            'Queue': 42,
+        }
+        obtained = utils.create_change(1, utils.SCHEDULED, queue=42)
+        self.assertEqual(expected, obtained)
+
+    def test_error(self):
+        # The change includes a deployment error.
+        expected = {
+            'DeploymentId': 2,
+            'Status': utils.COMPLETED,
+            'Time': 1234,
+            'Error': 'an error',
+        }
+        obtained = utils.create_change(2, utils.COMPLETED, error='an error')
+        self.assertEqual(expected, obtained)
+
+    def test_all_params(self):
+        # The change includes all the parameters.
+        expected = {
+            'DeploymentId': 3,
+            'Status': utils.COMPLETED,
+            'Time': 1234,
+            'Queue': 47,
+            'Error': 'an error',
+        }
+        obtained = utils.create_change(
+            3, utils.COMPLETED, queue=47, error='an error')
+        self.assertEqual(expected, obtained)
+
+
+class TestObserver(unittest.TestCase):
+
+    def setUp(self):
+        self.observer = utils.Observer()
+
+    def assert_deployment(self, deployment_id):
+        """Ensure the given deployment id is being observed.
+
+        Also check that a watcher is associated with the given deployment id.
+        Return the watcher.
+        """
+        deployments = self.observer.deployments
+        self.assertIn(deployment_id, deployments)
+        watcher = deployments[deployment_id]
+        self.assertIsInstance(watcher, watchers.AsyncWatcher)
+        return watcher
+
+    def assert_watcher(self, watcher_id, deployment_id):
+        """Ensure the given watcher id is associated with the deployment id."""
+        watchers = self.observer.watchers
+        self.assertIn(watcher_id, watchers)
+        self.assertEqual(deployment_id, watchers[watcher_id])
+
+    def test_initial(self):
+        # A newly created observer does not contain deployments.
+        self.assertEqual({}, self.observer.deployments)
+        self.assertEqual({}, self.observer.watchers)
+
+    def test_add_deployment(self):
+        # A new deployment is correctly added to the observer.
+        deployment_id = self.observer.add_deployment()
+        self.assertEqual(1, len(self.observer.deployments))
+        self.assert_deployment(deployment_id)
+
+    def test_add_multiple_deployments(self):
+        # Multiple deployments can be added to the observer.
+        deployment1 = self.observer.add_deployment()
+        deployment2 = self.observer.add_deployment()
+        self.assertNotEqual(deployment1, deployment2)
+        self.assertEqual(2, len(self.observer.deployments))
+        watcher1 = self.assert_deployment(deployment1)
+        watcher2 = self.assert_deployment(deployment2)
+        self.assertNotEqual(watcher1, watcher2)
+
+    def test_add_watcher(self):
+        # A new watcher is correctly added to the observer.
+        deployment_id = self.observer.add_deployment()
+        watcher_id = self.observer.add_watcher(deployment_id)
+        self.assertEqual(1, len(self.observer.watchers))
+        self.assert_watcher(watcher_id, deployment_id)
+
+    def test_add_multiple_watchers(self):
+        # Multiple watchers can be added to the observer.
+        deployment1 = self.observer.add_deployment()
+        deployment2 = self.observer.add_deployment()
+        watcher1 = self.observer.add_watcher(deployment1)
+        watcher2 = self.observer.add_watcher(deployment2)
+        self.assertNotEqual(watcher1, watcher2)
+        self.assertEqual(2, len(self.observer.watchers))
+        self.assert_watcher(watcher1, deployment1)
+        self.assert_watcher(watcher2, deployment2)
+
+    @mock.patch('time.time', mock.Mock(return_value=1234))
+    def test_notify_scheduled(self):
+        # It is possible to notify a new queue position for a deployment.
+        deployment_id = self.observer.add_deployment()
+        watcher = self.observer.deployments[deployment_id]
+        self.observer.notify_position(deployment_id, 3)
+        expected = {
+            'DeploymentId': deployment_id,
+            'Status': utils.SCHEDULED,
+            'Time': 1234,
+            'Queue': 3,
+        }
+        self.assertEqual(expected, watcher.getlast())
+        self.assertFalse(watcher.closed)
+
+    @mock.patch('time.time', mock.Mock(return_value=12345))
+    def test_notify_started(self):
+        # It is possible to notify that a deployment is (about to be) started.
+        deployment_id = self.observer.add_deployment()
+        watcher = self.observer.deployments[deployment_id]
+        self.observer.notify_position(deployment_id, 0)
+        expected = {
+            'DeploymentId': deployment_id,
+            'Status': utils.STARTED,
+            'Time': 12345,
+            'Queue': 0,
+        }
+        self.assertEqual(expected, watcher.getlast())
+        self.assertFalse(watcher.closed)
+
+    @mock.patch('time.time', mock.Mock(return_value=123456))
+    def test_notify_completed(self):
+        # It is possible to notify that a deployment is completed.
+        deployment_id = self.observer.add_deployment()
+        watcher = self.observer.deployments[deployment_id]
+        self.observer.notify_completed(deployment_id)
+        expected = {
+            'DeploymentId': deployment_id,
+            'Status': utils.COMPLETED,
+            'Time': 123456,
+        }
+        self.assertEqual(expected, watcher.getlast())
+        self.assertTrue(watcher.closed)
+
+    @mock.patch('time.time', mock.Mock(return_value=1234567))
+    def test_notify_error(self):
+        # It is possible to notify that an error occurred during a deployment.
+        deployment_id = self.observer.add_deployment()
+        watcher = self.observer.deployments[deployment_id]
+        self.observer.notify_completed(deployment_id, error='bad wolf')
+        expected = {
+            'DeploymentId': deployment_id,
+            'Status': utils.COMPLETED,
+            'Time': 1234567,
+            'Error': 'bad wolf',
+        }
+        self.assertEqual(expected, watcher.getlast())
+        self.assertTrue(watcher.closed)
+
+
 class TestRequireAuthenticatedUser(
         helpers.BundlesTestMixin, LogTrapTestCase, AsyncTestCase):
 

=== modified file 'server/guiserver/tests/test_utils.py'
--- server/guiserver/tests/test_utils.py	2013-07-25 14:22:50 +0000
+++ server/guiserver/tests/test_utils.py	2013-08-21 16:12:55 +0000
@@ -17,14 +17,63 @@
 """Tests for the Juju GUI server utilities."""
 
 import json
+import os
+import shutil
+import tempfile
 import unittest
 
 import mock
-from tornado.testing import ExpectLog
+from tornado import (
+    concurrent,
+    gen,
+)
+from tornado.testing import (
+    AsyncTestCase,
+    ExpectLog,
+    gen_test,
+)
 
 from guiserver import utils
 
 
+class TestAddFuture(AsyncTestCase):
+
+    def setUp(self):
+        # Set up a future object and a result attribute where tests will store
+        # their results.
+        super(TestAddFuture, self).setUp()
+        self.future = concurrent.Future()
+        self.result = None
+
+    @gen.coroutine
+    def assert_done(self, result):
+        """Fire the future and ensure the callback has been called.
+
+        Callbacks in this test case store their results in self.result.
+        """
+        self.assertTrue(self.future.done())
+        yield self.future
+        self.assertEqual(result, self.result)
+
+    @gen_test
+    def test_without_args(self):
+        # A callback without args is correctly called.
+        def callback(future):
+            self.result = 'future said: ' + future.result()
+        utils.add_future(self.io_loop, self.future, callback)
+        self.future.set_result('I am done')
+        yield self.assert_done('future said: I am done')
+
+    @gen_test
+    def test_with_args(self):
+        # A callback with args is correctly called.
+        def callback(arg1, arg2, future):
+            self.result = [arg1, arg2, future.result()]
+        utils.add_future(self.io_loop, self.future, callback, 1, 2)
+        self.future.set_result(3)
+        yield self.assert_done([1, 2, 3])
+
+
 class TestGetHeaders(unittest.TestCase):
 
     def test_propagation(self):
@@ -64,6 +113,54 @@
             self.assertIsNone(utils.json_decode_dict('"not-a-dict"'))
 
 
+class TestMkdir(unittest.TestCase):
+
+    def setUp(self):
+        self.playground = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.playground)
+
+    def test_create_dir(self):
+        # A directory is correctly created.
+        path = os.path.join(self.playground, 'foo')
+        utils.mkdir(path)
+        self.assertTrue(os.path.isdir(path))
+
+    def test_intermediate_dirs(self):
+        # All intermediate directories are created.
+        path = os.path.join(self.playground, 'foo', 'bar', 'leaf')
+        utils.mkdir(path)
+        self.assertTrue(os.path.isdir(path))
+
+    def test_expand_user(self):
+        # The ~ construction is expanded.
+        with mock.patch('os.environ', {'HOME': self.playground}):
+            utils.mkdir('~/in/my/home')
+        path = os.path.join(self.playground, 'in', 'my', 'home')
+        self.assertTrue(os.path.isdir(path))
+
+    def test_existing_dir(self):
+        # The function exits without errors if the target directory exists.
+        path = os.path.join(self.playground, 'foo')
+        os.mkdir(path)
+        utils.mkdir(path)
+
+    def test_existing_file(self):
+        # An OSError is raised if a file already exists in the target path.
+        path = os.path.join(self.playground, 'foo')
+        with open(path, 'w'):
+            with self.assertRaises(OSError):
+                utils.mkdir(path)
+
+    def test_failure(self):
+        # Errors are correctly re-raised.
+        path = os.path.join(self.playground, 'foo')
+        os.chmod(self.playground, 0000)
+        self.addCleanup(os.chmod, self.playground, 0700)
+        with self.assertRaises(OSError):
+            utils.mkdir(os.path.join(path))
+        self.assertFalse(os.path.exists(path))
+
+
 class TestRequestSummary(unittest.TestCase):
 
     def test_summary(self):

=== modified file 'server/guiserver/utils.py'
--- server/guiserver/utils.py	2013-07-24 12:35:50 +0000
+++ server/guiserver/utils.py	2013-08-21 16:12:55 +0000
@@ -17,12 +17,24 @@
 """Juju GUI server utility functions and classes."""
 
 import collections
+import errno
+import functools
 import logging
+import os
 import urlparse
 
 from tornado import escape
 
 
+def add_future(io_loop, future, callback, *args):
+    """Schedule a callback on the IO loop when the given Future is finished.
+
+    The callback will receive the given optional args and the completed Future.
+    """
+    partial_callback = functools.partial(callback, *args)
+    io_loop.add_future(future, partial_callback)
+
+
 def get_headers(request, websocket_url):
     """Return additional headers to be included in the client connection.
 
@@ -55,6 +67,21 @@
     return data
 
 
+def mkdir(path):
+    """Create a leaf directory and all intermediate ones.
+
+    Also expand ~ and ~user constructions.
+    If path exists and it's a directory, return without errors.
+    """
+    path = os.path.expanduser(path)
+    try:
+        os.makedirs(path)
+    except OSError as err:
+        # Re-raise the error if the target path exists but it is not a dir.
+        if (err.errno != errno.EEXIST) or (not os.path.isdir(path)):
+            raise
+
+
 def request_summary(request):
     """Return a string representing a summary for the given request."""
     return '{} {} ({})'.format(request.method, request.uri, request.remote_ip)