Make the GUI aware of the new Subordinate info. #558

frankban · 2014-09-16T17:20:21Z

After a recent change in juju-core, the subordinate
info is available through the mega-watcher for
services and units. This branch enables the GUI to
fully support this where possible.

Also fixed the code so that uses of "subordinate" vs
"is_subordinate" are consistent.

After a recent change in juju-core, the subordinate info is available through the mega-watcher for services and units. This branch enables the GUI to fully support this where possible. Also fixed the code so that uses of "subordinate" vs "is_subordinate" are consistent.

frankban · 2014-09-16T17:21:54Z

QA:

in sandbox mode and in a real env, use the mv flag;
deploy a subordinate charm, e.g. storage or puppet;
check that the inspector (both for the ghost and the deployed service)
does not expose the ability to scale up the service;
in machine view, the service unit does not appear;
deploy another non-subordinate unit, and check you can still place and
scale up the non-subordinate unit.

It's not easy to exercize the new juju-core mega-watcher stream, but code includes tests for that, so I don't think QAing that is important right now.

Thank you!

jujugui · 2014-09-16T19:23:28Z

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1864/

jcsackett · 2014-09-16T20:54:10Z

app/store/env/sandbox.js

@@ -943,7 +944,8 @@ YUI.add('juju-env-sandbox', function(Y) {
        Ports: 'open_ports',
        Status: 'agent_state',
        StatusInfo: 'agent_state_info',
-        StatusData: 'agent_state_data'
+        StatusData: 'agent_state_data',
+        Subordinate: 'subordinate'


This clearly predates your branch, but do you know why all the keys in this object are capitalized? Everywhere else we use camelCase.

Because this maps fields in the juju-core stream (capitalized) to internal JS model fields.

Ah, thanks.

jcsackett · 2014-09-16T21:06:47Z

Code looks good, though I have a few questions. I am QAing now.

jcsackett · 2014-09-16T21:15:59Z

Sandbox QA notes:

I deployed puppet; the scale up wasn't available, as expected. The inspector as a side effect looks a little goofy, but we can circle back with UX. Deploying worked as expected.
In machine view, I could scale up puppet--the adding units stuff was available there. I don't know if it's supposed to be. If the goal is to block scaling up entirely, I think we need a follow up branch to address machine view. I do not feel this blocks this branch's QA--merely that there might be further work.
Non-subordinate services had scale up as expected, and could properly be scaled up.

jcsackett · 2014-09-16T23:33:27Z

QA ec2 notes:

On ec2, I observed exactly the same behavior as in sandbox.

QA OK, with provisos of possible follow up work.

Thanks @frankban! 👍

frankban · 2014-09-17T12:06:43Z

Thanks for the review and QA JC. Yeah the scale up in mv needs to be fixed, but was out of scope for this branch.

jujugui · 2014-09-17T12:45:08Z

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1878/

hatched · 2014-09-17T15:28:29Z

app/models/handlers.js

@@ -191,7 +191,11 @@ YUI.add('juju-delta-handlers', function(Y) {
        agent_state_data: change.StatusData,
        public_address: change.PublicAddress,
        private_address: change.PrivateAddress,
-        open_ports: utils.convertOpenPorts(change.Ports)
+        open_ports: utils.convertOpenPorts(change.Ports),
+        // Since less recent versions of juju-core do not include the


Could you add the version of Juju that this will become available?

hatched · 2014-09-17T16:07:31Z

👍 with some comments. Thanks for jumping on this fix so fast!

jujugui · 2014-09-17T17:16:11Z

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1879/

frankban · 2014-09-17T17:17:31Z

Thanks for the reviews!

frankban · 2014-09-17T18:17:41Z

Retrying, failure seems spurious.

jujugui · 2014-09-17T18:18:00Z

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

Make the GUI aware of the new Subordinate info. After a recent change in juju-core, the subordinate info is available through the mega-watcher for services and units. This branch enables the GUI to fully support this where possible. Also fixed the code so that uses of "subordinate" vs "is_subordinate" are consistent.

jcsackett reviewed Sep 16, 2014
View reviewed changes

Add default value to units model subordinate attribute.

e35a443

hatched reviewed Sep 17, 2014
View reviewed changes

Improve the comments reguarding subordinate support in juju-core.

b6c210c

jujugui merged commit 00d9ba7 into juju:develop Sep 17, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the GUI aware of the new Subordinate info. #558

Make the GUI aware of the new Subordinate info. #558

frankban commented Sep 16, 2014

frankban commented Sep 16, 2014

jujugui commented Sep 16, 2014

jcsackett Sep 16, 2014

frankban Sep 17, 2014

jcsackett Sep 17, 2014

jcsackett commented Sep 16, 2014

jcsackett commented Sep 16, 2014

jcsackett commented Sep 16, 2014

frankban commented Sep 17, 2014

jujugui commented Sep 17, 2014

hatched Sep 17, 2014

frankban Sep 17, 2014

hatched commented Sep 17, 2014

jujugui commented Sep 17, 2014

frankban commented Sep 17, 2014

frankban commented Sep 17, 2014

jujugui commented Sep 17, 2014

Make the GUI aware of the new Subordinate info. #558

Make the GUI aware of the new Subordinate info. #558

Conversation

frankban commented Sep 16, 2014

frankban commented Sep 16, 2014

jujugui commented Sep 16, 2014

jcsackett Sep 16, 2014

Choose a reason for hiding this comment

frankban Sep 17, 2014

Choose a reason for hiding this comment

jcsackett Sep 17, 2014

Choose a reason for hiding this comment

jcsackett commented Sep 16, 2014

jcsackett commented Sep 16, 2014

jcsackett commented Sep 16, 2014

frankban commented Sep 17, 2014

jujugui commented Sep 17, 2014

hatched Sep 17, 2014

Choose a reason for hiding this comment

frankban Sep 17, 2014

Choose a reason for hiding this comment

hatched commented Sep 17, 2014

jujugui commented Sep 17, 2014

frankban commented Sep 17, 2014

frankban commented Sep 17, 2014

jujugui commented Sep 17, 2014