| axw | blahdeblah: if the default should be to do X, what's a better approach? | 00:01 |
|---|---|---|
| axw | forcing someone to say "--do-X" when X is the obvious default isn't very friendly | 00:01 |
| axw | (whether the obvious default is to destroy on remove is debatable, but it's the default already and I'm not sure we can change *that* behaviour) | 00:02 |
| blahdeblah | axw: Personally, whenever it's destructive, I wouldn't have a default. But like you say, may not be something you can change... | 00:02 |
| axw | blahdeblah: that has come up as an option, it's probably what we're going to do with destroy-model/destroy-controller (force you to choose between destroying/keeping storage). so *maybe* for remove-storage too | 00:03 |
| babbageclunk | axw: Yeah, I think I've talked myself around in the state case - it *is* destroying the storage in the model, it's just not destroying the underlying cloud storage. But there are also places in the providers and provisioner that I think would be clearer not calling the method Destroy. | 00:03 |
| axw | blahdeblah: FWIW, the spelling at this stage for destroy-model will be --keep-storage/--destroy-storage | 00:04 |
| blahdeblah | That makes good sense | 00:04 |
| axw | so no double negatives in sight :) | 00:04 |
| blahdeblah | \o/ | 00:04 |
| axw | babbageclunk: ok. I'll see what can be done to clarify | 00:05 |
| babbageclunk | axw: Anyway, just wanted to give you a heads-up before I finish the review - it started to feel a bit like I was harping on about it, but it's really my only issue with the PR. | 00:07 |
| axw | babbageclunk: no problem, I'm always happy to make the code easier to comprehend | 00:08 |
| axw | hml: would you kindly take a quick look at https://github.com/juju/juju/pull/7653? | 00:21 |
| hml | axw: looking | 00:21 |
| axw | hml: thanks | 00:29 |
| axw | babbageclunk hml: standup | 00:32 |
| babbageclunk | sorry! | 00:32 |
| axw | babbageclunk: thanks for the review. ReleaseVolumeParams doesn't really work because release = non-descructive remove. how about "RemoveVolumeParams"? i.e. params for destroying or removing the (cloud) volume | 01:32 |
| axw | er, non-destructive | 01:32 |
| babbageclunk | axw: Yeah, I like remove too | 01:36 |
| babbageclunk | axw: That makes sense - so remove becomes the general term that could denote a release or a destroy. | 01:39 |
| axw | babbageclunk: yep. except in state :) | 01:40 |
| babbageclunk | axw: sure, not much we can do about that one. | 01:40 |
| === mup_ is now known as mup | ||
| axw | babbageclunk: can you PTAL at https://github.com/juju/juju/pull/7649/commits, I haven't rebased yet so you can see the changes I've made today | 03:49 |
| babbageclunk | axw: thanks - looking | 03:49 |
| axw | babbageclunk menn0: do you know any tricks for removing a method from a newer version of a facade? I'm embedding v3 in v4 for hte Storage API, but I want drop a method... I'd rather not duplicate methods for the ones I want to keep | 03:56 |
| babbageclunk | axw: If you put a method on the facade that takes 2 args, the RPC layer will ignore it. | 03:57 |
| babbageclunk | axw: That effectively removes it. | 03:57 |
| babbageclunk | axw: (This hack brought to you by wpk) | 03:58 |
| menn0 | axw: what babbageclunk said. I think the Uniter facade does that. | 03:58 |
| axw | babbageclunk: that'll work, I think I saw something like that fly by | 03:58 |
| axw | babbageclunk menn0: ok, ta | 03:58 |
| menn0 | axw: see the bottom of apiserver/uniter/uniter.go | 03:58 |
| babbageclunk | axw: what he said | 03:58 |
| menn0 | axw: it's kinda awful but it works | 03:58 |
| axw | cool, got it | 03:58 |
| axw | yeah | 03:58 |
| babbageclunk | yeah, it's gross but better than the alternative. | 03:59 |
| * babbageclunk really should make an emacs func that builds a github url for a given go file. | 04:01 | |
| babbageclunk | axw: Reviewed - for some reason it marked my comments as outdated, not sure why - maybe a rebase? | 04:23 |
| babbageclunk | axw: lgtm, anyway | 04:23 |
| babbageclunk | axw: Thanks for making those changes, sorry if it was a pain! | 04:24 |
| axw | babbageclunk: not at all | 04:31 |
| axw | babbageclunk: thank you. not sure why outdated, I didn't rebase | 04:31 |
| ashipika | menn0: hey.. i fixed https://github.com/juju/juju/pull/7642 , if you could please take another look at it.. | 07:16 |
| menn0 | ashipika: reviewed! | 08:37 |
| menn0 | just one suggestion | 08:38 |
| menn0 | but ship it | 08:38 |
| ashipika | menn0: tyvm | 08:38 |
| rogpeppe | jam: I just responded to https://github.com/juju/testing/pull/129 FWIW | 11:48 |
| mattyw | anyone around to discuss update status? | 15:06 |
| niedbalski | wpk, ping | 15:07 |
| mattyw | or more generally how uniters respond to lost connections with controllers | 15:07 |
| bdx | hello everyone | 22:14 |
| bdx | how are new series enabled in metadata for Juju to use? | 22:14 |
| niedbalski | wpk, hey | 22:35 |
| wpk | hello | 22:36 |
| niedbalski | wpk, we were testing the reload-spaces functionality and I wondered what's the limitation/constraint for not updating the already existing space names and just the ids/subnets? i.e. we changed the name of a space on maas, run reload-spaces but the name remains as the original | 22:37 |
| wpk | niedbalski: it's the next step - we identify the space by name so in case it is changed we need to trace all places in which it's used and change it too | 22:40 |
| wpk | niedbalski: it is on our roadmap | 22:40 |
| niedbalski | wpk, ok, so its a known constraint.. | 22:41 |
| niedbalski | wpk, is there a LP bug for tracking this implementation? | 22:41 |
| niedbalski | anastasiamac, ^^ do you know? | 22:49 |
| wpk | niedbalski: I'm looking and there is one for space/subnet remove but I can't find one for rename, you're free to create one | 22:54 |
| niedbalski | wpk, ok | 22:54 |
| niedbalski | wpk, thanks! | 22:54 |
| anastasiamac | niedbalski: not that i can immediately recall ;) so probably not :D | 23:07 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!