davecheneywallyworld: can you get to launchpad atm ?01:54
* wallyworld checks01:55
wallyworlddavecheney: appears ok, what error are you seeing?01:55
davecheneyhttps://launchpad.net/juju-core times out01:55
wallyworldworks quickly for me01:55
wallyworldmaybe they were doing a deployment?01:56
wallyworldstill broken?01:56
davecheneytrying from another site01:56
davecheneyoh, finally01:56
wallyworldgood luck01:56
davecheneylaunchpad is slow as fuck for me most days01:56
davecheneybut this was unusually lethargic01:56
wallyworldit's not bad for me01:57
wallyworldthere's been a lot of work into performance improvements over the last 12 months01:57
davecheneywhat times to they schedule deployments ?01:57
wallyworldif you have any particular pages that are slow, let me know and i'll look into it01:57
davecheneyeverythign is slow for me01:58
wallyworldno down time deployments are done as needed, and should be transparent01:58
davecheney2-5 second page loag times01:58
davecheneymultiple locations01:58
davecheneymultiple computers01:58
wallyworldfast down time deployments are done 3 times a day and result in about a 2 second outage01:58
wallyworldjuju-core just loaded for me in 1 second01:58
wallyworldmaybe your connection has higher latency to the data centres?01:59
davecheneyhttps://launchpad.net/juju-core/+milestone/1.9.3 4 seconds01:59
wallyworld0.52 for me01:59
davecheneyi'm on iinet, your on tpg02:00
davecheneyneither of those are top shelf isps02:00
davecheneybut this happens for me from different locations02:00
wallyworldbut cheap :-)02:00
davecheneydifferent computers02:00
davecheneywas in melbourne over the weekend on optus cable02:00
wallyworldhmmm. i can't easily explain the difference02:00
davecheneydon't worry02:00
davecheneyi'm used to it02:01
wallyworldmakes it hard to work efficiently02:01
wallyworldthe guys are focused on performance issues, but they do need more than one data sample to be able to do anything concrete02:02
wallyworldespecially if it is not slow across the board02:02
davecheneywho should I complain too ?02:02
wallyworldPurple squad is on maintenance and hang out in #launchpad-dev - they are very responsive02:02
wallyworldanother option is to go to a page on qastaging.launchpad.net and turn on tracing, i can help you with that if you want02:03
wallyworldqastaging is slower than prod, but it may show some issues worth looking at02:04
wallyworldhave you checked your latency to the data centre?02:04
davecheneysolid 312ms02:05
davecheneyanyway, it's not your job to debug lp problems02:06
davecheneythanks for checking02:06
wallyworldno problems, but it would be nice to sort it out02:08
wallyworlddavecheney: so, out of interest, in the light blue bar, what's the waiting time vs connecting time?02:09
davecheneyconnection and ssl neg is in excess of 2.5 seconds on the second screenshot02:10
davecheneyi have no idea how you are negoating faster02:10
davecheneyit's a function of rtt02:10
davecheneywallyworld: you're using firefox, right ?02:10
wallyworlddavecheney: 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 you02:11
davecheneydunno how you got, 12:59 < wallyworld> 0.52 for me then02:11
wallyworldmy 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 guess02:12
wallyworldthat 0.52 for +milestone above was also the lp reported processing tim02:12
davecheneyall lies i tell ya!02:13
davecheneyanyway, seriously, i don't care about this02:13
davecheneyslow I can handle, as long as it's not down02:13
wallyworldfair 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.302:14
davecheneyping https://codereview.appspot.com/6855101/05:50
wallyworlddavecheney: i'd +1 it but i fear i don't know enough to be able to offer anything worthwhile. it looks ok to me though05:57
davecheneywallyworld: thanks05:57
davecheneyyou coudl always dist-upgrade and test it :)05:57
wallyworldwant me to +1 it anyway?05:57
wallyworldi'm on quantal already05:57
davecheneydo the juju tests pass for you ?05:58
wallyworldlet me test it then05:58
wallyworldi have not run them, will do so now05:58
wallyworldi've just been running goose tests05:58
davecheneygo test launchpad.net/juju-core/...05:58
wallyworldyeah, will do after i merge your branch05:58
davecheneynah, do it before05:58
davecheneyotherwise you won't know if I fixed anything05:58
wallyworldok, need to pull tip first05:59
davecheneygo get -v launchpad.net/juju-core/... will do that for you05:59
davecheneyif you haven't already checked it out05:59
wallyworldi have the code checked out etc, i'm just used to using bzr05:59
wallyworldis go get the preferred way?05:59
davecheneyif you have never checkout out the code, it is a fast way to get all the deps06:00
davecheneyif you already have a working copy06:00
davecheneyit'll screw it up royally06:00
wallyworldyeah, i have everything checked out and built06:00
wallyworldi used go get right at the start06:00
davecheneyi have trunk under a cobzr branch called trunk06:01
davecheneyso whenever I want to know what others are doing i do06:01
davecheneycobzr switch trunk ; cobzr pull06:01
wallyworldyeah, me too06:01
davecheneywas, http://bazaar.launchpad.net/~gophers/juju-core/trunk/view/head:/README useful at all ?06:02
wallyworldi used to use light weight checkouts etc, and bzr switch, but am trying cobzr06:02
wallyworldit stores the branches in a hidden dir which is both convenient and annoying06:02
wallyworlddavecheney: yes! i used that to get going06:03
davecheneyglad it was useful06:03
wallyworldthanks for writing it :-)06:03
davecheneynp, you can/should/might steal it for goose06:03
wallyworldok, tests fail as expected06:03
wallyworldnow to try your branch06:03
davecheneyit'll be whinging about can't find tools06:03
davecheneyno, from memory the actual error is it can't find a suitable ami to 'fake' boot06:04
davecheneythis change fills in the fixtures to make that happen06:04
wallyworlderror was: cannot find image satisfying constraints: error getting instance types: 404 Not Found06:05
davecheneywallyworld: yup, that is the error06:05
wallyworlddavecheney: tests running but taking a loooooong time - appears hung running the updated test06:10
wallyworldie nothing after "ok      launchpad.net/juju-core/environs/config 0.373s"06:11
davecheneypresences tests take over 70 seconds on my machine06:13
davecheneygocheck reports when the test is done, not when it starts06:13
davecheneycheck top06:13
davecheneyyou should see a number of someting.test processes running06:13
wallyworlddavecheney: what do i look for?06:13
davecheneya child of a process call go06:14
davecheney     │        ├─bash───go─┬─2*[6g]06:14
davecheney     │        │           ├─container.test─┬─mongod06:14
davecheney     │        │           │                └─2*[{container.test}]06:14
davecheney     │        │           ├─downloader.test───4*[{downloader.test}]06:14
davecheney     │        │           └─9*[{go}]06:14
davecheneysomething like this, if pstree is your poison06:14
wallyworldi have a dummy.test06:14
davecheneyfrom memory the default test timeout is 120s06:15
davecheneyit'll fail eventually06:15
wallyworldok, it's been a fair bit longer06:16
davecheneygotta go06:17
davecheneywill be online in an hour or so06:17
davecheneyif tests are fucked, email juju-dev with the output06:17
davecheneyor raise an issue06:17
wallyworldok, bye06:17
wallyworldjust died now!06:18
wallyworldbut works when run again06:22
TheMuefwereade, davecheney: Hiya.08:14
fwereadeTheMue, davecheney, morning08:14
TheMueOur new https://juju.ubuntu.com really looks good.08:17
rogpeppefwereade, TheMue: mornin'08:46
TheMueHello, Mr Peppe08:46
fwereaderogpeppe, heyhey08:50
fwereaderogpeppe, you know the TxnRevno field, that we need to use to start document watches?08:51
rogpeppefwereade: yeah08:51
fwereaderogpeppe, it appears to me as though you can *actually* just pass 0 and it'll still work anyway08:51
rogpeppefwereade: except you'll maybe get an event you don't need, right?08:51
fwereaderogpeppe, I guess that perhaps you'll get an extra event sometimes08:51
rogpeppefwereade: maybe you don't care though08:52
fwereaderogpeppe, it doesn't have any macro-level effects that I am sophisticated enough to detect08:52
rogpeppefwereade: it would mean doing strictly more work on restart08:53
fwereaderogpeppe, yeah, I'm not proposing actually doing it08:53
fwereaderogpeppe, the actual problem is that I was trying to use it, and doing it wrong, and couldn't see that from my tests08:53
rogpeppefwereade: interesting thought though - we might decide the simplicity is worth the network traffic08:53
fwereaderogpeppe, and I am wondering if there's any way I can reliably goose it into doing the wrong thing08:53
rogpeppefwereade: it's just an optimisation, right?08:54
fwereaderogpeppe, I dunno -- first time I used the API I just bunged 0 in and niemeyer was most unimpressed08:54
rogpeppefwereade: well, the API doesn't actually give you the values - it just tells you that something has changed, no?08:55
fwereaderogpeppe, I forget08:55
fwereaderogpeppe, .Id is all I ever look at08:55
rogpeppefwereade: if that's true, it's never going to make any difference as long as you get at least the required events08:55
rogpeppefwereade: 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
fwereaderogpeppe, indeed -- anyway, I just thought it was interesting08:56
rogpeppefwereade: second is https://codereview.appspot.com/6856105/ (which is the last in the series)08:57
rogpeppefwereade: in terms of seeing problems, you'd see problems if the revno field was too high08:57
fwereaderogpeppe, yeah08:57
rogpeppefwereade: so you might be able to goose it that way08:57
fwereaderogpeppe, not from outside I think08:58
rogpeppefwereade: aren't you storing revnos in files?08:58
fwereaderogpeppe, the problem was that I used the txnrevno field, instead of txn-revno, and got 0 every time08:58
fwereaderogpeppe, I am but that's in a different area08:58
rogpeppefwereade: yeah, in that case, i'm not sure i can think of a test that makes any difference.08:59
rogpeppefwereade: 'cos if it's zero, you'll get a spurious first event, find that there's nothing to do, then do nothing08:59
fwereaderogpeppe, you might be interested to take a look at https://codereview.appspot.com/6850105/ and https://codereview.appspot.com/6851110/ while I read yours properly09:00
fwereaderogpeppe, yeah, exactly09:00
rogpeppefwereade: looking09:00
rogpeppefwereade: ah, i had taken a look at the first - i had one unpublished comment09:02
rogpeppefwereade: LGTM09:03
fwereaderogpeppe, cheers, you have one too09:03
rogpeppes/dong/ding/ lol09:04
rogpeppefwereade: thanks09:05
fwereaderogpeppe, and another LGTM, that look fantastic09:10
rogpeppefwereade: thanks!09:10
rogpeppefwereade: the window really is very short indeed. i think it must be on the order of <0.1s09:12
rogpeppefwereade: i've seen the "unauthorized access" problem precisely once09:12
fwereaderogpeppe, fantastic09:17
rogpeppefwereade: basically the remote client and bootstrap-state are both racing for first access to the mongodb09:18
fwereaderogpeppe, ah, right, ofc09:18
rogpeppefwereade: we're more likely to see the problem now because mgo redials about twice a second09:19
dimiternjam, mgz: would you take a look https://codereview.appspot.com/6851112/ pls?09:49
jamdimitern: looking09:51
TheMuerogpeppe, fwereade: Would you take a look at https://codereview.appspot.com/6853075/? I changed and simplified the way it's working.10:15
rogpeppeTheMue: looking10:16
TheMuedimitern, jam: Good morning.10:16
jammorning TheMue10:16
dimiternTheMue:  morning10:16
jamdimitern: done10:19
dimiternjam: thanks10:20
rogpeppeTheMue: i like the idea (of just sourcing the file), but i'm not sure that env is the right way to print the env vars10:25
rogpeppeTheMue: what happens if one of the vars has a newline in?10:25
TheMuerogpeppe: Good hint, yes.10:26
TheMuerogpeppe: Todays Py code is only grepping two values, but as a general purpose package I would like to provide all.10:27
TheMuerogpeppe: Do you know a command to get the names of all environment variables?10:28
rogpeppeTheMue: you could use awk10:28
TheMuerogpeppe: Uuuh, last time I used awk has been last century. ;)10:29
rogpeppeTheMue: something like this might do the job:10:31
rogpeppeawk 'BEGIN {for(v in ENVIRON){s = ENVIRON[v]; gsub("\\", "\\\\", s); gsub("\n", "\\n", s); printf("%s=\"%s\"\n", v, s)}}'10:31
rogpeppeTheMue: then you could use strconv.Unquote to read 'em in10:31
wallyworldjam: dimitern: mgz: wanna have the standup now?10:32
TheMuerogpeppe: Thx, I'll try.10:32
jamworks for me, is mgz here?10:32
rogpeppeTheMue: there's probably a better tool around for doing this though10:32
rogpeppeTheMue: you may even be able to use bash directly10:32
jamwallyworld: I'm in mumble10:33
dimiternwallyworld, jam: I'm ok with that10:33
* wallyworld opens mumble10:33
wallyworldjam: dimitern: i've hit that mumble bug again where it won't start, i have to reboot, give me a sec10:34
rogpeppeha that's funny, you learn something new every day. i always thought that $'foo' was the same as $foo10:34
jamrogpeppe: do you mean $foo vs '$foo' ?10:37
jamI haven't really seen $'foo' before.10:37
rogpeppejam: no10:37
rogpeppejam: neither had i10:37
rogpeppejam: except in rc10:37
rogpeppejam: and i'd presumed sh was similar like that10:37
rogpeppejam: try: echo $'foo\nbar'10:38
rogpeppeTheMue: looks like "typeset -p -x" might give you what you need10:45
TheMuerogpeppe: Looks nice, yes.10:46
TheMuerogpeppe: Just fetched my old shell programming book. ;)10:47
rogpeppeTheMue: alternatively, you could just parse the file. it wouldn't be too hard.10:47
TheMuerogpeppe: Which one? The /etc/default/lxc?10:48
rogpeppeTheMue: yeah10:48
TheMuerogpeppe: Values in it could be based on environment variables too. ;)10:48
rogpeppeTheMue: that's not too hard either10:49
rogpeppeTheMue: os.Expand might help10:49
rogpeppeTheMue: hmm, i dunno. at least using typeset you're guaranteed a standard format10:49
rogpeppeTheMue: i wonder about the security implications of sourcing the shell script, but since it's in /etc, it's probably ok10:50
TheMuerogpeppe: Yes, here this approach seems better. I started with parsing. But niemeyer had remarks.10:50
TheMuerogpeppe: The Py code just sources it too.10:51
rogpeppeTheMue: ok, that's interesting.10:51
rogpeppeTheMue: what values do you need from it, BTW?10:51
TheMuerogpeppe: 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
rogpeppeTheMue: one issue with your current approach is that you'll see env vars that are defined in the testing environment too AFAICS10:55
rogpeppeTheMue: i think you need to start the shell command with a clean environment10:55
TheMuerogpeppe: I'm sourcing an own file with self-defined values for testing.10:56
TheMuerogpeppe: It's a copy, but with different values than the default values.10:57
TheMuerogpeppe: At least some of them.10:57
rogpeppeTheMue: i bet if you defined LXC_BRIDGE in your shell and deleted it from the "env" var, your test would still pass10:58
TheMuerogpeppe: If it has the right value, yes. :/11:00
wallyworld_jam: mgz: test one two three11:06
niemeyermgz: Heya11:45
niemeyermgz: When's our next squash game? :-)11:45
mgzwell, that would be one excuse for a holiday in brazil :)11:47
niemeyermgz: Consider yourself invited :)11:47
niemeyermgz: Just yesterday I was playing a pretty good match against a friend and reminding of that game in Copenhagen.. it was great11:48
niemeyerfwereade: Heya11:48
niemeyerfwereade: https://codereview.appspot.com/6850105 is ready to go in11:52
fwereadeniemeyer, sweet! tyvm11:52
niemeyerfwereade: Thank you!11:52
* dimitern => lunch11:59
* fwereade also12:02
rogpeppeniemeyer: morning!12:11
TheMuerogpeppe: Found an even simpler way. Wonna look again?12:39
rogpeppeTheMue: sure12:40
TheMuerogpeppe: Thanks.12:40
rogpeppeTheMue: what are those double quotes doing at the start of script?12:43
rogpeppeTheMue: (printenv -0 looks good BTW)12:44
TheMuerogpeppe: Otherwise it isn't passed to sh as one argument. I wondered too.12:44
rogpeppeTheMue: huh?12:45
rogpeppeTheMue: have you tried it without them?12:45
TheMuerogpeppe: Yep.12:45
TheMuerogpeppe: Aargh, you meant now double quotes.12:46
TheMuerogpeppe: That works, will remove and repropose it. *facepalm*12:46
rogpeppeTheMue: s/now/no/ ?12:46
TheMuerogpeppe: Yep, fingers too fast. ;)12:46
TheMuerogpeppe: So, removed. Took them too automatically.12:48
niemeyerTheMue: I think we should move on to MAAS instead of continuing to spend cycles on this12:49
TheMueniemeyer: 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
niemeyerTheMue: 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 soon12:51
TheMueniemeyer: OK, but could you please take a look today to see if the direction now is better?12:52
niemeyerTheMue: What happens if there is an LXC variable in the Go process?12:55
rogpeppeniemeyer: i've raised that issue12:57
TheMueniemeyer: 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
rogpeppeTheMue: you've got some more comments12:58
TheMuerogpeppe: Thanks.12:58
rogpeppeniemeyer: i think it should clear environment variables before sourcing the shell script12:58
TheMuerogpeppe: The sourcing is only in a subshell for reading the values.13:03
rogpeppeTheMue: that subshell inherits environment variables from the caller13:03
niemeyerTheMue: That's not how environment works13:03
niemeyerWhat roger said13:04
TheMueniemeyer: Wanted to express that for the execution of lxc commands the environment is unchanged.13:04
niemeyerTheMue: I don't understand how that changes the points made13:05
niemeyerTheMue: A child cannot change a parent's environment, ever13:05
niemeyerTheMue: But I'm not sure if that's what you're saying13:05
niemeyerI mean, at least in computing13:05
niemeyerIN the real world a child revamps a parent's environment in its entirety13:06
TheMueniemeyer: *lol*13:06
TheMueniemeyer: Yes, I know. I'm only using the way of the Py code to read values out of /etc/default/lxc13:06
TheMueniemeyer: Here they've done the same, but grepping only for the LXC_ values to then read ADDR and BRIDGE.13:07
niemeyerTheMue: Still doesn't change the points made13:07
niemeyerTheMue: and the point is reather simple really13:08
TheMueniemeyer: 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 script13:09
niemeyer<rogpeppe> TheMue: that subshell inherits environment variables from the caller13:09
niemeyerTheMue: This is right13:09
TheMueniemeyer: So the environment I'm parsing is too large, but it contains the values of /etc/default/lxc. Still right?13:10
TheMueniemeyer: And by clearing the environment before I would reduce it to the values of that file.13:11
niemeyerTheMue: Right13:12
TheMueniemeyer: OK, then I know where we gor our wires crossed. (You say so?)13:14
niemeyerrogpeppe: Can you please have a look at William's follow up when you have a second: https://codereview.appspot.com/685111013:35
niemeyerWell, 10 minutes perhaps13:35
rogpeppeniemeyer: will do13:35
niemeyerrogpeppe: Thanks13:35
niemeyerrogpeppe: One branch reviewed and double LGTMed with trivials13:52
niemeyerrogpeppe: Only one left in the queue.. will get to that after lunch13:52
rogpeppeniemeyer: thanks!13:52
rogpeppeniemeyer: whee!13:52
* rogpeppe should have some lunch too13:53
fwereaderogpeppe, ----------------------------------------------------------------------14:45
fwereadeFAIL: cert_test.go:50: certSuite.TestNewCA14:45
fwereade    c.Assert(caCert.NotAfter.Equal(expiry), Equals, true)14:45
fwereade... obtained bool = false14:45
fwereade... expected bool = true14:45
fwereadeFAIL: cert_test.go:66: certSuite.TestNewServer14:45
fwereade    c.Assert(srvCert.NotAfter.Equal(expiry), Equals, true)14:45
fwereade... obtained bool = false14:45
fwereade... expected bool = true14:45
fwereadeFAIL: cert_test.go:95: certSuite.TestVerify14: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
fwereadeOOPS: 3 passed, 3 FAILED14:45
fwereade--- FAIL: TestAll (1.96 seconds)14:45
fwereaderogpeppe, on trunk -- should I install something?14:45
rogpeppefwereade: hmm, passes for me14:46
rogpeppefwereade: i'll try against a different go version14:46
fwereaderogpeppe, I'm still using 1.0.214:46
fwereaderogpeppe, (er, did we decide to standardize on 1.0.3?)14:46
rogpeppefwereade: i'm not sure14:47
rogpeppefwereade: i think maybe 1.0.2 as that's what's bundled14:47
fwereaderogpeppe, ah, yes, then I did that deliberately14:47
* fwereade looks around shiftily14:47
* rogpeppe builds 1.0.214:49
rogpeppefwereade: it passes on 1.0.3 BTW14:49
fwereaderogpeppe, I did indeed assume you wouldn't have merged stuff that didn't work for you ;)14:50
rogpeppefwereade: i was compiling against tip, so it may not have14:50
rogpeppefwereade: hmm, 1.0.2 passes for me too14:50
* fwereade can't keep up with the cool kids and their crazy avant-garde go versions14:51
* fwereade goes off to run it more verbosely14:51
rogpeppefwereade: could you get it to print out the actual times you're seeing, please14:51
rogpeppefwereade: i.e. c.Logf("cert notafter: %v, expiry: %v", caCert.NotAfter, expiry)14:52
fwereaderogpeppe, cert notafter: 2012-11-29 14:53:57 +0100 CET, expiry: 2012-11-29 15:53:57 +0100 CET14:54
fwereaderogpeppe, the otherone's equivalent14:54
rogpeppefwereade: interesting. something to do with daylight savings14:55
fwereaderogpeppe, and/or timezones :)14:55
rogpeppefwereade: yeah14:55
fwereaderogpeppe, anotheradvantage of the distributed team :)14:55
rogpeppefwereade: yeah. i am GMT atm so particularly vulnerable to that kind of error14:56
fwereaderogpeppe, 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
rogpeppefwereade: yeah, i guess so.14:59
rogpeppefwereade: i'm just trying to reproduce the behaviour14:59
rogpeppefwereade: i can't seem to reproduce the behaviour, which is a bit off15:08
fwereaderogpeppe, huh, weird15:09
fwereadeTheMue, can you repro? you're in my timezone I think15:09
rogpeppefwereade: i tried using a time which i parsed from your timestamp15:09
TheMuefwereade: Will try.15:09
rogpeppefwereade: could you try this: where the test calls time.Now(), could you make it call time.Now().UTC() instead.15:10
fwereaderogpeppe, bingo15:11
fwereaderogpeppe, fixes all 315:12
rogpeppefwereade: hmm, it shouldn't make a difference15:12
rogpeppefwereade: i'll push a fix see if i can fix the underlying Go too15:12
rogpeppes/see/and see/15:13
fwereaderogpeppe, great, thanks, consider it pre-LGTMed if that's all you do :)15:13
TheMuefwereade, rogpeppe: Same error: http://paste.ubuntu.com/1394620/15:13
fwereaderogpeppe, sorry, I have no reason to still be waiting for your fix, do I? can I merge?15:32
rogpeppefwereade: you can and may15:33
fwereaderogpeppe, cheers15:34
rogpeppefwereade: 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
TheMuerogpeppe: 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
rogpeppeTheMue: ok15:40
* TheMue is stepping out a bit earlier today. Had no lunch due to pre-christmas-dinner with friends.15:42
fwereadeTheMue, enjoy15:43
TheMuefwereade: Thanks.15:44
niemeyerrogpeppe: Woohay TLS15:58
niemeyerrogpeppe: Just reviewed that last one15:58
rogpeppeniemeyer: thanks!15:58
rogpeppeniemeyer: BTW 0.5 seconds should be perfectly sufficient - mgo redials many times a second, and bootstrap-state takes very little time to complete16:00
rogpeppeniemeyer: i can raise the length of time anyway if you'd like.16:01
niemeyerrogpeppe: 0.5 second is spent in a trivial sneeze of AWS16:01
rogpeppeniemeyer: fair enough. BTW what in the ssh logic were you thinking we might miss?16:02
niemeyerrogpeppe: The ssh logic :-)16:02
rogpeppeniemeyer: if we're not using ssh, why should we care about that?16:03
niemeyerrogpeppe: ssh is battle tested as public doors.. I just feel a bit more comfortable there.16:04
rogpeppeniemeyer: i quoted the serverPEMPath because cfg.DataDir may reasonably contain spaces. quotes or backslashes are much more unlikely though.16:13
rogpeppeniemeyer: do you still think it's a bad idea to do that?16:14
niemeyerrogpeppe: We know it doesn't contain spaces, in the same way we know it doesn't contain quotes or backslashes16:14
rogpeppeniemeyer: do we know that ioutil.TempDir will never return a name with a space in?16:14
niemeyerrogpeppe: Pretending we're being safer isn't necessary16:14
niemeyerrogpeppe: Do we know it will never return a name with quotes?16:14
rogpeppeniemeyer: tbh, i'd prefer to know what the quoting rules *are* for upstart...16:15
niemeyerrogpeppe: Isn't that a line passed to a shell?16:15
rogpeppeniemeyer: no, it goes into the upstart config file which has its own syntax16:16
niemeyerrogpeppe: Isn't upstart taking that line from its config file and passing it to a shell?16:16
rogpeppeniemeyer: i don't think so.16:16
niemeyerrogpeppe: I'd be surprised16:16
rogpeppeniemeyer: i think it invokes the command directly16:16
niemeyerrogpeppe: But it doesn't really matter16:16
rogpeppeniemeyer: i did delve into the source once to try to find out16:17
niemeyerrogpeppe: The points made still hold16:17
rogpeppeniemeyer: does that mean i should remove all the shquote calls elsewhere?16:17
niemeyerrogpeppe: You're not using shquote there16:18
rogpeppeniemeyer: i'm using it to quote the same file name elsewhere16:18
rogpeppeniemeyer: 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
rogpeppes/maybe i'll add /16:19
niemeyerrogpeppe: Sure.. whatever suits. The point made is pretty trivial: dumb-quoting and no-quoting work the same16:19
niemeyerrogpeppe: Hmm.. isn't DataDIr coming from our own code?16:20
niemeyerrogpeppe: Where's the tempdir coming from?16:20
rogpeppeniemeyer: i kinda presumed that c.MkDir() called ioutil.TempDir somewhere along the line.16:21
rogpeppeniemeyer: i may be wrong16:21
niemeyerrogpeppe: Ah, for testing.. sure.. I'd be eagerly awaiting for the first bug report. :-)16:22
rogpeppeniemeyer: :-)16:22
niemeyerrogpeppe: This works in upstart, btw: exec echo "foo" > /tmp/foo16:22
niemeyerrogpeppe: So it's surely a shell.16:22
rogpeppeniemeyer: yes, i think > is special syntax.16:23
niemeyerrogpeppe: For a shell? :-)16:23
rogpeppeniemeyer: try echo "foo`echo bar`"16:23
niemeyerexec echo "foo" 2>&1 > /tmp/foo16:23
niemeyerrogpeppe: Is that special syntax too? :)16:24
rogpeppeniemeyer: quite possibly16:24
rogpeppeniemeyer: did you try the above example?16:24
rogpeppeniemeyer: if it succeeds, then i'm more convinced16:25
niemeyerexec echo "`cat /etc/passwd`" 2>&1 > /tmp/foo16:25
niemeyerrogpeppe: Try it.. :)16:25
rogpeppeniemeyer: in that case, cool, we can just use shquote16:29
niemeyerrogpeppe: Scott is a pretty sharp guy.. I'd be surprised if he had reimplemented a shell inside upstart16:30
rogpeppeniemeyer: 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 again16:32
rogpeppeniemeyer: it loses newlines from within the quotes, for example, so it's not exactly the same16:33
niemeyerrogpeppe: It's trivial to pick an exec line out of a file, but we're not really reimplementing upstart today :)16:34
rogpeppeniemeyer: it can be more than one line16:34
rogpeppeniemeyer: but yeah...16:34
rogpeppeniemeyer: 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
niemeyerrogpeppe: Of course16:41
rogpeppefwereade: could you confirm that this branch still fails when testing cert, please? lp:~rogpeppe/juju-core/174-fix-cert-times16:46
fwereaderogpeppe, branching16:47
fwereaderogpeppe, passes16:47
rogpeppefwereade: weird16:48
rogpeppefwereade: because it looks like x509 calls .UTC itself16:48
rogpeppefwereade: ah! but only in tip, not in 1.0.3 or earlier16:55
rogpeppefwereade: and now i can reproduce the issue, i'm happy to make the fix16:56
fwereaderogpeppe, ah, cool16:58
=== niemeyer_ is now known as niemeyer
fwereaderogpeppe, niemeyer: unexpected snag with environs.InstanceId type: state.Machine.InstanceId() should surely return one?17:07
rogpeppefwereade: i'd make it state.InstanceId17:07
fwereaderogpeppe, doesn't feel quite right tbh17:08
fwereaderogpeppe, but I guess I can live with it :)17:08
rogpeppefwereade: environs already uses state types17:08
fwereaderogpeppe, indeed so17:08
fwereaderogpeppe, it's just that InstanceId really doesn't feel very statey :)17:08
rogpeppefwereade: if it wasn't statey, the state wouldn't want to talk about it :-)17:09
* fwereade shrugs17:09
fwereaderogpeppe, fair enough17:09
niemeyerfwereade: I agree with both points.. it'd indeed fit better in environs, I think having it in state is also fine17:10
fwereadeniemeyer, sgtm17:10
niemeyerrobbiew: I guess that meeting isn't happening?17:10
robbiewniemeyer: it is17:10
robbiewwe are running late in another call17:10
niemeyerrobbiew: Hmm, ok17:10
rogpeppefwereade: do you wanna take a look at this, for form's sake? https://codereview.appspot.com/685809017:12
* fwereade looks17:12
fwereaderogpeppe, LGTM17:13
rogpeppefwereade: (i verified that the new tests failed on my machine against 1.0.2)17:13
rogpeppefwereade: ta17:13
rogpeppefwereade: submitting as i deem it trivial :-)17:13
fwereaderogpeppe, SGTM17:14
rogpeppeniemeyer: i don't think that waiting for a minute when we get an unauthorized error is a good idea17:28
rogpeppeniemeyer: that means the first connection will always take at least a minute17:29
niemeyerrogpeppe: Why?17:29
rogpeppeniemeyer: because we try with the admin password, and if that fails, we try with the password hash17:29
niemeyerrogpeppe: I thougth you said it takes less than 0.5 seconds?17:29
rogpeppeniemeyer: so for the first connection we will always get ErrUnauthorized17:29
niemeyerrogpeppe: So we tell the user unauthorized?17:29
rogpeppeniemeyer: it's part of our standard login heuristics17:30
rogpeppeniemeyer: see juju.NewConn17:30
niemeyerrogpeppe: So I don't understand what's going on there17:30
niemeyerrogpeppe: WHy are we retrying at all if we're retrying anyway?17:30
rogpeppeniemeyer: it's all as we discussed earlier17:30
rogpeppeniemeyer: ages ago, that is17:30
niemeyerrogpeppe: I don't think we discussed this?17:30
niemeyerrogpeppe: I don't recall talking about that 0.5 delay17:31
rogpeppeniemeyer: we're retrying in that specific circumstance, using a different password each time17:31
niemeyerrogpeppe: Yeah, but that's flaky17:31
niemeyerrogpeppe: Vastly changing results if the server takes 0.5 seconds to answer something is really bad17:31
rogpeppeniemeyer: 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
niemeyerrogpeppe: That's exactly what I mean17:32
niemeyerrogpeppe: and actually, that's not right either17:32
rogpeppeniemeyer: maybe juju.NewConn should do the timed retry actually17:33
niemeyerrogpeppe: What happens when bootstrap-state dials that starts that period?17:33
rogpeppeniemeyer: as it's retrying anyway17:33
niemeyerrogpeppe: Yeah, that would probably be less flaky17:33
rogpeppeniemeyer: mgo continually redials with no delay17:33
rogpeppeniemeyer: (i think that's not right actually)17:33
niemeyerrogpeppe: I don't know what that means in this context17:34
niemeyerrogpeppe: It doesn't matter what mgo does17:34
niemeyerrogpeppe: What happens when bootstrap-state dials to kick that 0.5 period?17:34
rogpeppeniemeyer: i don't understand the question17: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
niemeyerrogpeppe: "from dialling"!?17:35
niemeyerrogpeppe: What happens when it dials to get that 0.5 period kicked off?17:35
niemeyerrogpeppe: I can't see how that influences the period at all17:35
rogpeppeniemeyer: bootstrap-state continually redials the mgo server while it's coming up.17:35
niemeyerrogpeppe: MongoDB starts, and you'd get unauthorized, even if bootstrap-state hasn't even started running17:35
rogpeppeniemeyer: yes17:35
niemeyerrogpeppe: Yes, I don't think that's relevant17:35
rogpeppeniemeyer: the client is also continually redialling the mgo server17:36
niemeyerrogpeppe: Exactly17:36
niemeyerrogpeppe: bootstrap-state doesn't have to even start for that 0.5 period to pass by17:36
rogpeppeniemeyer: bootstrap-state starts well before mongodb is accepting connections17:36
rogpeppeniemeyer: but i agree the timeout in Open isn't right17:37
niemeyerrogpeppe: Because?  MongoDB is started before that17:37
niemeyerrogpeppe: You're trusting on external times of things that can take whatever time to run depending on scheduling and whatnot17:37
rogpeppeniemeyer: mongodb takes a while to answer connections. bootstrap-state is started immediately after starting mongo.17:37
rogpeppeniemeyer: yeah, that's true17:37
niemeyerrogpeppe: There's nothing about that "from dialling to initialising the state"17:37
rogpeppeniemeyer: i'm much happier putting the timeout in juju.NewConn17:38
niemeyerrogpeppe: Sounds good, we just have to make sure this is more reliable17:38
rogpeppeniemeyer: it's a pity we can't get bootstrap-state to tell mongodb to open a new port17:39
fwereaderogpeppe, niemeyer: https://codereview.appspot.com/6844103 should be trivial (state.InstanceId)18:01
niemeyerfwereade: On it18:03
rogpeppeniemeyer: 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
niemeyerfwereade: Why do we have a method that returns a public field?18:05
niemeyerfwereade: Did you spot why when doing it?18:05
fwereadeniemeyer, to satisfy environs.Instance, I think18:06
niemeyerfwereade: A field can't be part of an interfae18:06
fwereadeniemeyer, hence the method18:06
fwereadeniemeyer, which is part of the interface18:06
niemeyerfwereade: Ah, it's because that field is in ec2.Instance, actually18:06
niemeyerfwereade: Sure, I know why the method exist18:07
niemeyerfwereade: It wasn't clear why the field existed18:07
fwereadeniemeyer, oh, yes, sorry, that too18:07
niemeyerfwereade: CHeers18:07
* rogpeppe has to go now.18:14
rogpeppeg'night all18:15
rogpeppeniemeyer: if you like the new change, i'll submit a bit later.18:15
fssniemeyer: lol18:15
fssniemeyer: ops, wrong message18:15
fssniemeyer: sorry x)18:15
niemeyerrogpeppe: Super, cheers man18:16
niemeyerfwereade: LGTM with a couple of trivials. Thanks18:17
niemeyerfss: :)18:17
fwereadeniemeyer, cheers18:17
fwereadeniemeyer, the casting in the tests was justified in my mind on the basis that, well, we know the method signature and therefore the type18:18
fwereadeniemeyer, and it's slightly more readable IMO18:19
fwereadeniemeyer, no big deal, I'll change them18:19
niemeyerfwereade: In general that should be fine, but these tests are precisely the tests verifying that we know the method signature18:20
niemeyerfwereade: Note that the comment is specifically on the test of the InstanceId method itself18:20
fwereadeniemeyer, ah, yes, true18:20
fwereadeniemeyer, ok, sgtm, thanks18:21
niemeyerfwereade: My pleasure18:21
fwereaderogpeppe, can you precis the thinking behind having an explicitly required MachinerWorker but not an UpgraderWorker?20:29
fwereaderogpeppe, I would just as soon make them both implicit...20:29
niemeyerfwereade: The upgrader is a bit more attached to the details of the agent itself21:15
niemeyerfwereade: Although I'm not sure either if there's enough justification21:15
niemeyerdavecheney: Good morning Dave21:24
niemeyerdavecheney: Please ping me when you have a moment for a call21:25
davecheneyniemeyer: morning21:26
davecheneylemmie get my headset21:26
davecheneyready to go, g+ ?21:31
niemeyerdavecheney: Yep21:32
niemeyerdavecheney: https://plus.google.com/hangouts/_/449c0b5562132d520a43332aaa7f1eb67ec41bd1?authuser=0&hl=en21:33
davecheneyta, for some reason you never show as online on g+ for me21:33
niemeyerdavecheney: https://codereview.appspot.com/6854098/diff/2001/environs/ec2/local_test.go?column_width=9021:41
niemeyerdavecheney: ping21:41
davecheneyniemeyer: ack21:42
davecheneylost you21:42
niemeyerdavecheney: Okay, let me reconnect.. are you still up?21:42
davecheneyyeah, hangout is still working21:42
niemeyerdavecheney: Cool, was my side only then21:42

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