| thumper | wallyworld_: you around? | 01:24 |
|---|---|---|
| wallyworld_ | yep | 01:24 |
| thumper | good | 01:25 |
| * thumper is proposing the lxc-container branch *again* | 01:25 | |
| wallyworld_ | where else would i be | 01:25 |
| thumper | wallyworld_: I dunno, fucking around? | 01:25 |
| bigjools | enjoying a hot shower? | 01:25 |
| wallyworld_ | well, the weather is nice outside | 01:25 |
| * thumper snorts | 01:25 | |
| wallyworld_ | not funny | 01:25 |
| thumper | well, it is shit here | 01:25 |
| thumper | wallyworld_: 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 first | 01:26 |
| bigjools | hope you took pictures | 01:27 |
| wallyworld_ | yep | 01:27 |
| thumper | wow | 01:27 |
| thumper | been watching some home renovation programs on sky, damn scary what some builders do | 01:27 |
| thumper | hoping to get some that don't suck for our work | 01:28 |
| bigjools | I can only think of one profession that doesn't con customers more than builders | 01:28 |
| * thumper waits for lbox to continue generating the diff | 01:28 | |
| wallyworld_ | yeah. and i got the worst tiler in queensland according to people who i have talked to since | 01:28 |
| bigjools | that does, I mean | 01:28 |
| thumper | bigjools: software developers? | 01:28 |
| bigjools | thumper: got it in one | 01:28 |
| wallyworld_ | politician | 01:28 |
| bigjools | I don't consider politics to be a profession | 01:28 |
| thumper | ok, perhaps politicians | 01:28 |
| thumper | haha | 01:28 |
| bigjools | serial troughers maybe | 01:29 |
| thumper | wallyworld_: https://codereview.appspot.com/10370044 another round? | 01:29 |
| wallyworld_ | alright | 01:29 |
| thumper | ta | 01:29 |
| thumper | dog is being a nutter today | 01:30 |
| thumper | is outside | 01:30 |
| bigjools | so much for this being a family channel :) | 01:30 |
| thumper | yeah, I felt guilty as soon as I swore :) | 01:31 |
| thumper | you guys are a bad influence | 01:31 |
| bigjools | I thought I was bad until I met Ian | 01:32 |
| thumper | heh | 01:32 |
| thumper | I was much worse when I first went to the UK | 01:32 |
| thumper | had to tone things down a lot | 01:32 |
| thumper | and not use swearing as punctuation, or as adjectives too often | 01:32 |
| bigjools | I expect the politically correct HR drones had some policy on it ... | 01:33 |
| bigjools | everyone got asked to go on some "sensitivity" training at one of the banks I was at | 01:33 |
| bigjools | as a contractor I told them to get fucked | 01: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 |
| thumper | kk | 01:34 |
| thumper | bigjools: haha | 01:35 |
| bigjools | thumper: there's no crisis that can't be solved by sending in the diversity coordinators, apparently | 01:35 |
| thumper | pfft | 01:36 |
| bigjools | was that a fart? | 01:36 |
| thumper | no | 01:36 |
| thumper | like *snort* | 01:37 |
| thumper | things you miss by typing stuff | 01:37 |
| thumper | miss all that non-word verbal communication | 01:37 |
| bigjools | aye, it's hard to talk without waving your arms around | 01:38 |
| thumper | wallyworld_: 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 |
| thumper | wallyworld_: what happens if I return nil, or a real struct where all the values are nil? | 01:47 |
| wallyworld_ | just return nil | 01:47 |
| wallyworld_ | it will create an empty record in the db | 01:48 |
| wallyworld_ | with just the InstanceId and Nonce | 01:48 |
| wallyworld_ | and mem, cpu cores etc will be blank | 01:48 |
| bigjools | sigh | 02:13 |
| bigjools | seems like Go doesn;t coerce ints to floats | 02:13 |
| thumper | haha | 02:13 |
| thumper | no | 02:13 |
| thumper | it doesn't even promote ints | 02:14 |
| thumper | into bigger ints as needed | 02:14 |
| bigjools | O_o | 02:14 |
| thumper | needs explicit cast | 02:14 |
| thumper | wallyworld_: https://codereview.appspot.com/10409049 for the broker | 02:14 |
| thumper | davecheney: you could cast your "on call reviewer" eye over too maybe? | 02:15 |
| thumper | davecheney: are you still doing on-call reviewing in your current role? | 02:15 |
| thumper | wallyworld_: ta | 02:25 |
| wallyworld_ | np | 02:25 |
| thumper | wallyworld_: what is the point of the PublicAddress, PrivateAddress, and InstanceId functions on environs.EnvironProvider? | 02:47 |
| thumper | they don't seem to make any sense | 02:47 |
| thumper | why 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 sense | 02:49 |
| thumper | huh? | 02:49 |
| thumper | that is nuts | 02:49 |
| thumper | sounds broken | 02:50 |
| wallyworld_ | i'm not across the design decisions there, before my time | 02:50 |
| * thumper nods | 02:50 | |
| wallyworld_ | but i agree with you | 02:50 |
| thumper | I'll be up late tonight to talk with fwereade__ anyway | 02:50 |
| thumper | I'll add it to my WTF question list | 02:50 |
| wallyworld_ | if i'm around i'll listen in | 02:50 |
| wallyworld_ | i think william wants to refactor all this stuff | 02:51 |
| wallyworld_ | there's a few interfaces with inappropriate methods | 02:51 |
| davecheney | thumper: am I on call today ? | 02:56 |
| davecheney | as to the other question, NFI | 02:56 |
| thumper | davecheney: according to the juju google calendar | 02:57 |
| * thumper does school run | 02:57 | |
| davecheney | my bad | 02:57 |
| bigjools | wallyworld_: 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 code | 03:13 |
| bigjools | wallyworld_: is there any use for public at all then? | 03:13 |
| wallyworld_ | yes, to store the tools | 03:13 |
| wallyworld_ | snd simplestreams metadata | 03:14 |
| bigjools | does juju upload simplestreams stuff? | 03:14 |
| wallyworld_ | no | 03:14 |
| bigjools | thought that was seaprate | 03:14 |
| bigjools | ok | 03:14 |
| wallyworld_ | it is expected to be created to match the cloud | 03:14 |
| wallyworld_ | there's a tool which can be used to make a starting metadata file | 03:14 |
| wallyworld_ | the tooling needs to be refined | 03:14 |
| bigjools | so if one person uploads tools to a public place and then removes them, does someone else get to auto-upload tools? | 03:15 |
| bigjools | and how is authenticity guaranteed | 03:15 |
| wallyworld_ | it's not | 03:15 |
| bigjools | \o/ | 03:15 |
| wallyworld_ | design flaw | 03:15 |
| wallyworld_ | the idea right now is that only authorised people get to write to the public bucket | 03:15 |
| wallyworld_ | so those people need to be trusted | 03:16 |
| bigjools | so there's just one public storage | 03:16 |
| wallyworld_ | one per cloud | 03:16 |
| wallyworld_ | there's one for ec2, canonistack, hp cloud etc | 03:16 |
| wallyworld_ | the tools tarballs are currently replicated across each | 03:17 |
| bigjools | so 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 buckets | 03:18 |
| bigjools | ah ok | 03:18 |
| wallyworld_ | the new upcoming simplstreams stuff makes it a bit better | 03:18 |
| wallyworld_ | and the tarballs will be signed also | 03:18 |
| bigjools | so for our new provider, someone would upload the tools before any deployments would take place | 03:18 |
| wallyworld_ | yes | 03:19 |
| wallyworld_ | for certified public cloud partners, the tools are uploaded/maintained as part of the release process | 03:19 |
| bigjools | presumably *someone* must be thinking about some sort of GPG signature downloadable over https from a trusted site? | 03:19 |
| wallyworld_ | yes | 03:19 |
| wallyworld_ | for private clouds, we will provide tooling to make setup easy | 03:20 |
| bigjools | ok so I'll work on a change I need to make public storage available then | 03:20 |
| bigjools | was wondering whether it was really neede | 03:20 |
| bigjools | d | 03: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 from | 03:24 |
| bigjools | wallyworld_: right | 03:24 |
| bigjools | and juju at some point will gpg verify the md5 of the tools. | 03:24 |
| wallyworld_ | yes | 03:25 |
| bigjools | please tell me that is going to happen | 03:25 |
| wallyworld_ | real soon now | 03:25 |
| wallyworld_ | working on it | 03:25 |
| bigjools | splendid | 03:25 |
| * bigjools thinks about booking flights to Europe | 03:25 | |
| wallyworld_ | a few things are happening in that space, might be a release or two away | 03:25 |
| thumper | bigjools: you in IOM? | 03:27 |
| thumper | bigjools: you get to fly? | 03:27 |
| bigjools | thumper: supposed to be there | 03:27 |
| bigjools | still not sure about going | 03: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 |
| thumper | wallyworld_: are you going to land the instance metadata work? | 04:42 |
| * thumper off to take kids ice skating | 04:42 | |
| thumper | back after dinner | 04:42 |
| * thumper away | 04:42 | |
| === thumper is now known as thumper-afk | ||
| bigjools | wallyworld_: easy, all that stuff is already supported for the python provider | 04:49 |
| bigjools | so 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 branch | 04:56 |
| bigjools | yeah no worries | 04:56 |
| bigjools | it was on our hitlist when doing the new provider | 04:57 |
| wallyworld_ | bigjools: bug 1193998 | 05:15 |
| _mup_ | Bug #1193998: mass provider doesn't return hardware characteristics of started instances <juju-core:Triaged> <https://launchpad.net/bugs/1193998> | 05:15 |
| bigjools | ta. if we get to it, we get to it | 05:16 |
| wallyworld_ | fwereade__: ping | 06:04 |
| jam | wallyworld_: 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 out | 06:41 |
| * wallyworld_ still has no finished bathroom :-( | 06:41 | |
| jam | wallyworld_: wow. That bathroom is causing you so much grief. Time to just put in an Outhouse and be done with it. :) | 06:42 |
| jam | wallyworld_: we can reschedule for tomorrow, no problem | 06:42 |
| jam | can someone give a quick look at https://codereview.appspot.com/10465043/ it is a pretty small patchd | 06:43 |
| rogpeppe1 | mornin' all | 07:02 |
| rogpeppe1 | fwereade__: ping | 07: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 happen | 07:03 |
| jam | wallyworld_: you have to push, and let Launchpad see the new revs, and then Approve | 07:03 |
| jam | I re-approved it for you | 07:03 |
| jam | wallyworld_: you might also need to reload the MP | 07:03 |
| wallyworld_ | ok, thanks. i thought i had pushed but maybe not | 07:03 |
| jam | as it stores the 'current' revision when it is loaded, and sets the approved revision to the one it saw | 07:04 |
| jam | wallyworld_: you did push | 07:04 |
| jam | you didn't wait for the MP to refresh before you marked it Approved | 07:04 |
| wallyworld_ | ah ok. in too much of a hurry | 07:04 |
| jam | so the MP was at eg rev 10, you pushed rev 11, and marked rev 10 as approved, but not rev 11 | 07:04 |
| rogpeppe | wallyworld_: 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. sorry | 07:14 |
| rogpeppe | wallyworld_: np | 07:14 |
| rogpeppe | wallyworld_: thought i was probably too late, just pushing my luck :-) | 07:14 |
| rvba | wallyworld_: 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, heyhey | 07:17 |
| rogpeppe | fwereade__: yo! | 07:17 |
| rogpeppe | fwereade__: wondering if you fancy a chat about moving the api connect stuff forward | 07:17 |
| fwereade__ | rogpeppe, yeah, sounds sensible, would you start a hangout please? | 07:18 |
| rogpeppe | fwereade__: doing | 07:18 |
| rogpeppe | fwereade__: https://plus.google.com/hangouts/_/b51c68bb41b0ebd90ac5ae51531a89f50e298495?authuser=0&hl=en-GB | 07:19 |
| jam | rogpeppe, fwereade__: Mind if I listen in? I probably won't add much, but I'd like to follow the conversation. | 07:19 |
| jam | danilos: be there in just a second | 08:00 |
| danilos | jam: ack | 08:01 |
| === thumper-afk is now known as thumper | ||
| thumper | fwereade__: hey | 08:34 |
| fwereade__ | thumper, heyhey | 08:34 |
| thumper | fwereade__: got some time? I've got lots of questions | 08:35 |
| fwereade__ | thumper, how's it going? | 08:35 |
| fwereade__ | thumper, sure, just a sec | 08:35 |
| fwereade__ | thumper, ready | 08:36 |
| * thumper starts a hangout | 08:36 | |
| bigjools | fwereade__: have you got provider-specific constraints on the plan? | 09:09 |
| fwereade__ | bigjools, sorry, 5 mins | 09:13 |
| bigjools | fwereade__: we lost you | 09:52 |
| fwereade__ | heh, I can still hear you | 09:53 |
| fwereade__ | bigjools, rvba, I was just asking if it was just maas-name and maas-tags, or if we missed something else | 09:53 |
| fwereade__ | bigjools, rvba, if you've ok piggybacking name on tags I'm ok with adding tags | 09:53 |
| rvba | fwereade__: that's all: maas-name (can probably be supported as tags) and maas-tags. | 09:54 |
| jtv | fwereade__: 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 back | 10:18 |
| fwereade__ | bah | 10:18 |
| jtv | fwereade__: true, the place where the responsibility really belongs is missing, isn't it? | 10:18 |
| jtv | I 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 practice | 10:20 |
| jtv | Functionality 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 |
| jtv | already did — didn't the diff update? | 10:21 |
| fwereade__ | jtv, can't see the changes myself | 10:21 |
| fwereade__ | jtv, it doesn't happen automatically on push, you need to `lbox propose` again | 10:22 |
| jtv | Argh | 10:22 |
| jtv | Our lives would become so much easier if we could just use one kind of reviews... | 10:23 |
| jtv | It'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 that | 10:23 |
| rvba | wallyworld_: 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-gating | 10:24 |
| * fwereade__ refrains from comment | 10:25 | |
| jtv | What's trunk-gating? | 10:25 |
| jtv | It sounds lumber-related. | 10:25 |
| fwereade__ | jtv, we now have tarmac to land approved branches | 10:25 |
| jtv | Ah that | 10:25 |
| jtv | Do the two interact? | 10:25 |
| fwereade__ | jtv, not very much, no -- `lbox submit -tarmac` might be nice, though | 10:26 |
| fwereade__ | jtv, we've had a few branches get in without being `go fmt`ed | 10:26 |
| jtv | It'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, really | 10:27 |
| fwereade__ | jtv, but nobody's actually working on changing it at the moment | 10:27 |
| jtv | I 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 |
| jtv | Well, it looks like I've re-proposed. Let's see if the branches will marry. | 10:29 |
| jam1 | fwereade__: 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 now | 11: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 |
| fwereade__ | jam,^ | 11:26 |
| mgz | double underscore versus 1... | 11:26 |
| jam | fwereade__: 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 |
| mgz | running go fmt as part of tarmac seems a little risky, as that has changed between go versions so is inconsistent | 11:27 |
| jam | mgz: 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 |
| mgz | but does it actually commit post fmt? | 11:28 |
| jam | mgz: it does the fmt pre merge check | 11:28 |
| jam | so it should land it post fmt | 11:28 |
| mgz | I guess we trust fmt not the break stuff... | 11:28 |
| jam | mgz: I just checked the last 20 revs, and none of them had complaints for *my* go fmt. | 11:28 |
| mgz | it's 1.0 from distro on the bot right? | 11:30 |
| jam | fwereade__: 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 |
| jam | to verify, I manually mutated a file while it was running, and it complaine.d | 11:31 |
| jam | fwereade__: so if you see lbox complain, let me know | 11:31 |
| jam | but I can't reproduce (and I'm guessing it might be because of your own local changes) | 11:32 |
| jam | fwereade__: could it be a go 1.1 vs 1.0.3 formatting issue? | 11:32 |
| jam | mgz: https://plus.google.com/hangouts/_/8868e66b07fa02bdc903be4601200d470dae9ee3 | 11:33 |
| mgz | ya | 11:33 |
| fwereade__ | jam, yeah, I was wondering about that | 11:33 |
| fwereade__ | jam, I'm on 1.1 | 11:33 |
| jam1 | fwereade__: so go 1.0.3 is happy with all of the last 100 commits. | 11:36 |
| jam1 | fwereade__: 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 out | 11:36 |
| fwereade__ | jam, 1.1 vs 1.0.3 seems likely though | 11:37 |
| rogpeppe1 | any chance of a review of https://codereview.appspot.com/10447047/, please? it's considerably more deleted code than added code. | 11:38 |
| rogpeppe1 | jam: there are definitely gofmt 1.1 vs 1.0.3 differences | 11:43 |
| rogpeppe1 | jam: there's no difference between gofmt and go fmt (they use the same tool) | 11:43 |
| jam | rogpeppe1: they take different args, at least. | 11:45 |
| rogpeppe1 | jam: go fmt calls gofmt | 11:45 |
| * rogpeppe1 quickly goes to check he's not talking out of his arse | 11:46 | |
| rogpeppe1 | yes, that's true | 11:46 |
| rogpeppe1 | jam: it's even in the docs: " | 11:46 |
| rogpeppe1 | Fmt runs the command 'gofmt -l -w' on the packages named | 11:46 |
| rogpeppe1 | by the import paths. | 11:47 |
| rogpeppe1 | " | 11:47 |
| === rogpeppe1 is now known as rogpeppe | ||
| rogpeppe | jam: i have the go1.0.3 version of gofmt in my PATH, so that's what i always use to format code | 11:47 |
| TheMue | rogpeppe: you've got a review | 11:51 |
| rogpeppe | TheMue: 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 call | 11:59 |
| rogpeppe | mgz: any chance you'll be able to land https://codereview.appspot.com/10439043/ today? i have a branch that depends on it. | 12:16 |
| jam | fwereade__: you asked for some more tests about the Machiner watcher code (to test the API .Next() behavioR) | 12:18 |
| jam | fwereade__: from what I can see, there is 0 client code that follows any watcher that was set up in the API | 12:19 |
| fwereade__ | jam, ah, yes -- I guess there's some subtlety that I'm missing? | 12:19 |
| jam | there is a function "state/api/watcher.go" that has "newEntityWatcher" which is not an exposed function, and nothing calls it. | 12:19 |
| jam | fwereade__: so the code adds an ability to start a watcher on the server | 12:20 |
| jam | but 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 something | 12:20 |
| fwereade__ | jam, the client side is a different problem, I think | 12:20 |
| rvba | wallyworld_: I think the new mp is there: https://codereview.appspot.com/10478045/ | 12:20 |
| jam | fwereade__: so there should be a APIServer.resources[id] object | 12:21 |
| fwereade__ | jam, I'm only thinking of the unit-ish-testing of the server side | 12:21 |
| jam | fwereade__: is that reasonable? | 12:21 |
| mgz | rogpeppe: as jam says, trying to do that now, but working out how to get reasonable test coverage is frustrating me | 12:21 |
| jam | fwereade__: also, *I* am a little confused about what Next() is vs what Changes() is . | 12:21 |
| * rogpeppe goes for lunch | 12:21 | |
| rvba | wallyworld_: 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 now | 12:21 |
| fwereade__ | jam, so we can completely control that, I think | 12:21 |
| fwereade__ | jam, Next() means please-read-from-Changes()-and-return-what-you-got | 12:22 |
| fwereade__ | jam, but in fact I don't even care about that here, now I think about it | 12:22 |
| fwereade__ | jam, the Resourcerer or whatever interface will presumably be expected to have a watcher registered | 12:23 |
| wallyworld_ | rvba: done. thanks for adding the notfound code | 12: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 back | 12:23 |
| fwereade__ | jam, sane? I may be forgetting something about the resources, I don't know it well | 12:24 |
| rvba | wallyworld_: thanks for spotting that problem :) | 12:24 |
| wallyworld_ | no wuckers | 12:24 |
| fwereade__ | jam, fwiw, there is some rambling in the -wip https://codereview.appspot.com/10495043 that I was working on over the weekend | 12:25 |
| jam | fwereade__: right now the API for Resources is that they have a "Stop()" method. | 12:25 |
| jam | however, he could probably cast it to the actual Watcher | 12:25 |
| jam | because we humans know its actual type | 12:25 |
| fwereade__ | jam, +1 | 12:25 |
| fwereade__ | jam, in fact, independent of its relevance this second, I would appreciate your thoughts on that CL | 12:26 |
| fwereade__ | jam, it's a bit dashed off because I have other stuff to do but I hope to land something related eventually | 12:27 |
| jam | fwereade__: I will try to look at the code you mention. | 12:30 |
| jam | fwereade__: in the mean time, you mention Next | 12:30 |
| fwereade__ | jtv, LGTM, just one tweak | 12:30 |
| jam | but all I specifically see is "Changes" | 12:30 |
| jtv | Thanks fwereade__ | 12:30 |
| fwereade__ | jam, yeah, I had my layers confused | 12:30 |
| jam | the thing stored in the resources is a state.EntityWatcher | 12: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 present | 12:32 |
| fwereade__ | jam, but it's not actually needed for testing this | 12:32 |
| fwereade__ | jam, because we can write unitier tests anyway without bringing it into the mix | 12:33 |
| fwereade__ | jam, sorry, I didn't think that one all the way through | 12:33 |
| jam | fwereade__: 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 |
| jam | fwereade__: 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 that | 12:35 |
| jam | but hasn't been implemented | 12:35 |
| fwereade__ | jam, I think it was removed with the change | 12:35 |
| jam | fwereade__: there is nothing in trunk today that I could find. | 12:35 |
| jam | fwereade__: well, there is for the AllWatcher | 12:36 |
| fwereade__ | jam, it would have gone in one of dimitern's commits the week before he left | 12:36 |
| jam | but not for the EntityWatchers that we have. | 12:36 |
| fwereade__ | jam, if you find that code it will serve as a useful model | 12:36 |
| fwereade__ | jam, but it had a couple of issues -- I think the entitywatcher variant was ok, but the lifecyclewatcher would drop events | 12:37 |
| fwereade__ | jam, there are a couple of possible approaches to that | 12:38 |
| jam | fwereade__: drop all events so nobody notices? | 12:38 |
| jam | :) | 12:38 |
| fwereade__ | jam, the description in CL above really is relevant there :) | 12:38 |
| fwereade__ | jam, sorted | 12:38 |
| fwereade__ | jam, let's go shopping | 12:38 |
| fwereade__ | jam, basically we can either implement client-side coalescence when a second change comes in before we've delivered the first | 12: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 mode | 12: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 call | 12:40 |
| fwereade__ | jam, this is pretty nice, and a hell of a lot simpler | 12:41 |
| jam | fwereade__: 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 server | 12:41 |
| fwereade__ | jam, because the watcher coalesces internally until someone reads from Changes, and that only happens when we call Next | 12:42 |
| fwereade__ | jam, however, there is going to be an unintended consequence there with the code as it stands | 12: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 changes | 12:44 |
| jam | fwereade__: do you see a block of changes, or just one-by-one ? | 12:45 |
| fwereade__ | jam, in theory you can see blocks | 12:45 |
| fwereade__ | jam, how much you do in practice depends on the specific watcher | 12:45 |
| jam | fwereade__: EntityWatcher.Changes() has a channel that is essentially just a boolean "something happened" | 12:46 |
| jam | I 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 events | 12:46 |
| jam | s/array/slice/ | 12:46 |
| fwereade__ | jam, yeah | 12:47 |
| fwereade__ | jam, coalescence is indeed much easier for the entity watchers | 12:48 |
| fwereade__ | jam, but even then, it tends twoards suboptimal behaviour | 12:48 |
| fwereade__ | jam, hmm, actually, maybe not so much there | 12:48 |
| fwereade__ | jam, maybe single documents are ok across the board | 12:48 |
| jam | fwereade__: 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, agreed | 12:49 |
| fwereade__ | jam, that shouldn't be a problem with the entity watcher | 12:50 |
| fwereade__ | jam, it is a problem I recently discovered in the scope watcher, though | 12:50 |
| fwereade__ | jam, that's that CL | 12:50 |
| === fwereade__ is now known as fwereade | ||
| jam | fwereade__: 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 |
| fwereade | jam, mgz, that looks perfect in essence but I'm suspicious of getting that event out of it; just a sec | 12:57 |
| mgz | that event? | 12:58 |
| mgz | NotNil is all I;m asking of it | 12:58 |
| mgz | I'd *like* to make that a useful assertion | 12:58 |
| fwereade | mgz, according to the previous implementation you wouldn't have got one at all | 12:58 |
| mgz | and maybe have proper checks for yeah, no event at all/timeout/whatver | 12:59 |
| fwereade | mgz, because that one read and returned the initial guaranteed Changes() event as part of the Watch() call | 12:59 |
| fwereade | mgz, 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 state | 13:00 |
| fwereade | mgz, and it would be nice to help the client watcher implementation by delivering the first event straight-off | 13:00 |
| fwereade | mgz, 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 machine | 13:02 |
| mgz | fwereade: I would *really* like to see an example test along those lines to work off | 13:02 |
| fwereade | mgz, just as you do, but with one extra check verifying the bit of behaviour you missed because you had no way to know about it | 13:02 |
| fwereade | mgz, I just (at least partially) unfucked most of the ones in state actually | 13:03 |
| fwereade | mgz, state/watcher_test.go | 13:03 |
| mgz | thanks | 13:03 |
| fwereade | mgz, they're internal at the moment but I'm fine promoting them somewhere else if they're useful to you | 13:04 |
| mgz | what do you mean by internal? | 13:06 |
| fwereade | mgz, NotifyWatcherC looks like it might actually be *exactly* what you need | 13:06 |
| fwereade | mgz, in the state_test package | 13:07 |
| mgz | but... I could just use that, no? | 13:07 |
| fwereade | mgz, hmm, I thought you didn't get _test.go code compiled in except for the package currently under test | 13:07 |
| jam | fwereade: correct | 13:08 |
| mgz | surely I can just import state/state_test | 13:08 |
| jam | you have to move it to a testing module | 13:08 |
| jam | mgz: nope | 13:08 |
| mgz | ;_; | 13:08 |
| jam | it would be 'state_test' but that won't look up in the module path | 13:09 |
| mgz | okay, so I'll land this with copy-code, then maybe move that | 13:09 |
| * fwereade grudgingly approves of mg's plan, so long as he follows up with the fix asap ;p | 13:14 | |
| fwereade | mgz's | 13:14 |
| mgz | ...I keep getting "panic: watcher was stopped cleanly" instead of any useful failures | 13:23 |
| rogpeppe | mgz: just looked at your branch | 13:23 |
| rogpeppe | mgz: one thing: | 13:23 |
| mgz | I guess the stop needs do be a defer or something | 13:23 |
| rogpeppe | mgz: the event coming off the entity watcher channel can never be nil | 13:24 |
| rogpeppe | mgz: it's a struct{} | 13:24 |
| mgz | rogpeppe: that was an entirely stub assert | 13:24 |
| rogpeppe | mgz: ah | 13:24 |
| rogpeppe | mgz: 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 |
| rogpeppe | niemeyer: hiya | 13:25 |
| niemeyer | rogpeppe: Hey, good morning | 13:26 |
| mgz | rogpeppe: I tried that... (roughly) | 13:26 |
| mgz | I don't see why I'm getting into MustErr at all... | 13:27 |
| rogpeppe | mgz: and that's when you got your "watcher stopped cleanly" error? | 13:27 |
| rogpeppe | mgz: hmm | 13:27 |
| rogpeppe | mgz: where were you seeing that error? | 13:27 |
| mgz | http://paste.ubuntu.com/5795495/ | 13:29 |
| mgz | I get that with my branch just by making an assert fail inside the select | 13:32 |
| mgz | rogpeppe: ^ | 13:34 |
| mgz | I'll have some lunch, then everything will make sense | 13:34 |
| * rogpeppe pulls mgz's branch | 13:35 | |
| rogpeppe | mgz: 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 suspicious | 13:44 |
| rogpeppe | mgz: hmm, the tests pass for me (when testing that package on its own, at any rate) | 13:50 |
| rogpeppe | mgz: tests pass for me altogether - i cannot reproduce your issue | 13:54 |
| === wedgwood_away is now known as wedgwood | ||
| rvba | Could 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 |
| mgz | hm, not having any joy isolating this | 14:49 |
| rvba | mgz: 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 |
| rvba | jam: ^ ? I know you're the tarmac master :). | 14:58 |
| fwereade | rogpeppe, LGTM | 15:05 |
| rogpeppe | fwereade: ta! | 15:05 |
| mgz | rvba: sorry, didn't answer because I'd need to find out | 15:05 |
| rogpeppe | fwereade: i was surprised how easy it was actually | 15:06 |
| mgz | I think it's pretty trivial to bump the dep | 15:06 |
| rvba | mgz: 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 |
| mgz | hm, can't find the details anywhere, I'll bug jam when he's around again | 15:11 |
| rvba | mgz: okay, thanks. | 15:12 |
| rogpeppe | mgz: are you still having the "watcher died cleanly" issue? | 15:16 |
| mgz | rogpeppe: yeah, with trunk mgo | 15:16 |
| rogpeppe | mgz: go1.0.3 ? | 15:17 |
| mgz | it's fine if the test passes, I just can't get it to fail | 15:17 |
| rogpeppe | mgz: ah, i think i know what your problem might be | 15:18 |
| mgz | rogpeppe: 1.0.2-2 from dist on raring | 15:18 |
| rogpeppe | mgz: if your test fails, you will never stop the watcher | 15:19 |
| rogpeppe | mgz: so the state will be torn down and the watcher will see its state/watcher torn down | 15:19 |
| rogpeppe | mgz: tbh the error message should be better | 15:19 |
| mgz | right, that was my assumption, need the stop in some kind of defer | 15:20 |
| rogpeppe | mgz: i think using MustErr in EntityWatcher.loop is misleading | 15:20 |
| mgz | but... the error/expection being backwards... and a panic, is mysterious | 15:20 |
| rogpeppe | mgz: there's no indication that the *underlying* state/watcher Watcher might have been torn down | 15:21 |
| rogpeppe | mgz: i think it's worth changing the error message there, probably | 15:21 |
| rogpeppe | mgz: yes, you need to defer the Stop | 15:21 |
| jam | rvba: will updating gwacl break juju core without your branch? | 15:46 |
| rvba | jam: no | 15:47 |
| jam | rvba: 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 |
| jam | rvba: now on 129 | 15:48 |
| rvba | jam: 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 |
| jam | rvba: we don't have dep managment, so it is either always run tip and update on every revision, or do it manually. | 15:48 |
| jam | rvba: then we need to land your patch as we update the dependency | 15:48 |
| rvba | jam: all right :) | 15:48 |
| jam | rvba: we want to end up with a file that says what version of each dependency to use, but we don't have that yet | 15:49 |
| mgz | jam: can you record the ip of the machine somewhere? | 15:49 |
| jam | mgz: 'nova list lcy02' when you use the shared credentials | 15:49 |
| mgz | I'm assuming I could get in if I knew where it was :) | 15:49 |
| jam | mgz: 10.55.63.190 | 15:49 |
| mgz | ah, yeah, that'd work | 15:49 |
| rvba | jam: thanks for your help… I'm landing my branch now. | 15:50 |
| jam | mgz: ssh in as ubuntu, sudo su - tarmac | 15:50 |
| rogpeppe | fwereade: you've got a review of https://codereview.appspot.com/10495043/ | 16:01 |
| rogpeppe | right, time to go and trim a hedge | 19:10 |
| rogpeppe | g'night all | 19:10 |
| thumper | morning folks | 21:19 |
| hatch | morning thumper | 21:29 |
| mramm2 | morning thumper | 21:38 |
| thumper | mramm2: writing an email :) | 21:38 |
| thumper | mramm2: but thought you might like to catch up after that? | 21:38 |
| mramm2 | sure | 21:39 |
| thumper | wallyworld: can we chat when you are around? | 22:11 |
| wallyworld | thumper: give me 30 mins or so. gotta have breakfast and tidy the house for the cleaner | 22:11 |
| thumper | wallyworld: sure, np | 22:11 |
| thumper | fwereade: around in your evening? | 22:41 |
| dpb1 | thumper: hi, if I have some feedback on an MP, where should I put it? | 22:50 |
| dpb1 | this one specifically, I just tested it: https://code.launchpad.net/~themue/juju-core/029-config-get-all/+merge/170789 | 22:50 |
| thumper | dpb1: at the moment, in the reitveld review | 22:50 |
| thumper | otherwise the dev will likely miss it | 22:50 |
| dpb1 | thumper: ok | 22:50 |
| thumper | dpb1: bonus points if you log in to reitveld with an email address that launchpad knows about :) | 22:51 |
| dpb1 | ahh, ok will do | 22:51 |
| wallyworld | thumper: hi, did you want a chat now? https://plus.google.com/hangouts/_/8868e66b07fa02bdc903be4601200d470dae9ee3 | 22:52 |
| thumper | wallyworld: ack | 22:52 |
| thumper | wallyworld: https://codereview.appspot.com/10478043/ | 22:59 |
| wallyworld | thumper: in the above mp, the 2nd patch set doesn't seem to be pushed up | 23:27 |
| thumper | wallyworld: it hasn't | 23:27 |
| thumper | wallyworld: because it is a one word addition to an error | 23:27 |
| thumper | wallyworld: and a documentation change | 23:27 |
| thumper | s/function/method | 23:27 |
| wallyworld | sure, normally when i see Done i look for the change to be there | 23:28 |
| thumper | :) | 23:29 |
| === wedgwood is now known as wedgwood_away | ||
| * thumper considers food | 23:33 | |
| thumper | and perhaps another coffee | 23:33 |
| * thumper goes to inspect the fridge | 23:34 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!