Merge lp:~bac/juju-gui/send-bundle-id-to-deployer into lp:juju-gui/experimental

Proposed by Brad Crittenden
Status: Merged
Merged at revision: 1202
Proposed branch: lp:~bac/juju-gui/send-bundle-id-to-deployer
Merge into: lp:juju-gui/experimental
Diff against target: 195 lines (+34/-21)
5 files modified
app/app.js (+2/-1)
app/assets/javascripts/bundle-import-helpers.js (+9/-3)
app/store/env/go.js (+11/-7)
app/subapps/browser/views/bundle.js (+1/-1)
test/test_bundle_import_helpers.js (+11/-9)
To merge this branch: bzr merge lp:~bac/juju-gui/send-bundle-id-to-deployer
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+195598@code.launchpad.net

Description of the change

Send the bundle ID when deploying.

By sending the bundle ID to the server, the deployment statistics can be
updated on Charmworld for that bundle.

QA:

In the latest charm trunk:
% juju switch ec2
% juju bootstrap
% make deploy
% juju set juju-gui "juju-gui-source=lp:~bac/juju-gui/send-bundle-id-to-deployer"
% juju set juju-gui "charmworld-url=http://staging.jujucharms.com"

Search for a simple bundle. Jorge's are good. In this example I'll use mediawiki-simple.

Look at the bundle on staging:
http://staging.jujucharms.com/bundle/~jorge/mediawiki-simple/mediawiki-simple

Note the Downloads statistics.

Go to your juju-gui instance on ec2.
Search for jorge.
Pick the bundle you want (mediawiki-simple in this case).
Deploy it.
Only when all service are successfully deployed, green, and relations built
will the deployment statistics get updated. At that time go back to staging
and see that the downloads have been incremented.

https://codereview.appspot.com/28440043/

To post a comment you must log in.
1204. By Brad Crittenden

Fix param doc for deployBundle.

Revision history for this message
Brad Crittenden (bac) wrote :
Download full text (8.1 KiB)

Reviewers: mp+195598_code.launchpad.net,

Message:
Please take a look.

Description:
Send the bundle ID when deploying.

By sending the bundle ID to the server, the deployment statistics can be
updated on Charmworld for that bundle.

QA:

In the latest charm trunk:
% juju switch ec2
% juju bootstrap
% make deploy
% juju set juju-gui
"juju-gui-source=lp:~bac/juju-gui/send-bundle-id-to-deployer"
% juju set juju-gui "charmworld-url=http://staging.jujucharms.com"

Search for a simple bundle. Jorge's are good. In this example I'll use
mediawiki-simple.

Look at the bundle on staging:
http://staging.jujucharms.com/bundle/~jorge/mediawiki-simple/mediawiki-simple

Note the Downloads statistics.

Go to your juju-gui instance on ec2.
Search for jorge.
Pick the bundle you want (mediawiki-simple in this case).
Deploy it.
Only when all service are successfully deployed, green, and relations
built
will the deployment statistics get updated. At that time go back to
staging
and see that the downloads have been incremented.

https://code.launchpad.net/~bac/juju-gui/send-bundle-id-to-deployer/+merge/195598

(do not edit description out of merge proposal)

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

Affected files (+26, -14 lines):
   A [revision details]
   M app/app.js
   M app/assets/javascripts/bundle-import-helpers.js
   M app/store/env/go.js
   M app/subapps/browser/views/bundle.js
   M test/test_bundle_import_helpers.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: app/app.js
=== modified file 'app/app.js'
--- app/app.js 2013-11-15 13:46:35 +0000
+++ app/app.js 2013-11-18 13:27:49 +0000
@@ -595,13 +595,14 @@
        // Grab a reference of these for the nested event calls below.
        var env = this.env;
        var db = this.db;
