davecheney | wallyworld: can you get to launchpad atm ? | 01:54 |
---|---|---|
* wallyworld checks | 01:55 | |
wallyworld | davecheney: appears ok, what error are you seeing? | 01:55 |
davecheney | https://launchpad.net/juju-core times out | 01:55 |
wallyworld | works quickly for me | 01:55 |
wallyworld | maybe they were doing a deployment? | 01:56 |
wallyworld | still broken? | 01:56 |
davecheney | yup | 01:56 |
davecheney | trying from another site | 01:56 |
davecheney | oh, finally | 01:56 |
wallyworld | good luck | 01:56 |
wallyworld | \o/ | 01:56 |
davecheney | launchpad is slow as fuck for me most days | 01:56 |
davecheney | but this was unusually lethargic | 01:56 |
wallyworld | it's not bad for me | 01:57 |
wallyworld | there's been a lot of work into performance improvements over the last 12 months | 01:57 |
davecheney | what times to they schedule deployments ? | 01:57 |
wallyworld | if you have any particular pages that are slow, let me know and i'll look into it | 01:57 |
davecheney | everythign is slow for me | 01:58 |
wallyworld | no down time deployments are done as needed, and should be transparent | 01:58 |
davecheney | 2-5 second page loag times | 01:58 |
davecheney | multiple locations | 01:58 |
davecheney | multiple computers | 01:58 |
wallyworld | fast down time deployments are done 3 times a day and result in about a 2 second outage | 01:58 |
wallyworld | juju-core just loaded for me in 1 second | 01:58 |
wallyworld | cold | 01:59 |
wallyworld | maybe your connection has higher latency to the data centres? | 01:59 |
davecheney | https://launchpad.net/juju-core/+milestone/1.9.3 4 seconds | 01:59 |
wallyworld | 0.52 for me | 01:59 |
davecheney | i'm on iinet, your on tpg | 02:00 |
wallyworld | yeah | 02:00 |
davecheney | neither of those are top shelf isps | 02:00 |
wallyworld | nope | 02:00 |
davecheney | but this happens for me from different locations | 02:00 |
wallyworld | but cheap :-) | 02:00 |
davecheney | different computers | 02:00 |
davecheney | was in melbourne over the weekend on optus cable | 02:00 |
davecheney | same | 02:00 |
wallyworld | hmmm. i can't easily explain the difference | 02:00 |
davecheney | don't worry | 02:00 |
davecheney | i'm used to it | 02:01 |
wallyworld | :-( | 02:01 |
wallyworld | makes it hard to work efficiently | 02:01 |
davecheney | yup | 02:01 |
wallyworld | the guys are focused on performance issues, but they do need more than one data sample to be able to do anything concrete | 02:02 |
wallyworld | especially if it is not slow across the board | 02:02 |
davecheney | who should I complain too ? | 02:02 |
wallyworld | Purple squad is on maintenance and hang out in #launchpad-dev - they are very responsive | 02:02 |
wallyworld | another option is to go to a page on qastaging.launchpad.net and turn on tracing, i can help you with that if you want | 02:03 |
wallyworld | qastaging is slower than prod, but it may show some issues worth looking at | 02:04 |
wallyworld | have you checked your latency to the data centre? | 02:04 |
davecheney | http://d.pr/KwdU | 02:04 |
davecheney | http://d.pr/i/JVOI | 02:05 |
davecheney | solid 312ms | 02:05 |
davecheney | anyway, it's not your job to debug lp problems | 02:06 |
davecheney | thanks for checking | 02:06 |
wallyworld | no problems, but it would be nice to sort it out | 02:08 |
wallyworld | davecheney: so, out of interest, in the light blue bar, what's the waiting time vs connecting time? | 02:09 |
davecheney | connection and ssl neg is in excess of 2.5 seconds on the second screenshot | 02:10 |
davecheney | i have no idea how you are negoating faster | 02:10 |
davecheney | it's a function of rtt | 02:10 |
davecheney | wallyworld: you're using firefox, right ? | 02:10 |
wallyworld | davecheney: so, the 1.6s time i gave comes from an ajax info widget on lp, which i think developers get. i also have the connect time like you | 02:11 |
davecheney | dunno how you got, 12:59 < wallyworld> 0.52 for me then | 02:11 |
wallyworld | my graph says a waiting time of 2.2 seconds, and lp tells it it spent 1.6 seconds processing, so that leaves 0.6 seconds to get the data back to my browser i guess | 02:12 |
wallyworld | that 0.52 for +milestone above was also the lp reported processing tim | 02:12 |
davecheney | all lies i tell ya! | 02:13 |
davecheney | anyway, seriously, i don't care about this | 02:13 |
davecheney | slow I can handle, as long as it's not down | 02:13 |
wallyworld | fair enough. as a data point then, my connect time (incl ssl) is around 1.5, lp render/processing time is between 0.5 and 1.6, and receiving data to browser takes about 0.3 | 02:14 |
davecheney | ping https://codereview.appspot.com/6855101/ | 05:50 |
wallyworld | davecheney: i'd +1 it but i fear i don't know enough to be able to offer anything worthwhile. it looks ok to me though | 05:57 |
davecheney | wallyworld: thanks | 05:57 |
davecheney | you coudl always dist-upgrade and test it :) | 05:57 |
wallyworld | want me to +1 it anyway? | 05:57 |
wallyworld | i'm on quantal already | 05:57 |
davecheney | do the juju tests pass for you ? | 05:58 |
wallyworld | let me test it then | 05:58 |
wallyworld | i have not run them, will do so now | 05:58 |
wallyworld | i've just been running goose tests | 05:58 |
davecheney | go test launchpad.net/juju-core/... | 05:58 |
wallyworld | yeah, will do after i merge your branch | 05:58 |
davecheney | nah, do it before | 05:58 |
davecheney | otherwise you won't know if I fixed anything | 05:58 |
wallyworld | ok, need to pull tip first | 05:59 |
davecheney | go get -v launchpad.net/juju-core/... will do that for you | 05:59 |
davecheney | if you haven't already checked it out | 05:59 |
wallyworld | i have the code checked out etc, i'm just used to using bzr | 05:59 |
wallyworld | is go get the preferred way? | 05:59 |
davecheney | if you have never checkout out the code, it is a fast way to get all the deps | 06:00 |
davecheney | if you already have a working copy | 06:00 |
davecheney | it'll screw it up royally | 06:00 |
wallyworld | yeah, i have everything checked out and built | 06:00 |
wallyworld | i used go get right at the start | 06:00 |
davecheney | +1 | 06:00 |
davecheney | i have trunk under a cobzr branch called trunk | 06:01 |
davecheney | so whenever I want to know what others are doing i do | 06:01 |
davecheney | cobzr switch trunk ; cobzr pull | 06:01 |
wallyworld | yeah, me too | 06:01 |
davecheney | was, http://bazaar.launchpad.net/~gophers/juju-core/trunk/view/head:/README useful at all ? | 06:02 |
wallyworld | i used to use light weight checkouts etc, and bzr switch, but am trying cobzr | 06:02 |
wallyworld | it stores the branches in a hidden dir which is both convenient and annoying | 06:02 |
wallyworld | davecheney: yes! i used that to get going | 06:03 |
davecheney | cool | 06:03 |
davecheney | glad it was useful | 06:03 |
wallyworld | thanks for writing it :-) | 06:03 |
davecheney | np, you can/should/might steal it for goose | 06:03 |
wallyworld | yeah | 06:03 |
wallyworld | ok, tests fail as expected | 06:03 |
wallyworld | now to try your branch | 06:03 |
davecheney | huzzah! | 06:03 |
davecheney | it'll be whinging about can't find tools | 06:03 |
davecheney | no, from memory the actual error is it can't find a suitable ami to 'fake' boot | 06:04 |
davecheney | this change fills in the fixtures to make that happen | 06:04 |
wallyworld | error was: cannot find image satisfying constraints: error getting instance types: 404 Not Found | 06:05 |
davecheney | wallyworld: yup, that is the error | 06:05 |
wallyworld | davecheney: tests running but taking a loooooong time - appears hung running the updated test | 06:10 |
wallyworld | ie nothing after "ok launchpad.net/juju-core/environs/config 0.373s" | 06:11 |
davecheney | presences tests take over 70 seconds on my machine | 06:13 |
davecheney | gocheck reports when the test is done, not when it starts | 06:13 |
davecheney | check top | 06:13 |
davecheney | you should see a number of someting.test processes running | 06:13 |
wallyworld | davecheney: what do i look for? | 06:13 |
davecheney | a child of a process call go | 06:14 |
davecheney | │ ├─bash───go─┬─2*[6g] | 06:14 |
davecheney | │ │ ├─container.test─┬─mongod | 06:14 |
davecheney | │ │ │ └─2*[{container.test}] | 06:14 |
davecheney | │ │ ├─downloader.test───4*[{downloader.test}] | 06:14 |
davecheney | │ │ └─9*[{go}] | 06:14 |
davecheney | something like this, if pstree is your poison | 06:14 |
wallyworld | i have a dummy.test | 06:14 |
davecheney | from memory the default test timeout is 120s | 06:15 |
davecheney | it'll fail eventually | 06:15 |
wallyworld | ok, it's been a fair bit longer | 06:16 |
davecheney | gotta go | 06:17 |
davecheney | will be online in an hour or so | 06:17 |
davecheney | if tests are fucked, email juju-dev with the output | 06:17 |
davecheney | or raise an issue | 06:17 |
wallyworld | ok, bye | 06:17 |
wallyworld | just died now! | 06:18 |
wallyworld | but works when run again | 06:22 |
TheMue | Morning. | 07:47 |
fwereade | mornings | 08:05 |
TheMue | fwereade, davecheney: Hiya. | 08:14 |
fwereade | TheMue, davecheney, morning | 08:14 |
TheMue | Our new https://juju.ubuntu.com really looks good. | 08:17 |
davecheney | morning | 08:24 |
rogpeppe | fwereade, TheMue: mornin' | 08:46 |
TheMue | Hello, Mr Peppe | 08:46 |
fwereade | rogpeppe, heyhey | 08:50 |
fwereade | rogpeppe, you know the TxnRevno field, that we need to use to start document watches? | 08:51 |
rogpeppe | fwereade: yeah | 08:51 |
fwereade | rogpeppe, it appears to me as though you can *actually* just pass 0 and it'll still work anyway | 08:51 |
rogpeppe | fwereade: except you'll maybe get an event you don't need, right? | 08:51 |
fwereade | rogpeppe, I guess that perhaps you'll get an extra event sometimes | 08:51 |
rogpeppe | fwereade: maybe you don't care though | 08:52 |
fwereade | rogpeppe, it doesn't have any macro-level effects that I am sophisticated enough to detect | 08:52 |
rogpeppe | fwereade: it would mean doing strictly more work on restart | 08:53 |
fwereade | rogpeppe, yeah, I'm not proposing actually doing it | 08:53 |
fwereade | rogpeppe, the actual problem is that I was trying to use it, and doing it wrong, and couldn't see that from my tests | 08:53 |
rogpeppe | fwereade: interesting thought though - we might decide the simplicity is worth the network traffic | 08:53 |
fwereade | rogpeppe, and I am wondering if there's any way I can reliably goose it into doing the wrong thing | 08:53 |
rogpeppe | fwereade: it's just an optimisation, right? | 08:54 |
fwereade | rogpeppe, I dunno -- first time I used the API I just bunged 0 in and niemeyer was most unimpressed | 08:54 |
rogpeppe | lol | 08:54 |
rogpeppe | fwereade: well, the API doesn't actually give you the values - it just tells you that something has changed, no? | 08:55 |
fwereade | rogpeppe, I forget | 08:55 |
fwereade | rogpeppe, .Id is all I ever look at | 08:55 |
rogpeppe | fwereade: if that's true, it's never going to make any difference as long as you get at least the required events | 08:55 |
rogpeppe | fwereade: BTW, i've got a couple of CLs you might want to have a glance at if you fancy it. first is: https://codereview.appspot.com/6854107/ | 08:56 |
fwereade | rogpeppe, indeed -- anyway, I just thought it was interesting | 08:56 |
rogpeppe | fwereade: second is https://codereview.appspot.com/6856105/ (which is the last in the series) | 08:57 |
rogpeppe | fwereade: in terms of seeing problems, you'd see problems if the revno field was too high | 08:57 |
fwereade | rogpeppe, yeah | 08:57 |
rogpeppe | fwereade: so you might be able to goose it that way | 08:57 |
fwereade | rogpeppe, not from outside I think | 08:58 |
rogpeppe | fwereade: aren't you storing revnos in files? | 08:58 |
fwereade | rogpeppe, the problem was that I used the txnrevno field, instead of txn-revno, and got 0 every time | 08:58 |
fwereade | rogpeppe, I am but that's in a different area | 08:58 |
rogpeppe | fwereade: yeah, in that case, i'm not sure i can think of a test that makes any difference. | 08:59 |
rogpeppe | fwereade: 'cos if it's zero, you'll get a spurious first event, find that there's nothing to do, then do nothing | 08:59 |
fwereade | rogpeppe, you might be interested to take a look at https://codereview.appspot.com/6850105/ and https://codereview.appspot.com/6851110/ while I read yours properly | 09:00 |
fwereade | rogpeppe, yeah, exactly | 09:00 |
rogpeppe | fwereade: looking | 09:00 |
rogpeppe | fwereade: ah, i had taken a look at the first - i had one unpublished comment | 09:02 |
rogpeppe | fwereade: LGTM | 09:03 |
fwereade | rogpeppe, cheers, you have one too | 09:03 |
rogpeppe | s/dong/ding/ lol | 09:04 |
rogpeppe | fwereade: thanks | 09:05 |
fwereade | rogpeppe, and another LGTM, that look fantastic | 09:10 |
rogpeppe | fwereade: thanks! | 09:10 |
rogpeppe | fwereade: the window really is very short indeed. i think it must be on the order of <0.1s | 09:12 |
rogpeppe | fwereade: i've seen the "unauthorized access" problem precisely once | 09:12 |
fwereade | rogpeppe, fantastic | 09:17 |
rogpeppe | fwereade: basically the remote client and bootstrap-state are both racing for first access to the mongodb | 09:18 |
fwereade | rogpeppe, ah, right, ofc | 09:18 |
rogpeppe | fwereade: we're more likely to see the problem now because mgo redials about twice a second | 09:19 |
dimitern | jam, mgz: would you take a look https://codereview.appspot.com/6851112/ pls? | 09:49 |
jam | dimitern: looking | 09:51 |
TheMue | rogpeppe, fwereade: Would you take a look at https://codereview.appspot.com/6853075/? I changed and simplified the way it's working. | 10:15 |
rogpeppe | TheMue: looking | 10:16 |
TheMue | dimitern, jam: Good morning. | 10:16 |
jam | morning TheMue | 10:16 |
dimitern | TheMue: morning | 10:16 |
jam | dimitern: done | 10:19 |
dimitern | jam: thanks | 10:20 |
rogpeppe | TheMue: i like the idea (of just sourcing the file), but i'm not sure that env is the right way to print the env vars | 10:25 |
rogpeppe | TheMue: what happens if one of the vars has a newline in? | 10:25 |
TheMue | rogpeppe: Good hint, yes. | 10:26 |
TheMue | rogpeppe: Todays Py code is only grepping two values, but as a general purpose package I would like to provide all. | 10:27 |
TheMue | rogpeppe: Do you know a command to get the names of all environment variables? | 10:28 |
rogpeppe | TheMue: you could use awk | 10:28 |
TheMue | rogpeppe: Uuuh, last time I used awk has been last century. ;) | 10:29 |
rogpeppe | TheMue: something like this might do the job: | 10:31 |
rogpeppe | awk 'BEGIN {for(v in ENVIRON){s = ENVIRON[v]; gsub("\\", "\\\\", s); gsub("\n", "\\n", s); printf("%s=\"%s\"\n", v, s)}}' | 10:31 |
rogpeppe | TheMue: then you could use strconv.Unquote to read 'em in | 10:31 |
wallyworld | jam: dimitern: mgz: wanna have the standup now? | 10:32 |
TheMue | rogpeppe: Thx, I'll try. | 10:32 |
jam | works for me, is mgz here? | 10:32 |
rogpeppe | TheMue: there's probably a better tool around for doing this though | 10:32 |
rogpeppe | TheMue: you may even be able to use bash directly | 10:32 |
jam | wallyworld: I'm in mumble | 10:33 |
dimitern | wallyworld, jam: I'm ok with that | 10:33 |
* wallyworld opens mumble | 10:33 | |
wallyworld | jam: dimitern: i've hit that mumble bug again where it won't start, i have to reboot, give me a sec | 10:34 |
rogpeppe | ha that's funny, you learn something new every day. i always thought that $'foo' was the same as $foo | 10:34 |
jam | rogpeppe: do you mean $foo vs '$foo' ? | 10:37 |
jam | I haven't really seen $'foo' before. | 10:37 |
rogpeppe | jam: no | 10:37 |
rogpeppe | jam: neither had i | 10:37 |
rogpeppe | jam: except in rc | 10:37 |
rogpeppe | jam: and i'd presumed sh was similar like that | 10:37 |
rogpeppe | jam: try: echo $'foo\nbar' | 10:38 |
rogpeppe | TheMue: looks like "typeset -p -x" might give you what you need | 10:45 |
TheMue | rogpeppe: Looks nice, yes. | 10:46 |
TheMue | rogpeppe: Just fetched my old shell programming book. ;) | 10:47 |
rogpeppe | TheMue: alternatively, you could just parse the file. it wouldn't be too hard. | 10:47 |
TheMue | rogpeppe: Which one? The /etc/default/lxc? | 10:48 |
rogpeppe | TheMue: yeah | 10:48 |
TheMue | rogpeppe: Values in it could be based on environment variables too. ;) | 10:48 |
rogpeppe | TheMue: that's not too hard either | 10:49 |
rogpeppe | TheMue: os.Expand might help | 10:49 |
rogpeppe | TheMue: hmm, i dunno. at least using typeset you're guaranteed a standard format | 10:49 |
rogpeppe | TheMue: i wonder about the security implications of sourcing the shell script, but since it's in /etc, it's probably ok | 10:50 |
TheMue | rogpeppe: Yes, here this approach seems better. I started with parsing. But niemeyer had remarks. | 10:50 |
TheMue | rogpeppe: The Py code just sources it too. | 10:51 |
rogpeppe | TheMue: ok, that's interesting. | 10:51 |
rogpeppe | TheMue: what values do you need from it, BTW? | 10:51 |
TheMue | rogpeppe: For Juju only two, LXC_BRIDGE and LXC_ADDR. So they grepped it from env. But golxc is intended to be more general. So I would like to provide them all. | 10:52 |
rogpeppe | TheMue: one issue with your current approach is that you'll see env vars that are defined in the testing environment too AFAICS | 10:55 |
rogpeppe | TheMue: i think you need to start the shell command with a clean environment | 10:55 |
TheMue | rogpeppe: I'm sourcing an own file with self-defined values for testing. | 10:56 |
TheMue | rogpeppe: It's a copy, but with different values than the default values. | 10:57 |
TheMue | rogpeppe: At least some of them. | 10:57 |
rogpeppe | TheMue: i bet if you defined LXC_BRIDGE in your shell and deleted it from the "env" var, your test would still pass | 10:58 |
TheMue | rogpeppe: If it has the right value, yes. :/ | 11:00 |
wallyworld_ | jam: mgz: test one two three | 11:06 |
mgz | ta! | 11:06 |
niemeyer | Yo! | 11:39 |
mgz | hey! | 11:39 |
niemeyer | mgz: Heya | 11:45 |
niemeyer | mgz: When's our next squash game? :-) | 11:45 |
mgz | well, that would be one excuse for a holiday in brazil :) | 11:47 |
niemeyer | mgz: Consider yourself invited :) | 11:47 |
niemeyer | mgz: Just yesterday I was playing a pretty good match against a friend and reminding of that game in Copenhagen.. it was great | 11:48 |
niemeyer | fwereade: Heya | 11:48 |
niemeyer | fwereade: https://codereview.appspot.com/6850105 is ready to go in | 11:52 |
fwereade | niemeyer, sweet! tyvm | 11:52 |
niemeyer | fwereade: Thank you! | 11:52 |
* dimitern => lunch | 11:59 | |
* fwereade also | 12:02 | |
rogpeppe | niemeyer: morning! | 12:11 |
TheMue | rogpeppe: Found an even simpler way. Wonna look again? | 12:39 |
rogpeppe | TheMue: sure | 12:40 |
TheMue | rogpeppe: Thanks. | 12:40 |
rogpeppe | TheMue: what are those double quotes doing at the start of script? | 12:43 |
rogpeppe | TheMue: (printenv -0 looks good BTW) | 12:44 |
TheMue | rogpeppe: Otherwise it isn't passed to sh as one argument. I wondered too. | 12:44 |
rogpeppe | TheMue: huh? | 12:45 |
rogpeppe | TheMue: have you tried it without them? | 12:45 |
TheMue | rogpeppe: Yep. | 12:45 |
TheMue | rogpeppe: Aargh, you meant now double quotes. | 12:46 |
TheMue | rogpeppe: That works, will remove and repropose it. *facepalm* | 12:46 |
rogpeppe | TheMue: s/now/no/ ? | 12:46 |
TheMue | rogpeppe: Yep, fingers too fast. ;) | 12:46 |
TheMue | rogpeppe: So, removed. Took them too automatically. | 12:48 |
niemeyer | TheMue: I think we should move on to MAAS instead of continuing to spend cycles on this | 12:49 |
TheMue | niemeyer: I'm reading the MAAS source in parallel. The remaining two branches are really small and you would do me a favour if I could complete them. | 12:50 |
niemeyer | TheMue: I'd agree if they were ready to land, but last time I've seen there were several issues, so it spends your time, my time, Roger's time, for a branch we won't be using any time soon | 12:51 |
TheMue | niemeyer: OK, but could you please take a look today to see if the direction now is better? | 12:52 |
niemeyer | TheMue: What happens if there is an LXC variable in the Go process? | 12:55 |
rogpeppe | niemeyer: i've raised that issue | 12:57 |
TheMue | niemeyer: Ah, yeah, IC. That's a problem in the Py code too, but there only for LXC_ADDR and LXC_BRIDGE. /etc/default/lxc overwrites the processes variable. | 12:57 |
rogpeppe | TheMue: you've got some more comments | 12:58 |
TheMue | rogpeppe: Thanks. | 12:58 |
rogpeppe | niemeyer: i think it should clear environment variables before sourcing the shell script | 12:58 |
TheMue | rogpeppe: The sourcing is only in a subshell for reading the values. | 13:03 |
rogpeppe | TheMue: that subshell inherits environment variables from the caller | 13:03 |
niemeyer | TheMue: That's not how environment works | 13:03 |
niemeyer | What roger said | 13:04 |
TheMue | niemeyer: Wanted to express that for the execution of lxc commands the environment is unchanged. | 13:04 |
niemeyer | TheMue: I don't understand how that changes the points made | 13:05 |
niemeyer | TheMue: A child cannot change a parent's environment, ever | 13:05 |
niemeyer | TheMue: But I'm not sure if that's what you're saying | 13:05 |
niemeyer | I mean, at least in computing | 13:05 |
niemeyer | IN the real world a child revamps a parent's environment in its entirety | 13:06 |
TheMue | niemeyer: *lol* | 13:06 |
TheMue | niemeyer: Yes, I know. I'm only using the way of the Py code to read values out of /etc/default/lxc | 13:06 |
TheMue | niemeyer: Here they've done the same, but grepping only for the LXC_ values to then read ADDR and BRIDGE. | 13:07 |
niemeyer | TheMue: Still doesn't change the points made | 13:07 |
niemeyer | TheMue: and the point is reather simple really | 13:08 |
niemeyer | rather | 13:08 |
TheMue | niemeyer: IMHO sourcing that file overwrites the processes environment when fetchiing the values out of it. Am I right? | 13:09 |
niemeyer | <rogpeppe> niemeyer: i think it should clear environment variables before sourcing the shell script | 13:09 |
niemeyer | <rogpeppe> TheMue: that subshell inherits environment variables from the caller | 13:09 |
niemeyer | TheMue: This is right | 13:09 |
TheMue | niemeyer: So the environment I'm parsing is too large, but it contains the values of /etc/default/lxc. Still right? | 13:10 |
TheMue | niemeyer: And by clearing the environment before I would reduce it to the values of that file. | 13:11 |
niemeyer | TheMue: Right | 13:12 |
TheMue | niemeyer: OK, then I know where we gor our wires crossed. (You say so?) | 13:14 |
TheMue | s/gor/got/ | 13:14 |
niemeyer | rogpeppe: Can you please have a look at William's follow up when you have a second: https://codereview.appspot.com/6851110 | 13:35 |
niemeyer | Well, 10 minutes perhaps | 13:35 |
niemeyer | :) | 13:35 |
rogpeppe | niemeyer: will do | 13:35 |
niemeyer | rogpeppe: Thanks | 13:35 |
niemeyer | rogpeppe: One branch reviewed and double LGTMed with trivials | 13:52 |
niemeyer | rogpeppe: Only one left in the queue.. will get to that after lunch | 13:52 |
rogpeppe | niemeyer: thanks! | 13:52 |
rogpeppe | niemeyer: whee! | 13:52 |
* rogpeppe should have some lunch too | 13:53 | |
niemeyer | +1 | 13:53 |
niemeyer | :) | 13:53 |
fwereade | rogpeppe, ---------------------------------------------------------------------- | 14:45 |
fwereade | FAIL: cert_test.go:50: certSuite.TestNewCA | 14:45 |
fwereade | cert_test.go:60: | 14:45 |
fwereade | c.Assert(caCert.NotAfter.Equal(expiry), Equals, true) | 14:45 |
fwereade | ... obtained bool = false | 14:45 |
fwereade | ... expected bool = true | 14:45 |
fwereade | ---------------------------------------------------------------------- | 14:45 |
fwereade | FAIL: cert_test.go:66: certSuite.TestNewServer | 14:45 |
fwereade | cert_test.go:81: | 14:45 |
fwereade | c.Assert(srvCert.NotAfter.Equal(expiry), Equals, true) | 14:45 |
fwereade | ... obtained bool = false | 14:45 |
fwereade | ... expected bool = true | 14:45 |
fwereade | ---------------------------------------------------------------------- | 14:45 |
fwereade | FAIL: cert_test.go:95: certSuite.TestVerify | 14:45 |
fwereade | cert_test.go:104: | 14:45 |
fwereade | c.Assert(err, IsNil) | 14:45 |
fwereade | ... value x509.CertificateInvalidError = x509.CertificateInvalidError{Cert:(*x509.Certificate)(0xf8400892c0), Reason:1} ("x509: certificate has expired or is not yet valid") | 14:45 |
fwereade | OOPS: 3 passed, 3 FAILED | 14:45 |
fwereade | --- FAIL: TestAll (1.96 seconds) | 14:45 |
fwereade | FAIL | 14:45 |
fwereade | FAILlaunchpad.net/juju-core/cert1.974s | 14:45 |
fwereade | rogpeppe, on trunk -- should I install something? | 14:45 |
rogpeppe | fwereade: hmm, passes for me | 14:46 |
rogpeppe | fwereade: i'll try against a different go version | 14:46 |
fwereade | rogpeppe, I'm still using 1.0.2 | 14:46 |
fwereade | rogpeppe, (er, did we decide to standardize on 1.0.3?) | 14:46 |
rogpeppe | fwereade: i'm not sure | 14:47 |
rogpeppe | fwereade: i think maybe 1.0.2 as that's what's bundled | 14:47 |
fwereade | rogpeppe, ah, yes, then I did that deliberately | 14:47 |
* fwereade looks around shiftily | 14:47 | |
* rogpeppe builds 1.0.2 | 14:49 | |
rogpeppe | fwereade: it passes on 1.0.3 BTW | 14:49 |
fwereade | rogpeppe, I did indeed assume you wouldn't have merged stuff that didn't work for you ;) | 14:50 |
rogpeppe | fwereade: i was compiling against tip, so it may not have | 14:50 |
rogpeppe | fwereade: hmm, 1.0.2 passes for me too | 14:50 |
* fwereade can't keep up with the cool kids and their crazy avant-garde go versions | 14:51 | |
* fwereade goes off to run it more verbosely | 14:51 | |
rogpeppe | fwereade: could you get it to print out the actual times you're seeing, please | 14:51 |
rogpeppe | fwereade: i.e. c.Logf("cert notafter: %v, expiry: %v", caCert.NotAfter, expiry) | 14:52 |
fwereade | rogpeppe, cert notafter: 2012-11-29 14:53:57 +0100 CET, expiry: 2012-11-29 15:53:57 +0100 CET | 14:54 |
fwereade | rogpeppe, the otherone's equivalent | 14:54 |
rogpeppe | fwereade: interesting. something to do with daylight savings | 14:55 |
fwereade | rogpeppe, and/or timezones :) | 14:55 |
rogpeppe | fwereade: yeah | 14:55 |
fwereade | rogpeppe, anotheradvantage of the distributed team :) | 14:55 |
rogpeppe | fwereade: yeah. i am GMT atm so particularly vulnerable to that kind of error | 14:56 |
fwereade | rogpeppe, I will assume that the fix will be simple but not immediate, and merge over the top of it, ok? | 14:59 |
* TheMue wonders …oooOOO( Interesting behavior. ) | 14:59 | |
rogpeppe | fwereade: yeah, i guess so. | 14:59 |
rogpeppe | fwereade: i'm just trying to reproduce the behaviour | 14:59 |
rogpeppe | fwereade: i can't seem to reproduce the behaviour, which is a bit off | 15:08 |
rogpeppe | odd | 15:08 |
fwereade | rogpeppe, huh, weird | 15:09 |
fwereade | TheMue, can you repro? you're in my timezone I think | 15:09 |
rogpeppe | fwereade: i tried using a time which i parsed from your timestamp | 15:09 |
TheMue | fwereade: Will try. | 15:09 |
rogpeppe | fwereade: could you try this: where the test calls time.Now(), could you make it call time.Now().UTC() instead. | 15:10 |
fwereade | rogpeppe, bingo | 15:11 |
fwereade | rogpeppe, fixes all 3 | 15:12 |
rogpeppe | fwereade: hmm, it shouldn't make a difference | 15:12 |
rogpeppe | fwereade: i'll push a fix see if i can fix the underlying Go too | 15:12 |
rogpeppe | s/see/and see/ | 15:13 |
fwereade | rogpeppe, great, thanks, consider it pre-LGTMed if that's all you do :) | 15:13 |
TheMue | fwereade, rogpeppe: Same error: http://paste.ubuntu.com/1394620/ | 15:13 |
fwereade | rogpeppe, sorry, I have no reason to still be waiting for your fix, do I? can I merge? | 15:32 |
rogpeppe | fwereade: you can and may | 15:33 |
fwereade | rogpeppe, cheers | 15:34 |
rogpeppe | fwereade: i'm trying to understand the problem a little more before i commit a fix. it's weird that i can't reproduce the problem here. | 15:35 |
TheMue | rogpeppe: Clearing the environment is handled now. But no need to review so far, I'm currently diving into MAAS. Only has been a litle fix to answer your last review. | 15:40 |
rogpeppe | TheMue: ok | 15:40 |
* TheMue is stepping out a bit earlier today. Had no lunch due to pre-christmas-dinner with friends. | 15:42 | |
fwereade | TheMue, enjoy | 15:43 |
TheMue | fwereade: Thanks. | 15:44 |
niemeyer | rogpeppe: Woohay TLS | 15:58 |
niemeyer | rogpeppe: Just reviewed that last one | 15:58 |
rogpeppe | niemeyer: thanks! | 15:58 |
rogpeppe | niemeyer: BTW 0.5 seconds should be perfectly sufficient - mgo redials many times a second, and bootstrap-state takes very little time to complete | 16:00 |
rogpeppe | niemeyer: i can raise the length of time anyway if you'd like. | 16:01 |
niemeyer | rogpeppe: 0.5 second is spent in a trivial sneeze of AWS | 16:01 |
rogpeppe | niemeyer: fair enough. BTW what in the ssh logic were you thinking we might miss? | 16:02 |
niemeyer | rogpeppe: The ssh logic :-) | 16:02 |
rogpeppe | niemeyer: if we're not using ssh, why should we care about that? | 16:03 |
niemeyer | rogpeppe: ssh is battle tested as public doors.. I just feel a bit more comfortable there. | 16:04 |
rogpeppe | niemeyer: i quoted the serverPEMPath because cfg.DataDir may reasonably contain spaces. quotes or backslashes are much more unlikely though. | 16:13 |
rogpeppe | niemeyer: do you still think it's a bad idea to do that? | 16:14 |
niemeyer | rogpeppe: We know it doesn't contain spaces, in the same way we know it doesn't contain quotes or backslashes | 16:14 |
rogpeppe | niemeyer: do we know that ioutil.TempDir will never return a name with a space in? | 16:14 |
niemeyer | rogpeppe: Pretending we're being safer isn't necessary | 16:14 |
niemeyer | rogpeppe: Do we know it will never return a name with quotes? | 16:14 |
rogpeppe | niemeyer: tbh, i'd prefer to know what the quoting rules *are* for upstart... | 16:15 |
niemeyer | rogpeppe: Isn't that a line passed to a shell? | 16:15 |
rogpeppe | niemeyer: no, it goes into the upstart config file which has its own syntax | 16:16 |
niemeyer | rogpeppe: Isn't upstart taking that line from its config file and passing it to a shell? | 16:16 |
rogpeppe | niemeyer: i don't think so. | 16:16 |
niemeyer | rogpeppe: I'd be surprised | 16:16 |
rogpeppe | niemeyer: i think it invokes the command directly | 16:16 |
niemeyer | rogpeppe: But it doesn't really matter | 16:16 |
rogpeppe | niemeyer: i did delve into the source once to try to find out | 16:17 |
niemeyer | rogpeppe: The points made still hold | 16:17 |
rogpeppe | niemeyer: does that mean i should remove all the shquote calls elsewhere? | 16:17 |
niemeyer | rogpeppe: You're not using shquote there | 16:18 |
rogpeppe | niemeyer: i'm using it to quote the same file name elsewhere | 16:18 |
rogpeppe | niemeyer: i think maybe i'll add a sanity check early on that checks that DataDir is ok might be good. then i can relax. | 16:18 |
rogpeppe | s/maybe i'll add / | 16:19 |
niemeyer | rogpeppe: Sure.. whatever suits. The point made is pretty trivial: dumb-quoting and no-quoting work the same | 16:19 |
niemeyer | rogpeppe: Hmm.. isn't DataDIr coming from our own code? | 16:20 |
niemeyer | rogpeppe: Where's the tempdir coming from? | 16:20 |
rogpeppe | niemeyer: i kinda presumed that c.MkDir() called ioutil.TempDir somewhere along the line. | 16:21 |
rogpeppe | niemeyer: i may be wrong | 16:21 |
niemeyer | rogpeppe: Ah, for testing.. sure.. I'd be eagerly awaiting for the first bug report. :-) | 16:22 |
rogpeppe | niemeyer: :-) | 16:22 |
niemeyer | rogpeppe: This works in upstart, btw: exec echo "foo" > /tmp/foo | 16:22 |
niemeyer | rogpeppe: So it's surely a shell. | 16:22 |
rogpeppe | niemeyer: yes, i think > is special syntax. | 16:23 |
niemeyer | rogpeppe: For a shell? :-) | 16:23 |
rogpeppe | niemeyer: try echo "foo`echo bar`" | 16:23 |
niemeyer | exec echo "foo" 2>&1 > /tmp/foo | 16:23 |
niemeyer | rogpeppe: Is that special syntax too? :) | 16:24 |
rogpeppe | niemeyer: quite possibly | 16:24 |
rogpeppe | niemeyer: did you try the above example? | 16:24 |
rogpeppe | niemeyer: if it succeeds, then i'm more convinced | 16:25 |
niemeyer | exec echo "`cat /etc/passwd`" 2>&1 > /tmp/foo | 16:25 |
niemeyer | rogpeppe: Try it.. :) | 16:25 |
rogpeppe | niemeyer: in that case, cool, we can just use shquote | 16:29 |
niemeyer | rogpeppe: Scott is a pretty sharp guy.. I'd be surprised if he had reimplemented a shell inside upstart | 16:30 |
rogpeppe | niemeyer: well, there's *something* going on, because the upstart file itself isn't a shell script, so it has to parse the shell line to some degree before bundling it up to pass to the shell again | 16:32 |
rogpeppe | niemeyer: it loses newlines from within the quotes, for example, so it's not exactly the same | 16:33 |
niemeyer | rogpeppe: It's trivial to pick an exec line out of a file, but we're not really reimplementing upstart today :) | 16:34 |
rogpeppe | niemeyer: it can be more than one line | 16:34 |
rogpeppe | niemeyer: but yeah... | 16:34 |
niemeyer | Heh | 16:34 |
rogpeppe | niemeyer: so with the above in mind, is it ok if i use shquote, rather than leaving it unquoted. it would make me feel more comfortable. | 16:41 |
niemeyer | rogpeppe: Of course | 16:41 |
rogpeppe | fwereade: could you confirm that this branch still fails when testing cert, please? lp:~rogpeppe/juju-core/174-fix-cert-times | 16:46 |
fwereade | rogpeppe, branching | 16:47 |
fwereade | rogpeppe, passes | 16:47 |
rogpeppe | fwereade: weird | 16:48 |
rogpeppe | fwereade: because it looks like x509 calls .UTC itself | 16:48 |
rogpeppe | fwereade: ah! but only in tip, not in 1.0.3 or earlier | 16:55 |
rogpeppe | fwereade: and now i can reproduce the issue, i'm happy to make the fix | 16:56 |
fwereade | rogpeppe, ah, cool | 16:58 |
=== niemeyer_ is now known as niemeyer | ||
fwereade | rogpeppe, niemeyer: unexpected snag with environs.InstanceId type: state.Machine.InstanceId() should surely return one? | 17:07 |
rogpeppe | fwereade: i'd make it state.InstanceId | 17:07 |
fwereade | rogpeppe, doesn't feel quite right tbh | 17:08 |
fwereade | rogpeppe, but I guess I can live with it :) | 17:08 |
rogpeppe | fwereade: environs already uses state types | 17:08 |
fwereade | rogpeppe, indeed so | 17:08 |
fwereade | rogpeppe, it's just that InstanceId really doesn't feel very statey :) | 17:08 |
rogpeppe | fwereade: if it wasn't statey, the state wouldn't want to talk about it :-) | 17:09 |
* fwereade shrugs | 17:09 | |
fwereade | rogpeppe, fair enough | 17:09 |
niemeyer | fwereade: I agree with both points.. it'd indeed fit better in environs, I think having it in state is also fine | 17:10 |
fwereade | niemeyer, sgtm | 17:10 |
niemeyer | robbiew: I guess that meeting isn't happening? | 17:10 |
robbiew | niemeyer: it is | 17:10 |
robbiew | we are running late in another call | 17:10 |
niemeyer | robbiew: Hmm, ok | 17:10 |
rogpeppe | fwereade: do you wanna take a look at this, for form's sake? https://codereview.appspot.com/6858090 | 17:12 |
* fwereade looks | 17:12 | |
fwereade | rogpeppe, LGTM | 17:13 |
rogpeppe | fwereade: (i verified that the new tests failed on my machine against 1.0.2) | 17:13 |
rogpeppe | fwereade: ta | 17:13 |
rogpeppe | fwereade: submitting as i deem it trivial :-) | 17:13 |
fwereade | rogpeppe, SGTM | 17:14 |
rogpeppe | niemeyer: i don't think that waiting for a minute when we get an unauthorized error is a good idea | 17:28 |
rogpeppe | niemeyer: that means the first connection will always take at least a minute | 17:29 |
niemeyer | rogpeppe: Why? | 17:29 |
rogpeppe | niemeyer: because we try with the admin password, and if that fails, we try with the password hash | 17:29 |
niemeyer | rogpeppe: I thougth you said it takes less than 0.5 seconds? | 17:29 |
rogpeppe | niemeyer: so for the first connection we will always get ErrUnauthorized | 17:29 |
niemeyer | rogpeppe: So we tell the user unauthorized? | 17:29 |
rogpeppe | niemeyer: it's part of our standard login heuristics | 17:30 |
rogpeppe | niemeyer: see juju.NewConn | 17:30 |
niemeyer | rogpeppe: So I don't understand what's going on there | 17:30 |
niemeyer | rogpeppe: WHy are we retrying at all if we're retrying anyway? | 17:30 |
rogpeppe | niemeyer: it's all as we discussed earlier | 17:30 |
rogpeppe | niemeyer: ages ago, that is | 17:30 |
niemeyer | rogpeppe: I don't think we discussed this? | 17:30 |
niemeyer | rogpeppe: I don't recall talking about that 0.5 delay | 17:31 |
rogpeppe | niemeyer: we're retrying in that specific circumstance, using a different password each time | 17:31 |
niemeyer | rogpeppe: Yeah, but that's flaky | 17:31 |
niemeyer | rogpeppe: Vastly changing results if the server takes 0.5 seconds to answer something is really bad | 17:31 |
rogpeppe | niemeyer: it's not if the server takes 0.5 seconds to answer. it's if bootstrap-state takes more than 0.5 seconds from dialling to initialising the state. | 17:32 |
niemeyer | rogpeppe: That's exactly what I mean | 17:32 |
niemeyer | rogpeppe: and actually, that's not right either | 17:32 |
rogpeppe | niemeyer: maybe juju.NewConn should do the timed retry actually | 17:33 |
niemeyer | rogpeppe: What happens when bootstrap-state dials that starts that period? | 17:33 |
rogpeppe | niemeyer: as it's retrying anyway | 17:33 |
niemeyer | rogpeppe: Yeah, that would probably be less flaky | 17:33 |
rogpeppe | niemeyer: mgo continually redials with no delay | 17:33 |
rogpeppe | niemeyer: (i think that's not right actually) | 17:33 |
niemeyer | rogpeppe: I don't know what that means in this context | 17:34 |
niemeyer | rogpeppe: It doesn't matter what mgo does | 17:34 |
niemeyer | rogpeppe: What happens when bootstrap-state dials to kick that 0.5 period? | 17:34 |
rogpeppe | niemeyer: i don't understand the question | 17:34 |
niemeyer | <rogpeppe> niemeyer: it's not if the server takes 0.5 seconds to answer. it's if bootstrap-state takes more than 0.5 seconds from dialling to initialising the state. | 17:35 |
niemeyer | rogpeppe: "from dialling"!? | 17:35 |
niemeyer | rogpeppe: What happens when it dials to get that 0.5 period kicked off? | 17:35 |
niemeyer | rogpeppe: I can't see how that influences the period at all | 17:35 |
rogpeppe | niemeyer: bootstrap-state continually redials the mgo server while it's coming up. | 17:35 |
niemeyer | rogpeppe: MongoDB starts, and you'd get unauthorized, even if bootstrap-state hasn't even started running | 17:35 |
rogpeppe | niemeyer: yes | 17:35 |
niemeyer | rogpeppe: Yes, I don't think that's relevant | 17:35 |
rogpeppe | niemeyer: the client is also continually redialling the mgo server | 17:36 |
niemeyer | rogpeppe: Exactly | 17:36 |
niemeyer | rogpeppe: bootstrap-state doesn't have to even start for that 0.5 period to pass by | 17:36 |
rogpeppe | niemeyer: bootstrap-state starts well before mongodb is accepting connections | 17:36 |
rogpeppe | niemeyer: but i agree the timeout in Open isn't right | 17:37 |
niemeyer | rogpeppe: Because? MongoDB is started before that | 17:37 |
niemeyer | rogpeppe: You're trusting on external times of things that can take whatever time to run depending on scheduling and whatnot | 17:37 |
rogpeppe | niemeyer: mongodb takes a while to answer connections. bootstrap-state is started immediately after starting mongo. | 17:37 |
rogpeppe | niemeyer: yeah, that's true | 17:37 |
niemeyer | rogpeppe: There's nothing about that "from dialling to initialising the state" | 17:37 |
rogpeppe | niemeyer: i'm much happier putting the timeout in juju.NewConn | 17:38 |
niemeyer | rogpeppe: Sounds good, we just have to make sure this is more reliable | 17:38 |
rogpeppe | niemeyer: it's a pity we can't get bootstrap-state to tell mongodb to open a new port | 17:39 |
fwereade | rogpeppe, niemeyer: https://codereview.appspot.com/6844103 should be trivial (state.InstanceId) | 18:01 |
niemeyer | fwereade: On it | 18:03 |
rogpeppe | niemeyer: if you could take a brief look at https://codereview.appspot.com/6856105 before i submit; in particular the new code in juju.NewConn, that would be great. | 18:04 |
niemeyer | +func (inst *instance) Id() state.InstanceId { | 18:05 |
niemeyer | + return state.InstanceId(inst.InstanceId) | 18:05 |
niemeyer | } | 18:05 |
niemeyer | fwereade: Why do we have a method that returns a public field? | 18:05 |
niemeyer | fwereade: Did you spot why when doing it? | 18:05 |
fwereade | niemeyer, to satisfy environs.Instance, I think | 18:06 |
niemeyer | fwereade: A field can't be part of an interfae | 18:06 |
niemeyer | interface | 18:06 |
fwereade | niemeyer, hence the method | 18:06 |
fwereade | niemeyer, which is part of the interface | 18:06 |
niemeyer | fwereade: Ah, it's because that field is in ec2.Instance, actually | 18:06 |
niemeyer | fwereade: Sure, I know why the method exist | 18:07 |
niemeyer | s | 18:07 |
niemeyer | fwereade: It wasn't clear why the field existed | 18:07 |
fwereade | niemeyer, oh, yes, sorry, that too | 18:07 |
niemeyer | fwereade: CHeers | 18:07 |
* rogpeppe has to go now. | 18:14 | |
rogpeppe | g'night all | 18:15 |
rogpeppe | niemeyer: if you like the new change, i'll submit a bit later. | 18:15 |
fss | niemeyer: lol | 18:15 |
fss | niemeyer: ops, wrong message | 18:15 |
fss | niemeyer: sorry x) | 18:15 |
niemeyer | rogpeppe: Super, cheers man | 18:16 |
niemeyer | fwereade: LGTM with a couple of trivials. Thanks | 18:17 |
niemeyer | fss: :) | 18:17 |
fwereade | niemeyer, cheers | 18:17 |
fwereade | niemeyer, the casting in the tests was justified in my mind on the basis that, well, we know the method signature and therefore the type | 18:18 |
fwereade | niemeyer, and it's slightly more readable IMO | 18:19 |
fwereade | niemeyer, no big deal, I'll change them | 18:19 |
niemeyer | fwereade: In general that should be fine, but these tests are precisely the tests verifying that we know the method signature | 18:20 |
niemeyer | fwereade: Note that the comment is specifically on the test of the InstanceId method itself | 18:20 |
fwereade | niemeyer, ah, yes, true | 18:20 |
fwereade | niemeyer, ok, sgtm, thanks | 18:21 |
niemeyer | fwereade: My pleasure | 18:21 |
fwereade | rogpeppe, can you precis the thinking behind having an explicitly required MachinerWorker but not an UpgraderWorker? | 20:29 |
fwereade | rogpeppe, I would just as soon make them both implicit... | 20:29 |
niemeyer | fwereade: The upgrader is a bit more attached to the details of the agent itself | 21:15 |
niemeyer | fwereade: Although I'm not sure either if there's enough justification | 21:15 |
niemeyer | davecheney: Good morning Dave | 21:24 |
niemeyer | davecheney: Please ping me when you have a moment for a call | 21:25 |
davecheney | niemeyer: morning | 21:26 |
davecheney | lemmie get my headset | 21:26 |
davecheney | ready to go, g+ ? | 21:31 |
niemeyer | davecheney: Yep | 21:32 |
niemeyer | davecheney: https://plus.google.com/hangouts/_/449c0b5562132d520a43332aaa7f1eb67ec41bd1?authuser=0&hl=en | 21:33 |
davecheney | ta, for some reason you never show as online on g+ for me | 21:33 |
niemeyer | davecheney: https://codereview.appspot.com/6854098/diff/2001/environs/ec2/local_test.go?column_width=90 | 21:41 |
niemeyer | davecheney: ping | 21:41 |
davecheney | niemeyer: ack | 21:42 |
davecheney | lost you | 21:42 |
niemeyer | davecheney: Okay, let me reconnect.. are you still up? | 21:42 |
davecheney | yeah, hangout is still working | 21:42 |
niemeyer | davecheney: Cool, was my side only then | 21:42 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!