Implement getService/getCharm

Implement getService/getCharm

getService (w/o relations)
getCharm (w/o validation)

https://code.launchpad.net/~bcsaller/juju-gui/inmem-env/+merge/157210

(do not edit description out of merge proposal)

[bcsaller, 2013-04-04 22:08:55]

Please take a look.

[jeff.pihach, 2013-04-04 22:40:25]

LGTM with trivial. Thanks for putting this together!

https://codereview.appspot.com/8358045/diff/2001/app/config-debug.js
File app/config-debug.js (right):

https://codereview.appspot.com/8358045/diff/2001/app/config-debug.js#newcode27
app/config-debug.js:27: sandbox: true,
not to be committed.

https://codereview.appspot.com/8358045/diff/2001/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (right):

https://codereview.appspot.com/8358045/diff/2001/app/store/env/fakebackend.js...
app/store/env/fakebackend.js:312: * @method getService
missing description

https://codereview.appspot.com/8358045/diff/2001/app/store/env/fakebackend.js...
app/store/env/fakebackend.js:317: var self = this;
unused var

https://codereview.appspot.com/8358045/diff/2001/app/store/env/fakebackend.js...
app/store/env/fakebackend.js:339: * @method getCharm
missing description

https://codereview.appspot.com/8358045/diff/2001/app/store/env/fakebackend.js...
app/store/env/fakebackend.js:353: },
Shouldn't this method be returning something?

https://codereview.appspot.com/8358045/diff/2001/app/store/env/sandbox.js
File app/store/env/sandbox.js (right):

https://codereview.appspot.com/8358045/diff/2001/app/store/env/sandbox.js#new...
app/store/env/sandbox.js:162: var ASYNC_OP = function(context, rpcName, args) {
I don't really see why this is necessary with the almost instant in memory
backend - maybe you can explain to me on irc.

https://codereview.appspot.com/8358045/diff/2001/app/store/env/sandbox.js#new...
app/store/env/sandbox.js:184: var OP = function(context, rpcName, args, data) {
These should probably be documented with yuidoc

https://codereview.appspot.com/8358045/diff/2001/test/test_sandbox.js
File test/test_sandbox.js (right):

https://codereview.appspot.com/8358045/diff/2001/test/test_sandbox.js#newcode761
test/test_sandbox.js:761: // Post deploy of wordpress we should be alboe to
able

https://codereview.appspot.com/8358045/diff/2001/test/test_sandbox.js#newcode781
test/test_sandbox.js:781: // Post deploy of wordpress we should be alboe to
able

https://codereview.appspot.com/8358045/diff/2001/test/test_sandbox.js#newcode821
test/test_sandbox.js:821: // Post deploy of wordpress we should be alboe to
able :)

https://codereview.appspot.com/8358045/diff/2001/undocumented
File undocumented (right):

https://codereview.appspot.com/8358045/diff/2001/undocumented#newcode1
undocumented:1: app/store/charm.js:166 "success"
uh oh +10 undocumented :(

[bcsaller, 2013-04-05 00:17:07]

Thanks for the review, if I didn't explain the ASYNC_OP template method well
enough let me know.

https://codereview.appspot.com/8358045/diff/2001/app/config-debug.js
File app/config-debug.js (right):

https://codereview.appspot.com/8358045/diff/2001/app/config-debug.js#newcode27
app/config-debug.js:27: sandbox: true,
On 2013/04/04 22:40:25, jeff.pihach wrote:
> not to be committed.
oops

https://codereview.appspot.com/8358045/diff/2001/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (right):

https://codereview.appspot.com/8358045/diff/2001/app/store/env/fakebackend.js...
app/store/env/fakebackend.js:317: var self = this;
On 2013/04/04 22:40:25, jeff.pihach wrote:
> unused var

Done.

https://codereview.appspot.com/8358045/diff/2001/app/store/env/fakebackend.js...
app/store/env/fakebackend.js:353: },
On 2013/04/04 22:40:25, jeff.pihach wrote:
> Shouldn't this method be returning something? 

This method is async because it can load from the charmstore. Its all handled by
the callbacks.

https://codereview.appspot.com/8358045/diff/2001/app/store/env/sandbox.js
File app/store/env/sandbox.js (right):

https://codereview.appspot.com/8358045/diff/2001/app/store/env/sandbox.js#new...
app/store/env/sandbox.js:82: Y.soon(this.receiveNow.bind(this, data, true));
Awww. comeon, that was a good find ;)

