Code review - Issue 8824044: Handle service lifehttps://codereview.appspot.com/2013-04-17T13:49:09+00:00rietveld
Message from unknown
2013-04-17T11:27:14+00:00frankbanurn:md5:eaef3888e8b6f2dcc399097e84621136
Message from francesco.banconi@canonical.com
2013-04-17T11:27:22+00:00frankbanurn:md5:6a0f431449031a843c685250e67bd4a5
Please take a look.
Message from benji.york@gmail.com
2013-04-17T12:37:59+00:00benjiurn:md5:74b88d3945135e03b467a705065878fc
This looks good (LGTM). I assume there is going to be a follow-on branch that wires up the alive status to the go back-end because I don't see any way that services can be anything but alive, thus-far. Is that right, or am I missing something?
https://codereview.appspot.com/8824044/diff/1/app/models/models.js
File app/models/models.js (right):
https://codereview.appspot.com/8824044/diff/1/app/models/models.js#newcode99
app/models/models.js:99: Return true if this service life is "alive", false otherwise.
It might be nice to explain (ether here or in "alive" below) what it means for a service to be "alive".
Message from gary.poster@canonical.com
2013-04-17T12:54:26+00:00gary.posterurn:md5:17e72a530b55d1c756c8ee6cc714d961
Looks good. QAing.
https://codereview.appspot.com/8824044/diff/1/app/models/models.js
File app/models/models.js (right):
https://codereview.appspot.com/8824044/diff/1/app/models/models.js#newcode99
app/models/models.js:99: Return true if this service life is "alive", false otherwise.
On 2013/04/17 12:37:59, benji wrote:
> It might be nice to explain (ether here or in "alive" below) what it means for a
> service to be "alive".
+1
https://codereview.appspot.com/8824044/diff/1/test/test_model.js
File test/test_model.js (right):
https://codereview.appspot.com/8824044/diff/1/test/test_model.js#newcode665
test/test_model.js:665: it('instances identify if they are alive', function() {
(and also verifies that the default state is alive :-) )
Message from gary.poster@canonical.com
2013-04-17T13:10:16+00:00gary.posterurn:md5:2a79b819fc168c51098e63eb9e32e00f
LGTM. QA went well. Thank you!
Gary
Message from unknown
2013-04-17T13:36:22+00:00frankbanurn:md5:2b4d312baaeb75f1f296232dcd49dbcc
Message from francesco.banconi@canonical.com
2013-04-17T13:39:01+00:00frankbanurn:md5:361de55e2a23e4ea953714be72a7158d
*** Submitted:
Handle service life
The Life change included in the juju-core delta stream is
now stored in the service model, and used to avoid displaying
dying services in the topology view.
QA:
- bootstrap a juju-core env;
- deploy a service;
- connect the GUI to the env;
- use the GUI to destroy the service;
- suddenly refresh (or visit the GUI from another browser tab).
At this point, even if the service could be still
present in the db, it should not be alive and you should
not see it in the topology view.
R=benji, gary.poster
CC=
https://codereview.appspot.com/8824044
https://codereview.appspot.com/8824044/diff/1/app/models/models.js
File app/models/models.js (right):
https://codereview.appspot.com/8824044/diff/1/app/models/models.js#newcode99
app/models/models.js:99: Return true if this service life is "alive", false otherwise.
On 2013/04/17 12:37:59, benji wrote:
> It might be nice to explain (ether here or in "alive" below) what it means for a
> service to be "alive".
Done.
https://codereview.appspot.com/8824044/diff/1/test/test_model.js
File test/test_model.js (right):
https://codereview.appspot.com/8824044/diff/1/test/test_model.js#newcode665
test/test_model.js:665: it('instances identify if they are alive', function() {
On 2013/04/17 12:54:26, gary.poster wrote:
> (and also verifies that the default state is alive :-) )
Indeed! Added a comment.
Message from francesco.banconi@canonical.com
2013-04-17T13:49:09+00:00frankbanurn:md5:d0a5d2e51f5f9263e580735c96a91d48
Thank you both for the reviews!