pyjuju

Merge lp://staging/~rogpeppe/pyjuju/go-state-units-under-service into lp://staging/pyjuju/go

  1. go-state-units-under-service
  2. Merge into go
Proposed by Roger Peppe
Status: Rejected
Rejected by: Roger Peppe
Proposed branch: lp://staging/~rogpeppe/pyjuju/go-state-units-under-service
Merge into: lp://staging/pyjuju/go
Diff against target: 20549 lines (+19973/-0) (has conflicts)
111 files modified
.lbox.OTHER (+1/-0)
charm/bundle_test.go.OTHER (+196/-0)
charm/charm_test.go.OTHER (+97/-0)
charm/config.go.OTHER (+141/-0)
charm/config_test.go.OTHER (+191/-0)
charm/dir_test.go.OTHER (+199/-0)
charm/export_test.go.OTHER (+15/-0)
charm/meta.go.OTHER (+201/-0)
charm/meta_test.go.OTHER (+137/-0)
charm/repo.go.OTHER (+226/-0)
charm/repo_test.go.OTHER (+339/-0)
charm/url_test.go.OTHER (+126/-0)
cloudinit/cloudinit_test.go.OTHER (+228/-0)
cmd/cmd.go.OTHER (+139/-0)
cmd/cmd_test.go.OTHER (+96/-0)
cmd/juju/bootstrap.go.OTHER (+37/-0)
cmd/juju/cmd_test.go.OTHER (+255/-0)
cmd/juju/deploy.go.OTHER (+79/-0)
cmd/juju/destroy.go.OTHER (+36/-0)
cmd/juju/main.go.OTHER (+40/-0)
cmd/jujuc/main.go.OTHER (+89/-0)
cmd/jujuc/main_test.go.OTHER (+159/-0)
cmd/jujuc/server/config-get.go.OTHER (+66/-0)
cmd/jujuc/server/config-get_test.go.OTHER (+125/-0)
cmd/jujuc/server/context.go.OTHER (+96/-0)
cmd/jujuc/server/context_test.go.OTHER (+145/-0)
cmd/jujuc/server/juju-log.go.OTHER (+57/-0)
cmd/jujuc/server/juju-log_test.go.OTHER (+82/-0)
cmd/jujuc/server/output.go.OTHER (+149/-0)
cmd/jujuc/server/ports.go.OTHER (+100/-0)
cmd/jujuc/server/ports_test.go.OTHER (+86/-0)
cmd/jujuc/server/server.go.OTHER (+133/-0)
cmd/jujuc/server/server_test.go.OTHER (+146/-0)
cmd/jujuc/server/unit-get.go.OTHER (+66/-0)
cmd/jujuc/server/unit-get_test.go.OTHER (+109/-0)
cmd/jujuc/server/util_test.go.OTHER (+149/-0)
cmd/jujud/agent.go.OTHER (+70/-0)
cmd/jujud/initzk.go.OTHER (+44/-0)
cmd/jujud/initzk_test.go.OTHER (+78/-0)
cmd/jujud/machine.go.OTHER (+36/-0)
cmd/jujud/machine_test.go.OTHER (+35/-0)
cmd/jujud/main.go.OTHER (+29/-0)
cmd/jujud/main_test.go.OTHER (+67/-0)
cmd/jujud/provisioning.go.OTHER (+142/-0)
cmd/jujud/provisioning_test.go.OTHER (+107/-0)
cmd/jujud/unit.go.OTHER (+40/-0)
cmd/jujud/unit_test.go.OTHER (+44/-0)
cmd/jujud/util_test.go.OTHER (+46/-0)
cmd/logging.go.OTHER (+46/-0)
cmd/logging_test.go.OTHER (+104/-0)
cmd/supercommand_test.go.OTHER (+96/-0)
cmd/util_test.go.OTHER (+78/-0)
environs/config_test.go.OTHER (+176/-0)
environs/dummy/environs.go.OTHER (+411/-0)
environs/dummy/environs_test.go.OTHER (+41/-0)
environs/dummy/storage.go.OTHER (+97/-0)
environs/ec2/auth_test.go.OTHER (+87/-0)
environs/ec2/cloudinit.go.OTHER (+165/-0)
environs/ec2/cloudinit_test.go.OTHER (+210/-0)
environs/ec2/config.go.OTHER (+98/-0)
environs/ec2/config_test.go.OTHER (+196/-0)
environs/ec2/ec2.go.OTHER (+634/-0)
environs/ec2/export_test.go.OTHER (+102/-0)
environs/ec2/image.go.OTHER (+111/-0)
environs/ec2/live_test.go.OTHER (+348/-0)
environs/ec2/local_test.go.OTHER (+254/-0)
environs/ec2/state.go.OTHER (+48/-0)
environs/ec2/storage.go.OTHER (+127/-0)
environs/ec2/suite_test.go.OTHER (+22/-0)
environs/interface.go.OTHER (+149/-0)
environs/jujutest/livetests.go.OTHER (+116/-0)
environs/jujutest/test.go.OTHER (+99/-0)
environs/jujutest/tests.go.OTHER (+179/-0)
environs/open_test.go.OTHER (+53/-0)
environs/tools.go.OTHER (+322/-0)
environs/tools_test.go.OTHER (+292/-0)
juju/conn.go.OTHER (+77/-0)
juju/conn_test.go.OTHER (+89/-0)
log/log_test.go.OTHER (+54/-0)
schema/schema_test.go.OTHER (+292/-0)
state/charm.go.OTHER (+88/-0)
state/export_test.go.OTHER (+40/-0)
state/internal_test.go.OTHER (+1109/-0)
state/machine.go.OTHER (+120/-0)
state/open.go.OTHER (+160/-0)
state/presence/presence_test.go.OTHER (+320/-0)
state/relation.go.OTHER (+86/-0)
state/service.go.OTHER (+268/-0)
state/ssh.go.OTHER (+282/-0)
state/ssh_test.go.OTHER (+494/-0)
state/state.go.OTHER (+393/-0)
state/state_test.go.OTHER (+1253/-0)
state/topology.go.OTHER (+587/-0)
state/unit.go.OTHER (+611/-0)
state/watcher.go.OTHER (+411/-0)
state/watcher/watcher_test.go.OTHER (+168/-0)
state/watcher_test.go.OTHER (+355/-0)
store/branch.go.OTHER (+146/-0)
store/branch_test.go.OTHER (+195/-0)
store/charmd/main.go.OTHER (+74/-0)
store/charmload/main.go.OTHER (+75/-0)
store/lpad.go.OTHER (+113/-0)
store/lpad_test.go.OTHER (+68/-0)
store/server.go.OTHER (+190/-0)
store/server_test.go.OTHER (+208/-0)
store/store.go.OTHER (+762/-0)
store/store_test.go.OTHER (+574/-0)
testing/charm.go.OTHER (+82/-0)
testing/log.go.OTHER (+25/-0)
testing/zk_test.go.OTHER (+58/-0)
upstart/upstart_test.go.OTHER (+211/-0)
Contents conflict in .lbox
Conflict adding files to charm.  Created directory.
Conflict because charm is not versioned, but has versioned children.  Versioned directory.
Contents conflict in charm/bundle_test.go
Contents conflict in charm/charm_test.go
Contents conflict in charm/config.go
Contents conflict in charm/config_test.go
Contents conflict in charm/dir_test.go
Contents conflict in charm/export_test.go
Contents conflict in charm/meta.go
Contents conflict in charm/meta_test.go
Contents conflict in charm/repo.go
Contents conflict in charm/repo_test.go
Contents conflict in charm/url_test.go
Conflict adding files to cloudinit.  Created directory.
Conflict because cloudinit is not versioned, but has versioned children.  Versioned directory.
Contents conflict in cloudinit/cloudinit_test.go
Conflict adding files to cmd.  Created directory.
Conflict because cmd is not versioned, but has versioned children.  Versioned directory.
Contents conflict in cmd/cmd.go
Contents conflict in cmd/cmd_test.go
Conflict adding files to cmd/juju.  Created directory.
Conflict because cmd/juju is not versioned, but has versioned children.  Versioned directory.
Contents conflict in cmd/juju/bootstrap.go
Contents conflict in cmd/juju/cmd_test.go
Contents conflict in cmd/juju/deploy.go
Contents conflict in cmd/juju/destroy.go
Contents conflict in cmd/juju/main.go
Conflict adding files to cmd/jujuc.  Created directory.
Conflict because cmd/jujuc is not versioned, but has versioned children.  Versioned directory.
Contents conflict in cmd/jujuc/main.go
Contents conflict in cmd/jujuc/main_test.go
Conflict adding files to cmd/jujuc/server.  Created directory.
Conflict because cmd/jujuc/server is not versioned, but has versioned children.  Versioned directory.
Contents conflict in cmd/jujuc/server/config-get.go
Contents conflict in cmd/jujuc/server/config-get_test.go
Contents conflict in cmd/jujuc/server/context.go
Contents conflict in cmd/jujuc/server/context_test.go
Contents conflict in cmd/jujuc/server/juju-log.go
Contents conflict in cmd/jujuc/server/juju-log_test.go
Contents conflict in cmd/jujuc/server/output.go
Contents conflict in cmd/jujuc/server/ports.go
Contents conflict in cmd/jujuc/server/ports_test.go
Contents conflict in cmd/jujuc/server/server.go
Contents conflict in cmd/jujuc/server/server_test.go
Contents conflict in cmd/jujuc/server/unit-get.go
Contents conflict in cmd/jujuc/server/unit-get_test.go
Contents conflict in cmd/jujuc/server/util_test.go
Conflict adding files to cmd/jujud.  Created directory.
Conflict because cmd/jujud is not versioned, but has versioned children.  Versioned directory.
Contents conflict in cmd/jujud/agent.go
Contents conflict in cmd/jujud/initzk.go
Contents conflict in cmd/jujud/initzk_test.go
Contents conflict in cmd/jujud/machine.go
Contents conflict in cmd/jujud/machine_test.go
Contents conflict in cmd/jujud/main.go
Contents conflict in cmd/jujud/main_test.go
Contents conflict in cmd/jujud/provisioning.go
Contents conflict in cmd/jujud/provisioning_test.go
Contents conflict in cmd/jujud/unit.go
Contents conflict in cmd/jujud/unit_test.go
Contents conflict in cmd/jujud/util_test.go
Contents conflict in cmd/logging.go
Contents conflict in cmd/logging_test.go
Contents conflict in cmd/supercommand_test.go
Contents conflict in cmd/util_test.go
Conflict adding files to environs.  Created directory.
Conflict because environs is not versioned, but has versioned children.  Versioned directory.
Contents conflict in environs/config_test.go
Conflict adding files to environs/dummy.  Created directory.
Conflict because environs/dummy is not versioned, but has versioned children.  Versioned directory.
Contents conflict in environs/dummy/environs.go
Contents conflict in environs/dummy/environs_test.go
Contents conflict in environs/dummy/storage.go
Conflict adding files to environs/ec2.  Created directory.
Conflict because environs/ec2 is not versioned, but has versioned children.  Versioned directory.
Contents conflict in environs/ec2/auth_test.go
Contents conflict in environs/ec2/cloudinit.go
Contents conflict in environs/ec2/cloudinit_test.go
Contents conflict in environs/ec2/config.go
Contents conflict in environs/ec2/config_test.go
Contents conflict in environs/ec2/ec2.go
Contents conflict in environs/ec2/export_test.go
Contents conflict in environs/ec2/image.go
Contents conflict in environs/ec2/live_test.go
Contents conflict in environs/ec2/local_test.go
Contents conflict in environs/ec2/state.go
Contents conflict in environs/ec2/storage.go
Contents conflict in environs/ec2/suite_test.go
Contents conflict in environs/interface.go
Conflict adding files to environs/jujutest.  Created directory.
Conflict because environs/jujutest is not versioned, but has versioned children.  Versioned directory.
Contents conflict in environs/jujutest/livetests.go
Contents conflict in environs/jujutest/test.go
Contents conflict in environs/jujutest/tests.go
Contents conflict in environs/open_test.go
Contents conflict in environs/tools.go
Contents conflict in environs/tools_test.go
Conflict adding files to juju.  Created directory.
Conflict because juju is not versioned, but has versioned children.  Versioned directory.
Contents conflict in juju/conn.go
Contents conflict in juju/conn_test.go
Conflict adding files to log.  Created directory.
Conflict because log is not versioned, but has versioned children.  Versioned directory.
Contents conflict in log/log_test.go
Conflict adding files to schema.  Created directory.
Conflict because schema is not versioned, but has versioned children.  Versioned directory.
Contents conflict in schema/schema_test.go
Conflict adding files to state.  Created directory.
Conflict because state is not versioned, but has versioned children.  Versioned directory.
Contents conflict in state/charm.go
Contents conflict in state/export_test.go
Contents conflict in state/internal_test.go
Contents conflict in state/machine.go
Contents conflict in state/open.go
Conflict adding files to state/presence.  Created directory.
Conflict because state/presence is not versioned, but has versioned children.  Versioned directory.
Contents conflict in state/presence/presence_test.go
Contents conflict in state/relation.go
Contents conflict in state/service.go
Contents conflict in state/ssh.go
Contents conflict in state/ssh_test.go
Contents conflict in state/state.go
Contents conflict in state/state_test.go
Contents conflict in state/topology.go
Contents conflict in state/unit.go
Conflict adding files to state/watcher.  Created directory.
Conflict because state/watcher is not versioned, but has versioned children.  Versioned directory.
Contents conflict in state/watcher.go
Contents conflict in state/watcher/watcher_test.go
Contents conflict in state/watcher_test.go
Conflict adding files to store.  Created directory.
Conflict because store is not versioned, but has versioned children.  Versioned directory.
Contents conflict in store/branch.go
Contents conflict in store/branch_test.go
Conflict adding files to store/charmd.  Created directory.
Conflict because store/charmd is not versioned, but has versioned children.  Versioned directory.
Contents conflict in store/charmd/main.go
Conflict adding files to store/charmload.  Created directory.
Conflict because store/charmload is not versioned, but has versioned children.  Versioned directory.
Contents conflict in store/charmload/main.go
Contents conflict in store/lpad.go
Contents conflict in store/lpad_test.go
Contents conflict in store/server.go
Contents conflict in store/server_test.go
Contents conflict in store/store.go
Contents conflict in store/store_test.go
Conflict adding files to testing.  Created directory.
Conflict because testing is not versioned, but has versioned children.  Versioned directory.
Contents conflict in testing/charm.go
Contents conflict in testing/log.go
Contents conflict in testing/zk_test.go
Conflict adding files to upstart.  Created directory.
Conflict because upstart is not versioned, but has versioned children.  Versioned directory.
Contents conflict in upstart/upstart_test.go
To merge this branch: bzr merge lp://staging/~rogpeppe/pyjuju/go-state-units-under-service
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+108341@code.staging.launchpad.net

