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
=== modified file 'app/app.js'
--- app/app.js 2013-06-25 14:31:16 +0000
+++ app/app.js 2013-06-25 19:48:30 +0000
@@ -235,12 +235,12 @@
235235
236 'S-d': {236 'S-d': {
237 callback: function(evt) {237 callback: function(evt) {
238 this.env.exportEnvironment(function(r) {238 var yaml;
239 var exportData = JSON.stringify(r.result, undefined, 2);239 var result = this.db.exportDeployer();
240 var exportBlob = new Blob([exportData],240 var exportData = jsyaml.dump(result);
241 {type: 'application/json;charset=utf-8'});241 var exportBlob = new Blob([exportData],
242 saveAs(exportBlob, 'export.json');242 {type: 'application/yaml;charset=utf-8'});
243 });243 saveAs(exportBlob, 'export.yaml');
244 },244 },
245 help: 'Export the environment'245 help: 'Export the environment'
246 },246 },
@@ -502,6 +502,7 @@
502 this.env.after('providerTypeChange', this.onProviderTypeChange, this);502 this.env.after('providerTypeChange', this.onProviderTypeChange, this);
503 this.env.after('environmentNameChange',503 this.env.after('environmentNameChange',
504 this.onEnvironmentNameChange, this);504 this.onEnvironmentNameChange, this);
505 this.env.after('defaultSeriesChange', this.onDefaultSeriesChange, this);
505506
506 // Once the user logs in, we need to redraw.507 // Once the user logs in, we need to redraw.
507 this.env.after('login', this.onLogin, this);508 this.env.after('login', this.onLogin, this);
@@ -1072,6 +1073,20 @@
1072 },1073 },
10731074
1074 /**1075 /**
1076 * Record environment default series changes in our model.
1077 *
1078 * The provider type arrives asynchronously. Instead of updating the
1079 * display from the environment code (a separation of concerns violation),
1080 * we update it here.
1081 *
1082 * @method onDefaultSeriesChange
1083 */
1084 onDefaultSeriesChange: function(evt) {
1085 this.db.environment.set('defaultSeries', evt.newVal);
1086 },
1087
1088
1089 /**
1075 Display the Environment Name.1090 Display the Environment Name.
10761091
1077 The environment name can arrive asynchronously. Instead of updating1092 The environment name can arrive asynchronously. Instead of updating
10781093
=== modified file 'app/models/models.js'
--- app/models/models.js 2013-06-24 19:43:15 +0000
+++ app/models/models.js 2013-06-25 19:48:30 +0000
@@ -105,6 +105,7 @@
105 ATTRS: {105 ATTRS: {
106 name: {},106 name: {},
107 provider: {},107 provider: {},
108 defaultSeries: {},
108 annotations: {109 annotations: {
109 valueFn: function() {return {};}110 valueFn: function() {return {};}
110 }111 }
@@ -169,6 +170,18 @@
169 // to help support scale.170 // to help support scale.
170 annotations: {value: {}},171 annotations: {value: {}},
171 constraints: {},172 constraints: {},
173 constraintsStr: {
174 'getter': function() {
175 var result = [];
176 Y.each(this.get('constraints'), function(v, k) {
177 result.push(k + '=' + v);
178 });
179 if (result.length) {
180 return result.join(',');
181 }
182 return undefined;
183 }
184 },
172 exposed: {185 exposed: {
173 value: false186 value: false
174 },187 },
@@ -825,12 +838,65 @@
825 self.units.update_service_unit_aggregates(service);838 self.units.update_service_unit_aggregates(service);
826 });839 });
827 this.fire('update');840 this.fire('update');
841 },
842
843 /**
844 * Export deployer formatted dump of the current environment.
845 * Note: When we have a selection UI in place this should honor
846 * that.
847 *
848 * @method exportDeployer
849 * @param {Object} db for application.
850 * @return {Object} export object suitable for serialization.
851 */
852 exportDeployer: function() {
853 var self = this,
854 serviceList = this.services,
855 relationList = this.relations,
856 result = {
857 envExport: {
858 series: this.environment.get('defaultSeries'),
859 services: [],
860 relations: []
861 }
862 };
863
864 serviceList.each(function(s) {
865 var units = s.units;
866 var charm = self.charms.getById(s.get('charm'));
867 if (s.get('pending') === true) {return;}
868 var serviceData = {
869 // Using package name here so the default series
870 // is picked up. This will likely have to be the full
871 // path in the future.
872 charm: charm.get('package_name'),
873 options: s.get('config'),
874 num_units: units && units.size() || 1
875 };
876 // Add constraints
877 var constraints = s.get('constraintsStr');
878 if (constraints) {
879 serviceData.constraints = constraints;
880 }
881 result.envExport.services.push(serviceData);
882 });
883
884 relationList.each(function(r) {
885 var relationData = [];
886 Y.each(r.get('endpoints'), function(data, name) {
887 relationData.push([data[0], data[1].name]);
888 });
889 // Skip peer, they should add automatically.
890 if (relationData.length === 1) {
891 return;
892 }
893 result.envExport.relations.push(relationData);
894 });
895
896 return result;
828 }897 }
829
830 });898 });
831
832 models.Database = Database;899 models.Database = Database;
833
834}, '0.1.0', {900}, '0.1.0', {
835 requires: [901 requires: [
836 'model',902 'model',
837903
=== modified file 'app/store/env/fakebackend.js'
--- app/store/env/fakebackend.js 2013-06-25 12:57:53 +0000
+++ app/store/env/fakebackend.js 2013-06-25 19:48:30 +0000
@@ -1211,7 +1211,7 @@
1211 services: [], relations: []},1211 services: [], relations: []},
1212 blackLists = {1212 blackLists = {
1213 service: ['id', 'aggregated_status', 'clientId', 'initialized',1213 service: ['id', 'aggregated_status', 'clientId', 'initialized',
1214 'destroyed', 'pending'],1214 'constraintsStr', 'destroyed', 'pending'],
1215 relation: ['id', 'relation_id', 'clientId', 'initialized',1215 relation: ['id', 'relation_id', 'clientId', 'initialized',
1216 'destroyed', 'pending']1216 'destroyed', 'pending']
1217 };1217 };
12181218
=== modified file 'app/views/topology/importexport.js'
--- app/views/topology/importexport.js 2013-06-05 16:58:11 +0000
+++ app/views/topology/importexport.js 2013-06-25 19:48:30 +0000
@@ -109,6 +109,7 @@
109 * @method update109 * @method update
110 */110 */
111 update: function() {111 update: function() {
112 var self = this;
112 // Check the feature flag113 // Check the feature flag
113 if (!this._dragHandle && window.flags.dndexport) {114 if (!this._dragHandle && window.flags.dndexport) {
114 var env = this.get('component').get('env');115 var env = this.get('component').get('env');
115116
=== modified file 'test/test_fakebackend.js'
--- test/test_fakebackend.js 2013-06-25 12:57:53 +0000
+++ test/test_fakebackend.js 2013-06-25 19:48:30 +0000
@@ -114,6 +114,7 @@
114 charm: 'cs:precise/wordpress-10',114 charm: 'cs:precise/wordpress-10',
115 config: undefined,115 config: undefined,
116 constraints: {},116 constraints: {},
117 constraintsStr: undefined,
117 destroyed: false,118 destroyed: false,
118 displayName: 'wordpress',119 displayName: 'wordpress',
119 exposed: false,120 exposed: false,
120121
=== modified file 'test/test_model.js'
--- test/test_model.js 2013-06-25 15:44:15 +0000
+++ test/test_model.js 2013-06-25 19:48:30 +0000
@@ -773,6 +773,44 @@
773 });773 });
774});774});
775775
776describe('database export', function() {
777 var models;
778 before(function(done) {
779 YUI(GlobalConfig).use(['juju-models'], function(Y) {
780 models = Y.namespace('juju.models');
781 done();
782 });
783 });
784
785 it('can export in deployer format', function() {
786 var db = new models.Database();
787 var mysql = db.services.add({id: 'mysql', charm: 'precise/mysql-1'});
788 var wordpress = db.services.add({
789 id: 'wordpress',
790 charm: 'precise/wordpress-1'});
791 var rel0 = db.relations.add({
792 id: 'relation-0',
793 endpoints: [
794 ['mysql', {name: 'db', role: 'server'}],
795 ['wordpress', {name: 'app', role: 'client'}]],
796 'interface': 'db'
797 });
798
799 db.environment.set('defaultSeries', 'precise');
800
801 // Add the charms so we can resolve them in the export.
802 db.charms.add([{id: 'precise/mysql-1'}, {id: 'precise/wordpress-1'}]);
803 var result = db.exportDeployer().envExport;
804 var relation = result.relations[0];
805
806 assert.equal(result.series, 'precise');
807 assert.equal(result.services[0].charm, 'mysql');
808 assert.equal(result.services[1].charm, 'wordpress');
809
810 assert.deepEqual(relation[0], ['mysql', 'db']);
811 assert.deepEqual(relation[1], ['wordpress', 'app']);
812 });
813});
776814
777describe('service models', function() {815describe('service models', function() {
778 var models, list, django, rails, wordpress, mysql;816 var models, list, django, rails, wordpress, mysql;

Subscribers

People subscribed via source and target branches