| wallyworld_ | thumper: TestSimple also does it's own cleanup. i think it's just to check that the EnsureDead/Stop etc methods work | 00:00 |
|---|---|---|
| thumper | ok | 00:01 |
| bigjools | wallyworld_: is the openstack provider configuring a cloud-drive ? | 01:33 |
| bigjools | or config drive | 01:33 |
| wallyworld_ | no | 01:33 |
| wallyworld_ | the instance just uses the storage attached to the instance | 01:33 |
| bigjools | what format is that storage? | 01:34 |
| wallyworld_ | it depends on the instance | 01:34 |
| bigjools | I got pointed at this: http://bazaar.launchpad.net/~cloud-init-dev/cloud-init/trunk/view/head:/doc/sources/configdrive/README.rst | 01:34 |
| wallyworld_ | larger instances use emphmeral disks | 01:34 |
| wallyworld_ | perhaps that's a feature of cloud-init the provider doesn't yet support | 01:35 |
| wallyworld_ | not that i am aware of anyway | 01:36 |
| bigjools | how do you get your user data into openstack? | 01:36 |
| wallyworld_ | magic | 01:36 |
| bigjools | can you wave your wand on my azure stuff | 01:36 |
| wallyworld_ | i think it is passed via cloud-init somehow, but need to check that | 01:36 |
| bigjools | yes, no shit :) | 01:36 |
| * wallyworld_ waves his stick | 01:37 | |
| wallyworld_ | sorry, not sure of the exact mechanism | 01:37 |
| wallyworld_ | it's been ages since i looked at that code | 01:37 |
| bigjools | my question is how does cloud-init get it. normally you set a url for the kernel and the userdata is in provider storage at that url | 01:37 |
| bigjools | but we can't set kernel params in azure | 01:37 |
| bigjools | hence the cloud-drive thing - which I thought openstack used as well | 01:38 |
| wallyworld_ | bigjools: justed checked - openstack uses params passed when starting an instance to hold the user data | 01:39 |
| wallyworld_ | there's a userdata field in the struct | 01:39 |
| bigjools | ah so it's direct | 01:39 |
| wallyworld_ | so it's not via cloud-init | 01:39 |
| bigjools | cloud-init picks them up eventually | 01:39 |
| wallyworld_ | server, err = e.nova().RunServer(nova.RunServerOpts{ | 01:40 |
| wallyworld_ | Name: e.machineFullName(scfg.machineId), | 01:40 |
| wallyworld_ | FlavorId: spec.InstanceType.Id, | 01:40 |
| wallyworld_ | ImageId: spec.Image.Id, | 01:40 |
| wallyworld_ | UserData: userData, | 01:40 |
| wallyworld_ | SecurityGroupNames: groupNames, | 01:40 |
| wallyworld_ | }) | 01:40 |
| wallyworld_ | i'm not sure of the internals | 01:40 |
| bigjools | you have a userData() func | 01:40 |
| wallyworld_ | yes, but i'm not sure how the instance works internally | 01:41 |
| bigjools | which pretty much answers my question now | 01:41 |
| bigjools | it renders to []byte | 01:41 |
| bigjools | I need to render to a "clouddrive" | 01:42 |
| bigjools | configdrive, even | 01:42 |
| wallyworld_ | wave your wand and it will happen | 01:43 |
| thumper | oh FFS!!!! | 02:09 |
| thumper | why do we insist on making testing things so fucking hard? | 02:10 |
| * bigjools wonders why there's two cloudinit packages | 02:12 | |
| bigjools | thumper: if you want to take a break from fucking hard, can I have a pre-imp for something that is only moderately hard | 02:17 |
| thumper | bigjools sure | 02:25 |
| bigjools | thumper: gimme 2 mins and I'll call you | 02:26 |
| * bigjools prods thumper to answer | 02:29 | |
| === tasdomas_afk is now known as tasdomas | ||
| davecheney | 05:02 | |
| davecheney | Google Drive | 05:02 |
| davecheney | The app is currently unreachable | 05:02 |
| davecheney | shitter | 05:02 |
| wallyworld_ | thumper: for your todo list (tomorrow maybe) https://codereview.appspot.com/10534043/ and https://codereview.appspot.com/10447045/ | 05:02 |
| wallyworld_ | davecheney: they must have used some microsoft code in there somewhere | 05:03 |
| davecheney | wallyworld_: http://instantrimshot.com/classic/?sound=rimshot | 05:04 |
| * wallyworld_ laughs | 05:05 | |
| jam | thumper: poke | 05:30 |
| jam | I'm trying to sort out what an Envorin is, and why he knows a guy named 'eric' very intimately. (testing/environ_test.go) | 05:31 |
| jam | I guess the goal is to just have a known home to compare against. Though we should probably s/Envorin/Environ/ | 05:32 |
| jam | rogpeppe: I just found that "go1.1" is a build constraint. If you wanted to write your Sliceof code in 2 modules and have a '// +build go1.1' and a '// +build !go1.1' | 06:23 |
| jam | I don't think it is worth divergence just yet | 06:23 |
| jam | but we do have a mechanism to take advantage of features when available | 06:24 |
| === tasdomas is now known as tasdomas_afk | ||
| === tasdomas_afk is now known as tasdomas | ||
| rogpeppe | jam: that doesn't really solve my problem - my workaround for the SliceOf code is to have every module register the slice types - the go1.1 constraint would poison lots of packages. | 07:24 |
| jam | sure | 07:25 |
| rogpeppe | jam: mornin' BTW! | 07:25 |
| jam | I just hadn't seen it before | 07:25 |
| jam | morning | 07:25 |
| rogpeppe | i've had this out for review all week and i've still only had one review of it. if i'm to get the API stuff in, i'd really like another review, please! https://codereview.appspot.com/10259049/ | 07:28 |
| rogpeppe | to whomsoever it may concern :-) | 07:28 |
| jam | rogpeppe: to be honest, I've gone to look at it at least 3 times, it is just big enough and involves enough stuff I have trouble consuming it. I will certainly try again. | 07:29 |
| rogpeppe | jam: sorry about that - there are probably a couple of trivial things that could be split off (the params.Life definitions for example) but i couldn't think of a way of splitting up the main change to agent.go and machine.go | 07:31 |
| jam | rogpeppe: probably a lot of it is my unfamiliarity with parts of the internals, so I can't always grasp *why* a change is being made until I dig into it for a bit. | 07:32 |
| rogpeppe | jam: well, it would be nice for the future if you acquire some familiarity with this part of the code, so please feel free to ask anything about what's going on - we could have chat about it if you want. | 07:33 |
| jam | rogpeppe: so why did you need to add the worker.Worker.Kill implementation for the api server? It wasn't a worker before but should be? It was just useful for testing? | 07:33 |
| rogpeppe | jam: it wasn't a worker before but now it is - rather than having explicit run loops in three places, the CL changes things to use worker.Runner in more places | 07:34 |
| rogpeppe | jam: so in the machiner we've got the top level Runner that runs two workers; one of them runs any tasks that connect to the state; the other runs any tasks that connect to the API. each of them creates a runner themselves to run the tasks (so the runners are nested) | 07:35 |
| jam | rogpeppe: and the presumably we iterate until one of those isn't necessary. | 07:36 |
| jam | However, I would have thought that we would have tasks that couldn't quite be all API while we transition | 07:36 |
| jam | so we could implement *an* API call and start using it | 07:36 |
| jam | rather than having to finish off "everything a task might possibly need" before it can start using API. | 07:36 |
| jam | Too much chance of split brain? | 07:37 |
| rogpeppe | jam: i thought that too originally, but i think the idea is to transition an entire agent at a time | 07:37 |
| rogpeppe | s/agent/worker | 07:37 |
| jam | rogpeppe: Workers returns a worker that runs the unit agent workers. ??? | 07:37 |
| jam | why is the method plural, but the result singular | 07:37 |
| jam | And the doc string doesn't seem to explain much. | 07:38 |
| rogpeppe | jam: which method? | 07:38 |
| jam | rogpeppe: https://codereview.appspot.com/10259049/patch/5001/6006 | 07:38 |
| rogpeppe | jam: ah, in unit.go | 07:38 |
| jam | If it 'runs' it, I would have thought it would be a Runner | 07:38 |
| jam | but a Runner is also a Worker? | 07:39 |
| rogpeppe | jam: the unit agent has two levels of runner too | 07:39 |
| rogpeppe | jam: yes | 07:39 |
| rogpeppe | jam: that's an important point | 07:39 |
| jam | For the docstring, I think it is trivial to see that Workers returns a worker, so focusing more about what the thing it returns will be used for might be helpful | 07:40 |
| rogpeppe | jam: the Workers method is probably badly named | 07:40 |
| jam | I also wouldn't make the method plural when it returns a single object, even if that object wraps multiple others. | 07:40 |
| rogpeppe | jam: yeah, it's kind of a single object that encapsulates all the workers, but i agree | 07:41 |
| jam | rogpeppe: MetaWorker ? | 07:41 |
| rogpeppe | jam: TopLevelWorker? | 07:42 |
| rogpeppe | jam: StateConnectingWorkerRunningOtherWorkers ? | 07:42 |
| jam | // MetaWorker returns the Worker responsible for managing all individial workers that this Agent needs. | 07:42 |
| jam | Just calling it Worker would eliminate the confusion on the returned type not being a slice | 07:42 |
| jam | rogpeppe: so the thing you return isn't actually a runner, it is a newCloseWorker() which wraps a runner. | 07:43 |
| rogpeppe | jam: StateWorker might be good enough | 07:43 |
| rogpeppe | jam: yeah | 07:43 |
| jam | rogpeppe: so do Runners actually implement the Worker api? | 07:44 |
| rogpeppe | jam: that's so we close the state connection eventually | 07:44 |
| rogpeppe | jam: yes | 07:44 |
| rogpeppe | jam: (all it requires is Kill and Wait) | 07:44 |
| jam | rogpeppe: why do you start the runner in cmd/jujud/machine/Init() rather than in Run | 07:47 |
| rogpeppe | jam: that's so the tests can kill it | 07:47 |
| jam | rogpeppe: I can see that as a reason to have it as an attribute, but Init seems more like setup stuff-without-starting anything, no? | 07:48 |
| rogpeppe | jam: because we're not using our own tomb now - we're leaching off the Runner's liveness | 07:48 |
| rogpeppe | jam: yeah, i take your point. | 07:49 |
| rogpeppe | jam: even though the runner is empty, calling Init without Run will leak | 07:49 |
| jam | rogpeppe: so you already have a method called StateWorker in MachineAgent code. Though perhaps the functionality is similiar enough that name sharing is a good thing? | 07:51 |
| rogpeppe | jam: yes, that's what i was thinking | 07:52 |
| rogpeppe | jam: and when we transition the unit agent, we'll probably have an APIWorker *and* a StateWorker in there until we can finally delete the StateWorker | 07:53 |
| jam | rogpeppe: I would *really* like to call this thing the StateRunner instead, though. | 07:53 |
| jam | That makes it clear the thing isn't actually doing anything on its own. | 07:53 |
| rogpeppe | jam: yeah, seems like a good idea | 07:53 |
| jam | But it is managing all the tasks that want access to State. | 07:54 |
| jam | rogpeppe: It also seems odd that APIWorker (maybe called APIRunner) has a side effect of starting the StateRunner. | 07:55 |
| jam | Rather than a sepacate | 07:55 |
| jam | MachineAgent.StartAPIRunner() and MachineAgent.MaybeStartStateRunner ? | 07:55 |
| jam | rogpeppe: naming aside, calling agent.APIFoo() and getting a runner, but having a side effect of mutating agent.runner seems a bad idea. | 07:56 |
| rogpeppe | jam: unfortunately it needs to happen like that, because in general we can't start the state worker until we've connected to the API | 07:56 |
| jam | certainly when reading the code it wasn't immediately obvious that "a.runner" isn't the same thing as the "runner" you are building up. | 07:57 |
| jam | rogpeppe: you can check and refuse if the API side isn't running yet | 07:57 |
| jam | and callers need to call them sequentially. | 07:57 |
| jam | or we have a StartRunners call that does both | 07:57 |
| dimitern | morning all! | 07:57 |
| jam | welcome back dimitern | 07:57 |
| rogpeppe | dimitern: yo! | 07:57 |
| dimitern | thanks! :) | 07:58 |
| rogpeppe | jam: we can't even know if we *need* to run a state server until we've connected to the API | 07:58 |
| rogpeppe | s/state server/state worker/ | 07:58 |
| rogpeppe | jam: so there has to be some communication between the two | 07:58 |
| rogpeppe | jam: i know it might seem awkward, but i haven't been able to think of a more elegant way of solving the problem | 07:59 |
| jam | rogpeppe: again, you can always start the state worker after the API. I'm objecting to having a side effect of "create this runner for me", also starting and forcibly instantiating another runner that isn't actually the one I asked for. | 07:59 |
| jam | rogpeppe: I'm just asking to move the code into another function that more clearly signals that it is potentially starting 2 things. | 08:00 |
| jam | rogpeppe: If I'm understanding the code, the block from "m := entity.(*machineagent.Machine)" until the end of if needsStateWorker could be done after you return the newCloseWorker | 08:01 |
| jam | which means it could be in a different function. | 08:01 |
| jam | you need the 'entity', but you could expose that. | 08:01 |
| rogpeppe | jam: i'm not sure it can be | 08:02 |
| rogpeppe | jam: in the future, the information on how to connect to the state will be held in the API | 08:02 |
| rogpeppe | jam: so we'll have to pass some information that we've obtained from the API into StateWorker | 08:03 |
| jam | rogpeppe: so after we have the API runner up, we ask the API if we also need to start the StateWorker. | 08:03 |
| rogpeppe | jam: yes | 08:03 |
| rogpeppe | jam: that's what we do | 08:03 |
| jam | rogpeppe: The point is you have a function labeled "Give me an APIWorker", that should be straightforward. But it has this side effect of maybe starting a StateWorker | 08:03 |
| jam | which is nowhere in the contract we've defined | 08:03 |
| jam | it doesn't even activate the APIWorker (doesn't add it to a.runner) | 08:04 |
| jam | so it seems doubly strange that it would add a different worker to a.runner. | 08:04 |
| rogpeppe | jam: unfortunately one of the things we have to do after connecting to the API is maybe start a new state worker | 08:04 |
| jam | rogpeppe: so I'm just asking you to have a function called "StartWorkers" which connects to the API, creates an APIWorker, and then checks to see if it needs to start a state worker (and does so) | 08:04 |
| rogpeppe | jam: how can it do that? - it won't have an API connection | 08:05 |
| jam | rogpeppe: you just created the API connection | 08:05 |
| jam | how does it not have one? | 08:05 |
| rogpeppe | jam: creating an API worker doesn't create the API connection until some time later | 08:06 |
| jam | rogpeppe: then how is your code doing it today? | 08:06 |
| rogpeppe | jam: the only time we know we have an API connection is inside APIWorker itself | 08:06 |
| jam | if it doesn't have the connection? | 08:06 |
| rogpeppe | jam: so you'd be happy if StartWorkers was called from within APIWorker? | 08:07 |
| jam | rogpeppe: wrong way around | 08:07 |
| rogpeppe | jam: but if StartWorkers adds APIWorker to the top level runner, how can it get access to the API connection that only APIWorker has access to? | 08:08 |
| rogpeppe | jam: perhaps i might understand better what you're suggesting if you paste some pseudocode | 08:10 |
| jam | rogpeppe: rough sketch: http://paste.ubuntu.com/5800860/ | 08:10 |
| jam | rogpeppe: yeah, I was working on that | 08:11 |
| rogpeppe | jam: what calls StartWorkers? | 08:11 |
| jam | rogpeppe: right now it is an immediate replacement for whatever called APIWorker | 08:12 |
| rogpeppe | jam: currently APIWorker is called by the top level Runner | 08:14 |
| rogpeppe | jam: which is important because it gives us our top level run loop (retrying on failure) | 08:14 |
| jam | rogpeppe: so MachineAgent is also a Worker? | 08:15 |
| rogpeppe | jam: no | 08:16 |
| rogpeppe | jam: machine.go:87,90 | 08:16 |
| jam | rogpeppe: is there a reason we couldn't just have Workers() here, which returns a slice of Workers, which might be 1 for just the API or 2 for both the API and State worker if we find we need it. | 08:17 |
| rogpeppe | jam: that's how APIWorker gets called | 08:17 |
| jam | Then we don't have to touch a.runner | 08:17 |
| rogpeppe | jam: shall we G+ this - it seems like a bit of higher bandwidth might be useful | 08:18 |
| rogpeppe | jam: i'm not sure what you mean with your slice suggestion. are you suggesting that MachineAgent.Run calls Workers directly? | 08:20 |
| jam | rogpeppe: just a sec, digesting a bit. | 08:21 |
| rogpeppe | jam: the difficulty is that eventually we *do* need two independent workers in the top level a.runner, but we only know whether to run the state worker *after* the first API connection has been made, which happens to be exactly what the other runner has to do as a first step. i suppose we could have another worker function which connects to the API and whose sole purpose is to add the other runners to the top-level runner | 08:24 |
| rogpeppe | jam: but that's more code and not necessarily any clearer | 08:24 |
| rogpeppe | jam: and there's also the point that at some point we might have to *remove* the state worker (if the machine jobs change), which will probably require the API worker to mess with the top-level runner in a similar way | 08:26 |
| dimitern | jam: so now with the go-bot do I need to pull from a different location? bzr pull --overwrite --remember lp:juju-core ? | 08:28 |
| rogpeppe | jam: would something like this help matters? http://paste.ubuntu.com/5800883/ | 08:29 |
| rogpeppe | dimitern: if you have any existing proposals, they need to be reproposed | 08:30 |
| rogpeppe | dimitern: but the pull location is still the same | 08:30 |
| dimitern | rogpeppe: no, save for the one mgz_ took over | 08:30 |
| rogpeppe | dimitern: did you have a good holiday, BTW? | 08:30 |
| dimitern | rogpeppe: oh yeah, relaxed - just what I needed | 08:31 |
| rogpeppe | dimitern: are you officially back now? (i thought you were back tomorrow) | 08:31 |
| jam | dimitern: it depends how you got your branch. If you got it from 'bzr branch lp:juju-core' that still points at the right branch, if you use 'go get launchpad.net/juju-core' that points to the wrong one (because it pre-resolves the full branch URL). you can always just do it and not worry, because it won't be the wrong thing to do (bzr pull --remember lp:juju-core) | 08:31 |
| dimitern | rogpeppe: i'm back today | 08:31 |
| rogpeppe | dimitern: cool! | 08:31 |
| rogpeppe | dimitern: we get a day and a half of overlap before i'm away... | 08:32 |
| jam | rogpeppe: thinking a different way, what do we gain by not starting the StateWorker ? | 08:32 |
| dimitern | rogpeppe: yeah | 08:32 |
| dimitern | jam: will bzr info tell me that? | 08:32 |
| rogpeppe | jam: if we do start the StateWorker, how does it know how to connect to the state? | 08:32 |
| jam | rogpeppe: the fundamental bit for me is that it feels ugly to have APIWorker (create an api worker for me to start tasks on) have a side effect of touching the top level runner. We can manage the distrust by having 'Here be dragons' comments, but I'm trying to sort out if we can decouple it. | 08:33 |
| jam | dimitern: it can | 08:33 |
| rogpeppe | jam: i definitely see your point, but i *think* this is a fundamental causality issue | 08:33 |
| rogpeppe | jam: only when we connect to the API can we work out if (and how) to start the state worker | 08:34 |
| rogpeppe | jam: and the APIWorker is the only place that connects to the API | 08:34 |
| jam | rogpeppe: well today we can just use a.Conf.Conf as you are already doing, no? | 08:34 |
| rogpeppe | jam: currently we can, but in the future we won't be able to | 08:34 |
| jam | And when we are at the point where API knows better how to connect to State, won't we be at the point where we refuse to let anything but the API server itself connect to state? | 08:34 |
| jam | and the API server won't be connecting to itself. | 08:35 |
| dimitern | jam: i have this: http://paste.ubuntu.com/5800889/ | 08:35 |
| jam | dimitern: parent branch: bzr+ssh://bazaar.launchpad.net/+branch/juju-core/ | 08:35 |
| jam | that is always "lp:juju-core" even when we point that at a different branch | 08:35 |
| rogpeppe | jam: in the future, we will only know whether to run an API server by connecting to an API server | 08:35 |
| dimitern | jam: so it's fine then? | 08:35 |
| jam | dimitern: 'go get' would end with: parent branch: http://launchpad.net/~juju/juju-core/trunk/ (which would be the old wrong branch) | 08:36 |
| jam | dimitern: yes, you are fine | 08:36 |
| rogpeppe | jam: i do indeed hope that we get to the stage where the API server is the only thing that needs a state connection | 08:36 |
| dimitern | jam: cheers | 08:36 |
| jam | dimitern: note though that juju-core now has a bunch more external dependencies | 08:36 |
| jam | lp:gwacl and lp:golxc come to mind | 08:36 |
| jam | which also needs 'apt-get install libcurl4-openssl-dev' | 08:36 |
| jam | and a branch from github | 08:37 |
| rogpeppe | [09:35:02] <jam> and the API server won't be connecting to itself. | 08:37 |
| jam | though 'go get launchpad.net/juju-core' will sort some of that out for you | 08:37 |
| rogpeppe | jam: it will be connecting to other instances of itself | 08:37 |
| dimitern | jam: yeah, just discovered these and i'm go getting them | 08:37 |
| jam | rogpeppe: how does the first one start? | 08:37 |
| rogpeppe | jam: bootstrap | 08:37 |
| rogpeppe | jam: that's the "0" case at the top of the MachineAgent.Run | 08:37 |
| rogpeppe | jam: that's the only time we can't first connect to an API server. | 08:38 |
| jam | rogpeppe: in which case, I would offer that the task which actually runs an API server should be the thing starting the StateWorker and not APIWorker | 08:38 |
| rogpeppe | jam: the StateWorker *is* the thing that runs an API server | 08:39 |
| jam | anyway, I don't really want to argue it for too long, but having a "Create me one of these" have a side effect of creating a second one and mutating the state of the caller is hard to keep track off and should be well guarged. | 08:39 |
| rogpeppe | jam: i agree with your discomfort, but i haven't seen a decent alternative yet | 08:40 |
| rvba | Hi jam, I replied to your comment on lp https://code.launchpad.net/~rvb/juju-core/az-public-storage/+merge/171251/comments/382236 | 08:40 |
| jam | rogpeppe: the ordering *could* be, "connect to the API, poll it for the jobs to run, one of those jobs will start up, and ask the agent to start another worker, which then gets a task to run the API server" | 08:40 |
| jam | rvba: I didn't see anything from jtv on the MP, sorry if I missed it. | 08:42 |
| rogpeppe | jam: so rather than adding to the MachineAgent runner directly, we start another worker that does the same thing? | 08:42 |
| jam | rvba: to be fair, I still don't see anything looking back again. | 08:42 |
| rvba | jam: https://codereview.appspot.com/10541044/ | 08:42 |
| jam | rvba: ugh, split brain between LP and Reitveld. Reitveld mirrors most comments back into Launchpad, except when the Reitveld identity isn't known to LP, then those messages just get dropped by LP | 08:43 |
| jam | rvba: if you look here: https://code.launchpad.net/~rvb/juju-core/az-public-storage/+merge/171251 | 08:43 |
| jam | it *also* means that JTV's message isn't in my email folder | 08:43 |
| jam | because Rietveld only sends messages for things which you've already commented on. | 08:43 |
| jam | sorry about that. | 08:43 |
| rogpeppe | jam: in the end, starting the state worker *has* to be a consequence of connecting to the API. hmm, one possibility to make things easier to understand: | 08:44 |
| jam | is it possible for jtv to register his alternative email with LP? | 08:44 |
| rogpeppe | jam: we could explicitly pass the top level runner into the APIWorker | 08:44 |
| rogpeppe | jam: so that it's more obvious that APIWorker can control it | 08:44 |
| rogpeppe | jam: would that be better for you? | 08:45 |
| jtv | Oh hi guys... I'll try to register that email then. | 08:47 |
| rogpeppe | jam: something like this: http://paste.ubuntu.com/5800919/ | 08:50 |
| jam | rogpeppe: I would be happy if the api was "add yourself to my runner", and then we just end up adding 2 things to the runner. rather than "create something and I'll take care of registering it" which then has a side effect of creating and registering an "unrelated" thing. | 08:50 |
| dimitern | mgz_: ping | 08:51 |
| rogpeppe | jam: the second thing has to be added to the runner as a side-effect of running the first thing. unless we have some entirely different code which connects to the API for the first time, starts *another* worker that connects to the API and a state worker if appropriate. | 08:51 |
| dimitern | fwereade: ping | 08:52 |
| jam | rvba: I just sent an email to juju-dev about the config naming question. I actually prefer your method, but we were explicitly asked to do the common-names thing. Which is why I proxied it to you. | 08:52 |
| fwereade | dimitern, pong | 08:53 |
| jam | rogpeppe: the issue is that APIWorker isn't *running* the worker yet, right? | 08:53 |
| fwereade | dimitern, welcome back :) | 08:53 |
| dimitern | fwereade: thanks :) | 08:53 |
| rogpeppe | jam: which worker? | 08:53 |
| dimitern | fwereade: i was thinking to pick up the deployer API stuff | 08:53 |
| jam | it seems odd to have the StateWorker registered with the top level runner before the API Worker has been registered | 08:53 |
| jam | calling APIWorker() creates a worker object | 08:53 |
| jam | but hasn't added it to the topLevelRunner yet | 08:53 |
| rvba | jam: okay, thanks for starting the discussion. | 08:53 |
| dimitern | fwereade: as agreed before, if mgz_ haven't started on it | 08:53 |
| fwereade | dimitern, +1but speak to danilos -- he's about to go away but hasn't yet and has, AIUI, been looking into it | 08:54 |
| fwereade | dimitern, I'm not up to date on where he is with it though | 08:54 |
| jam | dimitern: right, danilos has started to do some of the infrastructure. | 08:54 |
| rogpeppe | jam: erm, i don't quite understand. you don't actually add workers to runners, you add a function that starts a worker. | 08:54 |
| jam | and has 3 days of overlap with you to hand it off. | 08:54 |
| dimitern | fwereade, jam: ok | 08:54 |
| dimitern | danilos__: hey | 08:54 |
| rogpeppe | jam: the function that calls APIWorker is added to the top level runner immediately | 08:55 |
| rogpeppe | jam: machine.go:87 | 08:56 |
| dimitern | jam: i saw the mail about the objectives - what's the deadline for that? | 08:57 |
| jam | rogpeppe: so we have some more naming confusion. | 08:57 |
| jam | StartWorker doesn't start anything | 08:58 |
| jam | it registers something that you'll want to start later, right? | 08:58 |
| rogpeppe | jam: no, it will call the function immediately | 08:58 |
| jam | dimitern: officially the end of this week. With official recognition that it is likely we'll miss it by a bit. | 08:58 |
| jam | dimitern: though it doesn't have to take super long. You can do it in an hour or so. You have a lot of other people's objectives you can crib from. | 08:59 |
| dimitern | jam: which ones should I take as a template - your or tim's ? | 08:59 |
| rogpeppe | jam: the slight tension is that the name StartWorker implies only a single worker, but actually there's a sequential succession of workers | 08:59 |
| rogpeppe | jam: each one started some time after the last one has quit, assuming it didn't quit with a fatal error | 09:00 |
| jam | dimitern: actually, you're officially under Tim now | 09:00 |
| jam | according to directory.canonical.com | 09:00 |
| jam | I wash my hands of you :) | 09:00 |
| dimitern | jam: ok :) \o/ | 09:00 |
| jam | rvba: sorry if my comments came across as a bit attacking. It certainly wasn't meant that way. I only commented because of the naming thing, and then I was surprised about the 1 review bit. | 09:01 |
| rvba | jam: no worries ;) | 09:02 |
| jam | rogpeppe: so it feels like the 'right' fix is to change the StartWorkers api so that the function you call can optionally return more workers that you might want to start. However, your last paste seems reasonable enough for now, and has sufficient Here Be Dragons to avoid people getting lost as to why it is happening. | 09:03 |
| rogpeppe | jam: thanks. i'm not sure how changing the StartWorker API would help really. it seems like it would make the Runner API more complex for no particular gain. if you want another worker, just adding it to the runner directly seems like a reasonable way to do it. | 09:08 |
| fwereade | TheMue, config-get --all LGTM | 09:08 |
| jam | rogpeppe: runner.StartWorkers(..., func()) it doesn't feel like that func() should know what runner it is being attached to, so that it can be arbitrarily called by some other runner. | 09:08 |
| jam | rogpeppe: it is a 'do we have a singleton per process', imagine writing tests cases for this | 09:09 |
| jam | that want to start their own runner, and add this thing to them. | 09:09 |
| jam | that should get added to whatever runner they created | 09:09 |
| jam | rogpeppe: anyway, the bit you wrote is 'ok', it doesn't feel 'right', but it is acceptable | 09:09 |
| jam | your pastebin does at least let you set which one you want it to add any future work to. | 09:09 |
| rogpeppe | jam: yeah, it's pragmatic code - it's not beautifully regular or modular, but it encapsulates the task in hand and the scope is limited | 09:10 |
| thumper | night all | 09:11 |
| jam | rogpeppe: yeah, I think 1 exception is ok when well documented, if we end up with 2 we should wonder, and if we have 5 exceptions, then we probably have the design wrong. | 09:12 |
| rogpeppe | jam: when we do multi-tenant, i think we will probably be doing a lot of runner-manipulating - there will probably be a worker who's sole responsibility is to add and remove other workers from the API runner. | 09:14 |
| rogpeppe | jam: at least, that's my wand-wavy plan | 09:14 |
| TheMue | fwereade: thx | 09:19 |
| TheMue | fwereade: and also thx for the better doc ;) | 09:20 |
| rogpeppe | jam: this is, i think, something more like you were suggesting. i don't think it's a great improvement: http://paste.ubuntu.com/5800976/ | 09:23 |
| jam | rogpeppe: I'm pretty sure line 32 is a.APIWorker | 09:24 |
| rogpeppe | jam: indeed it is | 09:25 |
| jam | rogpeppe: so the idea is you don't need the for{} in your original code because topLevelRunner handles that? | 09:26 |
| rogpeppe | jam: yes | 09:26 |
| jam | rogpeppe: I'll also note in your current proposal of "how do we know what to connect to", there is no actual connection of a.StateWorker() to anything the API returns | 09:30 |
| rogpeppe | jam: not currently | 09:30 |
| jam | though I realize you want to put something there. | 09:30 |
| rogpeppe | jam: eventually the state server addresses will be accessible through the API | 09:31 |
| jam | rogpeppe: I think changing the signature of StartWorker such that the function you pass in can take the runner as an argument, allowing it to add units would be a good idea. Especially under your proposal that there will be a worker which does a lot of start/stopping of stuff in its parent runner (rather than as children of itself). | 09:32 |
| jam | However, your old pastebin is my current favorite for the time being. | 09:32 |
| jam | ( | 09:32 |
| jam | http://paste.ubuntu.com/5800919/ | 09:33 |
| rogpeppe | jam: we *could* do that, but i think i'm happy having a runner as a closure variable too - there's a certain purity in having a function with zero args | 09:34 |
| jam | rogpeppe: as long as we never have to do something like migrate the workers to another runner, etc | 09:34 |
| jam | it *seems* like workers shouldn't know what runner they are running inside | 09:34 |
| jam | which the closure breaks that | 09:34 |
| rogpeppe | jam: if we do, the code is small and easy to change | 09:34 |
| jam | mgz_: did your patch finally land? | 09:36 |
| rogpeppe | jam: how about this? http://paste.ubuntu.com/5800999/ | 09:37 |
| jam | rogpeppe: it is pretty much equivalent to http://paste.ubuntu.com/5800919/ for me. | 09:38 |
| jam | rogpeppe: you still have to wrap that in a closure | 09:38 |
| rogpeppe | jam: ok | 09:38 |
| jam | that closure still has to save a.runner as a const in its closure | 09:38 |
| jam | etc. | 09:38 |
| rogpeppe | jam: i was trying to make it so that the worker doesn't know what runner it's running in | 09:39 |
| jam | It is probably slightly better at having APIWorker not know the internals of what a StateWorker is | 09:39 |
| jam | rogpeppe: sure, but passing that in is the same thing. | 09:39 |
| rogpeppe | jam: that knowledge is held outside (in MachineAgent.Run) | 09:39 |
| jam | rogpeppe: so if a runner passes itself into the thing it is calling, then the thing it is calling doesn't "know" where it is running, it is "told" where it is running. | 09:40 |
| jam | which means if another runner got that task, it would just run on the other runner. | 09:40 |
| jam | in this case, the closure has to *know*, which is true of either form of your code. | 09:40 |
| rogpeppe | jam: that assumes that we always want to add the state worker to the same runner that's running the API worker. | 09:41 |
| jam | rogpeppe: if given both, I probably like http://paste.ubuntu.com/5800999/ more, it just doesn't solve the specific issue I was having troubles with. | 09:41 |
| jam | Having nested runners that can start other runners that *aren't* nested underneath them is also a little bit confusing | 09:42 |
| rogpeppe | jam: the startStateAPI func thing works out quite nicely actually: http://paste.ubuntu.com/5801013/ | 09:44 |
| rogpeppe | jam: i agree it's a bit confusing, but the whole look-at-api-then-connect-to-state-but-not-if-we-are-bootstrapping thing is a little inherently confusing, i think | 09:46 |
| rogpeppe | jam: and i *think* that the code is just a reflection of that fundamental awkwardness | 09:46 |
| mgz_ | jam: yup | 09:50 |
| jam | fwereade, rogpeppe: so what was decided on package naming vs tasks. Specifically, what package should I put upgrader in? Its own as state/apiserver/upgrader/upgrader.go, or sharing machine as state/apiserver/machine/upgrader.go? (I get the impression it may be run by machine or unit, so it should be its own thing) | 09:57 |
| rogpeppe | jam: it should be in its own package | 09:57 |
| fwereade | jam, rogpeppe: this is kinda the problem with the "let's segment-by-agent" scheme | 09:58 |
| rogpeppe | jam: this CL shows where i'm aiming https://codereview.appspot.com/10494043/ | 09:58 |
| fwereade | rogpeppe, would you remind me what your objection was to putting common test infrastructure in its own package? | 09:58 |
| fwereade | rogpeppe, (iirc that was the main factor in your decision -- I hope I'm not misrepresenting?) | 09:59 |
| jtv | rogpeppe: does this correctly reflect your notes on concurrency hazards in provider implementations? https://codereview.appspot.com/10602043 | 10:00 |
| rogpeppe | fwereade: that was part of it. the main thing was to try to keep packages from proliferating wildly. | 10:00 |
| rogpeppe | jtv: will look in a mo | 10:01 |
| fwereade | rogpeppe, small packages with clear purposes are usually considered a good thing | 10:01 |
| jtv | thx | 10:01 |
| rogpeppe | fwereade: the way we're going, we'll have apiserver/machine, apiserver/machiner and apiserver/machineagent | 10:01 |
| fwereade | rogpeppe, all the more so in go, surely, considering that the only encapsulation boundaries are at packages edges | 10:02 |
| rogpeppe | fwereade: i think it's reasonable to gather the things that will only ever run in one agent inside a package for that agent | 10:02 |
| * jam goes and hides in a hole to get actual coding done | 10:02 | |
| jam | will emerge around standup time | 10:02 |
| rogpeppe | fwereade: i'm happy to encapsulate by type as well as by package | 10:03 |
| fwereade | rogpeppe, that's all very well in theory but when not enforced by language or ultra-string convention it tends to degrade ;) | 10:03 |
| fwereade | rogpeppe, I think that the details of exactly what runs where will be more fluid than you anticipate | 10:04 |
| fwereade | rogpeppe, and I would prefer not to impede our flexibility by signalling that the most important feature of, say, upgrader, is which agent runs it | 10:04 |
| fwereade | rogpeppe, we have a distributed system with a bunch of responsibilities | 10:05 |
| rogpeppe | fwereade: i would definitely put reusable components inside their own packages | 10:05 |
| rogpeppe | fwereade: so the upgrader would get its own package | 10:05 |
| rogpeppe | fwereade: one mo, i'll paste a couple of sketches | 10:06 |
| fwereade | rogpeppe, and uniter? | 10:06 |
| fwereade | rogpeppe, I'll probably want to run some of those in machine agents at some point | 10:06 |
| rogpeppe | fwereade: it would go into apiserver/unit | 10:06 |
| rogpeppe | fwereade: really? | 10:07 |
| fwereade | rogpeppe, remove-unit --force | 10:07 |
| fwereade | rogpeppe, I'm not going to try a transaction to clean up the whole unit state | 10:07 |
| fwereade | rogpeppe, revoking the original unit's access and running a sandboxed uniter with a fake charm would work just fine though | 10:07 |
| rogpeppe | fwereade: woah | 10:07 |
| rogpeppe | fwereade: that seems a bit... heavyweight | 10:08 |
| rogpeppe | fwereade: interesting idea though | 10:08 |
| fwereade | rogpeppe, I only thought of it relatively recently, but it seemed like a possible end-run around a lot of the diffculty | 10:09 |
| fwereade | rogpeppe, it would definitely require that the uniter be decomposed a little but that's definitely not a bad thing | 10:11 |
| rogpeppe | fwereade: it's already decomposing, arf arf :-) | 10:12 |
| rogpeppe | fwereade: it sounds like an interesting approach | 10:12 |
| fwereade | rogpeppe, I'd love to be able to drop most of the giant integrationy tests there and be able to run detailed unit tests on all the modes, for example | 10:12 |
| rogpeppe | fwereade: using the unit agent for that purpose would presumably only require a small subset of the full uniter API? | 10:13 |
| fwereade | rogpeppe, yeah, that was the thought | 10:13 |
| rogpeppe | fwereade: so it might have its own API facade anyway? | 10:13 |
| rogpeppe | fwereade: ok, so if we go with the "packages for everything" approach, this is how i see the machine package looking: http://paste.ubuntu.com/5801079/ | 10:15 |
| fwereade | rogpeppe, that hadn't been my thought in particular -- allowing access to that facade for force-dying units on ManageState machines connections seems maybe plausible | 10:15 |
| fwereade | rogpeppe, sorry, the machine agent implementation? | 10:16 |
| rogpeppe | fwereade: what does cleaning up the unit state actually involve? is it that complex that it's a great help to have whole uniter around for it? | 10:16 |
| fwereade | rogpeppe, basically a load of scope-leaving | 10:17 |
| fwereade | rogpeppe, it may indeed not involve the whole thing, that's why I mention decomposing it | 10:18 |
| rogpeppe | fwereade: so, the machine package would just integrate together all the APIs that we want to present to the machine agent. | 10:18 |
| dimitern | danilos__: hey | 10:20 |
| danilos | dimitern, hey-hey | 10:20 |
| danilos | dimitern, welcome back, I hope it was nice two weeks off :) | 10:20 |
| rogpeppe | danilos: hiya | 10:20 |
| dimitern | danilos__: have you started some work on the deployer api stuff? | 10:20 |
| danilos | rogpeppe, hey | 10:20 |
| dimitern | danilos__: oh yeah it was :) | 10:20 |
| danilos | dimitern, yeah, barely | 10:20 |
| danilos | dimitern, basically, looking at that unification of watchUnits under Machine state object | 10:21 |
| dimitern | danilos__: because i was thinking of picking that up | 10:21 |
| dimitern | danilos__: if you don't mind | 10:21 |
| danilos | dimitern, hum, perhaps not a bad idea and I can focus on finishing the python-env stuff | 10:21 |
| dimitern | danilos__: sgtm | 10:21 |
| fwereade | rogpeppe, that doesn't feel unreasonable to me -- it puts that responsibility in one place while we're firming up the stuff around it, and keeping it separate for now makes it easier to move it somewhere else if and when the need becomes apparent | 10:21 |
| fwereade | rogpeppe, I'd really prefer to keep things separated by default, and only combined when it's clear that doing so fits the rest of the model so well that it's actively *bad* to keep them separate | 10:23 |
| rogpeppe | fwereade: do you want a separate package for each watcher type? | 10:23 |
| danilos | dimitern, some of unfinished code is up in lp:~danilo/juju-core/watch-units, though you might want to start over since it's not much (I've got some attempts at fixing tests uncommitted locally, but it doesn't solve them completely) | 10:23 |
| danilos | dimitern, want me to push that too or you don't care? :) | 10:23 |
| rogpeppe | fwereade: i was planning on putting them all into apiserver/watchers | 10:24 |
| dimitern | danilos__: i'll take a look at that and what we planned before i left to bring myself up to speed | 10:24 |
| fwereade | rogpeppe, depends where you mean... I think that what you describe may actually work pretty well for me -- IMO that's the one clear case we have of a really broadly shared capability | 10:25 |
| danilos | dimitern, some notes I gathered are up in http://paste.ubuntu.com/5741603/ (including a link to your pastebin) | 10:26 |
| danilos | dimitern, I meant https://docs.google.com/a/canonical.com/document/d/105xob7LVW63NoWoKoRhJNYN26_1GaAO-apiG8TKt_5s/edit :) | 10:26 |
| dimitern | danilos__: cheers | 10:26 |
| danilos | dimitern, shared it with you | 10:26 |
| jam | fwereade, rogpeppe: by the same token, should it be called state/apiserver/upgrader/upgrader.go:UpgraderAPI vs Upgrader ? | 10:26 |
| jam | given MachinerAPI | 10:26 |
| fwereade | rogpeppe, so you'd make the watcher API accessible separately to most of the other APIs, and those APIs' watch methods will return ids for use with the watcher service? | 10:27 |
| fwereade | jam, if it has it's own package it could just be upgrader.API, and if everything did it could be machiner.API, etc | 10:27 |
| fwereade | rogpeppe, jam, watcher.API :) | 10:28 |
| jam | fwereade: it does make using grep to find where this type is implemented pretty hard. | 10:28 |
| rogpeppe | fwereade: i was thinking watchers.EntityWatcherAPI, watchers.EnvironWatcherAPI, etc | 10:29 |
| rogpeppe | fwereade: because each one is actually really quite small | 10:29 |
| fwereade | rogpeppe, fwiw I think EnvironWatcher should just be an EntityWatcher really | 10:29 |
| rogpeppe | fwereade: whatever | 10:29 |
| rogpeppe | fwereade: it was the second watcher i could think of :-) | 10:29 |
| fwereade | rogpeppe, (bikeshed bikeshed: NotifyWatcher) | 10:30 |
| rogpeppe | fwereade: if you wanna repaint that bikeshed, go for it | 10:31 |
| rogpeppe | fwereade: (re-bikeshed: Watcher) | 10:31 |
| fwereade | rogpeppe, yeah, we can do all this later :) | 10:32 |
| rogpeppe | fwereade: +1 | 10:32 |
| rogpeppe | jam: don't use grep to find types :-) | 10:33 |
| fwereade | rogpeppe, I'm comfortable with the broad shape of what you propose | 10:33 |
| rogpeppe | fwereade: cool | 10:33 |
| rogpeppe | jam: also grep '^type SomeType' is a good way of finding types, if that's not how you do it already | 10:34 |
| fwereade | rogpeppe, so in *that* case can we make that machine package wither to almost nothing, and make it the individual workers' responsibility to pull the APIs they need off a single shared client object? | 10:35 |
| rogpeppe | fwereade: yes | 10:36 |
| fwereade | rogpeppe, this sgtm | 10:36 |
| jam | rogpeppe: sure, but if everything is defined as type API ... | 10:36 |
| jam | func.*FuncName is reasonable to find functions too | 10:36 |
| rogpeppe | jam: well, you'll only ever see it referred to a pkgname.API | 10:36 |
| rogpeppe | jam: which is fairly unambiguous as to where that particular API type is defined | 10:36 |
| jam | rogpeppe: except in the package itself, and you don't know *which* file it is defined in | 10:37 |
| jam | rogpeppe: gives you a directory | 10:37 |
| rogpeppe | jam: my favourite is ' Foo\(' for finding method definitions | 10:37 |
| jam | rogpeppe: func.*Foo finds both methods and free funcs | 10:37 |
| rogpeppe | jam: but it also finds functions with Foo in the name | 10:38 |
| rogpeppe | jam: i like ' Foo\(' because it's pretty exact | 10:38 |
| jam | rogpeppe: except it finds calls of a free func Foo | 10:38 |
| jam | :) | 10:38 |
| jam | I agree that I put ( on when I need it | 10:39 |
| rogpeppe | jam: yeah, it finds method and func defns | 10:39 |
| rogpeppe | jam: in general though, i use godef :-) | 10:39 |
| jam | rogpeppe: which while it is a tool that works, it is also almost by definition a "tool that works for rogpeppe" :) | 10:39 |
| jam | as 99.999999% of humans don't have it installed on their machine | 10:40 |
| rogpeppe | jam: other people use it too, honest :-) | 10:40 |
| rogpeppe | jam: go get works | 10:40 |
| jam | rogpeppe: I would argue that the number of people comfortable using godef, and the number comfortable using grep | 10:40 |
| jam | ... | 10:40 |
| rogpeppe | jam: it's idiomatic in Go to have type names that work well when qualified by the package identifier | 10:41 |
| rogpeppe | jam: i don't think it's necessary to make names that are unique across the code base because they're a bit awkward to find using grep. | 10:42 |
| rogpeppe | jam: BTW i tried to write godef such that it would be almost trivial to integrate into another editor - all you need is the current file cursor position and the current file contents and it'll tell you where the definition is. | 10:43 |
| rogpeppe | jam: perhaps i'll actually learn some vim programming and write some bindings for it at some point. | 10:44 |
| wallyworld_ | jam: mgz_: danilos__: can we do standup now? it's half time in the State Of Origin football and I want to watch the 2nd half | 10:59 |
| mgz_ | I could, not sure about de otros | 10:59 |
| * wallyworld_ is hopeful | 10:59 | |
| dimitern | wallyworld_: me too | 11:00 |
| wallyworld_ | dimitern: hey, welcome back | 11:00 |
| dimitern | wallyworld_: thanks | 11:00 |
| wallyworld_ | good holiday? | 11:00 |
| jam | wallyworld_, mgz_, danilos__, dimitern: I'm there https://plus.google.com/hangouts/_/8868e66b07fa02bdc903be4601200d470dae9ee3 | 11:01 |
| dimitern | wallyworld_: oh yeah, although it seemed shorter ;) | 11:01 |
| jam | mgz_: de otros? los otros? | 11:01 |
| mgz_ | something like that :) | 11:01 |
| jam | danilos__: poke? | 11:03 |
| dimitern | danilos_: https://plus.google.com/hangouts/_/8868e66b07fa02bdc903be4601200d470dae9ee3 | 11:10 |
| danilos_ | dimitern, coming in a bit | 11:10 |
| rogpeppe | jam: https://github.com/dgryski/vim-godef | 11:16 |
| rogpeppe | jam: it's not fantastic (it doesn't know about the contents of the current buffer, so it will muck up if you're currently editing, but it might work ok otherwise) | 11:17 |
| rogpeppe | jtv: reviewed | 11:23 |
| TheMue | fwereade: ping | 11:40 |
| fwereade | TheMue, pong | 11:40 |
| TheMue | fwereade: wonna talk about autosync? | 11:41 |
| fwereade | TheMue, sgtm, would you precis it here? would be worth the chance of other eyes passing over it I think | 11:41 |
| TheMue | fwereade: currently simply wanted to know how you see this feature. so far I only have this topic "auto sync-tools" and found one mail | 11:43 |
| TheMue | fwereade: as long as I understand it the need for an explicit sync-tools call shall be removed | 11:44 |
| TheMue | fwereade: instead it should be handled automatically during bootstrap if needed | 11:44 |
| fwereade | TheMue, I had been hoping you had been analysing the problem in the meantime -- when should we do it, when not, what are the drawbacks of the approach you recommend for various users, why do we consider them to be a price wrth paying | 11:45 |
| fwereade | TheMue, the problem is that sync-tools is an annoying step extra step for first-time users, and we want to make their experience better | 11:46 |
| TheMue | fwereade: so it's like I wrote. handle it automatically during bootstrap if needed (simplified) | 11:50 |
| === tasdomas is now known as tasdomas_afk | ||
| TheMue | fwereade: maybe I'm underestimating it and simply don't see the troubles you expect | 11:55 |
| TheMue | fwereade: so it would be helpful what problems you see | 11:56 |
| fwereade | TheMue, it's the "if needed" and the "simplified" I'd like to hear more about | 11:58 |
| fwereade | TheMue, what's the trigger condition? what version(s) do we copy? what's the impact of these decisions? | 11:59 |
| fwereade | TheMue, how do we make this a nice story for an isolated environment? | 11:59 |
| TheMue | fwereade: the latter is the largest problem, indeed | 12:01 |
| fwereade | TheMue, don't underestimate the former | 12:01 |
| TheMue | fwereade: for the first parts I would use the same mechanisms like in sync-tools to check the existing tools in the environment | 12:01 |
| TheMue | fwereade: using the same decision logic of which tools are to sync | 12:02 |
| fwereade | TheMue, sync-tools will sync tools even if you already have valid tools available | 12:04 |
| TheMue | fwereade: do we have known troubles with this logic (in sync-tools)? | 12:04 |
| fwereade | TheMue, I don't think it's quite the same use case, is it? | 12:05 |
| TheMue | fwereade: it uses tools.ErrNoTools when inspecting the environs storage with tools.ReadList() | 12:06 |
| TheMue | fwereade: but public or private is explicitely set | 12:06 |
| fwereade | TheMue, right -- there's a whole bunch of things to consider. what source we want now, whether we'll want alternative sources in the future, where we want to store the tools we autosync, etc | 12:07 |
| fwereade | TheMue, at what point we check for tools, what sort of errors we could encounter, how we report those errors | 12:08 |
| TheMue | fwereade: thought the source topic is an extra one | 12:09 |
| fwereade | TheMue, ok, but I'm asking you to solve a mid-scale problem, and I need you to think through the solutions | 12:09 |
| TheMue | fwereade: reasonable, ok | 12:10 |
| fwereade | TheMue, the isolated environment case is one aspect of the problem; even if we don't solve that bit first (we won't) I'd like us to at least consider the problem's existence as we design it | 12:10 |
| TheMue | fwereade: but then I need more than simply the three words "auto sync tools". that's why I asked you, to get more information which problem has to be solved | 12:11 |
| rogpeppe | jam, fwereade: i've changed things a little bit and added some more comments. you might want to take a look before i approve the branch: https://codereview.appspot.com/10259049/diff2/5001:16001/cmd/jujud/machine.go | 12:11 |
| fwereade | TheMue, so one consequence of isolated environments is that we'll have to, at some stage, have a pluggable tools source | 12:11 |
| fwereade | rogpeppe, cheers, just a mo | 12:11 |
| fwereade | TheMue, ok, I'm sorry, I thought I'd been clear that it's about streamlining the user's first experience | 12:12 |
| TheMue | fwereade: so let me simply create an issue for this topic (currently only a tiny kanban card exists) where we can collect everything that should be solved | 12:12 |
| fwereade | TheMue, ec2 has a nice story from that POV, the others not so much | 12:12 |
| TheMue | fwereade: yeah, it's now more clear | 12:13 |
| TheMue | fwereade: and where I underestimated it *sigh* | 12:13 |
| TheMue | fwereade: hidden behind three nice words ... | 12:14 |
| fwereade | TheMue, haha :) | 12:14 |
| TheMue | fwereade: ;) | 12:14 |
| jam | rogpeppe: you have some doc comments that are out of date (startStateWorker vs ensureStateWorker) otherwise LGTM | 12:15 |
| fwereade | TheMue, the questions and answers are probably not actually that hard, it's just that I'd like to minimise surprising consequences and anything that involves copying several MB around is going to be a bit noticeable | 12:15 |
| rogpeppe | jam: ah, i thought i'd fixed that, thanks | 12:15 |
| TheMue | fwereade: yep | 12:16 |
| TheMue | fwereade: what I got from the mail it sounded only like a "hey, we've got not tools. I want to bootstrap, so please call sync-tools" | 12:18 |
| fwereade | rogpeppe, LGTM,one thought | 12:24 |
| rogpeppe | trivial CL that fixes golang-tip govet against juju-core trunk: https://codereview.appspot.com/10607043/ | 12:28 |
| rogpeppe | please could someone have a look quickly, as the issue is stopping me from proposing anything currently. | 12:28 |
| rogpeppe | fwereade: i believe you're on call :-) | 12:34 |
| rogpeppe | fwereade: i invoke you | 12:34 |
| fwereade | rogpeppe, cheers | 12:34 |
| fwereade | rogpeppe, LGTM trivial | 12:35 |
| rogpeppe | fwereade: ta | 12:36 |
| jam | fwereade, rogpeppe: pulling out some of the testing code in apiserver/machine into apiserver/testing so that I can reuse it for upgrader: https://codereview.appspot.com/10608043/ | 12:45 |
| jam | rogpeppe: in the HA case, you'll still have the root machine that starts the HA off, right/ | 12:47 |
| jam | ? | 12:47 |
| rogpeppe | jam: yes | 12:47 |
| jam | so the newly started will-be-root-nodes still have an API server to connect to to find out what they will be doing | 12:47 |
| jam | so there is still only 1 'machine/0' that bootstraps the whole process | 12:47 |
| rogpeppe | jam: yes | 12:47 |
| wallyworld_ | fwereade: no urgency, could you look at https://codereview.appspot.com/10534043/? i'm off to bed soon so will check any comments tomorrow | 12:47 |
| rogpeppe | jam: yes | 12:48 |
| rogpeppe | jam: but | 12:48 |
| rogpeppe | jam: once it's bootstrapped the whole process, its jujud might restart | 12:48 |
| jam | wallyworld_: did you win? | 12:48 |
| wallyworld_ | yes! | 12:48 |
| fwereade | wallyworld_, ack | 12:48 |
| wallyworld_ | smashed them :-D | 12:48 |
| jam | wallyworld_: grats | 12:48 |
| wallyworld_ | 1-1 now. we need the 3rd game | 12:48 |
| rogpeppe | jam: and then it's no longer appropriate to open state without opening the API first | 12:49 |
| rogpeppe | jam: assuming there's at least one other API server out there | 12:49 |
| fwereade | rogpeppe, I'm wondering if there's some justification for always connecting to state if state info is available | 12:50 |
| rogpeppe | fwereade: in the future, state info won't be passed in cloudinit | 12:51 |
| fwereade | rogpeppe, if a machine comes up without it, it can request it and write it, and the state workers that it'll start anyway can just fail repatedly until it's written out | 12:51 |
| rogpeppe | fwereade: and it's better to connect with the latest info if we can | 12:51 |
| fwereade | rogpeppe, the bootstrap case remains special | 12:51 |
| rogpeppe | fwereade: and the API holds the freshest info | 12:52 |
| fwereade | rogpeppe, sure, but I imagine the worker that keeps an eye on state info is going to have to be writing out updated versions every so often regardless | 12:52 |
| jam | rogpeppe: why do we believe the information about what API server to connect to is any fresher than what State to connect to? | 12:52 |
| rogpeppe | fwereade: sure. but we might come up after being partitioned from the API for some time | 12:53 |
| fwereade | rogpeppe, in the bootstrap case only, write it in so the state workers can start and work immediately, and the api workers can sit and wait until there's an api available | 12:53 |
| rogpeppe | jam: that's all we've got | 12:53 |
| rogpeppe | fwereade: what happens when we can remove jobs from a machine? | 12:54 |
| fwereade | rogpeppe, delete the stateinfo and bounce the agent I guess? | 12:54 |
| rogpeppe | fwereade: the machine is disconnected and comes back up, thinking that it needs to connect to the state, but actually it longer has that privilege, so it connects to the state repeatedly and fruitlessly | 12:55 |
| jam | rogpeppe: until it manages to connect to the API and finds out it is no longer able to do so. | 12:55 |
| fwereade | rogpeppe, what jam said | 12:55 |
| rogpeppe | jam: yes, so either way the APIWorker code has to interfere with the StateWorker code. | 12:56 |
| fwereade | rogpeppe, at which point it gets deleted and everybody's happy | 12:56 |
| rogpeppe | jam, fwereade: i'm not sure that we gain much by connecting to the state server regardless of the API. | 12:57 |
| fwereade | rogpeppe, that way the two tasks can be completely orthogonal, surely? | 12:57 |
| rogpeppe | [13:56:27] <fwereade> rogpeppe, at which point it gets deleted and everybody's happy | 12:58 |
| fwereade | rogpeppe, none of this conditional complexity and passing statey things into api methods | 12:58 |
| rogpeppe | fwereade: the APIWorker has to do that deletion | 12:58 |
| rogpeppe | fwereade: which makes the two tasks non-orthogonal | 12:58 |
| fwereade | rogpeppe, there's an api worker solely responsible for getting, updating, deleting state info -- but that's not something the api runner needs to take into consideration, is it? | 12:59 |
| rogpeppe | fwereade: having the two tasks communicating via the shared stateinfo state seems wrong to me | 13:00 |
| rogpeppe | fwereade: when we know exactly when a state worker is needed | 13:00 |
| rogpeppe | fwereade: and can start it then | 13:00 |
| fwereade | rogpeppe, at the cost of forcing everyone who wants to know about the api tasks to also figure out wat the deal is with the state tasks | 13:01 |
| rogpeppe | fwereade: ?? | 13:01 |
| rogpeppe | fwereade: the only shared knowledge is those few lines in machine.go | 13:01 |
| rogpeppe | fwereade: noone writing an API-based worker needs to know about any of that stuff | 13:02 |
| rogpeppe | fwereade: we're talking < 10 lines of code here. | 13:02 |
| fwereade | rogpeppe, agreed -- but to figure out what the agent does wrt the api, you need to derail into the state code | 13:02 |
| rogpeppe | fwereade: i think that doing it directly is nicer than doing it indirectly by side-effect of changing the shared stateinfo | 13:03 |
| rogpeppe | fwereade: which seems more like magic to me | 13:04 |
| jtv | rogpeppe: thanks for the review — would you also have time for another? It's this one: https://codereview.appspot.com/10480045/ | 13:05 |
| fwereade | rogpeppe, the actual state info is only "shared" between the thing that reads it and the thing that writes it | 13:05 |
| fwereade | rogpeppe, we need both those things already | 13:05 |
| fwereade | rogpeppe, why explicitly couple two distinct components when they'll work just fine independently anyway? | 13:06 |
| fwereade | rogpeppe, if the api runner needs to know the state info, that's a smell | 13:06 |
| fwereade | rogpeppe, if a component whose entire purpose is writing the state info knows about it, and *that* is run by an api worker, no problem | 13:07 |
| fwereade | rogpeppe, distinction seem meaningful? | 13:07 |
| rogpeppe | fwereade: it still feels icky to me; that's perhaps because i haven't absorbed its possibilities. at the moment it feels like smearing responsibilities (changing the stateinfo not only changes the stateinfo, but also has the side effect, at some point in the future maybe, of triggering state-based tasks to connect and run. | 13:11 |
| rogpeppe | ) | 13:11 |
| rogpeppe | fwereade: it *may* be a better way of doing it, but i'd need to think hard about it for a while | 13:12 |
| rogpeppe | fwereade: and for the time being, i'm reasonably happy with the current approach, which i think works ok | 13:12 |
| rogpeppe | fwereade: and i really want to get this stuff in | 13:12 |
| fwereade | rogpeppe, yeah, I did LGTM it, I'm not trying to block you | 13:13 |
| rogpeppe | fwereade: thanks | 13:13 |
| fwereade | rogpeppe, I'm also just wittering on about how I'd like to see it evolve | 13:13 |
| ackk | hi all, I have a question about juju-core API: after sending a WatchAll request, does the first AllWatchersNext response contain the whole current environment state, or can it be broken up in multiple responses? | 13:14 |
| rogpeppe | fwereade: i'd rather that the component that was responsible for writing the stateinfo was also directly responsible for adding or deleting the state worker. | 13:14 |
| rogpeppe | jtv: looking | 13:15 |
| jtv | thanks | 13:15 |
| rogpeppe | jtv: reviewed | 13:18 |
| jtv | Thanks again! | 13:20 |
| jtv | rogpeppe: not sure I understand your review comment... you want me to move the initialization of the environ's "name" attribute down by one line? | 13:28 |
| rogpeppe | jtv: did you see what i did in the other providers to solve the same issue? | 13:28 |
| jtv | Yes, I was the first to review the branch. I did something as similar as I could here. | 13:28 |
| rogpeppe | jtv: i made the setting of the name field entirely independent of SetConfig | 13:29 |
| jtv | Yes, that's what I did too. | 13:29 |
| rogpeppe | jtv: you set the name field within SetConfig, no? | 13:29 |
| rogpeppe | jtv: i would set it in Open | 13:29 |
| jtv | That's what I did. I guess you just missed it then because it was so hassle-free. :) | 13:29 |
| jtv | env := azureEnviron{name: name} | 13:29 |
| jtv | ←initializes "name" right from the start, for maximum protection as the ads say. :) | 13:30 |
| rogpeppe | jtv: ha ha! | 13:30 |
| rogpeppe | jtv: i'd read the red code as green code | 13:30 |
| rogpeppe | jtv: sorry for the bogus comment | 13:30 |
| jtv | Red code much better than Green squad code! | 13:30 |
| rogpeppe | jtv: :-) | 13:30 |
| mgz_ | now now, let's not have arguments :P | 13:30 |
| rogpeppe | jtv: LGTM | 13:31 |
| TheMue | fwereade: thx for review | 13:31 |
| dimitern | fwereade: got a minute? | 13:31 |
| jtv | mgz_: Arguments much better than global state! | 13:32 |
| jtv | thanks again rogpeppe | 13:32 |
| mgz_ | death by joke! | 13:33 |
| jtv | In Soviet Russia, we kill joke. | 13:33 |
| jtv | I'm sorry — stress-induced giddiness. | 13:33 |
| fwereade | dimitern, heyhey | 13:34 |
| fwereade | dimitern, sure | 13:34 |
| dimitern | fwereade: I'm running into some issues with the deployer tests after removing the units watcher arg | 13:35 |
| fwereade | dimitern, oh yes? | 13:35 |
| dimitern | fwereade: more specifically - take a look at TestDeployRecallRemovePrincipals and TestDeployRecallRemoveSubordinates | 13:36 |
| dimitern | fwereade: I changed the deployer to use the machineId instead of a tag to verify whether it's responsible | 13:36 |
| dimitern | fwereade: so now unassigned units cannot be deployed | 13:36 |
| fwereade | dimitern, cannot be removed you mean? | 13:37 |
| dimitern | fwereade: and the tests assuming that seem kinda crackful | 13:37 |
| dimitern | fwereade: that as well | 13:37 |
| fwereade | dimitern, hmm, don't forget this code *does* need to work with units started with an old deployer | 13:37 |
| dimitern | fwereade: but now I have this check: http://paste.ubuntu.com/5801495/ | 13:38 |
| dimitern | fwereade: before it was deployerTag, ok := unit.DeployerTag(); ok { responsible == tag == d.tag } | 13:38 |
| fwereade | dimitern, ok, that looks reasonable, I thnk | 13:38 |
| dimitern | fwereade: the thing is - now the above 2 tests timeout right about there: http://paste.ubuntu.com/5801499/ | 13:39 |
| fwereade | dimitern, then the only impact is making sure that we list units of old-style deployers as well in the manager | 13:39 |
| dimitern | fwereade: and I can't get the idea behind unassigning and then checking whether it's still deployed | 13:40 |
| fwereade | dimitern, well, unassigning is kinda madness and crack actually | 13:40 |
| fwereade | dimitern, because it's all fundamentally unknown | 13:41 |
| dimitern | fwereade: it's definitely confusing - what should it be instead? | 13:41 |
| fwereade | dimitern, well, it's not a feature we've designed properly at all, it's an 18-month-oldguess we still haven't found a usefor | 13:41 |
| fwereade | dimitern, so, to be clear , how do the tests fail? they don't remove the unit when it got unassigned, because..? | 13:42 |
| === wedgwood_away is now known as wedgwood | ||
| dimitern | fwereade: http://paste.ubuntu.com/5801508/ - that's the test output | 13:43 |
| fwereade | dimitern, ah! | 13:43 |
| rogpeppe | fwereade, anyone: i'd appreciate reviews of https://codereview.appspot.com/10494043/ and https://codereview.appspot.com/10554043/ if possible | 13:43 |
| fwereade | dimitern, so you saw a change | 13:44 |
| fwereade | dimitern, asked the machine for its id | 13:44 |
| fwereade | dimitern, it gave you that error and it filtered down | 13:44 |
| fwereade | dimitern, instead of trapping that error (in your first paste) and setting responsible = false | 13:44 |
| dimitern | fwereade: ah! | 13:44 |
| dimitern | fwereade: good catch | 13:44 |
| dimitern | fwereade: so this works: http://paste.ubuntu.com/5801529/ - but only for principals, the subordinates test still fails with the same timeout error | 13:49 |
| dimitern | fwereade: log http://paste.ubuntu.com/5801531/ - the only thing I changed is to add these lines in prepareSubordinates: http://paste.ubuntu.com/5801536/ | 13:50 |
| === gary_poster is now known as gary_poster|away | ||
| fwereade | dimitern, I think that one now needs to make into account that the principal will also be deployed? | 13:51 |
| dimitern | fwereade: you mean waitFor(c, isDeployed(ctx, u.Name()) first, then the sub? | 13:52 |
| rogpeppe | jam: what does this mean: "No proposals found for merge of https://code.launchpad.net/~rogpeppe/juju-core/311-juju-bootstrap-state-change-password-1.6 into https://code.launchpad.net/~go-bot/juju-core/trunk." | 13:52 |
| fwereade | dimitern, I suspect so | 13:53 |
| rogpeppe | jam: i just got it as an error from the Go Bot | 13:53 |
| * rogpeppe wishes there was some way of knowing what tarmac is up to at any given moment | 13:55 | |
| fwereade | dimitern, by the way, you know the separate branch I suggested that allows principal units to become dead without waiting for subs? I'm not quite so sure that's a good idea, don't waste any time removing that code :) | 13:56 |
| dimitern | fwereade: I'll propose it shortly as a first step | 13:57 |
| fwereade | dimitern, remember you can't land the change you're making without removing deployer from the unit agent | 13:58 |
| dimitern | fwereade: sure | 13:59 |
| fwereade | dimitern, and do please spin something up and test it live with an actual subordinate :) | 13:59 |
| dimitern | fwereade: ok, will do | 13:59 |
| fwereade | dimitern, and try taking things down in various ways | 13:59 |
| fwereade | dimitern, destroy principal unit/service, destroy subordinate service, destroy relation | 13:59 |
| fwereade | dimitern, at least you can --force machine and run all the test sin the same place ;p | 14:00 |
| dimitern | fwereade: i might need some help on that | 14:00 |
| fwereade | dimitern, sure -- I have a kanban meeting to get to now though | 14:00 |
| mgz_ | rogpeppe: not sure, nothing seems obviously wrong, but tarmac seems to have not found the merge proposal | 14:00 |
| rogpeppe | mgz_: hmm, weird | 14:01 |
| mgz_ | ah, you reproposed after targetting the wrong branch? | 14:01 |
| mgz_ | that seems likely to be the issue | 14:01 |
| rogpeppe | mgz_: i don't *think* so, but it's possible | 14:01 |
| rogpeppe | fwereade: kanban? | 14:02 |
| === gary_poster|away is now known as gary_poster | ||
| fss | niemeyer: ping | 14:16 |
| niemeyer | fss: Yo | 14:31 |
| rogpeppe | jam: any idea about this message: "No proposals found for merge of https://code.launchpad.net/~rogpeppe/juju-core/311-juju-bootstrap-state-change-password-1.6 into https://code.launchpad.net/~go-bot/juju-core/trunk." | 14:36 |
| rogpeppe | jam: ? | 14:36 |
| rogpeppe | jam: the go-bot has actually already merged that prereq | 14:36 |
| rogpeppe | jam: despite it saying that it can't find it | 14:36 |
| sidnei | rogpeppe: sometimes the lp api lags behind | 14:37 |
| rogpeppe | jam: the weird thing is that the merge proposal has existed for days | 14:37 |
| rogpeppe | sidnei: ^ | 14:37 |
| rogpeppe | sidnei: but maybe it's a misleading error message | 14:38 |
| sidnei | in one case i had to delete the cache that the lazr.restfulclient keeps locally to get it to notice | 14:39 |
| * TheMue is stepping out for some time and will be back later | 14:43 | |
| fwereade | rogpeppe, both LGTM | 14:47 |
| rogpeppe | fwereade: thanks | 14:48 |
| rogpeppe | the API-connection branch has finally landed in trunk | 14:57 |
| rogpeppe | yay! | 14:57 |
| dimitern | rogpeppe: you've got 2 reviews | 14:57 |
| rogpeppe | dimitern: thanks | 14:57 |
| rogpeppe | dimitern: i'm just moving towards approval | 14:58 |
| dimitern | rogpeppe: only the panics are fishy | 14:58 |
| rogpeppe | dimitern: the panics are there because i don't expect those methods to be called in the tests | 14:58 |
| rogpeppe | dimitern: no point in writing code that's never used | 14:58 |
| dimitern | rogpeppe: hmm.. well, if it's there we should test it (at some point) | 14:59 |
| rogpeppe | dimitern: i'm sure that they'll be fleshed out later when it becomes common test code | 14:59 |
| rogpeppe | dimitern: all in good time :-) | 14:59 |
| dimitern | rogpeppe: ok then | 14:59 |
| mgz_ | rogpeppe: what did you do to get your merge unstuck? | 15:00 |
| rogpeppe | mgz_: nothing at all | 15:01 |
| rogpeppe | mgz_: ah, no | 15:01 |
| rogpeppe | mgz_: i re-approved it | 15:01 |
| mgz_ | okay, good to know for future reference | 15:01 |
| rogpeppe | "There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions." | 15:09 |
| rogpeppe | aarg | 15:09 |
| rogpeppe | h | 15:09 |
| rogpeppe | does that mean i can't make changes that people suggest and then just land the branch? | 15:09 |
| mgz_ | you just toggle the approval state for that | 15:09 |
| mgz_ | you can't approve then push (as I found, I think) | 15:10 |
| rogpeppe | mgz_: i don't think i did. but maybe i did. | 15:10 |
| rogpeppe | mgz_: sigh | 15:10 |
| mgz_ | there may also be a little lag | 15:10 |
| rogpeppe | mgz_: i think tying the approval to the revno that was loaded in the lp page is bogus. | 15:11 |
| rogpeppe | mgz_: too clever for its own good and ours | 15:11 |
| dimitern | fwereade: so once the deployer creation is removed from the unit agent, there's no need to test for deployment right? | 15:11 |
| rogpeppe | mgz_: i'm spending far too much time shepherding the submission process | 15:11 |
| dpb1 | fwereade: Thanks for the review. So, in pyjuju setting "" even on the command line (juju set "foo=") would set an empty string in the config. AFAIK, there was no way to get a nil/null/None into the charm config, since that state does not exist in Bash (lowest common denominator) | 15:16 |
| fwereade | dpb1, hmm, so what were we doing with "" values at config-get time? were we always stripping them out? | 15:23 |
| dpb1 | "" -> remained as is. something unset (no default in the config) was stripped out. The only way to get a null was something unset in the config.yaml that you probed directly with json (config-get --format=json unset_key => null) | 15:28 |
| dpb1 | fwereade: my brain hurts just typing that. :) | 15:29 |
| fwereade | dpb1, turned out pretty readable actually :) | 15:31 |
| dimitern | fwereade: https://codereview.appspot.com/10617043 - first attempt to see the general direction, will do live testing now | 15:31 |
| fwereade | dpb1, so, ok -- I guess there wasn't any way to explicitly clear a value in python and get back to the default then? | 15:31 |
| fwereade | dpb1, sorry, I've been doing go for too long | 15:32 |
| fwereade | dpb1, ah! right, I remember why we turned "" into nil on input -- because python couldn't distinguish between a default value and an explicit value that matched the default | 15:33 |
| fwereade | dpb1, and hence could not handle changing default settings on charm upgrade | 15:34 |
| dpb1 | fwereade: correct. once it is defined, no way to unset it, so "" was the defacto "unset" | 15:34 |
| dpb1 | oh. | 15:34 |
| fwereade | dpb1, sorry, it was IIRC an overheard conversation at a sprint about a year ago | 15:34 |
| * dpb1 parses | 15:34 | |
| fwereade | dpb1, I'm frantically loading state myself | 15:35 |
| fwereade | dpb1, so, we took the ""-means-unset convention and formalized it a bit | 15:35 |
| dpb1 | interesting | 15:35 |
| fwereade | dpb1, and reassured ourselves it couldn't possibly hurt anyone because the semantics were just the same | 15:35 |
| fwereade | dpb1, ofc you can see how well that turned out | 15:36 |
| dpb1 | hehe | 15:36 |
| dpb1 | well, it's a small thing, and easy to fix in the charms, its just... there are a lot of them. :) | 15:36 |
| fwereade | dpb1, but I think there's a germ of value in the idea somewhere | 15:36 |
| fwereade | dpb1, I firmly believe we need to fix this | 15:36 |
| fwereade | dpb1, we've broken people and we must unbreak them | 15:37 |
| fwereade | dpb1, to me the question is whether the fix is temporary, with a deprecation warning, or grandfathered in forever | 15:37 |
| dpb1 | fwereade: agreed. I was kind of surprised by it (as a charm author). I'm sure others would be too | 15:37 |
| dpb1 | fwereade: to me it seemed straightforward, "default: " in config.yaml maps to "" when you read it out. But you threw that wrinkle in about distinguishing at charm upgrade time | 15:38 |
| fwereade | TheMue, ping | 15:38 |
| fwereade | dpb1, yeah -- and the ability to reset to default is quite nice too | 15:39 |
| dpb1 | fwereade: I guess yaml is just not expressive enough to consider a nil case | 15:39 |
| fwereade | dpb1, null is perfectly valid yaml but we don;t want people to have to type that ;p | 15:40 |
| dpb1 | fwereade: ah... | 15:41 |
| fwereade | dpb1, yaml can probably express "please fire all the nuclear missiles" if you're not careful | 15:42 |
| * dpb1 wonders what would happen if he typed juju set key=null :) | 15:42 | |
| fwereade | dpb1, I don't *think* that goes through a yaml filter in python, and it certainly doesn't in go | 15:42 |
| dpb1 | fwereade: iirc, I've tried that and you are right. just strigifies to null | 15:43 |
| fwereade | dpb1, but if you used --config with a null value we'd interpret it as "please delete this setting" with the final impact being "replace with default" | 15:43 |
| mgz_ | fwereade: that's what safe_load is for :P | 15:43 |
| fwereade | dpb1, except I'm suddenly not sure what would happen there in python | 15:43 |
| fwereade | dpb1, it might well start coming out as None | 15:43 |
| dpb1 | fwereade: interesting. I've never used this feature. Since charms interpret "" as unset (for a variety of reasons we have discussed), that has always been how ive done it from the command line juju set "key=" | 15:44 |
| dpb1 | fwereade: in any case, having some change in behavior when using juju-core in this manner is fine, better than having to change charms in subtle ways, IMO. | 15:46 |
| fwereade | dpb1, in the very narrow context of "" defaults being valid I completely agree | 15:48 |
| fwereade | dpb1, you've brought up a more disturbing question though | 15:48 |
| dpb1 | I don't like the sound of that. | 15:49 |
| fwereade | dpb1, that `juju set option=` is now sometimes a *very* different operation across go and python | 15:49 |
| fwereade | dpb1, this only impacts string keys at least | 15:50 |
| fwereade | dpb1, for other values it's new and useful and I don't think it hurts | 15:51 |
| fwereade | dpb1, can you give a gut estimate for how often you used that for an option that had a non-empty default? | 15:52 |
| dpb1 | fwereade: so you are saying in juju-core if I do "juju set option=" it will revert to the default value? | 15:53 |
| fwereade | dpb1, yeah | 15:53 |
| fwereade | dpb1, that was something that happened so long ago I'd completely forgotten the original behaviour :/ | 15:53 |
| dpb1 | well, that isn't entirely unexpected, I suppose. | 15:53 |
| * fwereade wishes we hadn't developed in the dark for quite such a long time | 15:53 | |
| dpb1 | but it brings up how you would actually set the value to "" | 15:54 |
| dpb1 | as you are getting at. | 15:54 |
| fwereade | dpb1, quite so | 15:54 |
| mgz_ | `juju set option=\"\"` I guess | 15:54 |
| fwereade | mgz_, except when `""` itself is valid | 15:55 |
| * fwereade sighs | 15:55 | |
| dpb1 | For me, I use that idiom all the time, when I'm developing the charm. unset, set back, see what the charm does | 15:55 |
| fwereade | dpb1, and this is when the default is not ""? | 15:55 |
| dpb1 | but, as you can expect, that differs depending on how "dynamic" the charm is supposed to be. ie., are options used in install or config-changed. | 15:55 |
| dpb1 | fwereade: it's less important, since most options default to "" or have no default, especially as the charms grow. | 15:56 |
| fwereade | dpb1, I'm really worried about that whole issue in general actually -- I rather feel that a charm that *requires* options is somewhat naughty, and one that can't handle changes is very naughty indeed ;) | 15:57 |
| dpb1 | fwereade: but, if I had found that before you mentioned it? I probably wouldn't care an awfully lot. it does something interesting (sets back to default). | 15:57 |
| dpb1 | as long as there is a way to unset, I would just use that. modifying that behavior slightly isn't hard. | 15:57 |
| fwereade | dpb1, where by "unset" you mean "set to empty", right? | 15:58 |
| dpb1 | fwereade: yes. I think there are cases where you need options, but in general, it's something you should and do learn in charm building 101. Don't require anything, and respond to change dynamically. | 15:59 |
| fwereade | dpb1, ok, so, to summarize: `option=` is a nice way to reset, and you can live with that, but you need some way to express empty strings in general? | 16:00 |
| dpb1 | fwereade: hehe, well, that is the crux of what we are talking about. In pyjuju, the two were equivalent (except for the case of config-get --format=json unset_value) | 16:00 |
| dpb1 | fwereade: ok, summarize... let me see | 16:01 |
| dpb1 | fwereade: the bug specifically mentions "default:" and how that should go to empty string. | 16:02 |
| fwereade | dpb1, that's the easy bit -- if "" is valid, then we can keep that setting directly | 16:03 |
| dpb1 | beyond that, I think most of what we have been talking about it theory. I don't need "option=" on the command line to behave in any certain way. | 16:03 |
| fwereade | dpb1, so my issue is of global consistency | 16:03 |
| fwereade | dpb1, if we allow "" in charm defaults, which I think is good, we should be able to express "make this the empty string, not the default" for the cases where the default is *not* "" | 16:04 |
| fwereade | dpb1, as it is the range of possible values is constrained by input method and that kinda sucks | 16:04 |
| dpb1 | fwereade: yes, that makes sense. | 16:04 |
| dpb1 | fwereade: so something like , juju set --default option | 16:05 |
| dpb1 | (just throwing that out there) | 16:05 |
| dpb1 | afaik, pyjuju had no way of doing what you are saying, but I think it would be nice. you can get the info out of "juju get" and just set it by hand. | 16:06 |
| fwereade | dpb1, the specific scenario is for, say, tuning changes -- we'd like to update the preferred setting for users who haven't expressed a preference, but not for those who explicitly did | 16:07 |
| fwereade | dpb1, the theory is that those who don't express a preference are those least likely to fix it manually and we should make it do the Right Thing by default | 16:07 |
| dpb1 | fwereade: right, but in that case, they aren't going to be changing things at the command line in the first place, right? | 16:08 |
| dpb1 | they will just deploy and let it work | 16:08 |
| fwereade | dpb1, but it should be easy for them to fiddle with the settings and reset the ones they didn't want to -- but still get the benefits from upgrades without having to look | 16:10 |
| dpb1 | yes. | 16:10 |
| dpb1 | in that case having an explicit setting like --set-default, makes even more sense, avoid relying on "option=" doing something perhaps unexpected. | 16:11 |
| fwereade | dpb1, I think I am becoming convinced, loath as I am to drop that cure little `float=` to reset to default in the cases where it's not clear | 16:13 |
| dpb1 | fwereade: in any case, it's a separate issue. want me to file a bug about it? | 16:14 |
| fwereade | dpb1, ok, that sounds good to me -- we fix charm defaults, with no deprecation warning, and file a bug that `juju set stringoption=` does the wrong thing when the default is not "" | 16:15 |
| dpb1 | ok, great. bug coming along shortly. | 16:16 |
| fwereade | TheMue, were yuo following along there roughly? | 16:18 |
| dpb1 | fwereade: https://bugs.launchpad.net/juju-core/+bug/1194945 | 16:19 |
| _mup_ | Bug #1194945: juju set is overloaded <juju-core:New> <https://launchpad.net/bugs/1194945> | 16:19 |
| fwereade | dpb1, thanks | 16:19 |
| dpb1 | fwereade: thx for chatting. I'll be afk for a while now. | 16:20 |
| fwereade | dpb1, cheers, anytime | 16:20 |
| rogpeppe | dpb1, fwereade: juju unset ? | 16:26 |
| rogpeppe | fwereade: trivial, i think: https://codereview.appspot.com/10595044 | 16:26 |
| fwereade | rogpeppe, I want to be able to do it in the same transaction as the sets, really | 16:26 |
| fwereade | rogpeppe, trivial | 16:27 |
| fwereade | rogpeppe, `juju set option-`? :) | 16:29 |
| rogpeppe | fwereade: ? | 16:29 |
| fwereade | rogpeppe, "-" indicating removal ;p | 16:30 |
| rogpeppe | fwereade: i think i'd prefer juju set !option, but i think people with dodgy shells might object :-) | 16:31 |
| rogpeppe | juju set myflag- foo=bar bar=tdfv | 16:33 |
| rogpeppe | hmm, not entirely sure if that reads well | 16:33 |
| fwereade | rogpeppe, yeah, it's a better idea in my head than on the screen | 16:33 |
| * fwereade bbiab | 16:34 | |
| dpb1 | I like unset from a readability point of view, for sure | 16:43 |
| fwereade | dpb1, I kinda feel it's good to be able to make all config changes via the same command (and, under the hood transactionally) | 16:44 |
| dpb1 | fwereade: yes, I agree with you in that argument. | 16:47 |
| rogpeppe | fwereade: another trivial? https://codereview.appspot.com/10620043 | 16:54 |
| arosales | have folks seen that when a service unit goes down (outside of Juju) it is still made available to haproxy? | 17:00 |
| mgz_ | AssertStrop... funny tyop | 17:02 |
| dimitern | rogpeppe: not sure increasing the ping timeout to 5m is a good thing | 17:13 |
| rogpeppe | dimitern: why does it need to be faster? | 17:14 |
| rogpeppe | dimitern: what are we guarding against? | 17:14 |
| dimitern | rogpeppe: we want dead connections to die quickly | 17:14 |
| dimitern | rogpeppe: i.e. to be detected and closed earlier | 17:15 |
| rogpeppe | dimitern: if we're using them, they'll die quickly - if not, we don't care that much | 17:15 |
| rogpeppe | dimitern: 5 seconds is way too fast | 17:15 |
| dimitern | rogpeppe: why so? | 17:15 |
| rogpeppe | dimitern: because it's constant network traffic from every single node | 17:15 |
| rogpeppe | dimitern: and a connection going down is a rare event | 17:16 |
| dimitern | rogpeppe: how about 1m then? | 17:16 |
| rogpeppe | dimitern: that's probably ok | 17:16 |
| TheMue | fwereade: just returned to the screen and will now read the chat log | 17:16 |
| dimitern | rogpeppe: reviewed | 17:20 |
| rogpeppe | dimitern: ta | 17:20 |
| fwereade | rogpeppe, LGTM also | 17:21 |
| rogpeppe | fwereade: thanks | 17:22 |
| rogpeppe | fwereade: do you have an opinion on the ping frequency? | 17:22 |
| fwereade | rogpeppe, a minute sounded reasonable | 17:22 |
| fwereade | rogpeppe, but probably only because it's the middle of the 3 values I was shown :) | 17:23 |
| fwereade | rogpeppe, that's the sort of number I'm perfectly happy tuning in response to observation though | 17:24 |
| rogpeppe | fwereade: yeah. i changed it from 5 seconds because i was watching the request log and it seemed way too fast | 17:24 |
| fwereade | rogpeppe, fair enough, I feel like a minute is quite a nice resolution for now | 17:25 |
| mgz_ | fwereade: https://codereview.appspot.com/10623043 | 17:36 |
| mgz_ | I can't find a nicer way of making those helpers shared | 17:37 |
| TheMue | fwereade: so, read it, good discussion | 17:41 |
| TheMue | fwereade: so when default: is '' the value is kept and only in this case (default: is a string) set foo= sets foo to an empty string | 17:46 |
| TheMue | fwereade: so foo default: 'bar' and set foo= sets foo not to bar (the default) but to '' | 17:47 |
| TheMue | fwereade: and the new issue will introduce unset foo or or set !foo to reset it to the default value (or delete it if no default is specified) | 17:48 |
| TheMue | fwereade: correct summarize? | 17:48 |
| jam | mgz_: how was the Release meeting? | 17:52 |
| mgz_ | jam: fine, I'll link you the notes | 17:54 |
| jam | rogpeppe: https://code.launchpad.net/~rogpeppe/juju-core/312-alt-api-jobs/+merge/170135 | 18:00 |
| jam | has a prerequisite of https://code.launchpad.net/~rogpeppe/juju-core/311-alt-juju-bootstrap-state-change-password-1.5 | 18:00 |
| jam | *not* 1.6 | 18:00 |
| rogpeppe | jam: that's not the one that failed | 18:02 |
| jam | rogpeppe: which one failed? | 18:02 |
| rogpeppe | jam: this one: https://code.launchpad.net/~rogpeppe/juju-core/305-alt-jujud-use-tasks-package/+merge/170856 | 18:03 |
| jam | rogpeppe: so see if this fits what happened. You had a branch with a prereq, and they both got marked Approved at roughly the same time. Go-bot landed one of them, and then rejected the other with "No Proposals Found". | 18:06 |
| rogpeppe | jam: sounds right | 18:07 |
| rogpeppe | jam: i thought i could approve both branches at the same time and the prereq would get merged first | 18:07 |
| jam | rogpeppe: IIRC it doesn't sort the requests by prerequisite, so it tried to merge 305-alt-jujud first, which has an approved-but-not-merged-prereq, so it gets bounced, and then 311-alt-jujud- is triggered immediately after and lands. | 18:12 |
| rogpeppe | jam: ha, that's not great | 18:12 |
| rogpeppe | jam: so if there are prerequisites, you have to wait until the prereq is merged before approving the next in line | 18:13 |
| jam | rogpeppe: it probably doesn't help that the dependent branch comes alphabetically before the prereq branch. | 18:13 |
| jam | 305 depends on 311 | 18:13 |
| rogpeppe | ha ha ha | 18:13 |
| rogpeppe | it sorts alphabetically?! | 18:13 |
| rogpeppe | jam: it should really sort by approval time, if anything | 18:13 |
| jam | rogpeppe: I think it sorts by whatever order LP gives it, which could be alphabetically | 18:13 |
| jam | I don't think it does sort internally. | 18:13 |
| rogpeppe | jam: can we change it? | 18:13 |
| jam | rogpeppe: https://launchpad.net/tarmac code is available | 18:14 |
| jam | rogpeppe: https://bugs.launchpad.net/tarmac/+bug/845706 | 18:15 |
| _mup_ | Bug #845706: Tarmac fails to resolve pre-req branches when they've already been merged <Tarmac:Triaged by rockstar> <https://launchpad.net/bugs/845706> | 18:15 |
| jam | rogpeppe: looking at the bug report | 18:15 |
| rogpeppe | jam: but if we make changes, do they have to be back-ported to precise before we get the benefit? | 18:15 |
| jam | it appears launchpad actually does stop showing you the merge proposal for some reason. | 18:15 |
| jam | rogpeppe: I'm running from source, not a deb | 18:15 |
| rogpeppe | jam: if you want to have a look, this branch relates somewhat to an earlier branch you proposed. https://codereview.appspot.com/10611046/ | 18:16 |
| jam | I don't know the validaty of this bug comment: This is actually a bug in Launchpad, it seems. Sometimes it will fail to give the branch in the requested list of proposals. This seems to happen regardless of the prerequisite's merge status, but does seem to occur more often when it has been merged. | 18:16 |
| rogpeppe | jam: thanks for pointing me to that report | 18:25 |
| rogpeppe | jam: i've reached eod. time to do some packing... see you tomorrow morning. | 18:26 |
| rogpeppe | g'night all | 18:26 |
| jam | rogpeppe: have a good evening. I'm about 6 hours past mine :) | 18:26 |
| jam | dimitern: you are attributed the code roger is removing in https://codereview.appspot.com/10611046/ | 18:26 |
| jam | there is an intermediate object that adds the Id and a pointer back to the resources map | 18:27 |
| jam | which doesn't seem to be used | 18:27 |
| jam | If it isn't needed, we can remove it, I just wanted to check if you remember a need for it (that may just not have been written yet) | 18:27 |
| jam | It is also possible you are marked with the code because you moved it in a refactoring, and not because you wrote it. | 18:27 |
| dimitern | jam: what? | 18:29 |
| dimitern | jam: I introduced the interface yes | 18:31 |
| jam | dimitern: so you have a Resource interface, and a registry which tracks them with secret srvResource type | 18:32 |
| jam | the only thing srvResource provides over Resource | 18:32 |
| jam | is the 2 attributes Id, and a pointer back to the resource registry | 18:32 |
| jam | Roger's proposal removes those 2 attributes by just using a map of Resource directly. | 18:33 |
| jam | and I'm checking if you had a reason to have those attributes, which are currently unused. | 18:33 |
| dimitern | jam: well, originally I had to introduce the interface to decouple the srvRoot from the machiner facade | 18:34 |
| dimitern | jam: and I think that CL is just reorganizing things around, rather than removing functionality | 18:36 |
| jam | dimitern: well it specifically removes an intermediate object which has some attributes the new one doesn't have, but they don't seem to be used, and I didn't know if they had other plans to be used. | 18:36 |
| dimitern | jam: i don't think so - if we need these, we'll reintroduce them | 18:38 |
| === tasdomas_afk is now known as tasdomas | ||
| === mramm2 is now known as mramm | ||
| === tasdomas is now known as tasdomas_afk | ||
| === BradCrittenden is now known as bac | ||
| rogpeppe | jam: those attributes were needed only because srvResource was originally embedded as an API object directly | 20:54 |
| rogpeppe | jam: it implemented Stop | 20:54 |
| rogpeppe | jam: so it needed to be able to find the resources struct to remove itself | 20:54 |
| fss | niemeyer: hi, took me all day to see your pong x) | 21:00 |
| fss | niemeyer: are you going to fisl next week? | 21:00 |
| niemeyer | fss: I'll try to | 21:13 |
| thumper | morning | 21:22 |
| thumper | fwereade: around? | 21:33 |
| fss | niemeyer: I was wondering if we can chat about juju-core + vpc :) | 21:39 |
| thumper | stabby stabby | 22:28 |
| thumper | pretty happy that `bzr destroy-environment` doesn't do anything | 22:57 |
| fwereade | thumper, heyhey | 23:10 |
| thumper | fwereade: hey | 23:10 |
| * thumper makes a sad face | 23:10 | |
| thumper | missing reviews | 23:10 |
| fwereade | thumper, haha, I've done that bzr/juju thing | 23:10 |
| fwereade | thumper, that's why I'm on :) | 23:10 |
| thumper | ah | 23:10 |
| thumper | fwereade: I've just merged trunk | 23:11 |
| thumper | and made the lxc provisioner work with the new runner stuff | 23:11 |
| thumper | and tested on ec2 | 23:11 |
| fwereade | thumper, oh yes..? | 23:11 |
| thumper | had to fix one thing | 23:11 |
| fwereade | that sounds like the past tense which I infer is a good thing | 23:11 |
| thumper | where the api was assuming that for machine-0-lxc-0, that "0-lxc-0" was the amchine id | 23:11 |
| thumper | so used the MachineIdFromTag and it is all good | 23:12 |
| thumper | so we are back to where it works | 23:12 |
| thumper | did you want me to repropose | 23:12 |
| thumper | actually, I'll do that anyway | 23:12 |
| thumper | I do think however that the pinger is doing something a little weird... | 23:13 |
| thumper | oh, perhaps not | 23:14 |
| wallyworld_ | fwereade: hi, thanks for the extra comments on the instance metadata branch | 23:14 |
| thumper | it seems that the api writing code doesn't mention the source | 23:14 |
| thumper | and it is writted from the api serving side | 23:14 |
| thumper | not the client side | 23:14 |
| thumper | my dog is getting restless | 23:14 |
| thumper | I think she needs to go outside for a bit | 23:14 |
| thumper | fwereade, wallyworld_: Rietveld: https://codereview.appspot.com/10489043 | 23:14 |
| thumper | that is the branch that makes the lxc provisioner work out of the box | 23:15 |
| wallyworld_ | fwereade: if i keep Nonce on machine and write InstanceId to both machine and instanceData for compatibility, i think that may address your concerns? | 23:15 |
| thumper | all previous pipes in that pipeline have landed | 23:15 |
| * fwereade is working through it literally rght now | 23:15 | |
| thumper | this one branch makes us provision containers | 23:15 |
| fwereade | wallyworld_, yep, that sgtm | 23:15 |
| wallyworld_ | fwereade: are you +1 on the sig change to machine.InstanceId() - returns (string, bool, error) | 23:15 |
| thumper | I have my daughter at home today finishing off her science fair project | 23:15 |
| thumper | better a day off school, than staying up until midnight and getting tired and stressed out | 23:16 |
| thumper | wallyworld_: what? | 23:16 |
| thumper | wallyworld_: why three results? | 23:16 |
| fwereade | wallyworld_, ah, hmm, not so sure there -- but if you keep instanceid on the machine for now we get to dodge that question for a bit, right? | 23:16 |
| wallyworld_ | fwereade: sure, but i'd rather alter the sig now to match what is will be in the end | 23:17 |
| fwereade | wallyworld_, if it's an error that would conventionally replace the info encoded in the bool with a specific error | 23:17 |
| wallyworld_ | thumper: results is string (the value), bool (is te machine provisioned), error (was there an error finding the id) | 23:17 |
| thumper | fwereade: I agree | 23:17 |
| thumper | error NotProvisioned | 23:17 |
| thumper | and match against that | 23:17 |
| thumper | two, not three results | 23:17 |
| fwereade | wallyworld_, I wouldn't be comfortable predicting that change right now tbh | 23:17 |
| wallyworld_ | ok, i can do that. i did like the bool | 23:17 |
| wallyworld_ | i'll make it an error, that will cover both cases | 23:18 |
| thumper | wallyworld_: it just becomes one more step, error.IsNotProvisioned(err) | 23:18 |
| fwereade | wallyworld_, when we've got it behind the API I suspect we'll find out some interesting things about actual usage | 23:18 |
| wallyworld_ | thumper: yes, agreed | 23:18 |
| fwereade | wallyworld_, but if you feel strongly I'm not too bothered | 23:18 |
| * wallyworld_ doesn't feel strongly | 23:18 | |
| thumper | fwereade: also, I have looked at a very interesting library | 23:19 |
| thumper | fwereade: that may fix watchers | 23:19 |
| wallyworld_ | fwereade: with the constraints branch - i'm thinking i may need a deployment constraints struct that embeds the current constraints struct | 23:19 |
| thumper | fwereade: and give us distributed pub/sub | 23:19 |
| thumper | fwereade: http://code.google.com/p/go-router/ | 23:19 |
| thumper | fwereade: worth looking into I think, but perhaps for later | 23:19 |
| fwereade | thumper, at the lowest level the watcher have to remain, but the distribution of the events is absolutely open for improvement imo | 23:20 |
| thumper | fwereade: it could mean we have one watcher on things, and just publishes the change | 23:20 |
| wallyworld_ | fwereade: the deployment constraints stuct has the container constraint; that will avoid the need for rasing an error in places where the deployment constraint is not needed, like adding a machine | 23:20 |
| fwereade | thumper, yeah, definitely | 23:20 |
| thumper | listeners instead are then just subscribers | 23:20 |
| fwereade | wallyworld_, yeah, I was wondering about that | 23:20 |
| wallyworld_ | thumper: \o/ bring it on | 23:20 |
| === wedgwood is now known as wedgwood_away | ||
| thumper | I want to poke around with it for a while to test efficiency and whether it does what we need | 23:21 |
| wallyworld_ | fwereade: if you're not immediately -1 on the idea, i'll rework the brnach to see how it pans out | 23:21 |
| thumper | but the docs look promising | 23:21 |
| === davechen1y is now known as davecheney | ||
| fwereade | wallyworld_, but I'm a little reluctant too -- partly, I have a feeling that struct embedding may fuck unhelpfully with serialization | 23:21 |
| wallyworld_ | really? | 23:21 |
| fwereade | wallyworld_, I think you should check before you go too far | 23:21 |
| fwereade | wallyworld_, and we're still wearing these schema compatibility chains ;p | 23:22 |
| wallyworld_ | fwereade: you talking aout serialisation over the wire? | 23:22 |
| fwereade | wallyworld_, in mongo | 23:22 |
| wallyworld_ | fwereade: there's a separate mongo doc already and conversion functions | 23:22 |
| wallyworld_ | between the mongo doc and constraints entity | 23:22 |
| fwereade | wallyworld_, ah, that's nice, I knew I did that for a reason | 23:22 |
| wallyworld_ | :-) | 23:22 |
| wallyworld_ | so i think it will be all good | 23:23 |
| wallyworld_ | tw, i would have done the same design if i were doing it, so you must have done it right :-) | 23:23 |
| wallyworld_ | bte | 23:23 |
| wallyworld_ | tw | 23:23 |
| wallyworld_ | btw ffs | 23:23 |
| fwereade | wallyworld_, ok -- just one thought, though, that we're already expecting that different bits will pay attention to different constraints | 23:23 |
| fwereade | wallyworld_, and I'm not quite sure that there's quite such a clean separation, once you consider matching against existing instances as well | 23:24 |
| fwereade | wallyworld_, I might be wrong though | 23:24 |
| fwereade | wallyworld_, can't hurt to investigate | 23:24 |
| wallyworld_ | i'm not sure right now, i'll look at how it pans out | 23:24 |
| * fwereade goes back to thumper's branch | 23:25 | |
| * thumper appreciates that | 23:25 | |
| wallyworld_ | thanks for the input | 23:25 |
| * thumper takes the dog otu | 23:25 | |
| wallyworld_ | sorry thumper for jumping the queue :-) | 23:25 |
| davecheney | arosales: ping | 23:34 |
| fwereade | thumper, LGTM with waffling | 23:34 |
| thumper | fwereade: thanks I'll look at those shortly | 23:40 |
| thumper | need to head into town now to get science fair photos printed :-) | 23:40 |
| thumper | wallyworld_: can I get you to look over that branch too? | 23:40 |
| thumper | perhaps I'll get it landed this afternoon, which would be great | 23:40 |
| * thumper heads into town | 23:40 | |
| === thumper is now known as thumper-afk | ||
| arosales | davecheney, sorry we missed each other again | 23:47 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!