Use Fakebackend API to perform deployer imports

Use Fakebackend API to perform deployer imports

This branch, rather than direct database manipulation uses the fackbackend 
API methods to build out its virutal deployments. This also removes support
the old improv import Environment which has no UI bound to it currently (
as import/export are now deployer oriented).

As a minor this changes the policy around relation displayNames such that
on the canvas we see both names by default (that would be a one liner to 
revert) and displays the actual endpoint name on the relations tab rather
than just one of them.

https://code.launchpad.net/~bcsaller/juju-gui/importDeployerRedux/+merge/186646

(do not edit description out of merge proposal)

[bcsaller, 2013-09-19 20:31:41]

Please take a look.

[gary.poster, 2013-09-19 21:12:08]

Incomplete review.  Sorry

https://codereview.appspot.com/13771047/diff/1/app/models/models.js
File app/models/models.js (left):

https://codereview.appspot.com/13771047/diff/1/app/models/models.js#oldcode774
app/models/models.js:774: Takes two string endpoints and splits it into usable
parts.
This section and the below would have been a great opportunity for code review
helper comments.  I'm assuming that these are deleted because they are moved,
but that's just an assumption so far.

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

https://codereview.appspot.com/13771047/diff/1/app/models/models.js#newcode715
app/models/models.js:715: names.push(endpoint[1].name);
Cool, thank you

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

https://codereview.appspot.com/13771047/diff/1/app/store/env/fakebackend.js#n...
app/store/env/fakebackend.js:184: Deploy a charm.  Uses a callback for response!
Bah, I probably did this.  Deploy a service. :-P :-)

https://codereview.appspot.com/13771047/diff/1/app/store/env/fakebackend.js#n...
app/store/env/fakebackend.js:222: Return a promise to deploy a charm.
to deploy a service...

https://codereview.appspot.com/13771047/diff/1/app/store/env/fakebackend.js#n...
app/store/env/fakebackend.js:227: @return {Promise} Resolving to the deployed
service.
It doesn't resolve to the deployed service; it resolves to what the callback
says it gets, in deploy, above.

https://codereview.appspot.com/13771047/diff/1/app/store/env/fakebackend.js#n...
app/store/env/fakebackend.js:232: self.deploy(charmId, resolve, options);
This approach bypasses the Promise "rejected" function, which seems less than
ideal.  Expedient, but it would be nice if this parsed the callback result and
called "resolve"/fulfill or reject, as appropriate.

https://codereview.appspot.com/13771047/diff/1/app/store/env/fakebackend.js#n...
app/store/env/fakebackend.js:741: Takes two string endpoints and splits it into
usable parts.
I'm not reviewing this because I'm assuming that this is copy and paste from the
previous location.  Lemme know if I am wrong and I will review.

https://codereview.appspot.com/13771047/diff/1/app/store/env/fakebackend.js#n...
app/store/env/fakebackend.js:904: // Explicit Role labelling.
Cool.

[benjamin.saller, 2013-09-19 21:36:27]

Pushed the reject deploy failure change as requested. I can repropose if need
be.

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

https://codereview.appspot.com/13771047/diff/1/app/store/env/fakebackend.js#n...
app/store/env/fakebackend.js:232: self.deploy(charmId, resolve, options);
On 2013/09/19 21:12:09, gary.poster wrote:
> This approach bypasses the Promise "rejected" function, which seems less than
> ideal.  Expedient, but it would be nice if this parsed the callback result and
> called "resolve"/fulfill or reject, as appropriate.

I was only calling this in testing where is was expected to work, but I've made
the changes and its better for it, thanks.

[benjamin.saller, 2013-09-19 21:55:38]

Added some reviewer notes

https://codereview.appspot.com/13771047/diff/1/app/models/models.js
File app/models/models.js (left):

https://codereview.appspot.com/13771047/diff/1/app/models/models.js#oldcode774
app/models/models.js:774: Takes two string endpoints and splits it into usable
parts.
On 2013/09/19 21:12:09, gary.poster wrote:
> This section and the below would have been a great opportunity for code review
> helper comments.  I'm assuming that these are deleted because they are moved,
> but that's just an assumption so far.

