Merge lp:~frankban/juju-quickstart/watcher-logs into lp:juju-quickstart

[Francesco Banconi]

Re-enable watcher requests/responses logging.

Tests: `make check`.

QA:
Run `.venv/bin/python juju-quickstart --debug` and
check that all the "AllWatcher" requests/responses
are properly logged.

https://codereview.appspot.com/30810043/

[Francesco Banconi (frankban)]

Reviewers: mp+196262_code.launchpad.net,

Message:
Please take a look.

Description:
Re-enable watcher requests/responses logging.

Tests: `make check`.

QA:
Run `.venv/bin/python juju-quickstart --debug` and
check that all the "AllWatcher" requests/responses
are properly logged.

https://code.launchpad.net/~frankban/juju-quickstart/watcher-logs/+merge/196262

(do not edit description out of merge proposal)

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

Affected files (+85, -25 lines):
   A [revision details]
   M quickstart/juju.py
   M quickstart/tests/test_juju.py

[Brad Crittenden (bac)]

LGTM QA-OK

https://codereview.appspot.com/30810043/diff/1/quickstart/tests/test_juju.py
File quickstart/tests/test_juju.py (right):

https://codereview.appspot.com/30810043/diff/1/quickstart/tests/test_juju.py#newcode216
quickstart/tests/test_juju.py:216: # We are only interested on the calls
from now on.
s/on/in/

https://codereview.appspot.com/30810043/

[lp:~frankban/juju-quickstart/watcher-logs]

21. By Francesco Banconi on 2013-11-22

Changes as per review.

[Francesco Banconi (frankban)]

*** Submitted:

Re-enable watcher requests/responses logging.

Tests: `make check`.

QA:
Run `.venv/bin/python juju-quickstart --debug` and
check that all the "AllWatcher" requests/responses
are properly logged.

R=bac
CC=
https://codereview.appspot.com/30810043

https://codereview.appspot.com/30810043/diff/1/quickstart/tests/test_juju.py
File quickstart/tests/test_juju.py (right):

https://codereview.appspot.com/30810043/diff/1/quickstart/tests/test_juju.py#newcode216
quickstart/tests/test_juju.py:216: # We are only interested on the calls
from now on.
On 2013/11/22 13:10:08, bac wrote:
> s/on/in/

Done.

https://codereview.appspot.com/30810043/

[Francesco Banconi (frankban)]

Thanks for the review Brad!

https://codereview.appspot.com/30810043/

=== modified file 'quickstart/juju.py'
--- quickstart/juju.py	2013-11-18 20:34:09 +0000
+++ quickstart/juju.py	2013-11-22 13:15:18 +0000
@@ -75,6 +75,24 @@
         }
         return self._rpc(request)
 
+    def get_watcher(self):
+        """Return a connected/authenticated environment watcher.
+
+        This method is similar to jujuclient.Environment.get_watch, but it
+        enables logging on the resulting watcher requests/responses traffic.
+        """
+        # Logging is enabled by the connect factory function, which uses our
+        # customized WebSocketConnection. Note that, since jujuclient does not
+        # track request identifiers, it is not currently possible to avoid
+        # establishing a new connection for each watcher.
+        env = connect(self.endpoint)
+        # For the remaining bits, see jujuclient.Environment.get_watch.
+        env.login(**self._creds)
+        watcher = jujuclient.Watcher(env.conn)
+        self._watches.append(watcher)
+        watcher.start()
+        return watcher
+
     def get_status(self):
         """Return the current status of the environment.
 
@@ -86,7 +104,7 @@
               change (i.e. "change" or "remove");
             - data is a dict containing information about the releated entity.
         """
-        with self.get_watch() as watcher:
+        with self.get_watcher() as watcher:
             changeset = watcher.next()
         return changeset
 
@@ -96,7 +114,7 @@
         For each changeset, call the given processor callable, and yield
         the values returned by the processor.
         """
-        with self.get_watch() as watcher:
+        with self.get_watcher() as watcher:
             # The watcher closes when the context manager exit hook is called.
             for changeset in watcher:
                 changes = processor(changeset)

=== modified file 'quickstart/tests/test_juju.py'
--- quickstart/tests/test_juju.py	2013-11-19 12:22:32 +0000
+++ quickstart/tests/test_juju.py	2013-11-22 13:15:18 +0000
@@ -100,17 +100,17 @@
             'Params': params,
         }
 
-    def patch_get_watch(self, return_value):
-        """Patch the Environment.get_watch method.
+    def patch_get_watcher(self, return_value):
+        """Patch the Environment.get_watcher method.
 
         When the resulting mock is used as a context manager, the given return
         value is returned.
         """
-        get_watch_path = 'quickstart.juju.jujuclient.Environment.get_watch'
-        mock_get_watch = mock.MagicMock()
-        mock_get_watch().__enter__.return_value = iter(return_value)
-        mock_get_watch.reset_mock()
-        return mock.patch(get_watch_path, mock_get_watch)
+        get_watcher_path = 'quickstart.juju.Environment.get_watcher'
+        mock_get_watcher = mock.MagicMock()
+        mock_get_watcher().__enter__.return_value = iter(return_value)
+        mock_get_watcher.reset_mock()
+        return mock.patch(get_watcher_path, mock_get_watcher)
 
     def processor(self, changeset):
         self.changesets.append(changeset)