- cfg.deployBundle = function(bundle) {
+ cfg.deployBundle = function(bundle, bundleId) {
          // The other views will hand us an Object vs a YAML string. The
import
          // helpers want the yaml string instead.
          importHelpers.deployBundle(
              Y.JSON.stringify({
                bundle: bundle
              }),
+ bundleId,
              env,
              db
          );

Index: test/test_bundle_import_helpers.js
=== modified file 'test/test_bundle_import_helpers.js'
--- test/test_bundle_import_helpers.js 2013-11-15 13:46:35 +0000
+++ test/test_bundle_import_helpers.js 2013-11-18 13:57:40 +0000
@@ -55,7 +55,7 @@
        };

        // Start the process by deploying the bundle.
- ns.BundleHelpers.deployBundle('test bundle', env, db);
+ ns.BundleHelpers.deployBundle('test bundle', undefined, env, db);
      });

      it('errors when the bundle import fails from the env', function(done) {
@@ -65,14 +65,14 @@
          done();
        };

- env.deployerImport = function(bundle, name, callback) {
+ env.deployerImport = function(bundle, name, bundleid, callback) {...

Read more...

Revision history for this message
Jeff Pihach (hatch) wrote :

LGTM Thanks for the discussion on this!

You can either do the comment below or make a ticket for someone else to
follow-up with at a later time.

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

https://codereview.appspot.com/28440043/diff/1/app/store/env/go.js#newcode431
app/store/env/go.js:431: else if (Y.Lang.isFunction(bundleID) &&
callback === undefined) {
We should change this method to accept a configuration object instead of
relying on this munging.

https://codereview.appspot.com/28440043/

1205. By Brad Crittenden

Pass an object with bundle name and id to deploy functions.

1206. By Brad Crittenden

lint

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

*** Submitted:

Send the bundle ID when deploying.

By sending the bundle ID to the server, the deployment statistics can be
updated on Charmworld for that bundle.

QA:

In the latest charm trunk:
% juju switch ec2
% juju bootstrap
% make deploy
% juju set juju-gui
"juju-gui-source=lp:~bac/juju-gui/send-bundle-id-to-deployer"
% juju set juju-gui "charmworld-url=http://staging.jujucharms.com"

Search for a simple bundle. Jorge's are good. In this example I'll use
mediawiki-simple.

Look at the bundle on staging:
http://staging.jujucharms.com/bundle/~jorge/mediawiki-simple/mediawiki-simple

Note the Downloads statistics.

Go to your juju-gui instance on ec2.
Search for jorge.
Pick the bundle you want (mediawiki-simple in this case).
Deploy it.
Only when all service are successfully deployed, green, and relations
built
will the deployment statistics get updated. At that time go back to
staging
and see that the downloads have been incremented.

R=jeff.pihach
CC=
https://codereview.appspot.com/28440043

https://codereview.appspot.com/28440043/

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

I realize this is out of band but I'd like to change the API for deployer
to take a bundle archive similar to how charms work. We intend to have
bundles with custom hooks in the future and those should ship with the
deployer YAML. We can discuss this again as I get closer with some of the
other needed changes, but this is a heads up.

On Mon, Nov 18, 2013 at 10:33 AM, Brad Crittenden <email address hidden> wrote:

> *** Submitted:
>
> Send the bundle ID when deploying.
>
> By sending the bundle ID to the server, the deployment statistics can be
> updated on Charmworld for that bundle.
>
> QA:
>
> In the latest charm trunk:
> % juju switch ec2
> % juju bootstrap
> % make deploy
> % juju set juju-gui
> "juju-gui-source=lp:~bac/juju-gui/send-bundle-id-to-deployer"
> % juju set juju-gui "charmworld-url=http://staging.jujucharms.com"
>
> Search for a simple bundle. Jorge's are good. In this example I'll use
> mediawiki-simple.
>
> Look at the bundle on staging:
>
> http://staging.jujucharms.com/bundle/~jorge/mediawiki-simple/mediawiki-simple
>
> Note the Downloads statistics.
>
> Go to your juju-gui instance on ec2.
> Search for jorge.
> Pick the bundle you want (mediawiki-simple in this case).
> Deploy it.
> Only when all service are successfully deployed, green, and relations
> built
> will the deployment statistics get updated. At that time go back to
> staging
> and see that the downloads have been incremented.
>
> R=jeff.pihach
> CC=
> https://codereview.appspot.com/28440043
>
>
> https://codereview.appspot.com/28440043/
>
> --
>
> https://code.launchpad.net/~bac/juju-gui/send-bundle-id-to-deployer/+merge/195598
> Your team Juju GUI Hackers is requested to review the proposed merge of
> lp:~bac/juju-gui/send-bundle-id-to-deployer into lp:juju-gui.
>

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-11-15 13:46:35 +0000
3+++ app/app.js 2013-11-18 18:25:29 +0000
4@@ -595,13 +595,14 @@
5 // Grab a reference of these for the nested event calls below.
6 var env = this.env;
7 var db = this.db;
8- cfg.deployBundle = function(bundle) {
9+ cfg.deployBundle = function(bundle, bundleId) {
10 // The other views will hand us an Object vs a YAML string. The import
11 // helpers want the yaml string instead.
12 importHelpers.deployBundle(
13 Y.JSON.stringify({
14 bundle: bundle
15 }),
16+ bundleId,
17 env,
18 db
19 );
20
21=== modified file 'app/assets/javascripts/bundle-import-helpers.js'
22--- app/assets/javascripts/bundle-import-helpers.js 2013-11-15 19:04:20 +0000
23+++ app/assets/javascripts/bundle-import-helpers.js 2013-11-18 18:25:29 +0000
24@@ -27,12 +27,13 @@
25 to deploy the bundle to the environment.
26
27 @method deployBundle
28- @param {Object} bundle Bundle data.
29+ @param {String} bundle Bundle YAML data.
30+ @param {String} bundleId Bundle ID for Charmworld. May be null.
31 @param {Environment} env Environment with access to the bundle back end.
32 calls.
33 @param {Database} db The app Database with access to the NotificationList.
34 */
35- deployBundle: function(bundle, env, db, customCallback) {
36+ deployBundle: function(bundle, bundleId, env, db, customCallback) {
37 var notifications = db.notifications;
38
39 var defaultCallback = function(result) {
40@@ -57,9 +58,13 @@
41 };
42
43 if (Y.Lang.isFunction(env.deployerImport)) {
44+ var bundleData = {
45+ name: null,
46+ id: bundleId
47+ };
48 env.deployerImport(
49 bundle,
50- null,
51+ bundleData,
52 customCallback ? customCallback : defaultCallback
53 );
54 } else {
55@@ -104,6 +109,7 @@
56 reader.onload = function(e) {
57 ns.BundleHelpers.deployBundle(
58 e.target.result,
59+ undefined,
60 env,
61 db,
62 function(result) {
63
64=== modified file 'app/store/env/go.js'
65--- app/store/env/go.js 2013-11-15 13:46:35 +0000
66+++ app/store/env/go.js 2013-11-18 18:25:29 +0000
67@@ -416,27 +416,31 @@
68 *
69 * @method deployerImport
70 * @param {String} yamlData to import.
71- * @param {String} [bundleName] Bundle name to import.
72+ * @param {Object} bundleData Object describing the bundle. Has name and
73+ * id. May be null.
74 * @param {Function} callback to trigger.
75 * @return {Number} Request Id.
76 */
77- deployerImport: function(yamlData, bundleName, callback) {
78+ deployerImport: function(yamlData, bundleData, callback) {
79 var intermediateCallback;
80+ var name, id;
81
82- if (Y.Lang.isFunction(bundleName) && callback === undefined) {
83- callback = bundleName;
84- bundleName = undefined;
85- }
86 if (callback) {
87 intermediateCallback = Y.bind(this.handleDeployerImport,
88 this, callback);
89 }
90+ if (Y.Lang.isValue(bundleData)) {
91+ name = bundleData.name;
92+ id = bundleData.id;
93+ }
94+
95 this._send_rpc({
96 Type: 'Deployer',
97 Request: 'Import',
98 Params: {
99 YAML: yamlData,
100- Name: undefined
101+ Name: name,
102+ BundleID: id
103 }
104 }, intermediateCallback);
105 },
106
107=== modified file 'app/subapps/browser/views/bundle.js'
108--- app/subapps/browser/views/bundle.js 2013-11-14 18:53:43 +0000
109+++ app/subapps/browser/views/bundle.js 2013-11-18 18:25:29 +0000
110@@ -93,7 +93,7 @@
111 } else {
112 this.fire('viewNavigate', {change: {charmID: null}});
113 }
114- this.get('deployBundle')(bundle.get('data'));
115+ this.get('deployBundle')(bundle.get('data'), bundle.get('id'));
116 },
117
118 /**
119
120=== modified file 'test/test_bundle_import_helpers.js'
121--- test/test_bundle_import_helpers.js 2013-11-15 13:46:35 +0000
122+++ test/test_bundle_import_helpers.js 2013-11-18 18:25:29 +0000
123@@ -55,7 +55,7 @@
124 };
125
126 // Start the process by deploying the bundle.
127- ns.BundleHelpers.deployBundle('test bundle', env, db);
128+ ns.BundleHelpers.deployBundle('test bundle', undefined, env, db);
129 });
130
131 it('errors when the bundle import fails from the env', function(done) {
132@@ -65,14 +65,14 @@
133 done();
134 };
135
136- env.deployerImport = function(bundle, name, callback) {
137+ env.deployerImport = function(bundle, bundleData, callback) {
138 callback({
139 err: 'Abort abort!'
140 });
141 };
142
143 // Start the process by deploying the bundle.
144- ns.BundleHelpers.deployBundle('test bundle', env, db);
145+ ns.BundleHelpers.deployBundle('test bundle', '~jorge/wiki/wiki', env, db);
146 });
147
148 it('provides deployBundle helper for working through env', function(done) {
149@@ -87,10 +87,12 @@
150
151 // Stub out the env call to make sure we check the params and call the
152 // provided callback.
153- env.deployerImport = function(bundle, name, callback) {
154+ env.deployerImport = function(bundle, bundleData, callback) {
155 assert.equal(bundle, 'test bundle');
156+ assert.equal(bundleData.id, '~jorge/wiki/wiki');
157 assert.equal(
158- name, null, 'The name is not currently supported or passed.');
159+ bundleData.name, null,
160+ 'The name is not currently supported or passed.');
161 // This is the default callback from the deployBundle method.
162 callback({
163 err: undefined,
164@@ -108,12 +110,12 @@
165 ns.BundleHelpers._watchDeployment = _watchDeployment;
166
167 // Make sure we did in fact post our notification to the user.
168- assert.equal(hitNotifications, true);
169+ assert.equal(true, hitNotifications);
170 done();
171 };
172
173 // Start the process by deploying the bundle.
174- ns.BundleHelpers.deployBundle('test bundle', env, db);
175+ ns.BundleHelpers.deployBundle('test bundle', '~jorge/wiki/wiki', env, db);
176 });
177
178 it('provides a notification when a deploy watch updates', function(done) {
179@@ -223,7 +225,7 @@
180
181 // Stub out the env call to make sure we check the params and call the
182 // provided callback.
183- env.deployerImport = function(bundle, name, callback) {
184+ env.deployerImport = function(bundle, bundleData, callback) {
185 callback({
186 err: undefined,
187 DeploymentId: 10
188@@ -295,7 +297,7 @@
189 };
190
191 // Start the process by deploying the bundle.
192- ns.BundleHelpers.deployBundle('test bundle', env, db);
193+ ns.BundleHelpers.deployBundle('test bundle', undefined, env, db);
194
195 });
196

Subscribers

People subscribed via source and target branches