thumperwallyworld_: you around?01:24
* thumper is proposing the lxc-container branch *again*01:25
wallyworld_where else would i be01:25
thumperwallyworld_: I dunno, fucking around?01:25
bigjoolsenjoying a hot shower?01:25
wallyworld_well, the weather is nice outside01:25
* thumper snorts01:25
wallyworld_not funny01:25
thumperwell, it is shit here01:25
thumperwallyworld_: how's the bathroom?01:26
wallyworld_not quite done. tradesman is a total fuckwit. we'll be reporting him to the Building Services Authority, but need to get him to finish as much as we can first01:26
bigjoolshope you took pictures01:27
thumperbeen watching some home renovation programs on sky, damn scary what some builders do01:27
thumperhoping to get some that don't suck for our work01:28
bigjoolsI can only think of one profession that doesn't con customers more than builders01:28
* thumper waits for lbox to continue generating the diff01:28
wallyworld_yeah. and i got the worst tiler in queensland according to people who i have talked to since01:28
bigjoolsthat does, I mean01:28
thumperbigjools: software developers?01:28
bigjoolsthumper: got it in one01:28
bigjoolsI don't consider politics to be a profession01:28
thumperok, perhaps politicians01:28
bigjoolsserial troughers maybe01:29
thumperwallyworld_:  https://codereview.appspot.com/10370044 another round?01:29
thumperdog is being a nutter today01:30
thumperis outside01:30
bigjoolsso much for this being a family channel :)01:30
thumperyeah, I felt guilty as soon as I swore :)01:31
thumperyou guys are a bad influence01:31
bigjoolsI thought I was bad until I met Ian01:32
thumperI was much worse when I first went to the UK01:32
thumperhad to tone things down a lot01:32
thumperand not use swearing as punctuation, or as adjectives too often01:32
bigjoolsI expect the politically correct HR drones had some policy on it ...01:33
bigjoolseveryone got asked to go on some "sensitivity" training at one of the banks I was at01:33
bigjoolsas a contractor I told them to get fucked01:34
wallyworld_thumper: i had a branch that william has +1'ed to remove that Instance.Metadata() method. if you +1 it also, i can land it and you could tweak your branch before landing https://codereview.appspot.com/10384049/01:34
thumperbigjools: haha01:35
bigjoolsthumper: there's no crisis that can't be solved by sending in the diversity coordinators, apparently01:35
bigjoolswas that a fart?01:36
thumperlike *snort*01:37
thumperthings you miss by typing stuff01:37
thumpermiss all that non-word verbal communication01:37
bigjoolsaye, it's hard to talk without waving your arms around01:38
thumperwallyworld_: so right now, we are doing nothing with this metadata?01:46
wallyworld_thumper: i have a branch which has been reviewed which i need to fix some things on. the branch writes the metadata to state.01:47
thumperwallyworld_: what happens if I return nil, or a real struct where all the values are nil?01:47
wallyworld_just return nil01:47
wallyworld_it will create an empty record in the db01:48
wallyworld_with just the InstanceId and Nonce01:48
wallyworld_and mem, cpu cores etc will be blank01:48
bigjoolsseems like Go doesn;t coerce ints to floats02:13
thumperit doesn't even promote ints02:14
thumperinto bigger ints as needed02:14
thumperneeds explicit cast02:14
thumperwallyworld_:  https://codereview.appspot.com/10409049 for the broker02:14
thumperdavecheney: you could cast your "on call reviewer" eye over too maybe?02:15
thumperdavecheney: are you still doing on-call reviewing in your current role?02:15
thumperwallyworld_: ta02:25
thumperwallyworld_: what is the point of the PublicAddress, PrivateAddress, and InstanceId functions on environs.EnvironProvider?02:47
thumperthey don't seem to make any sense02:47
thumperwhy does a provider have an ip address or instance id?02:48
wallyworld_thumper: i think each machine has a provider instance. i agree it doesn't make sense02:49
thumperthat is nuts02:49
thumpersounds broken02:50
wallyworld_i'm not across the design decisions there, before my time02:50
* thumper nods02:50
wallyworld_but i agree with you02:50
thumperI'll be up late tonight to talk with fwereade__ anyway02:50
thumperI'll add it to my WTF question list02:50
wallyworld_if i'm around i'll listen in02:50
wallyworld_i think william wants to refactor all this stuff02:51
wallyworld_there's a few interfaces with inappropriate methods02:51
davecheneythumper: am I on call today ?02:56
davecheneyas to the other question, NFI02:56
thumperdavecheney: according to the juju google calendar02:57
* thumper does school run02:57
davecheneymy bad02:57
bigjoolswallyworld_: does the provider interface require a separation between public and private storage?  Or can it all be private?03:10
wallyworld_bigjools: it could all be private. may require a tweak to some auxiliary code03:13
bigjoolswallyworld_: is there any use for public at all then?03:13
wallyworld_yes, to store the tools03:13
wallyworld_snd simplestreams metadata03:14
bigjoolsdoes juju upload simplestreams stuff?03:14
bigjoolsthought that was seaprate03:14
wallyworld_it is expected to be created to match the cloud03:14
wallyworld_there's a tool which can be used to make a starting metadata file03:14
wallyworld_the tooling needs to be refined03:14
bigjoolsso if one person uploads tools to a public place and then removes them, does someone else get to auto-upload tools?03:15
bigjoolsand how is authenticity guaranteed03:15
wallyworld_it's not03:15
wallyworld_design flaw03:15
wallyworld_the idea right now is that only authorised people get to write to the public bucket03:15
wallyworld_so those people need to be trusted03:16
bigjoolsso there's just one public storage03:16
wallyworld_one per cloud03:16
wallyworld_there's one for ec2, canonistack, hp cloud etc03:16
wallyworld_the tools tarballs are currently replicated across each03:17
bigjoolsso what's the procedure for the first upload to a public place?03:17
wallyworld_the juju release manager uploads to the ec2 pubic bucket, then uses sync-tools cmd to help replicate to other public buckets03:18
bigjoolsah ok03:18
wallyworld_the new upcoming simplstreams stuff makes it a bit better03:18
wallyworld_and the tarballs will be signed also03:18
bigjoolsso for our new provider, someone would upload the tools before any deployments would take place03:18
wallyworld_for certified public cloud partners, the tools are uploaded/maintained as part of the release process03:19
bigjoolspresumably *someone* must be thinking about some sort of GPG signature downloadable over https from a trusted site?03:19
wallyworld_for private clouds, we will provide tooling to make setup easy03:20
bigjoolsok so I'll work on a change I need to make public storage available then03:20
bigjoolswas wondering whether it was really neede03:20
wallyworld_bigjools:  is this for a CPC partner?03:21
wallyworld_bigjools: if so, then the metadata for tools and images will be hosted on a canonical url, and that metadata will point to the cloud's pulic bucket to actually get the tools from03:24
bigjoolswallyworld_: right03:24
bigjoolsand juju at some point will gpg verify the md5 of the tools.03:24
bigjoolsplease tell me that is going to happen03:25
wallyworld_real soon now03:25
wallyworld_working on it03:25
* bigjools thinks about booking flights to Europe03:25
wallyworld_a few things are happening in that space, might be a release or two away03:25
thumperbigjools: you in IOM?03:27
thumperbigjools: you get to fly?03:27
bigjoolsthumper: supposed to be there03:27
bigjoolsstill not sure about going03:27
wallyworld_bigjools: in the maas provider, i'm going to add a NOP when it comes to returning hardware characteristics about a started instance (mem, cores etc). i'd like to raise a bug for it. ec2 and openstack will have this supported. will it take much to do for mass?04:40
thumperwallyworld_: are you going to land the instance metadata work?04:42
* thumper off to take kids ice skating04:42
thumperback after dinner04:42
* thumper away04:42
=== thumper is now known as thumper-afk
bigjoolswallyworld_: easy, all that stuff is already supported for the python provider04:49
bigjoolsso why not fix it :)04:50
wallyworld_bigjools: i'll just land the current branch for now and raise a bug. thumper is queued up behind this branch04:56
bigjoolsyeah no worries04:56
bigjoolsit was on our hitlist when doing the new provider04:57
wallyworld_bigjools: bug 119399805:15
_mup_Bug #1193998: mass provider doesn't return hardware characteristics of started instances <juju-core:Triaged> <https://launchpad.net/bugs/1193998>05:15
bigjoolsta.  if we get to it, we get to it05:16
wallyworld_fwereade__: ping06:04
jamwallyworld_: sorry I missed our 1:1. I rebooted my machine, and then wasn't paying attention. I'm making a coffee real quick and then I'll be around if you are available.06:37
wallyworld_jam: hi, no problem. i saw you log off. can we reschedule to tomorrow. a *new* tiler has just arrived and i have a little bit of stuff to sort out06:41
* wallyworld_ still has no finished bathroom :-(06:41
jamwallyworld_: wow. That bathroom is causing you so much grief. Time to just put in an Outhouse and be done with it. :)06:42
jamwallyworld_: we can reschedule for tomorrow, no problem06:42
jamcan someone give a quick look at https://codereview.appspot.com/10465043/ it is a pretty small patchd06:43
rogpeppe1mornin' all07:02
rogpeppe1fwereade__: ping07:02
=== rogpeppe1 is now known as rogpeppe
wallyworld_jam: question - i got a mp approved, and did a few small bits of followup after the lgtm, but now the bot is saying i have unapproved revisions :-(07:03
wallyworld_that never used to happen07:03
jamwallyworld_: you have to push, and let Launchpad see the new revs, and then Approve07:03
jamI re-approved it for you07:03
jamwallyworld_: you might also need to reload the MP07:03
wallyworld_ok, thanks. i thought i had pushed but maybe not07:03
jamas it stores the 'current' revision when it is loaded, and sets the approved revision to the one it saw07:04
jamwallyworld_: you did push07:04
jamyou didn't wait for the MP to refresh before you marked it Approved07:04
wallyworld_ah ok. in too much of a hurry07:04
jamso the MP was at eg rev 10, you pushed rev 11, and marked rev 10 as approved, but not rev 1107:04
rogpeppewallyworld_: any chance of a review of https://codereview.appspot.com/10447047/ or https://codereview.appspot.com/10259049/ ? would be much appreciated.07:13
wallyworld_rogpeppe: i'm running late for soccer. i'll be back in about 3 hrs. sorry07:14
rogpeppewallyworld_: np07:14
rogpeppewallyworld_: thought i was probably too late, just pushing my luck :-)07:14
rvbawallyworld_: Hi… jam's explanation on MP is enough (I think) for me to work on the missing URL() method… but I'd rather work on it in a different branch and get this one (https://codereview.appspot.com/10237046/) reviewed now… could you please have another look when you'll be back fro soccer?07:15
fwereade__rogpeppe, heyhey07:17
rogpeppefwereade__: yo!07:17
rogpeppefwereade__: wondering if you fancy a chat about moving the api connect stuff forward07:17
fwereade__rogpeppe, yeah, sounds sensible, would you start a hangout please?07:18
rogpeppefwereade__: doing07:18
rogpeppefwereade__: https://plus.google.com/hangouts/_/b51c68bb41b0ebd90ac5ae51531a89f50e298495?authuser=0&hl=en-GB07:19
jamrogpeppe, fwereade__: Mind if I listen in? I probably won't add much, but I'd like to follow the conversation.07:19
jamdanilos: be there in just a second08:00
danilosjam: ack08:01
=== thumper-afk is now known as thumper
thumperfwereade__: hey08:34
fwereade__thumper, heyhey08:34
thumperfwereade__: got some time? I've got lots of questions08:35
fwereade__thumper, how's it going?08:35
fwereade__thumper, sure, just a sec08:35
fwereade__thumper, ready08:36
* thumper starts a hangout08:36
bigjoolsfwereade__: have you got provider-specific constraints on the plan?09:09
fwereade__bigjools, sorry, 5 mins09:13
bigjoolsfwereade__: we lost you09:52
fwereade__heh, I can still hear you09:53
fwereade__bigjools, rvba, I was just asking if it was just maas-name and maas-tags, or if we missed something else09:53
fwereade__bigjools, rvba, if you've ok piggybacking name on tags I'm ok with adding tags09:53
rvbafwereade__: that's all: maas-name (can probably be supported as tags) and maas-tags.09:54
jtvfwereade__: does my MP from last week look OK now?  https://codereview.appspot.com/10407045/10:14
fwereade__jtv, huh, looks like you're right about the validation on SetConfig. grar.10:16
fwereade__jtv, I don't think it should really be the SetEnvironmentCommand's responsibility; I could accept it being *state*'s responsibility not to mess it up, but state can't do that very easily due to some decisions we made a while back10:18
jtvfwereade__: true, the place where the responsibility really belongs is missing, isn't it?10:18
jtvI was also wondering why the ec2 and openstack providers continue to write to  environ.name when it's being read without locking.10:19
fwereade__jtv, search me... I would honestly not hold up any of our providers as shining beacons of best practice10:20
jtvFunctionality is easier to retrofit than concurrency correctness.10:20
fwereade__jtv, would you be ok adding the validation in SetConfig regardless of what ec2/openstack do?10:20
jtvalready did — didn't the diff update?10:21
fwereade__jtv, can't see the changes myself10:21
fwereade__jtv, it doesn't happen automatically on push, you need to `lbox propose` again10:22
jtvOur lives would become so much easier if we could just use one kind of reviews...10:23
jtvIt's especially annoying if it means switching back to another branch while you're already working on another.10:23
fwereade__jtv, heh, yeah, I can understand that10:23
rvbawallyworld_: I just realized I forgot to lbox propose again (with the fixes) so you probably did not see the diff being updated, I've done it now (https://codereview.appspot.com/10237046/).10:24
jtv(Also, it'd be nice if lbox could actually bother to check for fatal errors...  someone once wrote that whoever doesn't do that in Go ought to be fired :)10:24
fwereade__jtv, I would be loath to give up the line-specific comments, but it seems to be fading in importance a little given the trunk-gating10:24
* fwereade__ refrains from comment10:25
jtvWhat's trunk-gating?10:25
jtvIt sounds lumber-related.10:25
fwereade__jtv, we now have tarmac to land approved branches10:25
jtvAh that10:25
jtvDo the two interact?10:25
fwereade__jtv, not very much, no -- `lbox submit -tarmac` might be nice, though10:26
fwereade__jtv, we've had a few branches get in without being `go fmt`ed10:26
jtvIt'd also need to submit a commit message, rather than just a description...10:26
fwereade__jtv, yeah, that's not such a huge change, really10:27
fwereade__jtv, but nobody's actually working on changing it at the moment10:27
jtvI also find the line-based comments rather hard to work with as a reviewee...  I'd be perfectly happy to go with the normal Canonical procedure completely.10:28
jtvWell, it looks like I've re-proposed.  Let's see if the branches will marry.10:29
jam1fwereade__: the tarmac rules should be "go fmt ./... && go build ./... && go test ./..." so we shouldn't have anything land without formatting. Are you talking about the lbox rules?11:24
fwereade__jam1, hmm, I've had lbox complain about go fmt issues on propose twice now11:25
=== jam1 is now known as jam
fwereade__jam1, does it definitely merge the code it runs, and not just merge the source branch from which it got the code?11:25
mgzdouble underscore versus 1...11:26
jamfwereade__: It should run the test suite, and then commit/not commit, It doesn't restage the commit a second time after the test suite passed. (AFAICT)11:26
mgzrunning go fmt as part of tarmac seems a little risky, as that has changed between go versions so is inconsistent11:27
jammgz: it just means we're 100% consistent in what lands on trunk :)11:27
fwereade__mgz, jam, ahh -- remind me, which go does tarmac use?11:28
mgzbut does it actually commit post fmt?11:28
jammgz: it does the fmt pre merge check11:28
jamso it should land it post fmt11:28
mgzI guess we trust fmt not the break stuff...11:28
jammgz: I just checked the last 20 revs, and none of them had complaints for *my* go fmt.11:28
mgzit's 1.0 from distro on the bot right?11:30
jamfwereade__: so I know lbox check uses 'gofmt' rather than 'go fmt' I don't know if there is a difference there. But I just checked the last 100 commits to trunk, and 100% of them have 0 output running 'go fmt ./...'11:31
jamto verify, I manually mutated a file while it was running, and it complaine.d11:31
jamfwereade__: so if you see lbox complain, let me know11:31
jambut I can't reproduce (and I'm guessing it might be because of your own local changes)11:32
jamfwereade__: could it be a go 1.1 vs 1.0.3 formatting issue?11:32
jammgz: https://plus.google.com/hangouts/_/8868e66b07fa02bdc903be4601200d470dae9ee311:33
fwereade__jam, yeah, I was wondering about that11:33
fwereade__jam, I'm on 1.111:33
jam1fwereade__: so go 1.0.3 is happy with all of the last 100 commits.11:36
jam1fwereade__: so it must be you :)11:36
=== jam1 is now known as jam
fwereade__jam, ha, ok, I'll look closer next time I see it and try to figure it out11:36
fwereade__jam, 1.1 vs 1.0.3 seems likely though11:37
rogpeppe1any chance of a review of https://codereview.appspot.com/10447047/, please? it's considerably more deleted code than added code.11:38
rogpeppe1jam: there are definitely gofmt 1.1 vs 1.0.3 differences11:43
rogpeppe1jam: there's no difference between gofmt and go fmt (they use the same tool)11:43
jamrogpeppe1: they take different args, at least.11:45
rogpeppe1jam: go fmt calls gofmt11:45
* rogpeppe1 quickly goes to check he's not talking out of his arse11:46
rogpeppe1yes, that's true11:46
rogpeppe1jam: it's even in the docs: "11:46
rogpeppe1Fmt runs the command 'gofmt -l -w' on the packages named11:46
rogpeppe1by the import paths.11:47
=== rogpeppe1 is now known as rogpeppe
rogpeppejam: i have the go1.0.3 version of gofmt in my PATH, so that's what i always use to format code11:47
TheMuerogpeppe: you've got a review11:51
rogpeppeTheMue: thanks!11:51
wallyworld_rvba: hi, sorry i missed your earlier message. i was at soccer. i look at the codereview link but the 2nd patchset is not there. i looked at the diff in lp and it looks ok. i just have a more minor comment - use %q instead of '%s' to quote a string value in a printf call11:59
rogpeppemgz: any chance you'll be able to land https://codereview.appspot.com/10439043/ today? i have a branch that depends on it.12:16
jamfwereade__: you asked for some more tests about the Machiner watcher code (to test the API .Next() behavioR)12:18
jamfwereade__: from what I can see, there is 0 client code that follows any watcher that was set up in the API12:19
fwereade__jam, ah, yes -- I guess there's some subtlety that I'm missing?12:19
jamthere is a function "state/api/watcher.go" that has "newEntityWatcher" which is not an exposed function, and nothing calls it.12:19
jamfwereade__: so the code adds an ability to start a watcher on the server12:20
jambut AFAICT there is no functionality for actually following that watcher on the client.12:20
fwereade__jam, I was hoping that the unit tests for the machiner would grow the infrastructure necessary to make a naked Next call with the returned watcher id and check that it does something12:20
fwereade__jam, the client side is a different problem, I think12:20
rvbawallyworld_: I think the new mp is there: https://codereview.appspot.com/10478045/12:20
jamfwereade__: so there should be a APIServer.resources[id] object12:21
fwereade__jam, I'm only thinking of the unit-ish-testing of the server side12:21
jamfwereade__: is that reasonable?12:21
mgzrogpeppe: as jam says, trying to do that now, but working out how to get reasonable test coverage is frustrating me12:21
jamfwereade__: also, *I* am a little confused about what Next() is vs what Changes() is .12:21
* rogpeppe goes for lunch12:21
rvbawallyworld_: I did not realize re-proposing a branch creates yes a new MP.12:21
fwereade__jam, hmm, isn't the resources stuff done via an interface anyway?12:21
wallyworld_rvba: no problem, looking now12:21
fwereade__jam, so we can completely control that, I think12:21
fwereade__jam, Next() means please-read-from-Changes()-and-return-what-you-got12:22
fwereade__jam, but in fact I don't even care about that here, now I think about it12:22
fwereade__jam, the Resourcerer or whatever interface will presumably be expected to have a watcher registered12:23
wallyworld_rvba: done. thanks for adding the notfound code12:23
fwereade__jam, the mock can check the registered resource is sane, and pick an id to give back to the machiner, and the test can then check that id came back12:23
fwereade__jam, sane? I may be forgetting something about the resources, I don't know it well12:24
rvbawallyworld_: thanks for spotting that problem :)12:24
wallyworld_no wuckers12:24
fwereade__jam, fwiw, there is some rambling in the -wip https://codereview.appspot.com/10495043 that I was working on over the weekend12:25
jamfwereade__: right now the API for Resources is that they have a "Stop()" method.12:25
jamhowever, he could probably cast it to the actual Watcher12:25
jambecause we humans know its actual type12:25
fwereade__jam, +112:25
fwereade__jam, in fact, independent of its relevance this second, I would appreciate your thoughts on that CL12:26
fwereade__jam, it's a bit dashed off because I have other stuff to do but I hope to land something related eventually12:27
jamfwereade__: I will try to look at the code you mention.12:30
jamfwereade__: in the mean time, you mention Next12:30
fwereade__jtv, LGTM, just one tweak12:30
jambut all I specifically see is "Changes"12:30
jtvThanks fwereade__12:30
fwereade__jam, yeah, I had my layers confused12:30
jamthe thing stored in the resources is a state.EntityWatcher12:30
fwereade__jam, and there was/will be some sort of Next method somewhere that takes a resource Id and returns the next thing to come out of its changes channel (or an error if it's stopped)12:32
fwereade__jam, it appears that does not exist at present12:32
fwereade__jam, but it's not actually needed for testing this12:32
fwereade__jam, because we can write unitier tests anyway without bringing it into the mix12:33
fwereade__jam, sorry, I didn't think that one all the way through12:33
jamfwereade__: right, we were a bit confused because we couldn't find (a) anything client-side that could actually follow a watcher and (b) something called Next() :)12:34
jamfwereade__: apparently (a) has been designed and is called Next()12:35
fwereade__jam, that said, to write a client watcher, we will need to make Next calls -- and there is code in history that does that12:35
jambut hasn't been implemented12:35
fwereade__jam, I think it was removed with the change12:35
jamfwereade__: there is nothing in trunk today that I could find.12:35
jamfwereade__: well, there is for the AllWatcher12:36
fwereade__jam, it would have gone in one of dimitern's commits the week before he left12:36
jambut not for the EntityWatchers that we have.12:36
fwereade__jam, if you find that code it will serve as a useful model12:36
fwereade__jam, but it had a couple of issues -- I think the entitywatcher variant was ok, but the lifecyclewatcher would drop events12:37
fwereade__jam, there are a couple of possible approaches to that12:38
jamfwereade__: drop all events so nobody notices?12:38
fwereade__jam, the description in CL above really is relevant there :)12:38
fwereade__jam, sorted12:38
fwereade__jam, let's go shopping12:38
fwereade__jam, basically we can either implement client-side coalescence when a second change comes in before we've delivered the first12:39
fwereade__jam, (lots of work, duplicated from elsewhere, although it cold probably be factored into cleanliness)12:39
fwereade__jam, or we can flip-flop client-side watchers from send to receive mode12:40
fwereade__jam, so we get the result of Next(), then sit and wait until our client reads it from our channel, and only then make another Next call12:40
fwereade__jam, this is pretty nice, and a hell of a lot simpler12:41
jamfwereade__: doesn't that leave you with stale Next() results? Is it possible to not call Next until someone actually cares and we have the coallescing done in the server?12:41
fwereade__jam, well, the Next calls do in fact cause some coalescence on the server12:41
fwereade__jam, because the watcher coalesces internally until someone reads from Changes, and that only happens when we call Next12:42
fwereade__jam, however, there is going to be an unintended consequence there with the code as it stands12:42
fwereade__jam, specifically: (1) api server syncs state (2) client calls Next, no more changes (3) state syncs again, 100 changes come in (4) the first change is delivered on Changes and returned from Next (5) the client has to handle that one, and call Next *again* before he sees the rest of that block of 100 changes12:44
jamfwereade__: do you see a block of changes, or just one-by-one ?12:45
fwereade__jam, in theory you can see blocks12:45
fwereade__jam, how much you do in practice depends on the specific watcher12:45
jamfwereade__: EntityWatcher.Changes() has a channel that is essentially just a boolean "something happened"12:46
jamI guess LifeCycleWatcher gets an array of strings?12:46
fwereade__jam, and they are all currently implemented in a way that, given the characteristics of the underlying state/watcher implementation, tends towards single events12:46
fwereade__jam, yeah12:47
fwereade__jam, coalescence is indeed much easier for the entity watchers12:48
fwereade__jam, but even then, it tends twoards suboptimal behaviour12:48
fwereade__jam, hmm, actually, maybe not so much there12:48
fwereade__jam, maybe single documents are ok across the board12:48
jamfwereade__: well getting 100 'something has changed' calls all at once is a bit annoying. It really depends how the backlog is treated.12:49
fwereade__jam, agreed12:49
fwereade__jam, that shouldn't be a problem with the entity watcher12:50
fwereade__jam, it is a problem I recently discovered in the scope watcher, though12:50
fwereade__jam, that's that CL12:50
=== fwereade__ is now known as fwereade
jamfwereade__: mgz added a call to Changes, does it look reasonable to you? (https://code.launchpad.net/~gz/juju-core/057-api-machiner-watch/+merge/170586 line 212, CL is in the process of updating)12:56
fwereadejam, mgz, that looks perfect in essence  but I'm suspicious of getting that event out of it; just a sec12:57
mgzthat event?12:58
mgzNotNil is all I;m asking of it12:58
mgzI'd *like* to make that a useful assertion12:58
fwereademgz, according to the previous implementation you wouldn't have got one at all12:58
mgzand maybe have proper checks for yeah, no event at all/timeout/whatver12:59
fwereademgz, because that one read and returned the initial guaranteed Changes() event as part of the Watch() call12:59
fwereademgz, that approach makes some sense, I think, because all watchers are meant to return an initial event describing the difference between no-state and the current state13:00
fwereademgz, and it would be nice to help the client watcher implementation by delivering the first event straight-off13:00
fwereademgz, so in that case you'd be able to extract a resource that's an EntityWatcher for the machine you know about, and do a quick check that no events are produced until you write a change to the machine13:02
mgzfwereade: I would *really* like to see an example test along those lines to work off13:02
fwereademgz, just as you do, but with one extra check verifying the bit of behaviour you missed because you had no way to know about it13:02
fwereademgz, I just (at least partially) unfucked most of the ones in state actually13:03
fwereademgz, state/watcher_test.go13:03
fwereademgz, they're internal at the moment but I'm fine promoting them somewhere else if they're useful to you13:04
mgzwhat do you mean by internal?13:06
fwereademgz, NotifyWatcherC looks like it might actually be *exactly* what you need13:06
fwereademgz, in the state_test package13:07
mgzbut... I could just use that, no?13:07
fwereademgz, hmm, I thought you didn't get _test.go code compiled in except for the package currently under test13:07
jamfwereade: correct13:08
mgzsurely I can just import state/state_test13:08
jamyou have to move it to a testing module13:08
jammgz: nope13:08
jamit would be 'state_test' but that won't look up in the module path13:09
mgzokay, so I'll land this with copy-code, then maybe move that13:09
* fwereade grudgingly approves of mg's plan, so long as he follows up with the fix asap ;p13:14
mgz...I keep getting "panic: watcher was stopped cleanly" instead of any useful failures13:23
rogpeppemgz: just looked at your branch13:23
rogpeppemgz: one thing:13:23
mgzI guess the stop needs do be a defer or something13:23
rogpeppemgz: the event coming off the entity watcher channel can never be nil13:24
rogpeppemgz: it's a struct{}13:24
mgzrogpeppe: that was an entirely stub assert13:24
rogpeppemgz: ah13:24
rogpeppemgz: the test should probably be something like: select {case _, ok := <-channel: c.Assert(ok, Equals, true); case <-time.After(timeout): c.Fatal("timed out")}13:25
rogpeppeniemeyer: hiya13:25
niemeyerrogpeppe: Hey, good morning13:26
mgzrogpeppe: I tried that... (roughly)13:26
mgzI don't see why I'm getting into MustErr at all...13:27
rogpeppemgz: and that's when you got your "watcher stopped cleanly" error?13:27
rogpeppemgz: hmm13:27
rogpeppemgz: where were you seeing that error?13:27
mgzI get that with my branch just by making an assert fail inside the select13:32
mgzrogpeppe: ^13:34
mgzI'll have some lunch, then everything will make sense13:34
* rogpeppe pulls mgz's branch13:35
rogpeppemgz: what revision of mgo are you using? it's probably not a related issue, but all those goroutines stuck at labix.org/v2/mgo/server.go:227 look suspicious13:44
rogpeppemgz: hmm, the tests pass for me (when testing that package on its own, at any rate)13:50
rogpeppemgz: tests pass for me altogether - i cannot reproduce your issue13:54
=== wedgwood_away is now known as wedgwood
rvbaCould you guys please update the gwacl library in the landing environment (or tell me how to do it)?  I've made a change to gwacl recently and I need it to land a branch in juju-core.14:20
mgzhm, not having any joy isolating this14:49
rvbamgz: sorry to bother you, but would you happen to know how to update gwacl in juju-core's landing environment?  I can't land my approved branch because I need the last version of gwacl there.14:56
rvbajam: ^ ? I know you're the tarmac master :).14:58
fwereaderogpeppe, LGTM15:05
rogpeppefwereade: ta!15:05
mgzrvba: sorry, didn't answer because I'd need to find out15:05
rogpeppefwereade: i was surprised how easy it was actually15:06
mgzI think it's pretty trivial to bump the dep15:06
rvbamgz: I guess bumping the dep is easy… but I'm not sure who has access to the tarmac machine… I certainly don't.15:08
mgzhm, can't find the details anywhere, I'll bug jam when he's around again15:11
rvbamgz: okay, thanks.15:12
rogpeppemgz: are you still having the "watcher died cleanly" issue?15:16
mgzrogpeppe: yeah, with trunk mgo15:16
rogpeppemgz: go1.0.3 ?15:17
mgzit's fine if the test passes, I just can't get it to fail15:17
rogpeppemgz: ah, i think i know what your problem might be15:18
mgzrogpeppe: 1.0.2-2 from dist on raring15:18
rogpeppemgz: if your test fails, you will never stop the watcher15:19
rogpeppemgz: so the state will be torn down and the watcher will see its state/watcher torn down15:19
rogpeppemgz: tbh the error message should be better15:19
mgzright, that was my assumption, need the stop in some kind of defer15:20
rogpeppemgz: i think using MustErr in EntityWatcher.loop is misleading15:20
mgzbut... the error/expection being backwards... and a panic, is mysterious15:20
rogpeppemgz: there's no indication that the *underlying* state/watcher Watcher might have been torn down15:21
rogpeppemgz: i think it's worth changing the error message there, probably15:21
rogpeppemgz: yes, you need to defer the Stop15:21
jamrvba: will updating gwacl break juju core without your branch?15:46
rvbajam: no15:47
jamrvba: generally you have to ping someone who knows how to ssh into tarmac-bot, I've told some people (wallyworld has done it), but I'm a reasonable alternative.15:47
jamrvba: now on 12915:48
rvbajam: but that's a good question… what it I wanted to make a non-backward compatible change to gwacl (and then land the required change in the provider)?15:48
jamrvba: we don't have dep managment, so it is either always run tip and update on every revision, or do it manually.15:48
jamrvba: then we need to land your patch as we update the dependency15:48
rvbajam: all right :)15:48
jamrvba: we want to end up with a file that says what version of each dependency to use, but we don't have that yet15:49
mgzjam: can you record the ip of the machine somewhere?15:49
jammgz: 'nova list lcy02' when you use the shared credentials15:49
mgzI'm assuming I could get in if I knew where it was :)15:49
mgzah, yeah, that'd work15:49
rvbajam: thanks for your help… I'm landing my branch now.15:50
jammgz: ssh in as ubuntu, sudo su - tarmac15:50
rogpeppefwereade: you've got a review of https://codereview.appspot.com/10495043/16:01
rogpepperight, time to go and trim a hedge19:10
rogpeppeg'night all19:10
thumpermorning folks21:19
hatchmorning thumper21:29
mramm2morning thumper21:38
thumpermramm2: writing an email :)21:38
thumpermramm2: but thought you might like to catch up after that?21:38
thumperwallyworld: can we chat when you are around?22:11
wallyworldthumper: give me 30 mins or so. gotta have breakfast and tidy the house for the cleaner22:11
thumperwallyworld: sure, np22:11
thumperfwereade: around in your evening?22:41
dpb1thumper: hi, if I have some feedback on an MP, where should I put it?22:50
dpb1this one specifically, I just tested it: https://code.launchpad.net/~themue/juju-core/029-config-get-all/+merge/17078922:50
thumperdpb1: at the moment, in the reitveld review22:50
thumperotherwise the dev will likely miss it22:50
dpb1thumper: ok22:50
thumperdpb1: bonus points if you log in to reitveld with an email address that launchpad knows about :)22:51
dpb1ahh, ok  will do22:51
wallyworldthumper: hi, did you want a chat now? https://plus.google.com/hangouts/_/8868e66b07fa02bdc903be4601200d470dae9ee322:52
thumperwallyworld: ack22:52
thumperwallyworld: https://codereview.appspot.com/10478043/22:59
wallyworldthumper: in the above mp, the 2nd patch set doesn't seem to be pushed up23:27
thumperwallyworld: it hasn't23:27
thumperwallyworld: because it is a one word addition to an error23:27
thumperwallyworld: and a documentation change23:27
wallyworldsure, normally when i see Done i look for the change to be there23:28
=== wedgwood is now known as wedgwood_away
* thumper considers food23:33
thumperand perhaps another coffee23:33
* thumper goes to inspect the fridge23:34

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!