https://codereview.appspot.com/8358045/diff/2001/app/store/env/sandbox.js#new...
app/store/env/sandbox.js:162: var ASYNC_OP = function(context, rpcName, args) {
On 2013/04/04 22:40:25, jeff.pihach wrote:
> I don't really see why this is necessary with the almost instant in memory
> backend - maybe you can explain to me on irc.

Operations taking a callback do so because they have some async component. This
normalizes methods needing a callback.
If we switch to promises then we can wait for the operation to complete using
that system, but in practice it would be similar.

https://codereview.appspot.com/8358045/diff/2001/app/store/env/sandbox.js#new...
app/store/env/sandbox.js:184: var OP = function(context, rpcName, args, data) {
On 2013/04/04 22:40:25, jeff.pihach wrote:
> These should probably be documented with yuidoc 

Done.

https://codereview.appspot.com/8358045/diff/2001/test/test_sandbox.js
File test/test_sandbox.js (right):

https://codereview.appspot.com/8358045/diff/2001/test/test_sandbox.js#newcode761
test/test_sandbox.js:761: // Post deploy of wordpress we should be alboe to
On 2013/04/04 22:40:25, jeff.pihach wrote:
> able

Done.

https://codereview.appspot.com/8358045/diff/2001/test/test_sandbox.js#newcode781
test/test_sandbox.js:781: // Post deploy of wordpress we should be alboe to
On 2013/04/04 22:40:25, jeff.pihach wrote:
> able

Done.

https://codereview.appspot.com/8358045/diff/2001/test/test_sandbox.js#newcode821
test/test_sandbox.js:821: // Post deploy of wordpress we should be alboe to
On 2013/04/04 22:40:25, jeff.pihach wrote:
> able :)

Done.

https://codereview.appspot.com/8358045/diff/2001/undocumented
File undocumented (right):

https://codereview.appspot.com/8358045/diff/2001/undocumented#newcode1
undocumented:1: app/store/charm.js:166 "success"
On 2013/04/04 22:40:25, jeff.pihach wrote:
> uh oh +10 undocumented :(

Added more docs.

[bcsaller, 2013-04-05 00:21:29]

Please take a look.

[matthew.scott, 2013-04-05 15:52:31]

LGTM - thanks for this!  Some of the other TODOs may be card worthy, but this
one has a defined sequence of events, so I think a card might help.

https://codereview.appspot.com/8358045/diff/12001/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (right):

https://codereview.appspot.com/8358045/diff/12001/app/store/env/fakebackend.j...
app/store/env/fakebackend.js:332: // TODO: properly map relations to expected
format rather
May be card-worthy.

[bcsaller, 2013-04-05 15:58:20]

Thanks for the reviews, card added.

https://codereview.appspot.com/8358045/diff/12001/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (right):

https://codereview.appspot.com/8358045/diff/12001/app/store/env/fakebackend.j...
app/store/env/fakebackend.js:332: // TODO: properly map relations to expected
format rather
On 2013/04/05 15:52:31, matthew.scott wrote:
> May be card-worthy.

Added, thanks

[bcsaller, 2013-04-05 16:02:32]

*** Submitted:

getService/getCharm/setConfig/setConstraints

getService (w/o relations)
getCharm (w/o validation)
setConfig (w/o validation)
setConstraints (w/o validation)

R=jeff.pihach, matthew.scott
CC=
https://codereview.appspot.com/8358045