Code review comment for lp:~hatch/juju-gui/save-changed-1214087

Thanks for this branch Jeff. I think there is still some substantial
work/clean up to do here (see the comments below). The constraints
problem prevented me from deploying charms in juju-core most of the
times. In the go sandbox, I was able to deploy services and set
constraints, but a further look in the inspector showed the constraints
to be broken (e.g. a constraint named "1" with value "cpu-cores=4"). I
believe this is the same problem described below.

https://codereview.appspot.com/13252045/diff/1/app/views/ghost-inspector.js
File app/views/ghost-inspector.js (right):

https://codereview.appspot.com/13252045/diff/1/app/views/ghost-inspector.js#newcode141
app/views/ghost-inspector.js:141: // Format the constraints into the
proper format.
This is not the proper format for juju-core, e.g.:

D 130830 08:33:57 handlers:148] GET /ws (10.0.3.1) client -> juju:
{"Type":"Client","Request":"ServiceDeploy","Params":{"ServiceName":"mediawiki","Config":{},"Constraints":["cpu-power=","cpu-cores=4","mem=1","arch="],"CharmUrl":"cs:precise/mediawiki-9","NumUnits":1},"RequestId":17}
[D 130830 08:33:57 handlers:164] GET /ws (10.0.3.1) juju -> client:
{"RequestId":17,"Error":"json: cannot unmarshal array into Go value of
type constraints.Value","Response":{}}

The deploy method of the GoEnvironment, and the set_constraints as well,
requires an object to be passed. The juju-core constraints.Value itself
is a struct, not a slice. The deploy method of the PyEnvironment does
not mention "constraints" in its docstring (and this should be fixed).
Since we eventually want to stop maintaining the pyjuju backend, I'd
suggest to generate the constraints as an object (like it is currently
done), and make the proper conversions only ub the case the environment
is python (or the python sandbox).
Does this make sense?

https://codereview.appspot.com/13252045/diff/1/app/views/inspector.js
File app/views/inspector.js (right):

https://codereview.appspot.com/13252045/diff/1/app/views/inspector.js#newcode739
app/views/inspector.js:739: /*jshint -W089 */
This logic is used by both inspectors: I'd like to see this code
abstracted in a separate function, reused when required, and tested. A
comment explaining why we need implicit type conversion would be great.

https://codereview.appspot.com/13252045/