@@ -209,39 +209,79 @@
         }
         mock_rpc.assert_called_once_with(expected)
 
+    @patch_rpc
+    def test_get_watcher(self, mock_rpc):
+        # Environment watching is correctly started.
+        self.env.login('Secret!')
+        # We are only interested in the calls from now on.
+        mock_rpc.reset_mock()
+        connect_path = 'quickstart.juju.WebSocketConnection.connect'
+        watcher_rpc_path = 'quickstart.juju.jujuclient.Watcher._rpc'
+        with mock.patch(connect_path) as mock_connect:
+            with mock.patch(watcher_rpc_path) as mock_watcher_rpc:
+                watcher = self.env.get_watcher()
+        # The returned watcher is running.
+        self.assertTrue(watcher.running)
+        # The watcher uses our customized WebSocket connection with logging.
+        self.assertIsInstance(watcher.conn, juju.WebSocketConnection)
+        # A connection has been established with the API backend.
+        mock_connect.assert_called_once_with(self.api_url, origin=self.api_url)
+        # The connection used by the watcher is authenticated.
+        expected = {
+            'Type': 'Admin',
+            'Request': 'Login',
+            'Params': {'AuthTag': 'user-admin', 'Password': 'Secret!'},
+        }
+        mock_rpc.assert_called_once_with(expected)
+        # The watcher sent the correct start request.
+        mock_watcher_rpc.assert_called_with({
+            'Type': 'Client',
+            'Request': 'WatchAll',
+            'Params': {},
+        })
+
     def test_get_status(self):
         # The current status of the Juju environment is properly returned.
-        changeset1 = ['change1', 'change2']
-        changeset2 = ['change3']
-        with self.patch_get_watch([changeset1, changeset2]) as mock_get_watch:
+        changesets = [['change1', 'change2'], ['change3']]
+        with self.patch_get_watcher(changesets) as mock_get_watcher:
             status = self.env.get_status()
         # The get_status call only waits for the first changeset.
-        self.assertEqual(changeset1, status)
+        self.assertEqual(changesets[0], status)
         # The watcher is correctly closed.
-        self.assertEqual(1, mock_get_watch().__exit__.call_count)
+        self.assertEqual(1, mock_get_watcher().__exit__.call_count)
+
+    @patch_rpc
+    def test_login(self, mock_rpc):
+        # The login API call is properly generated.
+        self.env.login('Secret!')
+        expected = {
+            'Type': 'Admin',
+            'Request': 'Login',
+            'Params': {'AuthTag': 'user-admin', 'Password': 'Secret!'},
+        }
+        mock_rpc.assert_called_once_with(expected)
 
     def test_watch_changes(self):
         # It is possible to watch for changes using a processor callable.
-        changeset1 = ['change1', 'change2']
-        changeset2 = ['change3']
-        with self.patch_get_watch([changeset1, changeset2]) as mock_get_watch:
+        changesets = [['change1', 'change2'], ['change3']]
+        with self.patch_get_watcher(changesets) as mock_get_watcher:
             watcher = self.env.watch_changes(self.processor)
             # The first set of changes is correctly returned.
             changeset = watcher.next()
-            self.assertEqual(changeset1, changeset)
+            self.assertEqual(changesets[0], changeset)
             # The second set of changes is correctly returned.
             changeset = watcher.next()
-            self.assertEqual(changeset2, changeset)
+            self.assertEqual(changesets[1], changeset)
         # All the changes have been processed.
-        self.assertEqual([changeset1, changeset2], self.changesets)
+        self.assertEqual(changesets, self.changesets)
         # Ensure the API has been used properly.
-        mock_get_watch().__enter__.assert_called_once_with()
+        mock_get_watcher().__enter__.assert_called_once_with()
 
     def test_watch_changes_map(self):
         # The processor callable can be used to modify changes.
         changeset1 = ['change1', 'change2']
         changeset2 = ['change3']
-        with self.patch_get_watch([changeset1, changeset2]):
+        with self.patch_get_watcher([changeset1, changeset2]):
             watcher = self.env.watch_changes(len)
             changesets = list(watcher)
         self.assertEqual([len(changeset1), len(changeset2)], changesets)
@@ -251,7 +291,7 @@
         changeset1 = ['change1', 'change2']
         changeset2 = ['change3']
         processor = lambda changes: None if len(changes) == 1 else changes
-        with self.patch_get_watch([changeset1, changeset2]):
+        with self.patch_get_watcher([changeset1, changeset2]):
             watcher = self.env.watch_changes(processor)
             changesets = list(watcher)
         self.assertEqual([changeset1], changesets)
@@ -260,13 +300,13 @@
         # A stop API call on the AllWatcher is performed when the watcher is
         # garbage collected.
         changeset = ['change1', 'change2']
-        with self.patch_get_watch([changeset]) as mock_get_watch:
+        with self.patch_get_watcher([changeset]) as mock_get_watcher:
             watcher = self.env.watch_changes(self.processor)
             # The first set of changes is correctly returned.
             watcher.next()
             del watcher
         # Ensure the API has been used properly.
-        self.assertEqual(1, mock_get_watch().__exit__.call_count)
+        self.assertEqual(1, mock_get_watcher().__exit__.call_count)
 
 
 class TestWebSocketConnection(unittest.TestCase):