Description of the change

state: store units inside their respective services

Making this change simplifies a lot of code - not need
to keep a separate sequence numbering system for units,
for example.

https://codereview.appspot.com/6247066/

To post a comment you must log in.
Revision history for this message
Roger Peppe (rogpeppe) wrote :

Reviewers: mp+108341_code.launchpad.net,

Message:
Please take a look.

Description:
state: store units inside their respective services

Making this change simplifies a lot of code - not need
to keep a separate sequence numbering system for units,
for example.

https://code.launchpad.net/~rogpeppe/juju/go-state-units-under-service/+merge/108341

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6247066/

Affected files:
   A [revision details]
   M state/internal_test.go
   M state/machine.go
   M state/service.go
   M state/state.go
   M state/state_test.go
   M state/topology.go
   M state/unit.go

Revision history for this message
Roger Peppe (rogpeppe) wrote :

Please take a look.

https://codereview.appspot.com/6247066/

Revision history for this message
William Reade (fwereade) wrote :

LGTM modulo below.

https://codereview.appspot.com/6247066/diff/7001/state/service.go
File state/service.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/service.go#newcode120
state/service.go:120: return zkRemoveTree(s.st.zk, unit.zkPath())
Not relevant to this CL... but do we actually want to do this? The unit
agent will surely still be running; I'm not sure it'll handle this all
that well...