Yes, this code is moved back in to the fakebackend, these methods are not
changed, sorry this wasn't clear.

https://codereview.appspot.com/13771047/diff/1/app/views/topology/relation.js
File app/views/topology/relation.js (left):

https://codereview.appspot.com/13771047/diff/1/app/views/topology/relation.js...
app/views/topology/relation.js:234: decoratedRelation.display_name = pair[0][0];
Stop setting display_name and use the new default getter.

https://codereview.appspot.com/13771047/diff/1/test/data/large-improv.json
File test/data/large-improv.json (left):

https://codereview.appspot.com/13771047/diff/1/test/data/large-improv.json#ol...
test/data/large-improv.json:1: {
Improv style imports not supported anymore, cleaning up the fixtures.

https://codereview.appspot.com/13771047/diff/1/test/data/wp-deployer.yaml
File test/data/wp-deployer.yaml (right):

https://codereview.appspot.com/13771047/diff/1/test/data/wp-deployer.yaml#new...
test/data/wp-deployer.yaml:30: num_units: 2
So we can more easily test that deployer picked up the num_units.

https://codereview.appspot.com/13771047/diff/1/test/test_bundle_module.js
File test/test_bundle_module.js (right):

https://codereview.appspot.com/13771047/diff/1/test/test_bundle_module.js#new...
test/test_bundle_module.js:54: fakebackend.db.environment.set('defaultSeries',
'precise');
Almost all the test changes result from the moved code and the addition of some
helper methods.

https://codereview.appspot.com/13771047/diff/1/test/test_model.js
File test/test_model.js (left):

https://codereview.appspot.com/13771047/diff/1/test/test_model.js#oldcode810
test/test_model.js:810: 
These mostly moved to test_fakebackend. We had a few tests around the options
handling to the import call, but the server API doesn't take options so those
parts of the tests are normalized to the defaults.

https://codereview.appspot.com/13771047/diff/1/test/test_simulator.js
File test/test_simulator.js (right):

https://codereview.appspot.com/13771047/diff/1/test/test_simulator.js#newcode111
test/test_simulator.js:111: assert.equal(units.length, 12);
We are using a different import file with a different bundle than the old improv
based fixture. The fact that they differ by one is to confuse you.

https://codereview.appspot.com/13771047/diff/1/test/utils.js
File test/utils.js (right):

https://codereview.appspot.com/13771047/diff/1/test/utils.js#newcode172
test/utils.js:172: var fixture = jujuTests.utils.loadFixture(YAMLBundleURL);
Hopefully this method can be reused to quickly provide setups for integration
tests in the future.

[jeff.pihach, 2013-09-20 16:59:44]

LGTM with some trivials

QA OK 
Tested on Go sandbox and Rapi (it showed the error notification)

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

https://codereview.appspot.com/13771047/diff/1/app/store/env/fakebackend.js#n...
app/store/env/fakebackend.js:229: promiseDeploy: function(charmId, options) {
Is the idea of this method to only use it in tests? If so could you mark it as
such or make it private so that we don't start relying on it during dev

https://codereview.appspot.com/13771047/diff/1/test/test_fakebackend.js
File test/test_fakebackend.js (right):

https://codereview.appspot.com/13771047/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:519: assert.equal(fakebackend.db.relations.size(), 1,
Should we have a test here for proper config as well?

https://codereview.appspot.com/13771047/diff/1/test/utils.js
File test/utils.js (right):

https://codereview.appspot.com/13771047/diff/1/test/utils.js#newcode168
test/utils.js:168: promiseImport: function(YAMLBundleURL, name) {
Maybe some function docs

[bcsaller, 2013-09-20 18:08:15]

*** Submitted:

Use Fakebackend API to perform deployer imports

This branch, rather than direct database manipulation uses the fackbackend 
API methods to build out its virutal deployments. This also removes support
the old improv import Environment which has no UI bound to it currently (
as import/export are now deployer oriented).

As a minor this changes the policy around relation displayNames such that
on the canvas we see both names by default (that would be a one liner to 
revert) and displays the actual endpoint name on the relations tab rather
than just one of them.

R=gary.poster, benjamin.saller, jeff.pihach
CC=
https://codereview.appspot.com/13771047