Merge lp:~bcsaller/juju-gui/deployer-export into lp:juju-gui/experimental

Proposed by Benjamin Saller
Status: Merged
Merged at revision: 763
Proposed branch: lp:~bcsaller/juju-gui/deployer-export
Merge into: lp:juju-gui/experimental
Diff against target: 237 lines (+131/-10)
6 files modified
app/app.js (+21/-6)
app/models/models.js (+69/-3)
app/store/env/fakebackend.js (+1/-1)
app/views/topology/importexport.js (+1/-0)
test/test_fakebackend.js (+1/-0)
test/test_model.js (+38/-0)
To merge this branch: bzr merge lp:~bcsaller/juju-gui/deployer-export
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+171402@code.launchpad.net

Description of the change

Exports in Deployer format.

Switch default hotkey export to YAML based deployer format. It we don't import
this format yet.

https://codereview.appspot.com/10563043/

To post a comment you must log in.
Revision history for this message
Benjamin Saller (bcsaller) wrote :

Reviewers: mp+171402_code.launchpad.net,

Message:
Please take a look.

Description:
Exports in Deployer format.

Switch default hotkey export to YAML based deployer format. It we don't
import
this format yet.

https://code.launchpad.net/~bcsaller/juju-gui/deployer-export/+merge/171402

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/app.js
   M app/models/models.js
   M app/store/env/fakebackend.js
   M app/views/topology/importexport.js
   M test/test_fakebackend.js
   M test/test_model.js

Revision history for this message
Jeff Pihach (hatch) wrote :
Revision history for this message
Benjamin Saller (bcsaller) wrote :
Download full text (3.9 KiB)

Thanks for the reviews. Made most of the minor updates indicated. Will
submit soon.

https://codereview.appspot.com/10563043/diff/1/app/app.js
File app/app.js (right):

https://codereview.appspot.com/10563043/diff/1/app/app.js#newcode239
app/app.js:239: var result = this.db.exportDeployer();
On 2013/06/25 20:37:03, benji wrote:
> A small thought: "exportDeployer" (despite it's lack of capitalization
and
> having no "new") read like a constructor to me. Perhaps a more
"verby" name
> would be an improvement. Maybe just "export" since we have only one
export
> format (right?).

export is a reserved word in JS and makes linters unhappy. This does
return an object but its not really a constuctor as that objects
prototype is the default.

https://codereview.appspot.com/10563043/diff/1/app/models/models.js
File app/models/models.js (right):

https://codereview.appspot.com/10563043/diff/1/app/models/models.js#newcode183
app/models/models.js:183: }
On 2013/06/25 20:37:03, benji wrote:
> It looks like an empty string would work as well as undefined, so
lines 179-182
> could be replaced with just "return result.join(',');".

It would but I would rather not return a string if we don't expect to
parse it (comma delimited). If you don't feel strongly I'll leave this
for now.

https://codereview.appspot.com/10563043/diff/1/app/models/models.js#newcode849
app/models/models.js:849: * @param {Object} db for application.
On 2013/06/25 20:37:03, benji wrote:
> This was probably left from an earlier iteration.

Nice catch and exactly right.