https://codereview.appspot.com/6247066/diff/7001/state/service.go#newcode228
state/service.go:228: return s.zkPath() + "/config"
I strongly approve of all the changes like this one :).

https://codereview.appspot.com/6247066/diff/7001/state/topology.go
File state/topology.go (left):

https://codereview.appspot.com/6247066/diff/7001/state/topology.go#oldcode283
state/topology.go:283: // sequence number will be increased
monotonically for each service.
Update comment

https://codereview.appspot.com/6247066/diff/7001/state/unit.go
File state/unit.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode77
state/unit.go:77: func mkUnitKey(serviceKey string, unitId int) string {
What does "mk" signify?

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode86
state/unit.go:86: return "", fmt.Errorf("invalid unit key %q (1) %s",
unitKey, debug.Callers(0, 10))
Is this the proposed implementation or an oversight?

https://codereview.appspot.com/6247066/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Very nice change. LGTM, assuming you're happy with the adaptations
below. Otherwise, please reply accordingly and repropose.

Please note you'll have to propose+submit this to the new juju-core
project, since the old trunk is now gone (sorry!). You can do this by
creating a new branch from the new project as you'd usually do, merging
from this work onto it, and then just proposing as usual.

https://codereview.appspot.com/6247066/diff/7001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/machine.go#newcode88
state/machine.go:88: // keyId returns the id corresponding to the given
machine or unit id.
s/keyId/keySequence/ or s/keyId/keySeq/

This is not an identifier. "unit-0000001" is the identifier, and we use
the term "key" there, after much debate, precisely to avoid the
confusion with the idea of an "id" which is public. We can easily avoid
the confusion still by calling the sequence number what it really is (it
comes from the idea of sequence from ZooKeeper).

As a special case, we do call the *machine* sequence number an
identifier, because that's how we handle it across the board
(machine.Id(), etc).

https://codereview.appspot.com/6247066/diff/7001/state/service.go
File state/service.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/service.go#newcode13
state/service.go:13: pathPkg "path"
stdpath

https://codereview.appspot.com/6247066/diff/7001/state/service.go#newcode84
state/service.go:84: kprefix = kprefix[:strings.LastIndex(kprefix,
"-")+1]
There's no reason to go over the trouble here. It's already assuming
knowledge about how makeUnitKey works (who said it could skip 'til the
last dash like that?) so it may as well be a lot more clear and say
"/units/unit-" + s.key + "-".

https://codereview.appspot.com/6247066/diff/7001/state/unit.go
File state/unit.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode77
state/unit.go:77: func mkUnitKey(serviceKey string, unitId int) string {
On 2012/06/04 07:29:19, fwereade wrote:
> What does "mk" signify?

I assume "make", but I'd also prefer the word itself rather than saving
two chars. Even more when we have serviceKeyForUnitKey below. :-)

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode86
state/unit.go:86: return "", fmt.Errorf("invalid unit key %q (1) %s",
unitKey, debug.Callers(0, 10))
On 2012/06/04 07:29:19, fwereade wrote:
> Is this the proposed implementation or an oversight?

Yeah, looks like an oversight. We need something closer to the error
message of makeUnitKey.

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode518
state/unit.go:518: func parseUnitName(name string) (serviceName string,
serviceId int, err error) {
s/serviceId/serviceSeq/

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode523
state/unit.go:523: id, err := strconv.ParseInt(parts[1], 10, 0)
s/id/seq/

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode527
state/unit.go:527: return parts[0], int(id), nil
s/id/seq/

https://codereview.appspot.com/6247066/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

I have marked the branch in Launchpad as Rejected, but only because of
the move to juju-core. The above review is still valid and is what
counts.

https://codereview.appspot.com/6247066/

lp://staging/~rogpeppe/pyjuju/go-state-units-under-service updated
201. By Roger Peppe

state: fixes for review

202. By Roger Peppe

merge trunk

Revision history for this message
Roger Peppe (rogpeppe) wrote :
Download full text (3.7 KiB)

submitted in branch from juju-core

https://codereview.appspot.com/6247066/diff/7001/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/machine.go#newcode88
state/machine.go:88: // keyId returns the id corresponding to the given
machine or unit id.
On 2012/06/06 20:15:09, niemeyer wrote:
> s/keyId/keySequence/ or s/keyId/keySeq/

> This is not an identifier. "unit-0000001" is the identifier, and we
use the term
> "key" there, after much debate, precisely to avoid the confusion with
the idea
> of an "id" which is public. We can easily avoid the confusion still by
calling
> the sequence number what it really is (it comes from the idea of
sequence from
> ZooKeeper).

> As a special case, we do call the *machine* sequence number an
identifier,
> because that's how we handle it across the board (machine.Id(), etc).

Done.

https://codereview.appspot.com/6247066/diff/7001/state/service.go
File state/service.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/service.go#newcode84
state/service.go:84: kprefix = kprefix[:strings.LastIndex(kprefix,
"-")+1]
On 2012/06/06 20:15:09, niemeyer wrote:
> There's no reason to go over the trouble here. It's already assuming
 knowledge
> about how makeUnitKey works (who said it could skip 'til the last dash
 like
> that?) so it may as well be a lot more clear and say "/units/unit-" +
 s.key +
> "-".

it's not *quite* as simple as that (makeUnitKey strips off the initial
"service-" prefix) but done anyway.

https://codereview.appspot.com/6247066/diff/7001/state/service.go#newcode120
state/service.go:120: return zkRemoveTree(s.st.zk, unit.zkPath())
On 2012/06/04 07:29:19, fwereade wrote:
> Not relevant to this CL... but do we actually want to do this? The
 unit agent
> will surely still be running; I'm not sure it'll handle this all that
 well...

that's an interesting point (although, as you say, not relevant to this
CL). what *should* we do here? wait until the unit agent has gone away?
leave the directory around and GC it later? or perhaps the best approach
*is* to delete the directory and let the unit agent detect that.

