Add apiv4 bundle id support to deploy-target #695

Merged
merged 2 commits into from Feb 17, 2015

Conversation

Projects
None yet
4 participants
Owner

hatched commented Feb 17, 2015

The jujucharms.com website allows users to "add to demo" bundles to play around with. It has had to guess at the apiv3 bundle id for some time now which has caused a number of bugs. This adds apiv4 bundle support to the GUI's deploy-target handler so that jujucharms.com can now use the apiv4 bundle id's.

Owner

hatched commented Feb 17, 2015

To QA

make devel
visit <host>?deploy-target=bundle/django
It should deploy the bundle.

Member

jujugui commented Feb 17, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
http://ci.jujugui.org:8080//job/juju-gui/2372/
Test FAILed.

Owner

hatched commented Feb 17, 2015

Spurious failure, running manually.

Member

makyo commented Feb 17, 2015

👍 QA okay - works like a dream, tried a few different bundles.

Owner

hatched commented Feb 17, 2015

+ deploying a bundle via the deployer.
+
+ @method getBundleYAML
+ @param {String} id Bundle id in apiv4 format.
@jaycee

jaycee Feb 17, 2015

Member

I might make it clear we're talking charmstore here, e.g. ... id in v4 charmstore api format.

@hatched

hatched Feb 17, 2015

Owner

Good idea

+ @param {Function} failureCallback The failure callback.
+ @param {Array} bundle An array containing the requested bundle model.
+ */
+ _getBundleYAMLResponse: function(successCallback, failureCallback, bundle) {
@jaycee

jaycee Feb 17, 2015

Member

Aren't callbacks usually the last arguments, except maybe context/scope? This reads weirdly to me. I feel like it should be _getBundleYAMLResponse: function(bundle, success, failure).

@hatched

hatched Feb 17, 2015

Owner

The arguments which are passed to a bound function are passed first to the function, the function is essentially being curried. This is why when getEntity calls the _getBundleYAMLResponse method the arguments it passes to it get appended after the two callbacks.

+ getBundleYAML: function(id, successCallback, failureCallback) {
+ this.getEntity(
+ id, this._getBundleYAMLResponse.bind(
+ this, successCallback, failureCallback), failureCallback);
@jaycee

jaycee Feb 17, 2015

Member

I'm not sure this call is right, based on the def below. It looks like your passing in context first, the callbacks, and no bundle at all to the .bind call on _getBundleYAMLResponse. Am I misreading this?

@hatched

hatched Feb 17, 2015

Owner

bind returns a function which executes in the context of the object passed as the first argument. The call signature is bind(context, [arg [, arg[, ...])

Member

jaycee commented Feb 17, 2015

👍 @hatched, thanks for the explanations.

Owner

hatched commented Feb 17, 2015

Thanks for the reviews! :shipit:

Member

jujugui commented Feb 17, 2015

Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge

jujugui added a commit that referenced this pull request Feb 17, 2015

Merge pull request #695 from hatched/deploy-target-apiv4
Add apiv4 bundle id support to deploy-target

The jujucharms.com website allows users to "add to demo" bundles to play around with. It has had to guess at the apiv3 bundle id for some time now which has caused a number of bugs. This adds apiv4 bundle support to the GUI's deploy-target handler so that jujucharms.com can now use the apiv4 bundle id's.

@jujugui jujugui merged commit eedee1b into juju:develop Feb 17, 2015

1 check failed

default Merged build finished.
Details

@hatched hatched deleted the hatched:deploy-target-apiv4 branch Mar 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment