Merge lp:~makyo/juju-gui/go-remove-units-1168310 into lp:juju-gui/experimental

[Madison Scott-Clary]

Implement remove_units in go env

https://codereview.appspot.com/8721043/

[Madison Scott-Clary (makyo)]

Reviewers: mp+158688_code.launchpad.net,

Message:
Please take a look.

Description:
Implement remove_units in go env

https://code.launchpad.net/~makyo/juju-gui/go-remove-units-1168310/+merge/158688

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/store/env/go.js
   M test/test_env_go.js

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: test/test_env_go.js
=== modified file 'test/test_env_go.js'
--- test/test_env_go.js 2013-04-12 11:33:12 +0000
+++ test/test_env_go.js 2013-04-12 17:40:34 +0000
@@ -247,6 +247,44 @@
        });
      });

+ it('sends the correct DestroyServiceUnits message', function() {
+ env.remove_units(['django/2', 'django/3']);
+ var last_message = conn.last_message();
+ var expected = {
+ Type: 'Client',
+ Request: 'DestroyServiceUnits',
+ RequestId: 1,
+ Params: {UnitNames: ['django/2', 'django/3']}
+ };
+ assert.deepEqual(expected, last_message);
+ });
+
+ it('successfully removes units from a service', function(done) {
+ env.remove_units(['django/2', 'django/3'], function(data) {
+ assert.deepEqual(['django/2', 'django/3'], data.unit_names);
+ assert.isUndefined(data.err);
+ done();
+ });
+ // Mimic response.
+ conn.msg({
+ RequestId: 1,
+ Response: {}
+ });
+ });
+
+ it('handles failures removing units from a service', function(done) {
+ env.remove_units(['django/2'], function(data) {
+ assert.deepEqual(['django/2'], data.unit_names);
+ assert.strictEqual('unit django/2 does not exist', data.err);
+ done();
+ });
+ // Mimic response.
+ conn.msg({
+ RequestId: 1,
+ Error: 'unit django/2 does not exist'
+ });
+ });
+
      it('sends the correct expose message', function() {
        env.expose('apache');
        var last_message = conn.last_message();

Index: app/store/env/go.js
=== modified file 'app/store/env/go.js'
--- app/store/env/go.js 2013-04-12 12:30:52 +0000
+++ app/store/env/go.js 2013-04-12 17:40:34 +0000
@@ -427,6 +427,50 @@
      },

      /**
+ * Remove units from a service.
+ *
+ * @method remove_units
+ * @param {Array} unit_names The units to be removed.
+ * @param {Function} callback A callable that must be called once the
+ operation is performed.
+ * @return {undefined} Sends a message to the server only.
+ */
+ remove_units: function(unit_names, callback) {
+ var intermediateCallback;
+ if (callback) {
+ // Curry the callback and unit_names. No context is passed.
+ intermediateCallback = Y.bind(this.handleRemoveUnits, null,
+ callback, unit_names);
+ }
+ this._send_rpc({
+ Type: 'Client',
+ Request: 'DestroyServiceUnits',
+ ...

[Jeff Pihach (hatch)]

LGTM - let there be remove_units!

https://codereview.appspot.com/8721043/

[Brad Crittenden (bac)]

LGTM with a couple of small documentation requests.

https://codereview.appspot.com/8721043/diff/1/app/store/env/go.js
File app/store/env/go.js (right):

https://codereview.appspot.com/8721043/diff/1/app/store/env/go.js#newcode435
app/store/env/go.js:435: operation is performed.
Many methods list the items that will be passed to the callable here. I
find that pretty helpful.

https://codereview.appspot.com/8721043/diff/1/app/store/env/go.js#newcode436
app/store/env/go.js:436: * @return {undefined} Sends a message to the
server only.
rick_h claims yuidoc doesn't need this content-free @return. Might be
worth removing and seeing. I'd like to see them go away if we really
don't require them.

https://codereview.appspot.com/8721043/

[Madison Scott-Clary (makyo)]

*** Submitted:

Implement remove_units in go env

R=jeff.pihach, bac
CC=
https://codereview.appspot.com/8721043

https://codereview.appspot.com/8721043/

=== modified file 'app/store/env/go.js'
--- app/store/env/go.js	2013-04-12 12:30:52 +0000
+++ app/store/env/go.js	2013-04-12 17:47:32 +0000
@@ -427,6 +427,50 @@
     },
 
     /**
+     * Remove units from a service.
+     *
+     * @method remove_units
+     * @param {Array} unit_names The units to be removed.
+     * @param {Function} callback A callable that must be called once the
+         operation is performed.
+     * @return {undefined} Sends a message to the server only.
+     */
+    remove_units: function(unit_names, callback) {
+      var intermediateCallback;
+      if (callback) {
+        // Curry the callback and unit_names.  No context is passed.
+        intermediateCallback = Y.bind(this.handleRemoveUnits, null,
+            callback, unit_names);
+      }
+      this._send_rpc({
+        Type: 'Client',
+        Request: 'DestroyServiceUnits',
+        Params: {UnitNames: unit_names}
+      }, intermediateCallback);
+    },
+
+    /**
+     * Transform the data returned from the juju-core remove_units call into
+     * that suitable for the user callback.
+     *
+     * @method handleRemoveUnits
+     * @static
+     * @param {Function} userCallback The callback originally submitted by the
+     * call site.
+     * @param {Array} unitNames The names of the removed units.  Passed in
+     * since it is not part of the response.
+     * @param {Object} data The response returned by the server.
+     * @return {undefined} Nothing.
+     */
+    handleRemoveUnits: function(userCallback, unitNames, data) {
+      var transformedData = {
+        err: data.Error,
+        unit_names: unitNames
+      };
+      userCallback(transformedData);
+    },
+
+    /**
      * Expose the given service.
      *
      * @method expose

=== modified file 'test/test_env_go.js'
--- test/test_env_go.js	2013-04-12 11:33:12 +0000
+++ test/test_env_go.js	2013-04-12 17:47:32 +0000
@@ -247,6 +247,44 @@
       });
     });
 
+    it('sends the correct DestroyServiceUnits message', function() {
+      env.remove_units(['django/2', 'django/3']);
+      var last_message = conn.last_message();
+      var expected = {
+        Type: 'Client',
+        Request: 'DestroyServiceUnits',
+        RequestId: 1,
+        Params: {UnitNames: ['django/2', 'django/3']}
+      };
+      assert.deepEqual(expected, last_message);
+    });
+
+    it('successfully removes units from a service', function(done) {
+      env.remove_units(['django/2', 'django/3'], function(data) {
+        assert.deepEqual(['django/2', 'django/3'], data.unit_names);
+        assert.isUndefined(data.err);
+        done();
+      });
+      // Mimic response.
+      conn.msg({
+        RequestId: 1,
+        Response: {}
+      });
+    });
+
+    it('handles failures removing units from a service', function(done) {
+      env.remove_units(['django/2'], function(data) {
+        assert.deepEqual(['django/2'], data.unit_names);
+        assert.strictEqual('unit django/2 does not exist', data.err);
+        done();
+      });
+      // Mimic response.
+      conn.msg({
+        RequestId: 1,
+        Error: 'unit django/2 does not exist'
+      });
+    });
+
     it('sends the correct expose message', function() {
       env.expose('apache');
       var last_message = conn.last_message();