https://codereview.appspot.com/6247066/diff/7001/state/unit.go
File state/unit.go (right):

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode77
state/unit.go:77: func mkUnitKey(serviceKey string, unitId int) string {
On 2012/06/06 20:15:09, niemeyer wrote:
> On 2012/06/04 07:29:19, fwereade wrote:
> > What does "mk" signify?

> I assume "make", but I'd also prefer the word itself rather than
saving two
> chars. Even more when we have serviceKeyForUnitKey below. :-)

done. old C proclivities creeping in there... :-)

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode86
state/unit.go:86: return "", fmt.Errorf("invalid unit key %q (1) %s",
unitKey, debug.Callers(0, 10))
On 2012/06/06 20:15:09, niemeyer wrote:
> On 2012/06/04 07:29:19, fwereade wrote:
> > Is this the proposed implementation or an oversight?

> Yeah, looks like an oversight. We need something closer to the error
message of
> makeUnitKey.

oops. done.

https://codereview.appspot.com/6247066/diff/7001/state/unit.go#newcode518
sta...

Read more...

Revision history for this message
William Reade (fwereade) wrote :

On 2012/06/07 12:07:29, rog wrote:
> On 2012/06/04 07:29:19, fwereade wrote:
> > Not relevant to this CL... but do we actually want to do this? The
 unit agent
