Merge lp:~jimbaker/pyjuju/relation-ids-whitespace-separated into lp:pyjuju

[Jim Baker]

Support smart formatting for relation-ids command

Adds format_smart method for RelationIdsCli, which outputs whitespace separated (to be precise, per line) relation idents.

https://codereview.appspot.com/6118047/

[Jim Baker (jimbaker)]

Reviewers: mp+103387_code.launchpad.net,

Message:
Please take a look.

Description:
Support smart formatting for relation-ids command

Adds format_smart method for RelationIdsCli, which outputs whitespace
separated (to be precise, per line) relation idents.

https://code.launchpad.net/~jimbaker/juju/relation-ids-whitespace-separated/+merge/103387

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/hooks/commands.py
   M juju/hooks/tests/test_invoker.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision: <email address hidden>

Index: juju/hooks/commands.py
=== modified file 'juju/hooks/commands.py'
--- juju/hooks/commands.py 2012-04-10 16:42:18 +0000
+++ juju/hooks/commands.py 2012-04-24 21:47:40 +0000
@@ -117,6 +117,10 @@
              self.options.client_id, self.options.relation_name)
          returnValue(result)

+ def format_smart(self, result, stream):
+ for ident in result:
+ print >>stream, ident
+

  def relation_ids():
      """Entry point for relation-set."""

Index: juju/hooks/tests/test_invoker.py
=== modified file 'juju/hooks/tests/test_invoker.py'
--- juju/hooks/tests/test_invoker.py 2012-04-10 20:16:14 +0000
+++ juju/hooks/tests/test_invoker.py 2012-04-24 22:13:23 +0000
@@ -1102,7 +1102,31 @@

      @defer.inlineCallbacks
      def test_relation_ids(self):
- """Verify `relation-ids` command returns corresponding ids."""
+ """Verify `relation-ids` command returns white-space separated
ids."""
+ yield self.add_a_blog("wordpress2")
+ hook_log = self.capture_logging("hook")
+
+ # Invoker will be in the context of the mysql/0 service unit
+ exe = yield self.ua.get_invoker(
+ "db:0", "add", "mysql/0", self.relation,
+ client_id="client_id")
+
+ # Then verify the hook lists the relation ids corresponding to
+ # the relation name `db`
+ hook = self.create_hook("relation-ids", "db")
+ result = yield exe(hook)
+ self.assertEqual(result, 0)
+ yield exe.ended
+ # Smart formtting outputs one id per line
+ self.assertEqual(
+ hook_log.getvalue(), "db:0\ndb:1\n\n")
+ # But can be split on whitespace
+ self.assertEqual(
+ hook_log.getvalue().split(), ["db:0", "db:1"])
+
+ @defer.inlineCallbacks
+ def test_relation_ids_json_format(self):
+ """Verify `relation-ids --format=json` command returns ids in
json."""
          yield self.add_a_blog("wordpress2")
          yield self.add_db_admin_tool("admin")
          hook_log = self.capture_logging("hook")

[Kapil Thangavelu (hazmat)]

fix looks straightforwards, its just not clear its as intended

https://codereview.appspot.com/6118047/diff/1/juju/hooks/tests/test_invoker.py
File juju/hooks/tests/test_invoker.py (right):

https://codereview.appspot.com/6118047/diff/1/juju/hooks/tests/test_invoker.py#newcode1105
juju/hooks/tests/test_invoker.py:1105: """Verify `relation-ids` command
returns white-space separated ids."""
this isn't space separated its newline separated. what's the spec say
regarding?

https://codereview.appspot.com/6118047/

[Jim Baker (jimbaker)]

Please take a look.

https://codereview.appspot.com/6118047/

[lp:~jimbaker/pyjuju/relation-ids-whitespace-separated]

534. By Jim Baker on 2012-04-25

Update docstring/comments to describe more precisely relation-ids, when using smart formatting, outputs newline separated ids; and that this is also whitespace separated

[Jim Baker (jimbaker)]

Fixed in latest version of the MP

https://codereview.appspot.com/6118047/diff/1/juju/hooks/tests/test_invoker.py
File juju/hooks/tests/test_invoker.py (right):

https://codereview.appspot.com/6118047/diff/1/juju/hooks/tests/test_invoker.py#newcode1105
juju/hooks/tests/test_invoker.py:1105: """Verify `relation-ids` command
returns white-space separated ids."""
The spec says:
"The output is a whitespace separated list of relation ids, if any."
Per our conversation on IRC, updated docstring and comments to reflect
this is newline separated; but since shell/Python/presumably others
treat newline as just whitespace, this follows the letter of the spec.
It also follows the convention for relation-list.

https://codereview.appspot.com/6118047/

[Kapil Thangavelu (hazmat)]

lgtm, pls also update the spec as thats what most people will read for
docs.

https://codereview.appspot.com/6118047/

=== modified file 'juju/hooks/commands.py'
--- juju/hooks/commands.py	2012-04-10 16:42:18 +0000
+++ juju/hooks/commands.py	2012-04-25 00:23:20 +0000
@@ -117,6 +117,10 @@
             self.options.client_id, self.options.relation_name)
         returnValue(result)
 
+    def format_smart(self, result, stream):
+        for ident in result:
+            print >>stream, ident
+
 
 def relation_ids():
     """Entry point for relation-set."""

=== modified file 'juju/hooks/tests/test_invoker.py'
--- juju/hooks/tests/test_invoker.py	2012-04-10 20:16:14 +0000
+++ juju/hooks/tests/test_invoker.py	2012-04-25 00:23:20 +0000
@@ -1102,7 +1102,33 @@
 
     @defer.inlineCallbacks
     def test_relation_ids(self):
-        """Verify `relation-ids` command returns corresponding ids."""
+        """Verify `relation-ids` command returns ids separated by newlines."""
+        yield self.add_a_blog("wordpress2")
+        hook_log = self.capture_logging("hook")
+
+        # Invoker will be in the context of the mysql/0 service unit
+        exe = yield self.ua.get_invoker(
+            "db:0", "add", "mysql/0", self.relation,
+            client_id="client_id")
+
+        # Then verify the hook lists the relation ids corresponding to
+        # the relation name `db`
+        hook = self.create_hook("relation-ids", "db")
+        result = yield exe(hook)
+        self.assertEqual(result, 0)
+        yield exe.ended
+        # Smart formtting outputs one id per line
+        self.assertEqual(
+            hook_log.getvalue(), "db:0\ndb:1\n\n")
+        # But newlines are just whitespace to the shell or to Python,
+        # so they can be split accordingly, adhering to the letter of
+        # the spec
+        self.assertEqual(
+            hook_log.getvalue().split(), ["db:0", "db:1"])
+
+    @defer.inlineCallbacks
+    def test_relation_ids_json_format(self):
+        """Verify `relation-ids --format=json` command returns ids in json."""
         yield self.add_a_blog("wordpress2")
         yield self.add_db_admin_tool("admin")
         hook_log = self.capture_logging("hook")