https://codereview.appspot.com/10563043/diff/1/app/models/models.js#newcode857
app/models/models.js:857: envExport: {
On 2013/06/25 20:37:03, benji wrote:
> It seems to me that we don't need the "envExport" layer in here.

The deployer format labels each section within the YAML. For example
this might be 'blog' and there could be another block called
'productionBlog' which inherits blog and so on. We don't deal with
inheritance as we don't retain any of the initial structure (and in fact
don't import these things at all yet).

This is provided as a default name to match the format.

https://codereview.appspot.com/10563043/diff/1/app/models/models.js#newcode874
app/models/models.js:874: num_units: units && units.size() || 1
On 2013/06/25 20:37:03, benji wrote:
> A comment about the circumstances in which s.units is not defined
might be nice.

Testing or ghosts. 'units' is created on the delta event under each
service now automatically. Added to code as well.

https://codereview.appspot.com/10563043/diff/1/app/models/models.js#newcode884
app/models/models.js:884: relationList.each(function(r) {
On 2013/06/25 20:37:03, benji wrote:
> Single character variable names... have you been working on juju-core?
  I kid, I
> kid. It might be nice to make that "relation" and the "s" above
"service",
> though.

Done.

https://codereview.appspot.com/10563043/diff/1/app/views/topology/importexport.js
File app/views/topology/importexport.js (right):

https://codereview.appspot.com/10563043/diff/1/app/views/topology/importexport.js#newcode112
app/views/topology/importexport.js:112: var self = this;
On 2013/06/25 20:08:12, jeff.pihach wrote...

Read more...

Revision history for this message
Benjamin Saller (bcsaller) wrote :

*** Submitted:

Exports in Deployer format.

Switch default hotkey export to YAML based deployer format. It we don't
import
this format yet.

R=jeff.pihach, benji, benjamin.saller
CC=
https://codereview.appspot.com/10563043

https://codereview.appspot.com/10563043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/app.js'
2--- app/app.js 2013-06-25 14:31:16 +0000
3+++ app/app.js 2013-06-25 19:48:30 +0000
4@@ -235,12 +235,12 @@
5
6 'S-d': {
7 callback: function(evt) {
8- this.env.exportEnvironment(function(r) {
9- var exportData = JSON.stringify(r.result, undefined, 2);
10- var exportBlob = new Blob([exportData],
11- {type: 'application/json;charset=utf-8'});
12- saveAs(exportBlob, 'export.json');
13- });
14+ var yaml;
15+ var result = this.db.exportDeployer();
16+ var exportData = jsyaml.dump(result);
17+ var exportBlob = new Blob([exportData],
18+ {type: 'application/yaml;charset=utf-8'});
19+ saveAs(exportBlob, 'export.yaml');
20 },
21 help: 'Export the environment'
22 },
23@@ -502,6 +502,7 @@
24 this.env.after('providerTypeChange', this.onProviderTypeChange, this);
25 this.env.after('environmentNameChange',
26 this.onEnvironmentNameChange, this);
27+ this.env.after('defaultSeriesChange', this.onDefaultSeriesChange, this);
28
29 // Once the user logs in, we need to redraw.
30 this.env.after('login', this.onLogin, this);
31@@ -1072,6 +1073,20 @@
32 },
33
34 /**
35+ * Record environment default series changes in our model.
36+ *
37+ * The provider type arrives asynchronously. Instead of updating the
38+ * display from the environment code (a separation of concerns violation),
39+ * we update it here.
40+ *
41+ * @method onDefaultSeriesChange
42+ */
43+ onDefaultSeriesChange: function(evt) {
44+ this.db.environment.set('defaultSeries', evt.newVal);
45+ },
46+
47+
48+ /**
49 Display the Environment Name.
50
51 The environment name can arrive asynchronously. Instead of updating
52
53=== modified file 'app/models/models.js'
54--- app/models/models.js 2013-06-24 19:43:15 +0000
55+++ app/models/models.js 2013-06-25 19:48:30 +0000
56@@ -105,6 +105,7 @@
57 ATTRS: {
58 name: {},
59 provider: {},
60+ defaultSeries: {},
61 annotations: {
62 valueFn: function() {return {};}
63 }
64@@ -169,6 +170,18 @@
65 // to help support scale.
66 annotations: {value: {}},
67 constraints: {},
68+ constraintsStr: {
69+ 'getter': function() {
70+ var result = [];
71+ Y.each(this.get('constraints'), function(v, k) {
72+ result.push(k + '=' + v);
73+ });
74+ if (result.length) {
75+ return result.join(',');
76+ }
77+ return undefined;
78+ }
79+ },
80 exposed: {
81 value: false
82 },
83@@ -825,12 +838,65 @@
84 self.units.update_service_unit_aggregates(service);
85 });
86 this.fire('update');
87+ },
88+
89+ /**
90+ * Export deployer formatted dump of the current environment.
91+ * Note: When we have a selection UI in place this should honor
92+ * that.
93+ *
94+ * @method exportDeployer
95+ * @param {Object} db for application.
96+ * @return {Object} export object suitable for serialization.
97+ */
98+ exportDeployer: function() {
99+ var self = this,
100+ serviceList = this.services,
101+ relationList = this.relations,
102+ result = {
103+ envExport: {
104+ series: this.environment.get('defaultSeries'),
105+ services: [],
106+ relations: []
107+ }
108+ };
109+
110+ serviceList.each(function(s) {
111+ var units = s.units;
112+ var charm = self.charms.getById(s.get('charm'));
113+ if (s.get('pending') === true) {return;}
114+ var serviceData = {
115+ // Using package name here so the default series
116+ // is picked up. This will likely have to be the full
117+ // path in the future.
118+ charm: charm.get('package_name'),
119+ options: s.get('config'),
120+ num_units: units && units.size() || 1
121+ };
122+ // Add constraints
123+ var constraints = s.get('constraintsStr');
124+ if (constraints) {
125+ serviceData.constraints = constraints;
126+ }
127+ result.envExport.services.push(serviceData);
128+ });
129+
130+ relationList.each(function(r) {
131+ var relationData = [];
132+ Y.each(r.get('endpoints'), function(data, name) {
133+ relationData.push([data[0], data[1].name]);
134+ });
135+ // Skip peer, they should add automatically.
136+ if (relationData.length === 1) {
137+ return;
138+ }
139+ result.envExport.relations.push(relationData);
140+ });
141+
142+ return result;
143 }
144-
145 });
146-
147 models.Database = Database;
148-
149 }, '0.1.0', {
150 requires: [
151 'model',
152
153=== modified file 'app/store/env/fakebackend.js'
154--- app/store/env/fakebackend.js 2013-06-25 12:57:53 +0000
155+++ app/store/env/fakebackend.js 2013-06-25 19:48:30 +0000
156@@ -1211,7 +1211,7 @@
157 services: [], relations: []},
158 blackLists = {
159 service: ['id', 'aggregated_status', 'clientId', 'initialized',
160- 'destroyed', 'pending'],
161+ 'constraintsStr', 'destroyed', 'pending'],
162 relation: ['id', 'relation_id', 'clientId', 'initialized',
163 'destroyed', 'pending']
164 };
165
166=== modified file 'app/views/topology/importexport.js'
167--- app/views/topology/importexport.js 2013-06-05 16:58:11 +0000
168+++ app/views/topology/importexport.js 2013-06-25 19:48:30 +0000
169@@ -109,6 +109,7 @@
170 * @method update
171 */
172 update: function() {
173+ var self = this;
174 // Check the feature flag
175 if (!this._dragHandle && window.flags.dndexport) {
176 var env = this.get('component').get('env');
177
178=== modified file 'test/test_fakebackend.js'
179--- test/test_fakebackend.js 2013-06-25 12:57:53 +0000
180+++ test/test_fakebackend.js 2013-06-25 19:48:30 +0000
181@@ -114,6 +114,7 @@
182 charm: 'cs:precise/wordpress-10',
183 config: undefined,
184 constraints: {},
185+ constraintsStr: undefined,
186 destroyed: false,
187 displayName: 'wordpress',
188 exposed: false,
189
190=== modified file 'test/test_model.js'
191--- test/test_model.js 2013-06-25 15:44:15 +0000
192+++ test/test_model.js 2013-06-25 19:48:30 +0000
193@@ -773,6 +773,44 @@
194 });
195 });
196
197+describe('database export', function() {
198+ var models;
199+ before(function(done) {
200+ YUI(GlobalConfig).use(['juju-models'], function(Y) {
201+ models = Y.namespace('juju.models');
202+ done();
203+ });
204+ });
205+
206+ it('can export in deployer format', function() {
207+ var db = new models.Database();
208+ var mysql = db.services.add({id: 'mysql', charm: 'precise/mysql-1'});
209+ var wordpress = db.services.add({
210+ id: 'wordpress',
211+ charm: 'precise/wordpress-1'});
212+ var rel0 = db.relations.add({
213+ id: 'relation-0',
214+ endpoints: [
215+ ['mysql', {name: 'db', role: 'server'}],
216+ ['wordpress', {name: 'app', role: 'client'}]],
217+ 'interface': 'db'
218+ });
219+
220+ db.environment.set('defaultSeries', 'precise');
221+
222+ // Add the charms so we can resolve them in the export.
223+ db.charms.add([{id: 'precise/mysql-1'}, {id: 'precise/wordpress-1'}]);
224+ var result = db.exportDeployer().envExport;
225+ var relation = result.relations[0];
226+
227+ assert.equal(result.series, 'precise');
228+ assert.equal(result.services[0].charm, 'mysql');
229+ assert.equal(result.services[1].charm, 'wordpress');
230+
231+ assert.deepEqual(relation[0], ['mysql', 'db']);
232+ assert.deepEqual(relation[1], ['wordpress', 'app']);
233+ });
234+});
235
236 describe('service models', function() {
237 var models, list, django, rails, wordpress, mysql;

Subscribers

People subscribed via source and target branches