get charm and service now uses promises

get charm and service now uses promises

Three new primary methods were added to retrieve charm and
service data. ServiceList adds getFullService(), CharmList
adds getFullCharm, and Database gets a populateService()
method. These methods return promises for their respecive
data and populateService returns a fully populated service
and charm.

Endpoint code which used the old methods have been converted
over with the rest to be converted as time goes on.

Unit tests will be added in a follow-up branch.

https://code.launchpad.net/~hatch/juju-gui/load-srv-db/+merge/161243

(do not edit description out of merge proposal)

[jeff.pihach, 2013-04-26 21:41:04]

Please take a look.

[gary.poster, 2013-04-26 22:18:34]

Great improvement.  I don't want to have the db know about the env.  I suggest
putting the new functions on the app, or better, on a new ModelController
object.

LGTM if you want to divide this up.  If so, immediately go to the clean-up
please.

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

https://codereview.appspot.com/8839050/diff/1/app/models/charm.js#newcode313
app/models/charm.js:313: Y.bind(function(charmId, db, env, resolve, reject) {
As a matter of establishing our Promise practice, I think this would read better
as a closure.  IOW,

var charmList = this;
return new Y.promise(
    function(resolve, reject) {

and so on.  I think that is simpler and easier to read.  Could be wrong.

https://codereview.appspot.com/8839050/diff/1/app/models/charm.js#newcode333
app/models/charm.js:333: Refernce to the client env.
typo: Reference

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

https://codereview.appspot.com/8839050/diff/1/app/models/models.js#newcode173
app/models/models.js:173: Y.bind(this._servicePromise, this, serviceId,
As before, I personally think a closure would be nicer.  The better docs of your
current approach could be counter argument, but like the way I approach

[rharding, 2013-04-26 23:28:01]

LGTM, I'm with Gary that I think a follow up to try to break the env out would
be great. 

Thanks for getting that going.

[gary.poster, 2013-04-28 01:01:33]

Not LGTM. QA is bad.  This exposes a problem in the sandbox annotation code.  I
see the problem and will work on a sandbox fix tomorrow and get it to you.

[gary.poster, 2013-04-28 01:04:44]

On 2013/04/28 01:01:33, gary.poster wrote:
> Not LGTM. QA is bad.  This exposes a problem in the sandbox annotation code. 
I
> see the problem and will work on a sandbox fix tomorrow and get it to you.

FWIW, symptom is intermittent errors caused by too many recursions in Y.mix. 
cause is use of Y.mix in delta code in sandbox.  We should not be doing a deep
mix with full db objects, and when you added the env and db to the models we got
infinite recursion.  I think the plan is to take the db and env back off the
models.  Even so, we need the sandbox fix.  I have a sketch but will finish it
tomorrow.

[gary.poster, 2013-04-28 10:24:13]

This fixes associated qa problems: lp:~gary/juju-gui/load-srv .

I am ok with landing if you include my fixes above.

My changes are needed regardless of your branch: you just triggered the
associated problem.

Thanks,

Gary