Merge lp:~bac/juju-gui/1096230 into lp:juju-gui/experimental

[Brad Crittenden]

Add support for _annotation calls to env.

The calls update_annotations, get_annotations, and remove_annotations are
added to the env object.

As a drive-by, in test_env.js the unnecessary use of async calls was
eliminated by removing the 'done' parameter. Leaving it in place where not
required was confusing.

https://codereview.appspot.com/7064060/

[Brad Crittenden (bac)]

Reviewers: mp+142513_code.launchpad.net,

Message:
Please take a look.

Description:
Add support for _annotation calls to env.

The calls update_annotations, get_annotations, and remove_annotations
are
added to the env object.

As a drive-by, in test_env.js the unnecessary use of async calls was
eliminated by removing the 'done' parameter. Leaving it in place where
not
required was confusing.

https://code.launchpad.net/~bac/juju-gui/1096230/+merge/142513

(do not edit description out of merge proposal)

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

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

[Gary Poster (gary)]

Land with changes.

Looks good to me. Thank you!

Gary

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

https://codereview.appspot.com/7064060/diff/1/app/store/env.js#newcode276
app/store/env.js:276: * @param {Object} entity The name of a machine,
unit, service, or
Would be nice to specify what the name of these things is--examples.

https://codereview.appspot.com/7064060/diff/1/app/store/env.js#newcode307
app/store/env.js:307: * environment.
You don't describe the keys argument. Given that it appears to be
optional, it would be particularly nice to document.

https://codereview.appspot.com/7064060/

[lp:~bac/juju-gui/1096230]

307. By Brad Crittenden on 2013-01-09

Updated function documentation.

308. By Brad Crittenden on 2013-01-09

Updated use of environment entity in annotation command documentation.

[Brad Crittenden (bac)]

Please take a look.

https://codereview.appspot.com/7064060/

[Kapil Thangavelu (hazmat)]

lgtm, one minor of note

https://codereview.appspot.com/7064060/diff/5001/app/store/env.js
File app/store/env.js (right):

https://codereview.appspot.com/7064060/diff/5001/app/store/env.js#newcode296
app/store/env.js:296: * callback. The invocation of this command
returns nothing.
the doc string should also have a note that annotations come back in the
delta stream so explicit fetching should be rarely be needed or unesc.

https://codereview.appspot.com/7064060/

[lp:~bac/juju-gui/1096230]

309. By Brad Crittenden on 2013-01-09

Updated doc for get_annotations to note it is rarely required to be used.

310. By Brad Crittenden on 2013-01-09

Fixed typo

[Brad Crittenden (bac)]

*** Submitted:

Add support for _annotation calls to env.

The calls update_annotations, get_annotations, and remove_annotations
are
added to the env object.

As a drive-by, in test_env.js the unnecessary use of async calls was
eliminated by removing the 'done' parameter. Leaving it in place where
not
required was confusing.

R=gary.poster, hazmat
CC=
https://codereview.appspot.com/7064060

https://codereview.appspot.com/7064060/

=== modified file 'app/store/env.js'
--- app/store/env.js	2013-01-07 21:38:42 +0000
+++ app/store/env.js	2013-01-09 14:59:22 +0000
@@ -268,9 +268,60 @@
     get_endpoints: function(services, callback) {
       this._send_rpc({'op': 'get_endpoints', 'service_names': services},
                      callback);
+    },
+
+    /**
+     * Update the annotations for an entity by name.
+     *
+     * @param {Object} entity The name of a machine, unit, service, or
+     *   environment, e.g. '0', 'mysql/0', or 'mysql'.  To specify the
+     *   environment as the entity the magic string 'env' is used.
+     * @param {Object} data A dictionary of key, value pairs.
+     * @return {undefined} Nothing.
+     */
+    update_annotations: function(entity, data, callback) {
+      this._send_rpc({
+        op: 'update_annotations',
+        entity: entity,
+        data: data}, callback);
+    },
+
+    /**
+     * Get the annotations for an entity by name.
+     *
+     * Note that the annotations are returned as part of the delta stream, so
+     * the explicit use of this command should rarely be needed.
+     *
+     * @param {Object} entity The name of a machine, unit, service, or
+     *   environment, e.g. '0', 'mysql/0', or 'mysql'.  To specify the
+     *   environment as the entity the magic string 'env' is used.
+     * @return {Object} A dictionary of key,value pairs is returned in the
+     *   callback.  The invocation of this command returns nothing.
+     */
+    get_annotations: function(entity, callback) {
+      this._send_rpc({
+        op: 'get_annotations',
+        entity: entity}, callback);
+    },
+
+    /**
+     * Remove the annotations for an entity by name.
+     *
+     * @param {Object} entity The name of a machine, unit, service, or
+     *   environment, e.g. '0', 'mysql/0', or 'mysql'.  To specify the
+     *   environment as the entity the magic string 'env' is used.
+     * @param {Object} keys An optional list of annotation key names for the
+     *   annotations to be deleted.  If no keys are passed, all annotations
+     *   for the entity will be removed.
+     * @return {undefined} Nothing.
+     */
+    remove_annotations: function(entity, keys, callback) {
+      this._send_rpc({
+        op: 'remove_annotations',
+        entity: entity,
+        keys: keys || []}, callback);
     }
 
