wrtp | fwereade: hiya | 06:41 |
---|---|---|
fwereade | wrtp, heyhey | 06:41 |
fwereade | wrtp, actually, can I get your opinion on a naming issue in uniter? | 06:42 |
wrtp | fwereade: sure | 06:42 |
fwereade | wrtp, ok, this is about persistent states | 06:42 |
fwereade | wrtp, ATM I have Op (Install, Upgrade, RunHook, Abide) and Status (Queued, Pending, Committing) | 06:42 |
fwereade | wrtp, niemeyer suggests s/Status/OpStep/ (+!) and s/Committing/Done/ (not sure) | 06:43 |
fwereade | wrtp, I was wondering if a smarter naming scheme might burst fully-formed from your mind, because I think there's something wrong even with the changes that I can't put my finger on | 06:44 |
wrtp | fwereade: OpStep certainly seems a little odd | 06:44 |
fwereade | wrtp, well, Step perhaps -- I think it does beat Status -- but I think that maybe there's something larger that's wrong | 06:45 |
wrtp | fwereade: was this from comments in a review? | 06:45 |
fwereade | wrtp, yeah -- and its current form is at least partly a mix of what I originally thought and niemeyer's suggestions as well | 06:45 |
fwereade | wrtp, honestly I can live with it, with niemeyer's suggestions, and be perfectly happy | 06:46 |
wrtp | fwereade: can you link to his comment where he suggests OpStep? | 06:46 |
fwereade | wrtp, I'm just checking for a fresh perspective on it, because I think I had already overthought it a while ago | 06:46 |
fwereade | wrtp, just a mo | 06:46 |
fwereade | wrtp, https://codereview.appspot.com/6489083/diff/3001/worker/uniter/state.go#newcode31 and a comment just below as well | 06:47 |
wrtp | fwereade: i think he's suggesting s/Status/Op/ and s/Op/OpStep/ | 06:48 |
wrtp | fwereade: which makes more sense | 06:49 |
fwereade | wrtp, sorry, that makes no sense to me :( | 06:49 |
wrtp | fwereade: although i'm still not entirely sure | 06:49 |
fwereade | ISTM he's clearly referencing Status when he suggests OpStep... | 06:50 |
fwereade | and an Op does indeed pass through a number of steps/statuses/whatevers | 06:51 |
wrtp | fwereade: how about Op and OpStatus ? | 06:51 |
fwereade | wrtp, I *think* step is a win over status | 06:51 |
wrtp | fwereade: i'm suggesting that you just do s/Status/OpStatus/ | 06:52 |
wrtp | fwereade: because, if gustavo's right in that comment, the Status is the status *of* the op | 06:52 |
fwereade | ffs, sorry | 06:53 |
wrtp | last seen: [07:51:58] <fwereade> wrtp, I *think* step is a win over status | 06:54 |
fwereade | wrtp, ...the Status is the status *of* the op | 06:54 |
wrtp | fwereade: that's the last thing i said | 06:54 |
fwereade | wrtp, IMO step has a valid and helpful semantic payload over status | 06:54 |
wrtp | fwereade: i'm not suggesting we use step | 06:54 |
wrtp | fwereade: oh misread sorry | 06:55 |
wrtp | fwereade: i'm not suggesting we use step *instead of* status | 06:55 |
fwereade | wrtp, I know; but I think niemeyer is, and I think step is strictly etter than status | 06:55 |
fwereade | wrtp, what I'm whining about is a sense that it's still not *good* | 06:56 |
fwereade | wrtp, and I feel that I should probably recast the naming of all related states such that everything magically makes perfect sense | 06:56 |
wrtp | fwereade: it doesn't seem quite right to me, but i haven't looked at the code much. how is "queued" a "step"? | 06:56 |
fwereade | wrtp, but I can't figure out a better way of looking at them | 06:56 |
fwereade | wrtp, hm, yeah, ISWYM -- but they're steps in that they have a defined order to them | 06:57 |
fwereade | wrtp, an evil little part of me is suggesting Before/During/After might be a fruitful area for exploration | 06:58 |
fwereade | wrtp, anyway, don't let this seriously distract you | 06:58 |
fwereade | wrtp, I'm ok going with niemeyer's suggestions, but I'm hoping that a nice name-fixing followup will someday come to fruition | 06:59 |
wrtp | fwereade: i prefer OpStatus to OpStep - a step to me indicates an action not a condition | 06:59 |
wrtp | fwereade: as in "a step towards a better reality" :-) | 06:59 |
fwereade | wrtp, I see OpStep as specializing the Op action | 06:59 |
fwereade | wrtp, yeah, understood, might become convinced once it's percolated | 07:00 |
wrtp | fwereade: i see what you mean, but i think i see OpStatus as *about* the Op action | 07:00 |
wrtp | fwereade: and makes it clearer, perhaps, that an Op is something you *do* and an OpStatus is something you *move towards* | 07:01 |
fwereade | wrtp, is it though? I suspect you have an interesting view on this but I'm having difficulty meshing it with my own | 07:06 |
fwereade | wrtp, feels to me like an OpStep is something that you are *doing* (but, yeah, Queued doesn't fit there) | 07:08 |
fwereade | wrtp, steps: prepare, run, cleanup? | 07:08 |
fwereade | wrtp, not quite right either though :( | 07:08 |
wrtp | fwereade: what do you find not-quite-right about OpStatus? | 07:09 |
fwereade | wrtp, I find the whole naming scheme ...somewhat off | 07:10 |
fwereade | wrtp, that's as well as I can characterise it atm | 07:10 |
wrtp | fwereade: you're certainly coming from a deeper understanding of the problem than me | 07:11 |
fwereade | wrtp, heh, I think I'm way off in the Slough of Overthinking | 07:21 |
wrtp | fwereade: i know what you mean | 07:21 |
wrtp | fwereade: i'm just trying to convince myself of whether gustavo's right and my charm suite branch is in fact crack | 07:22 |
wrtp | fwereade: i'm just trying to convince myself of whether gustavo's right and my charm suite branch is in fact crack | 07:23 |
fwereade | wrtp, I *think* you're on the side of the angels there, but I'm not 100% sure | 07:24 |
wrtp | fwereade: it definitely *felt* good. the main part of his argument comes down to performance, i think, so i'm just checking if it has any measurable impact | 07:25 |
fwereade | wrtp, oh *crap* I've got to go out, completely forgot | 07:25 |
fwereade | wrtp, might be a couple of hours :( | 07:25 |
fwereade | and am late already, balls | 07:25 |
wrtp | fwereade: ok, see you later. have as much fun as you can doing what you're doing :-) | 07:25 |
Aram | morning. | 08:53 |
wrtp | Aram: morning | 10:22 |
wrtp | fwereade: i'm just about to add upgrading to the uniter - just checking that you're not already working on it. | 10:25 |
wrtp | fwereade: can you help me to interpret gustavo's last remark here? https://codereview.appspot.com/6501106/diff/1001/worker/machiner/machiner.go#newcode28 | 11:31 |
wrtp | fwereade: i'm not sure i can understand it enough to do something right | 11:31 |
fwereade | wrtp, heyhey | 11:47 |
wrtp | fwereade: i *think* i've worked it out | 11:47 |
fwereade | wrtp, sorry about that, that was quite the unexpectedly complicated morning | 11:47 |
wrtp | fwereade: np | 11:47 |
wrtp | fwereade: i think that a container API like this might work: http://paste.ubuntu.com/1200404/ | 11:48 |
fwereade | wrtp, ISWYM, it's not entirely clear t what extent he's agreeing with me and to what extent he's saying something else | 11:48 |
wrtp | fwereade: wdyt? | 11:48 |
* fwereade is thinking | 11:49 | |
fwereade | wrtp, I think that, yes, *probably* we will want it to look like that | 11:49 |
wrtp | fwereade: i've got to do something like that, as i need to pass in the VarDir somehow | 11:50 |
fwereade | wrtp, the trouble is I kinda feel the surroundings are still somewhat in flux so I'm still not quite sure | 11:50 |
fwereade | wrtp, indeed | 11:50 |
fwereade | wrtp, I'd like it if we did the same with LogDir too | 11:50 |
wrtp | fwereade: i tried a more minimal change before, but perhaps this will be better looked on | 11:50 |
wrtp | fwereade: LogDir? | 11:50 |
wrtp | fwereade: you mean we should have container.Config.LogDir? | 11:51 |
fwereade | wrtp, /var/log/juju, needed for --log-file in jujud upstart scripts | 11:51 |
fwereade | wrtp, I *think* so, yes | 11:51 |
wrtp | fwereade: can't we just derive it from VarDir ? | 11:51 |
fwereade | wrtp, vardir/../../log/juju? | 11:52 |
wrtp | fwereade: oh yeah | 11:52 |
wrtp | :-) | 11:52 |
fwereade | ;p | 11:52 |
wrtp | fwereade: you're probably right. | 11:52 |
wrtp | fwereade: i'll add a TODO | 11:52 |
fwereade | wrtp, cheers | 11:53 |
wrtp | fwereade: is the log directory currently configurable anywhere in fact? | 11:53 |
fwereade | wrtp, I think it's just hardcoded in cloudinit | 11:54 |
wrtp | fwereade: i thought so. i don't mind that too much tbh, but some day | 11:54 |
wrtp | fwereade: any chance you could have another quick once-over on https://codereview.appspot.com/6501106 ? | 13:54 |
wrtp | fwereade: i made that container change; i'm hoping it's not yet more crack. | 13:54 |
fwereade | wrtp, ofc | 13:55 |
fwereade | wrtp, it's tricky -- I *know* there's something nice waiting to emerge from the marble somewhere around there but I don't think either of us have quite nailed it yet (although I think you are looking in the right place) | 13:55 |
wrtp | fwereade: it doesn't fix all the container problems (i have another branch for that), but i feel the API entry points are about right now. | 13:56 |
fwereade | wrtp, +1 on Deploy/Destroy | 14:05 |
wrtp | fwereade: thanks | 14:05 |
fwereade | wrtp, just rereading everything else to try to remember what was involved | 14:05 |
fwereade | niemeyer, heyhey | 14:05 |
wrtp | niemeyer: hiya | 14:05 |
niemeyer | GOod mornings | 14:05 |
niemeyer | Or afternoons I guess | 14:05 |
fwereade | niemeyer, sorry I missed you last night -- I'm still a little bit unsure about the uniter op/step naming, but have resolved to stop vacillating and propose it again more-or-less as you suggest; if I come up with something better that can be a new CL later | 14:08 |
niemeyer | fwereade: Cool, I'm not super attached to those names either.. they just felt closer to what was going on than the State.Status stuff | 14:09 |
fwereade | niemeyer, yeah, I have a deep-seated feeling that there's some simple recasting of everything going on there that will make clear and obvious sense | 14:10 |
fwereade | niemeyer, the actual *logic* is really pretty trivial | 14:10 |
wrtp | niemeyer: i'm hoping that this isn't still crack: https://codereview.appspot.com/6501106 | 14:10 |
wrtp | niemeyer: and it would be nice if you could resolve the juju-dir/var-dir question with fwereade. i don't feel strongly either way. | 14:11 |
fwereade | niemeyer, well, I don't like VarDir unless it actually refers to /var, but I can live with any name really | 14:11 |
wrtp | if i had to choose, "jujuDir" feels more descriptive. varDir doesn't really mean anything other than "directory with some variable contents" to me | 14:12 |
wrtp | jujuRoot might work too | 14:13 |
niemeyer | """ | 14:13 |
niemeyer | It is vague because it is truly vague. The only thing it means is "juju's | 14:13 |
niemeyer | directory under var", because under it we have a bunch of different things with | 14:13 |
niemeyer | more precise naming (bundles, unit containers, etc). No single precise FooDir | 14:13 |
niemeyer | name will encompass it all. | 14:13 |
niemeyer | """ | 14:13 |
fwereade | niemeyer, well, it's *one* of juju's directories under var | 14:13 |
fwereade | niemeyer, if it were the only separate juju-specific dir under var I wouldn't be bothered | 14:14 |
wrtp | niemeyer: i'm not sure it's entirely relevant that it's under var. it *may* be under var, but that's a platform-specific decision. | 14:14 |
wrtp | niemeyer: hence we can configure it | 14:14 |
niemeyer | wrtp, fwereade: Okay, alternatives? | 14:14 |
wrtp | niemeyer: jujuRoot ? | 14:14 |
niemeyer | wrtp: That's / | 14:15 |
fwereade | niemeyer, we also used jujuHome in a couple of places in python iirc, but then that's not necessarily /var/lib/juju either | 14:15 |
niemeyer | wrtp: Since that's the only directory that shares ~/.juju, /var/log/juju, /var/lib/juju, and whatever else we need | 14:15 |
fwereade | niemeyer, no favour for LibDir/LogDir? | 14:15 |
wrtp | jujuDataDir ? | 14:16 |
wrtp | or just dataDir, perhaps | 14:16 |
niemeyer | DataDir? | 14:16 |
fwereade | niemeyer, +1 | 14:16 |
niemeyer | wrtp: +1 :-) | 14:16 |
wrtp | :-) | 14:16 |
wrtp | ok, i'll go for that then | 14:16 |
niemeyer | Woohay consensus | 14:16 |
wrtp | niemeyer: shall i change the flag name too? | 14:17 |
wrtp | perhaps i should do that in another CL actually | 14:17 |
wrtp | rather than bulking this one up more | 14:17 |
niemeyer | wrtp: --data-dir sounds sane | 14:20 |
* Aram food | 14:23 | |
wrtp | fwereade: container fixed, i hope: https://codereview.appspot.com/6498117 | 15:03 |
fwereade | wrtp, sweet,k looking | 15:04 |
fwereade | wrtp, LGTM | 15:13 |
fwereade | wrtp, very nice | 15:14 |
wrtp | fwereade: wonderful thanks | 15:14 |
wrtp | fwereade, niemeyer: i've made the change to dataDir from varDir. https://codereview.appspot.com/6501106 | 15:24 |
niemeyer | wrtp: Checking | 15:28 |
fwereade | wrtp, LGTM | 15:31 |
wrtp | fwereade: thanks | 15:31 |
niemeyer | wrtp: done | 15:41 |
wrtp | niemeyer: what's the difference between container.Simple(VarDir, InitDir) and container.Simple{VarDir: varDir, InitDir: initDir} ? | 15:43 |
wrtp | niemeyer: given that we're eschewing globals, i can't see what the former can do that the latter can't | 15:43 |
niemeyer | wrtp: This question seems to assume that we have Simple{}, which we don't.. so I don't understand why you're asking me that | 15:44 |
wrtp | niemeyer: we *did* have Simple, but you told me it was wrong. | 15:44 |
wrtp | niemeyer: so i changed to use the current scheme (which BTW i think is significantly better) | 15:44 |
niemeyer | wrtp: Sorry, I'm pretty lost | 15:45 |
niemeyer | wrtp: The branch is doing something else entirely | 15:45 |
niemeyer | wrtp: I've reviewed it, and pointed it has issues | 15:45 |
niemeyer | wrtp: Now you're blaming me for saying something else was improper, that is not what is in the branch, nor what I'm suggesting | 15:45 |
niemeyer | wrtp: I'd like to help, but I don't know hwo | 15:45 |
niemeyer | how | 15:45 |
wrtp | niemeyer: *somehow* i've got to pass VarDir and InitDir through to container. i changed container so that that was done in a nice way, i thought. | 15:46 |
niemeyer | wrtp: Yes, that's exactly what my comment is about | 15:46 |
niemeyer | wrtp: The package interface was changed entirely | 15:46 |
niemeyer | wrtp: unnecessarily | 15:46 |
niemeyer | wrtp: and I'm explaining that.. that's all | 15:46 |
niemeyer | wrtp: The original interface was better, as it allows containers to be implemented without changing the consumer interface | 15:47 |
wrtp | niemeyer: i'm not sure i understand that | 15:47 |
niemeyer | wrtp: It doesn't matter much, really | 15:47 |
niemeyer | wrtp: your branch is about changing a global to a local.. we don't need to change the package interface for that | 15:47 |
niemeyer | wrtp: We only need to provide simple with the location | 15:47 |
wrtp | niemeyer: i don't see why a function constructor is better than a data constructor in this case. | 15:48 |
niemeyer | wrtp: The original interface was put in place precisely so that we can have multiple containers, with different needs, and the same interface | 15:48 |
niemeyer | wrtp: We're losing that | 15:48 |
niemeyer | wrtp: Let's not, please, and let's focus on what you're claiming to do with the bracnh | 15:48 |
wrtp | niemeyer: k | 15:48 |
* wrtp rewinds the dataDir changes | 15:49 | |
niemeyer | wrtp: The data constructor is fine, btw | 15:49 |
niemeyer | wrtp: If you want Simple{}, that's fine | 15:49 |
niemeyer | wrtp: I don't think I argued against that | 15:49 |
wrtp | niemeyer: that's what i had that you objected to! | 15:49 |
niemeyer | wrtp: Where did I do this? | 15:50 |
wrtp | "" | 15:50 |
wrtp | This is true, but the decision is made considering settings, which means | 15:50 |
wrtp | that the API won't look like this (won't receive it through | 15:50 |
wrtp | constructor), so I agree with William that this seems premature. It's | 15:50 |
wrtp | also a bit unrelated to the CL topic. | 15:50 |
wrtp | "" | 15:50 |
niemeyer | wrtp: That's *completely* unrelated to the container package interface | 15:51 |
niemeyer | wrtp: That's in *machiner* | 15:51 |
niemeyer | wrtp: and is still true.. we don't have to pass a container through its constructor | 15:51 |
wrtp | niemeyer: ok, so why was that code wrong? i must have got the wrong end of the stick | 15:51 |
wrtp | (i did find that sentence difficult to parse, it's true) | 15:51 |
niemeyer | wrtp: The machiner will never make good use of a container being passed through its constructor | 15:51 |
niemeyer | wrtp: Because it needs to decide internally how it's supposed to deploy | 15:52 |
wrtp | niemeyer: there's only one Simple container though. | 15:52 |
niemeyer | wrtp: !? | 15:52 |
niemeyer | wrtp: and the sky is blue..? :) | 15:52 |
wrtp | niemeyer: there's only one Simple container that the machiner needs to use | 15:53 |
wrtp | niemeyer: it can use that for all local deployments | 15:53 |
wrtp | niemeyer: hence it can live in the Machiner struct AFAICS with no loss of generality | 15:54 |
niemeyer | wrtp: Sorry, I don't understand your point.. yes, there's only ever going to be one Simple container in juju | 15:54 |
niemeyer | wrtp: What does that mean? | 15:54 |
wrtp | niemeyer: it means i can store it in the Machiner struct, no? | 15:54 |
wrtp | niemeyer: which is what i was doing. i seem to be missing some fundamental issue here... | 15:54 |
niemeyer | wrtp: You can store it wherever you want.. | 15:55 |
niemeyer | wrtp: NewMachiner should not take a container | 15:55 |
wrtp | niemeyer: it doesn't | 15:55 |
niemeyer | wrtp: That's William's point, and that's my point | 15:55 |
wrtp | niemeyer: it never did | 15:55 |
wrtp | niemeyer: the *internal* constructor took a container | 15:55 |
wrtp | niemeyer: so that we can see when deploys happen | 15:55 |
wrtp | niemeyer: (there's otherwise no way of finding that out, i think) | 15:56 |
niemeyer | https://codereview.appspot.com/6501106/diff/1001/worker/machiner/export_test.go?column_width=90 | 15:56 |
niemeyer | wrtp: That's broken | 15:56 |
wrtp | niemeyer: ok, so how can i test that the machiner is actually doing something? | 15:57 |
niemeyer | wrtp: Sorry, can we stop the derail? | 15:57 |
niemeyer | wrtp: This CL is changing VarDir to a local.. | 15:57 |
niemeyer | wrtp: Can we do just that and move on? | 15:57 |
niemeyer | wrtp: We've been debating about changes in interfaces to various things so far | 15:57 |
wrtp | niemeyer: the old code changed the instance of Simple in the container package, so it could mock it. | 15:59 |
wrtp | niemeyer: we can't do that any more. | 15:59 |
wrtp | niemeyer: so this was my simplest idea for changing that | 15:59 |
niemeyer | wrtp: I don't understand.. we had a simple container being used | 15:59 |
niemeyer | wrtp: simple := container.Simple{DataDir: dataDir} | 15:59 |
niemeyer | wrtp: Done? | 15:59 |
niemeyer | wrtp: I don't understand where all the debate is coming from | 15:59 |
wrtp | niemeyer: if we do that, how can the testing code know when the machiner is actually doing a deploy? | 16:00 |
niemeyer | wrtp: How did we do that before? | 16:00 |
wrtp | niemeyer: Simple was a global variable of type container.Container. we changed its value in the test to our own local implementation. | 16:01 |
niemeyer | wrtp: Argh, ok.. so we were already mocking before :( | 16:01 |
wrtp | niemeyer: yes. and given that container can't work if you're not root, i don't see how we can avoid it | 16:01 |
niemeyer | wrtp: What's the simple container doing? | 16:02 |
* niemeyer looks | 16:02 | |
wrtp | niemeyer: it calls upstart.Install | 16:02 |
niemeyer | wrtp: Yeah, so why is that being mocked? If we're passing the directories being changed in.. ? | 16:02 |
niemeyer | wrtp: Hmm.. I suppose container is broken, since it should be starting the upstart script too? | 16:03 |
wrtp | niemeyer: sorry, i don't understand the second part of your question | 16:03 |
niemeyer | wrtp: DataDir and InitDir are both variables | 16:03 |
wrtp | niemeyer: it does start the upstart script too, i think | 16:03 |
niemeyer | wrtp: That we're now giving the machiner | 16:03 |
niemeyer | wrtp: It should.. but it's not clear if it does.. | 16:03 |
wrtp | niemeyer: only DataDir actually | 16:03 |
* niemeyer looks at what Install does | 16:03 | |
niemeyer | a installs and *starts*, ok | 16:04 |
niemeyer | wrtp: Okay, sorry, the confusion is my fault then | 16:04 |
niemeyer | wrtp: We are already stuck with the mocking of container, and once we introduce a second one we'll need to change the way we're testing the machiner | 16:05 |
niemeyer | wrtp: +1 on your original design of passing a container in for tests | 16:05 |
wrtp | niemeyer: i think the container package can decide which container is appropriate to use, based on the Unit | 16:05 |
niemeyer | wrtp: It wasn't clear to me that were were replacing what the container.Simpler global meant | 16:06 |
wrtp | niemeyer: that's the rationale for the most recent change | 16:06 |
niemeyer | wrtp: It can't | 16:06 |
wrtp | niemeyer: no? ok. | 16:06 |
niemeyer | wrtp: Container kind is an environment-defined setting, not per unit | 16:06 |
wrtp | niemeyer: what info does the uniter have that container doesn't? | 16:07 |
wrtp | niemeyer: and why couldn't that environment setting live in the Config? | 16:07 |
niemeyer | wrtp: Sorry, I don't understand the question... container is a deployment package.. uniter has a lot of information that container doesn't | 16:07 |
wrtp | niemeyer: ok, so do i take it that my original code was ok? | 16:10 |
niemeyer | wrtp: The original way in which you were testing machiner was ok, if that's your question | 16:11 |
niemeyer | wrtp: We'll need to rethink it when we introduce LXC, but it doesn't matter for now | 16:11 |
niemeyer | wrtp: We already have that problem | 16:11 |
wrtp | niemeyer: i think the most recent approach doesn't have the problems with LXC. the kind of container to use for isolation could be a parameter in the config. | 16:12 |
wrtp | niemeyer: and the container package could decide whether it's appropriate to isolate a unit | 16:12 |
wrtp | niemeyer: so the current machiner code would hardly need to change at all for LXC | 16:13 |
wrtp | niemeyer: i'm still pushing slightly for it because it's going to be a right hassle to rewind and go through another half-hour's worth of conflict resolution. | 16:13 |
niemeyer | wrtp: If that's all it takes, we can trivially pass a set of containers instead of a single one and get to the same place.. | 16:15 |
niemeyer | wrtp: The changes in design to the container package are not an improvement, and I'd appreciate if we didn't do that | 16:16 |
niemeyer | wrtp: You're basically dropping the concept of a Container interface, and putting it all inside the package itself as hidden details, with a single jumbo config that acts for all containers | 16:17 |
niemeyer | wrtp: It also takes away the ability for a container implementation to cache information, such as what are the things it has seen alive or not | 16:17 |
niemeyer | wrtp: we'll have to land that information in globals instead, if we want to do it | 16:18 |
niemeyer | wrtp: These aren't improvements | 16:18 |
wrtp | niemeyer: ok, i finally think i see where you're coming from on this. | 16:19 |
wrtp | niemeyer: thanks for explaining | 16:20 |
niemeyer | wrtp: np, and sorry for the partial detail on the test of machiner.. I misunderstood there | 16:21 |
niemeyer | s/detail/derail/ | 16:21 |
wrtp | niemeyer: that's ok. | 16:21 |
wrtp | niemeyer: luckily i can work late tonight | 16:21 |
niemeyer | wrtp: Ouch | 16:23 |
wrtp | niemeyer: it's true that i'm a bit sad about the derail, but i'm also stoked to get past these branches and actually get stuff working. we're really very close. | 16:29 |
niemeyer | wrtp: +1! | 16:33 |
wrtp | niemeyer: hopefully this will do the trick: https://codereview.appspot.com/6501106 | 16:46 |
niemeyer | wrtp: Done, cheers | 16:51 |
wrtp | niemeyer: thanks | 16:52 |
wrtp | niemeyer: do you know about this test failure? http://paste.ubuntu.com/1200932/ | 17:25 |
niemeyer | wrtp: ... Panic: no reachable servers (PC=0x4116D4) | 17:26 |
niemeyer | wrtp: This is the root | 17:26 |
niemeyer | wrtp: mgo.Dial is not finding the mongodb server running | 17:27 |
wrtp | niemeyer: is that a race thing? i only get this error sporadically | 17:27 |
wrtp | niemeyer: perhaps some timeout should be longer? | 17:27 |
niemeyer | wrtp: 15 seconds is the default IIRC.. I've never seen a mongodb server take that long to start | 17:27 |
wrtp | niemeyer: (speaking of which, i got an mstate/presence failure this morning - the test that waits for 1 second) | 17:27 |
niemeyer | wrtp: It's generally sub-second | 17:28 |
wrtp | niemeyer: i guess mongo just failed to start then | 17:28 |
niemeyer | wrtp: Yeah, wonder why | 17:29 |
wrtp | niemeyer: is there a log somewhere? | 17:29 |
niemeyer | wrtp: the suite will tell | 17:29 |
niemeyer | wrtp: Apparently not | 17:30 |
wrtp | niemeyer: yeah, looks like s.output is dropped on panic. | 17:31 |
wrtp | niemeyer: that might be a useful thing not to do :-) | 17:31 |
niemeyer | wrtp: Indeed | 17:32 |
wrtp | interesting. first time i've seen this live failure: | 17:41 |
wrtp | 07.05.602 /home/rog/src/go-alt/src/launchpad.net/juju-core/environs/jujutest/livetests.go:162: | 17:41 |
wrtp | 07.05.606 c.Assert(err, IsNil) | 17:41 |
wrtp | 07.05.606 ... value *errors.errorString = &errors.errorString{s:"session error: ZooKeeper connecting"} ("session error: ZooKeeper connecting") | 17:41 |
wrtp | niemeyer: i thought zk didn't time out. | 17:41 |
niemeyer | wrtp: ... don't know what to say | 17:45 |
wrtp | niemeyer: 's'alright. i wondered if you might have seen similar. | 17:45 |
niemeyer | wrtp: I haven't | 17:45 |
wrtp | niemeyer: it's not the world's most informative error message :-) | 17:45 |
wrtp | niemeyer: currently trying to reproduce. we'll see. | 17:46 |
niemeyer | wrtp: zk definitely has timeouts, and we definitely fail if we get an error from zk | 17:49 |
wrtp | niemeyer: looks like it was a sporadic failure. | 17:50 |
niemeyer | wrtp, fwereade: You'll probably like that one: https://codereview.appspot.com/6503109 | 17:56 |
wrtp | niemeyer: yay! LGTM | 17:59 |
wrtp | niemeyer: hmm, i did a apt-get update; apt-get install lbox and it hasn't found a newer version. should i use the Go version? | 18:02 |
niemeyer | wrtp: There's certainly a new version in the PPA | 18:03 |
niemeyer | wrtp: 1.0-56.64.39.11~precise1 | 18:03 |
niemeyer | wrtp: This is the latest | 18:03 |
* wrtp can't remember how to find the currently installed version | 18:04 | |
wrtp | niemeyer: 1.0-47.61.38.10~oneiric1 | 18:05 |
wrtp | is what i've got | 18:05 |
wrtp | hmm, i wonder why i'm on an oneiric version | 18:06 |
wrtp | darn | 18:09 |
wrtp | % go get launchpad.net/lbox | 18:09 |
wrtp | package launchpad.net/lbox | 18:10 |
wrtp | imports exp/html: unrecognized import path "exp/html" | 18:10 |
wrtp | oh well, will wait until it works. can't be bothered to update to go tip for now. | 18:10 |
wrtp | niemeyer: could we have a chat about this? i'd like to move forward with it if possible. https://codereview.appspot.com/6495086/ | 18:14 |
niemeyer | wrtp: | 18:23 |
niemeyer | [niemeyer@gopher ..nchpad.net/lbox]% grep html * | 18:23 |
niemeyer | [niemeyer@gopher ..nchpad.net/lbox]% | 18:23 |
niemeyer | wrtp: Your machine seems pretty unfriendly today :) | 18:23 |
niemeyer | wrtp: Ah, could be lpad.. | 18:24 |
wrtp | niemeyer: the actual error is | 18:24 |
wrtp | ../goetveld/rietveld/form.go:7:2: import "exp/html": cannot find package | 18:24 |
niemeyer | wrtp: form.go: "launchpad.net/goetveld/rietveld/html" | 18:25 |
niemeyer | wrtp: The package is in the PPA, and the goetveld package doesn't depend on exp/html for a while | 18:25 |
wrtp | niemeyer: ah! i should remove goetveld | 18:25 |
niemeyer | wrtp: Seems like things are out of date in both fronts there | 18:25 |
wrtp | niemeyer: pity go get -u doesn't work | 18:25 |
niemeyer | wrtp: Check if you have the PPA configured | 18:25 |
niemeyer | wrtp: apt-get update won't help otherwise | 18:26 |
wrtp | niemeyer: which PPA is it? | 18:26 |
niemeyer | wrtp: ppa:gophers/go | 18:26 |
wrtp | niemeyer: ah, that'll be the reason then. i guess it was probably lost when my system upgraded. | 18:28 |
wrtp | niemeyer: thanks | 18:29 |
niemeyer | wrtp: np | 18:29 |
wrtp | niemeyer: from the CL: | 19:18 |
wrtp | "" | 19:18 |
wrtp | Two options with the current interface - either I add a series | 19:18 |
wrtp | argument to all the entry points, or I add "WithSeries" variants | 19:18 |
wrtp | of all the functions. Any preference here before I go off and do the | 19:18 |
wrtp | wrong thing? | 19:18 |
wrtp | "" | 19:18 |
wrtp | have you got a preference here? | 19:18 |
niemeyer | wrtp: Your changes to the actual method interfaces seemed nice | 19:23 |
niemeyer | wrtp: My complaints was only about the whole refactoring on how the helper is instantiated and hooked everywhere | 19:23 |
wrtp | niemeyer: i wish i'd realised that. it sounded like you didn't like any change to the existing interface. | 19:24 |
niemeyer | wrtp: Having series seems fine | 19:24 |
wrtp | niemeyer: i think i'll go with reverting everything. it's much less work. | 19:25 |
niemeyer | wrtp: I realize you're trying to fix a problem you found | 19:25 |
niemeyer | wrtp: I was only arguing that the problem you found seems much simpler than the amount of work we've both put on this already | 19:25 |
wrtp | niemeyer: perhaps that's true. but i've already done that work, and now i have to do quite a bit more to undo it and put in another fix. | 19:26 |
niemeyer | wrtp: Yep, you've already done that work *several* times.. | 19:26 |
niemeyer | wrtp: unfortunately the last incarnation doesn't look good | 19:26 |
wrtp | niemeyer: twice only. | 19:26 |
niemeyer | wrtp: But I cant' be blamed for not liking every incarnation :) | 19:26 |
wrtp | niemeyer: i *thought* it was significantly better than the suite idea | 19:27 |
niemeyer | wrtp: There was a simple change in the beginning, then a refactoring, then another refactoring | 19:27 |
niemeyer | wrtp: While all you needed was a series argument | 19:27 |
wrtp | niemeyer: that's true. sometimes it seems like something is worthwhile when actually it's not. | 19:29 |
wrtp | niemeyer: BTW there *was* only one refactoring - note that most files have only two versions. | 19:31 |
niemeyer | wrtp: There were pastes before that | 19:32 |
niemeyer | wrtp: But it's ok, it's really not worth arguing further | 19:32 |
wrtp | niemeyer: agreed. i'm on it now. | 19:33 |
wrtp | dammit, it's not as easy as i thought. unmerging is a right pain. | 19:39 |
wrtp | niemeyer: this is trivial, as discussed earlier: https://codereview.appspot.com/6489117/ | 21:09 |
niemeyer | wrtp: I'm half-way through already.. just be ready in a moment | 21:09 |
wrtp | niemeyer: ta | 21:09 |
niemeyer | wrtp: done | 21:25 |
wrtp | niemeyer: thanks a lot. | 21:28 |
niemeyer | wrtp: My pleasure, thanks for the fixes | 21:29 |
wrtp | niemeyer: did you look at the branch i mentioned above (the series argument added to the Repo methods)? it should be trivial. | 21:30 |
wrtp | niemeyer: (just realised it looks like you might have assumed i was referring to another branch) | 21:31 |
wrtp | niemeyer: i can submit the authorize-internal-traffic branch which depends on it if it LGTY | 21:33 |
wrtp | niemeyer: i've discovered a little more about the bizarre bzr behaviour i encountered earlier. it's in the branch in launchpad. if i push to any other branch, it works. | 21:34 |
wrtp | % lbox propose -wip -req 058-testing-charm-series | 21:37 |
wrtp | error: Branch check failed: exit status 1 | 21:37 |
wrtp | ----- | 21:37 |
wrtp | gofmt is sad: | 21:37 |
wrtp | environs/jujutest/livetests.go | 21:37 |
wrtp | yay! | 21:37 |
niemeyer | !! | 21:37 |
niemeyer | wrtp: Hmm, interesting | 21:37 |
niemeyer | LOL | 21:38 |
niemeyer | wrtp: | 21:38 |
niemeyer | https://codereview.appspot.com/6498117/ | 21:38 |
niemeyer | https://codereview.appspot.com/6489117/ | 21:38 |
niemeyer | wrtp: Both are yours :-) | 21:38 |
wrtp | niemeyer: erm, yes. is there something i should notice there? | 21:39 |
niemeyer | wrtp: The numbers | 21:40 |
wrtp | niemeyer: cool! | 21:40 |
wrtp | niemeyer: now i see why you assumed i was referring to a different branch! | 21:40 |
niemeyer | wrtp: RIght :) | 21:41 |
wrtp | niemeyer: might the "dangling branch reference" be the source of my problem? http://paste.ubuntu.com/1201394/ | 21:42 |
wrtp | niemeyer: (not that i care any more; i'm abandoning that branch because my prereq has changed) | 21:42 |
niemeyer | wrtp: Uh.. probably | 21:43 |
niemeyer | wrtp: I don't recall seeing that before | 21:43 |
niemeyer | wrtp: review sent | 21:43 |
niemeyer | wrtp: Just suggested inverting the parameters so it matches how we use the two arguments elsewhere (sorry :() | 21:43 |
wrtp | niemeyer: damn, i wondered about that as i was doing it | 21:43 |
niemeyer | wrtp: Hopefully sed can do it for you | 21:45 |
wrtp | niemeyer: yeah, i did it before. shouldn't be much hassle. | 21:45 |
wrtp | niemeyer: structural regexps - even better than sed :-) | 21:45 |
niemeyer | wrtp: Oh? | 21:48 |
niemeyer | wrtp: How does that work? | 21:49 |
wrtp | niemeyer: http://bit.ly/RSC1iU | 21:51 |
niemeyer | CHeers | 21:55 |
wrtp | niemeyer: here was my expression for changing the args around BTW. it doesn't really leverage much though - sed would be just as easy here. | 22:03 |
wrtp | X/./,x/testing.Charms.*"series"\)/s/(,[^,]*), "series"\)/, "series"\1)/ | 22:03 |
wrtp | niemeyer: but the x pattern is a common one - narrow down scope until you've got something of known form. then hack at it | 22:03 |
wrtp | niemeyer: it occurs to me that gofmt would probably have been a better approach here! | 22:07 |
wrtp | niemeyer: gofmt -w -r 'testing.Charms.x(z, a, b) -> testing.Charms.x(z, b, a)' $gofiles | 22:14 |
wrtp | niemeyer: gofmt rules | 22:15 |
niemeyer | wrtp: Woah | 22:34 |
niemeyer | wrtp: Impressive indeed | 22:34 |
wrtp | niemeyer: the only thing it failed on was when it was using coretesting not testing | 22:35 |
wrtp | niemeyer: i think i need a UnitWatcher, same kind of thing as the MachineWatcher. sound reasonable to you? | 22:59 |
grantgm | hey folks! i'm giving a talk about PaaS and wondering if anyone has a reference for when Juju (or Ensemble, at that time) was first released? I don't see it on either the "about" page or the wikipedia page... | 23:07 |
SpamapS | grantgm: good question. :) | 23:08 |
grantgm | :) | 23:09 |
SpamapS | grantgm: If you count shipping it in the distro as "first released" then that would be when Ubuntu 11.10 was released | 23:09 |
SpamapS | grantgm: 0.5+bzr398-0ubuntu1 | 23:09 |
grantgm | hmmm...ok, that sounds like about when I first heard of it. but presumably it was usable via PPA before that, right? | 23:10 |
SpamapS | grantgm: yeah, the first time I saw it was January of 2011, from the PPA | 23:11 |
grantgm | just want to make sure i'm not short-changing it w.r.t. the big release announcements (before they were really usable) from the other players | 23:11 |
grantgm | SpamapS, ok, cool, i guess i'll go with that | 23:12 |
grantgm | SpamapS, thanks! | 23:12 |
SpamapS | grantgm: its still, in many ways, just a tech preview :) | 23:12 |
SpamapS | grantgm: http://juju.ubuntu.com/docs .. says so :) | 23:12 |
grantgm | well, it's a pretty damn impressive tech preview, then! but really, aren't they all? :) | 23:13 |
grantgm | SpamapS, thanks again! | 23:13 |
niemeyer | wrtp: Yeah, certainly | 23:13 |
niemeyer | grantgm: It was first seen working even around march 2010 | 23:14 |
niemeyer | s/even/ever | 23:14 |
niemeyer | Erm, sorry | 23:14 |
niemeyer | 2011 | 23:14 |
niemeyer | What SpamapS said, actually | 23:15 |
SpamapS | niemeyer: right, it was in the PPA in January, but we weren't really talking about it until March. :) | 23:15 |
SpamapS | it lived in public obscurity | 23:16 |
grantgm | SpamapS, niemeyer, ok, so maybe i'll go with March then. That still puts it out ahead of Cloud Foundry (April) and OpenShift (May) :) | 23:17 |
SpamapS | grantgm: I'd be interested to see how you're drawing a comparison between juju and cloudfoundry. | 23:20 |
SpamapS | since.. juju deploys cloudfoundry | 23:20 |
grantgm | yea, i know it's definitely not a perfect comparison...the first thing i'm talking about is that the definition of paas is very fuzzy | 23:20 |
grantgm | basically anything in the massive void between traditional IaaS and SaaS gets assigned the PaaS moniker, which is really rather silly | 23:21 |
SpamapS | its a silly human trait | 23:21 |
SpamapS | we can't have groups of 1 | 23:21 |
wrtp | niemeyer: done: https://codereview.appspot.com/6498124 | 23:22 |
wrtp | niemeyer: if it's ok i'll hold off on the mstate unit watcher until the generic watcher logic is back in place there. | 23:23 |
wrtp | davecheney: morning! | 23:24 |
davecheney | wrtp: hello | 23:32 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!