juju-gui

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

  1. 1096230
  2. Merge into trunk
Proposed by Brad Crittenden
Status: Merged
Merged at revision: 306
Proposed branch: lp:~bac/juju-gui/1096230
Merge into: lp:juju-gui/experimental
Diff against target: 205 lines (+94/-15)
2 files modified
app/store/env.js (+52/-1)
test/test_env.js (+42/-14)
To merge this branch: bzr merge lp:~bac/juju-gui/1096230
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+142513@code.launchpad.net

Description of the change

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/

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

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

Revision history for this message
Gary Poster (gary) wrote :

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 updated
307. By Brad Crittenden

Updated function documentation.

308. By Brad Crittenden

Updated use of environment entity in annotation command documentation.

Revision history for this message
Brad Crittenden (bac) wrote :

Please take a look.

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

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

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 updated
309. By Brad Crittenden

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

310. By Brad Crittenden

Fixed typo

Revision history for this message
Brad Crittenden (bac) wrote :

*** 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/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
268 get_endpoints: function(services, callback) {268 get_endpoints: function(services, callback) {
269 this._send_rpc({'op': 'get_endpoints', 'service_names': services},269 this._send_rpc({'op': 'get_endpoints', 'service_names': services},
270 callback);270 callback);
271 },
272
273 /**
274 * Update the annotations for an entity by name.
275 *
276 * @param {Object} entity The name of a machine, unit, service, or
277 * environment, e.g. '0', 'mysql/0', or 'mysql'. To specify the
278 * environment as the entity the magic string 'env' is used.
279 * @param {Object} data A dictionary of key, value pairs.
280 * @return {undefined} Nothing.
281 */
282 update_annotations: function(entity, data, callback) {
283 this._send_rpc({
284 op: 'update_annotations',
285 entity: entity,
286 data: data}, callback);
287 },
288
289 /**
290 * Get the annotations for an entity by name.
291 *
292 * Note that the annotations are returned as part of the delta stream, so
293 * the explicit use of this command should rarely be needed.
294 *
295 * @param {Object} entity The name of a machine, unit, service, or
296 * environment, e.g. '0', 'mysql/0', or 'mysql'. To specify the
297 * environment as the entity the magic string 'env' is used.
298 * @return {Object} A dictionary of key,value pairs is returned in the
299 * callback. The invocation of this command returns nothing.
300 */
301 get_annotations: function(entity, callback) {
302 this._send_rpc({
303 op: 'get_annotations',
304 entity: entity}, callback);
305 },
306
307 /**
308 * Remove the annotations for an entity by name.
309 *
310 * @param {Object} entity The name of a machine, unit, service, or
311 * environment, e.g. '0', 'mysql/0', or 'mysql'. To specify the
312 * environment as the entity the magic string 'env' is used.
313 * @param {Object} keys An optional list of annotation key names for the
314 * annotations to be deleted. If no keys are passed, all annotations
315 * for the entity will be removed.
316 * @return {undefined} Nothing.
317 */
318 remove_annotations: function(entity, keys, callback) {
319 this._send_rpc({
320 op: 'remove_annotations',
321 entity: entity,
322 keys: keys || []}, callback);
271 }323 }
272324
273
274 });325 });
275326
276327
277328
=== 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 @@
31 done();31 done();
32 });32 });
3333
34 it('can deploy a service', function(done) {34 it('can deploy a service', function() {
35 env.deploy('precise/mysql');35 env.deploy('precise/mysql');
36 msg = conn.last_message();36 msg = conn.last_message();
37 msg.op.should.equal('deploy');37 msg.op.should.equal('deploy');
38 msg.charm_url.should.equal('precise/mysql');38 msg.charm_url.should.equal('precise/mysql');
39 done();
40 });39 });
4140
42 it('can deploy a service with a config file', function(done) {41 it('can deploy a service with a config file', function() {
43 /*jshint multistr:true */42 /*jshint multistr:true */
44 var config_raw = 'tuning-level: \nexpert-mojo';43 var config_raw = 'tuning-level: \nexpert-mojo';
45 /*jshint multistr:false */44 /*jshint multistr:false */
@@ -48,16 +47,14 @@
48 msg.op.should.equal('deploy');47 msg.op.should.equal('deploy');
49 msg.charm_url.should.equal('precise/mysql');48 msg.charm_url.should.equal('precise/mysql');
50 msg.config_raw.should.equal(config_raw);49 msg.config_raw.should.equal(config_raw);
51 done();
52 });50 });
5351
54 it('can add a unit', function(done) {52 it('can add a unit', function() {
55 env.add_unit('mysql', 3);53 env.add_unit('mysql', 3);
56 msg = conn.last_message();54 msg = conn.last_message();
57 msg.op.should.equal('add_unit');55 msg.op.should.equal('add_unit');
58 msg.service_name.should.equal('mysql');56 msg.service_name.should.equal('mysql');
59 msg.num_units.should.equal(3);57 msg.num_units.should.equal(3);
60 done();
61 });58 });
6259
63 it('can accept a callback on its methods', function(done) {60 it('can accept a callback on its methods', function(done) {
@@ -76,7 +73,7 @@
76 'result': {'id': 'cs:precise/mysql'}});73 'result': {'id': 'cs:precise/mysql'}});
77 });74 });
7875
79 it('can resolve a problem with a unit', function(done) {76 it('can resolve a problem with a unit', function() {
80 var unit_name = 'mysql/0';77 var unit_name = 'mysql/0';
81 env.resolved(unit_name);78 env.resolved(unit_name);
82 msg = conn.last_message();79 msg = conn.last_message();
@@ -84,10 +81,9 @@
84 msg.unit_name.should.equal(unit_name);81 msg.unit_name.should.equal(unit_name);
85 var _ = expect(msg.relation_name).to.not.exist;82 var _ = expect(msg.relation_name).to.not.exist;
86 msg.retry.should.equal(false);83 msg.retry.should.equal(false);
87 done();
88 });84 });
8985
90 it('can resolve a problem with a unit relation', function(done) {86 it('can resolve a problem with a unit relation', function() {
91 var unit_name = 'mysql/0';87 var unit_name = 'mysql/0';
92 var rel_name = 'relation-0000000000';88 var rel_name = 'relation-0000000000';
93 env.resolved(unit_name, rel_name);89 env.resolved(unit_name, rel_name);
@@ -96,10 +92,9 @@
96 msg.unit_name.should.equal(unit_name);92 msg.unit_name.should.equal(unit_name);
97 msg.relation_name.should.equal(rel_name);93 msg.relation_name.should.equal(rel_name);
98 msg.retry.should.equal(false);94 msg.retry.should.equal(false);
99 done();
100 });95 });
10196
102 it('can retry a problem with a unit', function(done) {97 it('can retry a problem with a unit', function() {
103 var unit_name = 'mysql/0';98 var unit_name = 'mysql/0';
104 env.resolved(unit_name, null, true);99 env.resolved(unit_name, null, true);
105 msg = conn.last_message();100 msg = conn.last_message();
@@ -107,7 +102,6 @@
107 msg.unit_name.should.equal(unit_name);102 msg.unit_name.should.equal(unit_name);
108 var _ = expect(msg.relation_name).to.not.exist;103 var _ = expect(msg.relation_name).to.not.exist;
109 msg.retry.should.equal(true);104 msg.retry.should.equal(true);
110 done();
111 });105 });
112106
113 it('can retry a problem with a unit using a callback', function(done) {107 it('can retry a problem with a unit using a callback', function(done) {
@@ -124,7 +118,7 @@
124 request_id: msg.request_id});118 request_id: msg.request_id});
125 });119 });
126120
127 it('will populate the provider type and default series', function(done) {121 it('will populate the provider type and default series', function() {
128 var providerType = 'super provider',122 var providerType = 'super provider',
129 defaultSeries = 'oneiric',123 defaultSeries = 'oneiric',
130 evt =124 evt =
@@ -141,7 +135,6 @@
141 // After the message arrives the provider type is set.135 // After the message arrives the provider type is set.
142 assert.equal(env.get('providerType'), providerType);136 assert.equal(env.get('providerType'), providerType);
143 assert.equal(env.get('defaultSeries'), defaultSeries);137 assert.equal(env.get('defaultSeries'), defaultSeries);
144 done();
145 });138 });
146139
147 it('can get endpoints for a service', function() {140 it('can get endpoints for a service', function() {
@@ -151,6 +144,41 @@
151 msg.service_names.should.eql(['mysql']);144 msg.service_names.should.eql(['mysql']);
152 });145 });
153146
147 it('can update annotations', function() {
148 var unit_name = 'mysql/0';
149 env.update_annotations(unit_name, {name: 'A'});
150 msg = conn.last_message();
151 msg.op.should.equal('update_annotations');
152 msg.entity.should.equal(unit_name);
153 msg.data.name.should.equal('A');
154 });
155
156 it('can get annotations', function() {
157 var unit_name = 'mysql/0';
158 env.get_annotations(unit_name);
159 msg = conn.last_message();
160 msg.op.should.equal('get_annotations');
161 msg.entity.should.equal(unit_name);
162 });
163
164 it('can remove annotations with specified keys', function() {
165 var unit_name = 'mysql/0';
166 var keys = ['key1', 'key2'];
167 env.remove_annotations(unit_name, keys);
168 msg = conn.last_message();
169 msg.op.should.equal('remove_annotations');
170 msg.entity.should.equal(unit_name);
171 msg.keys.should.eql(keys);
172 });
173
174 it('can remove annotations with no specified keys', function() {
175 var unit_name = 'mysql/0';
176 env.remove_annotations(unit_name);
177 msg = conn.last_message();
178 msg.op.should.equal('remove_annotations');
179 msg.entity.should.equal(unit_name);
180 msg.keys.should.eql([]);
181 });
154182
155 });183 });
156})();184})();

Subscribers

People subscribed via source and target branches