-
   });
 
 

=== modified file 'test/test_env.js'
--- test/test_env.js	2012-10-14 16:30:50 +0000
+++ test/test_env.js	2013-01-09 14:59:22 +0000
@@ -31,15 +31,14 @@
       done();
     });
 
-    it('can deploy a service', function(done) {
+    it('can deploy a service', function() {
       env.deploy('precise/mysql');
       msg = conn.last_message();
       msg.op.should.equal('deploy');
       msg.charm_url.should.equal('precise/mysql');
-      done();
     });
 
-    it('can deploy a service with a config file', function(done) {
+    it('can deploy a service with a config file', function() {
       /*jshint multistr:true */
       var config_raw = 'tuning-level: \nexpert-mojo';
       /*jshint multistr:false */
@@ -48,16 +47,14 @@
       msg.op.should.equal('deploy');
       msg.charm_url.should.equal('precise/mysql');
       msg.config_raw.should.equal(config_raw);
-      done();
     });
 
-    it('can add a unit', function(done) {
+    it('can add a unit', function() {
       env.add_unit('mysql', 3);
       msg = conn.last_message();
       msg.op.should.equal('add_unit');
       msg.service_name.should.equal('mysql');
       msg.num_units.should.equal(3);
-      done();
     });
 
     it('can accept a callback on its methods', function(done) {
@@ -76,7 +73,7 @@
         'result': {'id': 'cs:precise/mysql'}});
     });
 
-    it('can resolve a problem with a unit', function(done) {
+    it('can resolve a problem with a unit', function() {
       var unit_name = 'mysql/0';
       env.resolved(unit_name);
       msg = conn.last_message();
@@ -84,10 +81,9 @@
       msg.unit_name.should.equal(unit_name);
       var _ = expect(msg.relation_name).to.not.exist;
       msg.retry.should.equal(false);
-      done();
     });
 
-    it('can resolve a problem with a unit relation', function(done) {
+    it('can resolve a problem with a unit relation', function() {
       var unit_name = 'mysql/0';
       var rel_name = 'relation-0000000000';
       env.resolved(unit_name, rel_name);
@@ -96,10 +92,9 @@
       msg.unit_name.should.equal(unit_name);
       msg.relation_name.should.equal(rel_name);
       msg.retry.should.equal(false);
-      done();
     });
 
-    it('can retry a problem with a unit', function(done) {
+    it('can retry a problem with a unit', function() {
       var unit_name = 'mysql/0';
       env.resolved(unit_name, null, true);
       msg = conn.last_message();
@@ -107,7 +102,6 @@
       msg.unit_name.should.equal(unit_name);
       var _ = expect(msg.relation_name).to.not.exist;
       msg.retry.should.equal(true);
-      done();
     });
 
     it('can retry a problem with a unit using a callback', function(done) {
@@ -124,7 +118,7 @@
         request_id: msg.request_id});
     });
 
-    it('will populate the provider type and default series', function(done) {
+    it('will populate the provider type and default series', function() {
       var providerType = 'super provider',
           defaultSeries = 'oneiric',
           evt =
@@ -141,7 +135,6 @@
       // After the message arrives the provider type is set.
       assert.equal(env.get('providerType'), providerType);
       assert.equal(env.get('defaultSeries'), defaultSeries);
-      done();
     });
 
     it('can get endpoints for a service', function() {
@@ -151,6 +144,41 @@
       msg.service_names.should.eql(['mysql']);
     });
 
+    it('can update annotations', function() {
+      var unit_name = 'mysql/0';
+      env.update_annotations(unit_name, {name: 'A'});
+      msg = conn.last_message();
+      msg.op.should.equal('update_annotations');
+      msg.entity.should.equal(unit_name);
+      msg.data.name.should.equal('A');
+    });
+
+    it('can get annotations', function() {
+      var unit_name = 'mysql/0';
+      env.get_annotations(unit_name);
+      msg = conn.last_message();
+      msg.op.should.equal('get_annotations');
+      msg.entity.should.equal(unit_name);
+    });
+
+    it('can remove annotations with specified keys', function() {
+      var unit_name = 'mysql/0';
+      var keys = ['key1', 'key2'];
+      env.remove_annotations(unit_name, keys);
+      msg = conn.last_message();
+      msg.op.should.equal('remove_annotations');
+      msg.entity.should.equal(unit_name);
+      msg.keys.should.eql(keys);
+    });
+
+    it('can remove annotations with no specified keys', function() {
+      var unit_name = 'mysql/0';
+      env.remove_annotations(unit_name);
+      msg = conn.last_message();
+      msg.op.should.equal('remove_annotations');
+      msg.entity.should.equal(unit_name);
+      msg.keys.should.eql([]);
+    });
 
   });
 })();