> > will surely still be running; I'm not sure it'll handle this all
 that well...

> that's an interesting point (although, as you say, not relevant to
this CL).
> what *should* we do here? wait until the unit agent has gone away?
leave the
> directory around and GC it later? or perhaps the best approach *is* to
delete
> the directory and let the unit agent detect that.

Offhand, I think I'd favour a dying/dead approach... eg this sets
"dying", which is handled by the UA (which cleans itself up, and sets
"dead"); MA can watch for "dead" on assigned units, tidy them up, and
finally delete the state once we're sure it's unused. Not sure though.

https://codereview.appspot.com/6247066/

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

taking out dirs from running processes is a bad idea. it needs to be
coordinate if your going to leave the process around.

On Thu, Jun 7, 2012 at 11:22 AM, William Reade
<email address hidden>wrote:

> On 2012/06/07 12:07:29, rog wrote:
> > On 2012/06/04 07:29:19, fwereade wrote:
> > > Not relevant to this CL... but do we actually want to do this? The
> unit agent
> > > will surely still be running; I'm not sure it'll handle this all
> that well...
>
> > that's an interesting point (although, as you say, not relevant to
> this CL).
> > what *should* we do here? wait until the unit agent has gone away?
> leave the
> > directory around and GC it later? or perhaps the best approach *is* to
> delete
> > the directory and let the unit agent detect that.
>
> Offhand, I think I'd favour a dying/dead approach... eg this sets
> "dying", which is handled by the UA (which cleans itself up, and sets
> "dead"); MA can watch for "dead" on assigned units, tidy them up, and
> finally delete the state once we're sure it's unused. Not sure though.
>
> https://codereview.appspot.com/6247066/
>
> --
>
> https://code.launchpad.net/~rogpeppe/juju/go-state-units-under-service/+merge/108341
> Your team juju hackers is requested to review the proposed merge of
> lp:~rogpeppe/juju/go-state-units-under-service into lp:juju/go.
>

Unmerged revisions

202. By Roger Peppe

merge trunk

201. By Roger Peppe

state: fixes for review

200. By Roger Peppe

minor tweaks

199. By Roger Peppe

state: use a string for unit key

198. By Roger Peppe

state: move function
;

197. By Roger Peppe

state: move units under service directory

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: