/srv/irclogs.ubuntu.com/2013/09/12/#juju-dev.txt

bigjools2FA is properly getting on my tits lately00:34
wallyworldbigjools: go bot didn't want to land my gwacl branch after i marked the mp as approved00:49
wallyworldretry_policy.go:7:5: cannot find package "launchpad.net/gwacl/fork/http" in any of:00:49
wallyworld/usr/lib/go/src/pkg/launchpad.net/gwacl/fork/http (from $GOROOT)00:49
wallyworld/home/tarmac/trees/gwacl-trees/src/launchpad.net/gwacl/fork/http (from $GOPATH)00:49
wallyworldany clues what i need to do?00:49
bigjoolswallyworld: ummm looks like dependencies were missed when jam set it up, although he did mention that particular one00:50
bigjoolsyou have ssh access to the bot I think?00:52
wallyworldyeah, connecting now00:52
bigjoolsoh wait - that one ought to be in the source00:52
wallyworldneed to figure out how to fix00:52
bigjoolsso I WTFing00:52
wallyworldah00:53
wallyworldit looks in /home/tarmac/trees/gwacl-trees/src/launchpad.net/gwacl/fork/http00:53
wallyworldbut should be /home/tarmac/gwacl-trees/src/launchpad.net/gwacl/fork/http00:53
bigjoolsah00:53
wallyworldso go path is wrong00:54
bigjoolsthis is odd since jam did a few test runs with the bot00:54
wallyworldhmmm00:54
bigjoolswallyworld: congraulations01:17
bigjoolsand congratulations too01:17
wallyworldhuh?01:17
bigjoolsyou landed it finally01:17
wallyworldwell, i hacked it01:18
bigjoolsswoosh01:18
wallyworldi sym linked the dir01:18
bigjoolshaha01:18
wallyworldand removed some .a files which were compiled with go 1.1.201:18
wallyworldwtf happened i have no idea01:18
wallyworldi'll followup with jam later01:18
bigjoolsok01:18
wallyworldthumper: i have a little something for you :-D https://codereview.appspot.com/13302053/01:26
davecheneywallyworld: don't use symlinks01:31
davecheneythe go tool doesn't like them01:31
wallyworlddavecheney: it was just as an emergency to get the build to run on tarmac01:31
wallyworldi'll fix when i talk to jam later cause i have nfi how the bot is set up and why gwacl was fooked01:31
davecheneyok, but symlinking may not unfuck01:38
davecheneyymmv01:38
wallyworlddavecheney: it all worked01:39
wallyworldi've landed gwacl and juju-core branches after my "fix"01:39
davecheneyjolly good01:43
thumperwallyworld: will look shortly01:47
wallyworldnp. thans01:47
wallyworldthanks even01:47
thumperwallyworld: hey02:25
thumperwallyworld: your diff on rietveld is all fucked up02:25
thumpererror: old chunk mismatch02:25
* thumper opens the lp review02:26
thumper1610 lines (+650/-371) 19 files modified02:26
thumperhmm...02:26
thumpergood think I like you02:26
bigjoolsdavecheney: does any state get stored on the client that bootstrapped?02:26
wallyworldthumper: otp to robbie02:26
thumperack02:27
thumperbigjools: what do you mean?02:27
bigjoolsIOW, can I replicate environments.yaml on another machine and expect it to work?02:27
davecheneybigjools: yes02:29
davecheneyyou basically need all of ~/.juju02:29
bigjoolsdavecheney: which question are you answering?02:29
bigjools:)02:29
davecheney21:26 < bigjools> davecheney: does any state get stored on the client that bootstrapped?02:29
bigjoolsok so it's a NO to the second one02:29
bigjoolsbugger02:29
davecheneythe truth is somewhere betwee all of ~/.juju and ~/.juju/environments.yaml02:30
davecheneybut the former is a good aproximation02:30
thumper:(02:30
* thumper tried to do inline reviews on lp02:30
thumperkept clicking wondering why it wasn't working02:30
bigjoolsoh dear02:31
bigjoolsinline reviews always felt like a convenience for the reviewer at the expense of the reviewee02:31
wallyworldthumper: so, do i re-submit? do you know how to resolve that diff issue?02:40
wallyworldthumper: it's only a net 200 line addition - a lot of it is moving code, so it looks bigger than it is :-)02:41
thumperwallyworld: NFI, currently just using LP02:41
* wallyworld can't wait to use use lp and not rietveld02:41
* thumper sighs02:45
thumperwallyworld: there will be changes02:45
wallyworldto my branch?02:46
thumperreview done02:54
wallyworldthumper: thanks for review. saucy is needed because "defaultseries" is not a valid series. runs fine on raring etc03:05
thumperwhere is saucy set?03:06
wallyworldin cmd_test i think, i'll check03:07
wallyworldno, actually just in the test itself03:08
wallyworldbecause when we sync tools we now write metadata, all series names must be valid03:09
wallyworldso "foo" etc won't work03:09
wallyworldthese tests just check output of things, they don't run anything as such03:09
jamwallyworld, bigjools: I had set up the configuration and then the machine rebooted, which caused juju to re "setup" all the config files. At the time I didn't have cert for machine-0 so I couldn't set the juju config, but I think I've sorted that out.03:11
wallyworldjam: ok. so i'll go and and remove the symlink i added03:12
wallyworldi wonder why son stuff had been compiled with Go 1.1.203:12
wallyworldsome03:12
wallyworldand when I triggered the bot it used 1.1.103:12
jamobviously I messed up that line03:14
wallyworldah, ok :-)03:14
jamI'll clean that up now03:14
wallyworldthumper: logging - i added loggo.GetLogger("").SetLogLevel(loggo.INFO) cause othewise infos weren't printed03:22
wallyworldis there a better way to do it?03:22
thumperdon't look for the logging info03:22
thumperand yes, use the LoggingSuite03:22
wallyworldthumper: the implementation changed - it all used to be printed to stdout, now there's a combination of stdout and logging03:23
wallyworldsince the code which used to be in the command is now in a lib03:23
wallyworldand the lib uses logging03:23
wallyworldthe cmd uses stdout03:23
wallyworldand the tests check the output03:23
thumperif the output should be writting out, don't use logging03:23
thumperit's just wrong03:24
wallyworldi don't want a lib to print to std out03:24
thumperagreed03:24
thumpertest the results03:24
thumpernot the logging03:24
wallyworldwhich i do - but jam likes to check logging output03:24
wallyworldand the tests were written that way before my changes03:24
wallyworldi'd be happy to kill that bit of the tests03:25
thumperI vote to kill those bits03:25
wallyworld\o/03:25
thumperas long as the results are tested03:25
thumperdon't check the logging03:25
wallyworldthey are03:25
thumperseems like a waste03:25
jamthumper: *today* we have a terrible UX because we don't tell the user anything. While I agree that logging shouldn't go to stdout, I think it is better to start putting some stuff out there and then fix stuff as we can.03:26
wallyworldto be clear - there's 2 place we check logging - in the command we are talking about here and also in simplestreams lib. i won't change the simplestreams lib in this cause it's part of different work03:26
jamHow many of us default to "juju $SOMETHING -v" ?03:26
thumperwell that is about to break03:27
* thumper is writing a branch03:27
thumperjam: I'm not against writing it out in logs03:27
thumperbut we shouldn't be testing the logging03:27
wallyworldjam: so your log output tests won't change in the stuff i am doing here - there was a separate test in the cmd which also did log checking03:27
* thumper taking another daughter to the doctor03:27
jamthumper: we've had multiple times where we've screwed up formatting in logs03:27
jamwhy wouldn't we want to test it03:27
jamwallyworld: isn't that "test what the user actually gets to see" which we probably want to keep?03:28
wallyworldjam: it used to be all stdout, but it not now03:28
wallyworldsince core functionality was moved to a lib03:28
jamwallyworld: If I remember the code, it originally went to ctx.stdout, but then the code got moved, and it accidentally got suppressed by default03:28
wallyworldand the lib logs03:28
jamwhich is why we wanted to test that the user actually gets feedback03:28
wallyworldtrouble is, libs should log, not print to std out03:29
jamwallyworld: we need a process in place to allow lib *stuff* to generate user visible results. If a lib is going to block for 10min, it should have a way to inform the user what is going on. Right now the only thing we have is logging.03:30
wallyworldjam: agreed. but here we are not blocking03:30
wallyworldthis is a command which writes some metadata03:31
jamwallyworld: when I was writing synctools it takes several seconds to download something and upload something, and it was *really* nice to get feedback03:31
jamI think users would appreciate it as well03:31
jamhence I made it visible.03:31
jamIn IOM thumper fwereade_ and I agreed that we should default commands to having helpful user feedback03:31
jamthe exact definition of that is in the air, but it is a change from silent-by-default03:31
wallyworldsure. so the loggig is done at INFO level03:31
jamsync-tools was the first experiment in that direction03:31
wallyworldwhich is not visible by default03:32
thumperso...03:32
wallyworldso we need to change that03:32
thumperit should be passing through the context03:32
thumperthat controls stdout and stderr03:32
thumperand write to those03:32
thumperI'd like to have it so only developers care about --debug (or --show-log)03:33
thumperwhich is what I'm writing03:33
thumper--verbose should set a flag in the execution context03:33
thumperas should --quiet03:33
thumpersqueezing time in to do this is tricky though03:33
wallyworldi like using execution contexts, they work well. we have a shit load of code to change in that case03:33
thumperwallyworld: there shouldn't be too much03:34
thumperas most of the code is server side03:34
thumperhowever yes, the client side code should accept a command context03:35
wallyworldyep03:35
jamthumper: we have a modest amount of shared code03:35
wallyworldi'll change my current branch03:35
thumperjam: less and less as the command line moves to the api03:35
jamthumper: perhaps more and more? :)03:35
jamactually shared execution of code, just not running in the client :)03:36
wallyworldthumper: so how do we envision the api getting status messages back to the caller03:36
wallyworldwe need a return path03:36
thumperwallyworld: pass through the cmd.Context from the Run method03:36
wallyworldi haven't looked at the code - so long as that is supported it's all good03:36
thumperI bet gnuflag doesn't like something like -vv03:37
thumpercalling the v flag twice03:37
wallyworldor -vvvv even :-)03:37
jamthumper: it likes it just fine, it just treats it as a single call, though03:38
thumperof the dick who goes -vvqqvqv03:38
jamthumper: it isn't too uncommon to have "last flag wins"03:38
jamso you can have an alias and overwrite it03:38
jambzr used that03:38
* jam goes on a walk before it gets too hot outside03:40
wallyworldthumper: it seems other code uses a different pattern03:44
wallyworldloggo.RegisterWriter("synctools", sync.NewSyncLogWriter(ctx.Stdout, ctx.Stderr), loggo.INFO)03:44
wallyworldso the sync tools command calls a lib function without needing to pass in a context03:45
wallyworldso the api stays clean03:45
wallyworldand can sensibly be used easily by client and server callers03:45
thumperyeah, that'd work03:45
wallyworldi'll do that then03:45
thumperhowever03:45
thumperthe synctools stuff will get sent to all the writers03:46
thumpernot just the synctools writer03:46
thumperit'll appear in the logs too03:46
wallyworldwhich is probably ok03:46
thumperwhat does the NewSyncLogWriter do?03:46
wallyworldreturn &syncLogWriter{out, err}03:46
wallyworldtype syncLogWriter struct {03:47
wallyworldout io.Writer03:47
wallyworlderr io.Writer03:47
wallyworld}03:47
wallyworldsyncLogWritter has a write method03:47
wallyworldwhich prints to out or err03:47
wallyworlddepending on level03:47
wallyworld< info goes to err03:47
thumperick03:48
thumperthat seems very messy03:48
thumperin that it is trying to do too many things03:48
wallyworldthere's also this line: if name == "juju.environs.sync" {03:49
wallyworldso it exits early if not the environs sync logger03:49
wallyworldi'll see if i can do something sensible for the tools metadata command, so i don't need to mess with logging in the tests03:50
thumperok, cool03:57
bigjoolsI'm struggling to publish a charm, it's looking for a config.yaml in the directory above the series name dir, is that right?03:57
thumperdafaq?03:58
* thumper got a weird test failure03:58
thumper DebugHooksServerSuite.TestRunHookExceptional03:58
thumper:(03:59
axwuh oh03:59
thumperintermittent test failure03:59
axwthat's my code :)03:59
axwwhat's the error?04:00
thumperno, it is the apiserver04:00
axwermm04:00
thumperno04:00
thumperapiuniter04:00
axwyeah it's the debug-hooks code04:00
thumperaxw: I'll paste it for you04:00
axwsome of those tests are time based, sadly... probably needs a tweak04:00
thumperaxw: http://pastebin.ubuntu.com/6095464/04:00
axwthumper: thanks04:01
thumperaxw: it succeeded when running alone04:01
thumpertwice04:01
axwthat error message is difficult to parse04:03
axw"type must before start value"04:03
axwwhat.04:03
thumperthat may have been me04:04
thumperway back04:04
thumperpossibly04:04
thumperif it is a checker04:04
thumperwallyworld: while you are there04:06
thumperthe sync tools test is very noisy04:06
thumperit writes to stdout04:06
thumperplz fix04:06
axwthumper: was your machine stressed when this failed?04:07
thumperaxw: no, not at all04:07
thumperwell, it was running the tests04:07
wallyworldthumper: will do. there's a couple of places that use that log writer for commands, so  i'll make it generic and we can tweak it later04:09
thumperacj04:09
thumperack04:09
axwthumper: I ask because it seems the test took >100ms between recording a timestamp and executing the debug hook... bleh. this will be a pain to fix04:10
* thumper tries again04:10
thumperaxw: my laptop is faster than average04:10
thumperso I'd be real surpised04:10
axwI can't get it to happen at all, and my laptop's probably slower than average... hrm.04:14
thumperwallyworld: your turn:  https://code.launchpad.net/~thumper/juju-core/show-log/+merge/18519304:14
thumperaxw: no idea how it happened04:14
wallyworldok04:14
thumpertiming tests suck04:14
axwthumper: I'll log a bug to make it not/less time sensitive04:15
thumperkk04:15
thumperwallyworld: Rietveld: https://codereview.appspot.com/13352052 if you'd prefer04:16
wallyworldi can click trhough the mp you know :-)04:16
* thumper wants to add a base test suite that has "AddCleanup(cleanup func())"04:17
axwthat would be nice.04:18
thumperwould mean we wouldn't need test tear downs04:18
thumpersould have an AddSuiteCleanup(cleanup func()) too04:18
thumpershould04:18
thumperfuck it04:21
thumperdoing it now04:21
thumperit has been on my list for too long04:21
wallyworldthumper: review done. quick, land it before anyone can object :-)04:24
thumperwallyworld: ok04:24
thumperdavecheney, axw: what is the fastest/easiest way to iterate backwards over a slice?04:34
davecheneyfor i := len(s) ; i != 0 ; i-- { ... }04:34
axwfor i := len(slice)-1; i >= 0; i-- {} ?04:34
axwor that04:34
davecheney^ do what he said04:34
axwor not, because of off by one04:35
axw:P04:35
* thumper was hoping for a reverse builtin04:35
axwno reversed for you I'm afraid04:35
axw:)04:35
thumpercan we use bound functions yet?04:38
thumperare we 1.1?04:38
axwI'm pretty certain I've seen changes from rogpeppe to use 1.1 syntax04:39
axwdavecheney: ?04:39
wallyworldyeah, we are 1.104:41
wallyworldthumper: if i register a log writer with level INFO, there are no messages cause getEffectiveLevel() is still WARNING. i'm not sure what i'm doing wrong04:42
wallyworldloggo.RegisterWriter("toolsmetadata", cmd.NewCommandLogWriter("juju.environs.tools", context.Stdout, context.Stderr), loggo.INFO)04:42
thumperwallyworld: the level specified there is the minimum level that the writer will write04:42
thumperit doesn't specify any log levels04:42
thumperso that writer won't write debug or trace04:42
thumperyou still need to explicitly specify the value for the logger04:43
thumperor set the root level04:43
wallyworldthe logger is specified elsewhere04:43
wallyworldok, i'll set root level, via loggo.GetLogger("") ?04:43
axwthumper: is there any good reason to allow --to=lxc:0 for the local provider?04:44
axwcontext: https://codereview.appspot.com/13632046/patch/4001/501304:44
thumperaxw: no04:44
thumpernot at all04:44
thumpernot yet04:44
axwnot... yet?04:44
thumpernested lxc doesn't work just now04:44
thumperit works later04:44
axwright, but 0 isn't a container04:45
axwI meant specifically 0, sorry04:45
axwI would say no, because you can just create a "machine"...04:45
thumperoh, specificlly 0, no never04:46
thumpermachine 0 for the local provider doesn't actually have the job "manage units"04:47
thumperso you can't04:47
thumperor at least there should be a check for that :)04:47
axwokey dokey04:47
thumperdamn05:02
thumperjust accidentally closed the emacs that opened for my lbox message05:03
thumpersecond side-tracked branch for the day proposed05:04
thumper wallyworld, axw:  https://code.launchpad.net/~thumper/juju-core/cleanup-suite/+merge/18519905:04
wallyworldlooking05:05
wallyworldthumper: i've pushed my changes to the branch you reviewed05:05
thumperwallyworld: want to lbox propose it to see if it fixes the diff?05:06
wallyworldok, doing now05:06
thumperta05:09
wallyworldthumper: done, and looks like fixed05:09
thumpercool05:09
axwthumper: lgtm05:10
wallyworldthumper: thanks :-)05:15
=== thumper is now known as thumper-afk
thumper-afkback later for meetings05:26
davecheneyjam: /me waves06:15
jamhey davecheney06:15
davecheneyjam: what do I need to do to land that mgo revno:240 change06:15
davecheneydo I need to backport the other branches on that issue ?06:15
jamdavecheney: to 1.14 ?06:15
davecheneyy06:15
jamdavecheney: you should be able to propose a merge against "lp:juju-core/1.14" and then mark it approved and add the commit message.06:17
jamI updated the bot to monitor it06:17
jamthe particular branch should be just that change, and not the branch you landed to trunk06:17
davecheneyyup, that was the branch you approved yesterday06:17
davecheneythen there was some muttering about it not passing06:18
jamdavecheney: can you point me to it, I'm not seeing it right away06:18
jamdavecheney: ah, sorry if my comment was short, we should be able to just Approve it and try again, if it still fails we can backport the "don't run this test" patch and land it06:19
jamthough it is marked approved, so let me look closer06:19
davecheneyhttps://bugs.launchpad.net/juju-core/+bug/122170506:19
_mup_Bug #1221705: relationunit_test.go: 2 tests fail with mgo >= 241 <juju-core:Fix Released> <juju-core 1.14:In Progress> <juju-core trunk:Fix Released> <mgo:Confirmed> <https://launchpad.net/bugs/1221705>06:19
davecheneymight need to flip it06:20
jamno, that's a different one06:20
jamdavecheney: looks like the bot succeeded in landing it, but failed to push it back up, give me a sec06:21
jamdavecheney: fixed, and should be fixed for future patches to 1.14 sorry about that.06:21
davecheneyjam: sweet06:26
davecheneyi'll make as comitted06:26
davecheneythumper-afk: does it say anyuwhere public that the local provider does not work with precise ?06:43
rogpeppemornin' all06:45
* davecheney waves06:46
mgzmorning!07:29
TheMuemgz: morning07:34
jamhi TheMue and mgz07:39
fwereade_jam, rogpeppe: https://codereview.appspot.com/13355044/08:36
rogpeppefwereade_: yes?08:37
fwereade_jam, rogpeppe: IIRC my reading was that it was basically sane, so I was about to do a pass with a view to LGTMing08:37
fwereade_jam, rogpeppe: just wanted to check with jam if there were any not-lgtmy concerns08:37
fwereade_jam, fwiw, I can't think of many cases in which one'd want to look in local .ssh for authorized_keys08:38
rogpeppefwereade_: jam doesn't seem to be around currently08:38
fwereade_rogpeppe, bah, so he doesn't08:39
fwereade_rogpeppe, I'll finish in case he scrolls back08:39
rogpeppefwereade_: why wouldn't you want to use your local ssh keys?08:39
fwereade_jam, I said something insane a moment ago but I'd like to chat about it to make sure we're on the same page08:40
rogpeppefwereade_: (what other keys would you use?)08:40
fwereade_rogpeppe, something about what he said made me worried we had the old authorized_keys confusion cropping up08:40
fwereade_rogpeppe, ie I wanted to be clear that local authorized_keys are not usually what you're looking for08:41
rogpeppefwereade_: i'm not sure i get that08:41
fwereade_rogpeppe, I do agree that we should indeed be getting the user's public key from .ssh08:41
fwereade_rogpeppe, but for a while we were reading authorized_keys from .ssh08:41
rogpeppefwereade_: ah!08:41
rogpeppefwereade_: yes, indeed08:42
rogpeppefwereade_: i'd forgotten the distinction08:42
fwereade_rogpeppe, and that's incoherent at worst and... surprising at best, even if there is a potentially valid use case08:42
fwereade_rogpeppe, ...which would itself be supported just fine with an explicit authorized-keys-path08:42
fwereade_jam, anyway, https://codereview.appspot.com/13355044/08:43
fwereade_jam, are you aware of any blockers or shall I go ahead and review with intent to aprove?08:43
jamfwereade_: I don't know of any specific blockers. I had spotted some stuff that looked like it would contradict the work to split out the files, but nothing that breaks anything today08:44
fwereade_jam, ok, thanks, I will be particularly sensitive to that08:44
rogpeppefwereade_: AFAICS we don't read ~/.ssh/authorized_keys by default08:45
fwereade_rogpeppe, indeed we don't, I suspect I just misread something jam said that suggested to me I should mention it explicitly08:45
fwereade_rogpeppe, https://codereview.appspot.com/13355044/ reviewed09:20
rogpeppefwereade_: thanks!09:20
fwereade_rogpeppe, it's basically awesome, just not quite LGTM09:21
rogpeppefwereade_: i already plan on making two constructors - but having one constructor as an intermediate step means that i can instantly catch all old-style calls to New09:21
rogpeppefwereade_: so i'd prefer to leave it like this for this CL, so that I can catch any new calls that have been added to trunk since I last merged09:22
fwereade_rogpeppe, ok, that makes sense if there's an easy followup coming09:23
mgzfwereade_: have you got a moment to explain why bug 1200267 happens to me?09:30
_mup_Bug #1200267: Expose when stable state is reached <canonical-webops> <papercut> <juju-core:Triaged> <https://launchpad.net/bugs/1200267>09:30
mgzas far as I can tell from the code, we cmd.Wait on the hook script, with no timeout, and uniter only sets the hook state to done after RunHook completes09:31
fwereade_mgz, I think that problem is much harder than it looks09:31
mgzit looks hard enough that I have no idea how it happens at all :)09:32
fwereade_mgz, first, what do you mean by "happens to me"?09:32
fwereade_mgz, it is true that the bug exists, and nobody can currently tell when a unit is "stable"09:33
mgzer... I don't, what did I mean... er, just how those symptoms happen at all09:33
fwereade_mgz, are we definitely talking about the same bug? the one you linked STM to be a feature request09:34
mgzif a hook script takes 30mins to run, I don't see how `juju status` would report anything other than pending till 30mins was up09:34
mgzbut apparently it does09:35
fwereade_mgz, if "install" takes that long, I think you're stuck in pending09:35
fwereade_mgz, then "installed" will last for a config-changed and a start09:35
fwereade_mgz, after that it's just "started" independent of what's going on09:36
fwereade_mgz, in theory we could just add a status -- started (busy) vs started (idle), say -- but actually setting that sanely is more challenging than one might hope09:37
mgzhm, and then relations are another thing again09:38
fwereade_mgz, right09:38
yolandahi, i'm trying to add some nagios functionality to a gerrit charm, and i need some advice. I see other charms like memcached, postgres... that are using nagios plugins for it, but there isn't a nagios plugin for gerrit, what should be the best way to proceed?09:39
fwereade_mgz, we can tell remotely when a unit agent's started participating in a relation, but we can't tell what it's doing very helpfully09:40
fwereade_yolanda, you may be better off asking in #juju, nagios is not something I know09:40
yolandafwereade_ ok, thx09:41
mgzfwereade_: thanks a lot, that made things much clearer for me09:42
=== frankban__ is now known as frankban
jammgz: https://plus.google.com/hangouts/_/8aa005e26ea2bcdf23aaade34a5e64182dcd04c310:04
rogpeppedavecheney: meeting10:04
rogpeppemgz: meeting?10:04
mgzgah, rebooting10:06
mgzwill be 2 mins at least, start without me10:06
=== frankban__ is now known as frankban
jamdimitern: I just sent you an email that fwereade_ remembered about the Uniter API. can you read it and see if it makes sense. If you need clarification, just poke me here.11:46
* dimitern whew.. irc is finally working! quassel betrayed me today totally11:48
dimiternjam, ok, will do11:48
dimiternjam, that's kinda strange - charm bundles were there log before the gui started using it11:51
dimiternjam, but we can rename it to archive i suppose - in a follow-up though11:51
jamdimitern: well, "Bundle" wasn't a publicly exposed name, and I think juju-deployer called them that11:51
jamanyway, the goal is to not have a stable release with Bundle in the API attributes11:52
jamso we can reserve it for future use.11:52
dimiternjam, what do you mean by "publicly exposed" ? it's in the source11:52
dimiternjam, yeah, I got that11:52
jamdimitern: you never referenced them as a bundle in command line syntax, etc11:53
jamit is the name in the source file11:53
jambut not in terminology someone not hacking on the source would have ever seen11:53
dimiternjam, perhaps because bundles are only useful when deploying stuff from the store11:53
rogpeppeweird behaviour when logging into wiki.canonical.com (seen it several times now) - it asks me for my password (ok), then my 2 factor key, which is says is invalid, but then when i go to wiki.canonical.com, i'm logged in ok12:07
rogpeppes/is says/it says/12:08
dimiternrogpeppe, do you use a ubikey or the android app for 2fa?12:09
rogpeppedimitern: yubikey12:09
dimiternrogpeppe, I had a similar issue with my yubikey some time ago - it got out of sync after pressing the button one time too many out of the context of the 2fa form12:10
dimiternrogpeppe, i had to remove and reauthorize it to get it working again - good that i had a backup device (my phone)12:10
rogpeppedimitern: if that happened, surely i wouldn't be able to then access the site successfully (unless our security is borked, i guess)12:10
rogpeppedimitern: yeah, i need to get my phone working as an auth device again12:11
arosalesjam, would you suggest trunk or 1.13.3 for testing Azure?12:12
jamarosales: so we know today that there are 1-2 bugs for 1.13.3 in Azure. You could try lp:juju-core/1.1412:12
arosalesjam, ok12:14
arosalesjam, and to confirm my understanding12:14
arosales    # public-storage-account-name: jujutools12:14
arosales    # public-storage-container-name: juju-tools12:14
jamarosales: those are commented out because that is the hard-coded defaults internally12:15
arosalesif that is in your env.yaml you still get those be default, correct?12:15
arosalesok12:15
jamarosales: so the one thing to confirm with juju-1.13 is that we actually get the tools from the shared public bucket12:19
jamsince 1.14 hasn't been released you have to --upload-tools12:19
jambut 1.13.3 should have tools in the bucket already12:19
arosalesjam I can give that a try12:20
fwereade_dimitern, jam: I just thought that we possibly *have* exposed the bundle terminology via the store, which sucks a bit12:28
jamfwereade_: well, we can try to minimize existing terminology to migrate to new terminology12:28
fwereade_jam, dimitern: indeed, +112:29
arosalesjam, as of 1.13.3 it looks to be correctly using the public tools bucket by default (ie I had the public-storage-account-name and public-storage-container-name commented out)12:39
arosaleshttp://pastebin.ubuntu.com/6096850/12:39
arosalesjam, also did bootstrap the correct precise release image by default12:39
jamarosales: that is by commenting out the image-stream, etc. Right? IIRC 1.13.3 still "juju init" with saucy in the config.12:40
arosalescorrect I commented it out12:40
jamwallyworld, davecheney: It looks like the azure public bucket didn't get the 1.13.3 tools, we'll need to be really careful with 1.14+ since after 1.14 we will require exact matches.12:41
arosalesspecifically image-stream and default-series I commented out12:41
arosalesya 1.13.2 was the latest found12:41
wallyworldjam: the release manager needs to upload them :-)12:42
jamwallyworld: I don't think davecheney has the azure creds, I know *I* don't. Do we know who controls the "jujutools" account?12:43
jamis it just bigjools and friends ?12:43
wallyworldi *think* so12:43
arosalesjam, I do12:43
wallyworldjam: when I say "needs to upload", that can include delegation12:44
arosalesjam, wallyworld, specifically the azure tools bucket12:44
wallyworldwe should have a socumented release process, preferrably automated12:44
wallyworldhopefully orange squad can progress that12:44
jamhp and ec2 both have 1.13.3, but it didn't get to azure12:48
arosalesjam, wallyworld, specifically the azure tools bucket tools juju-tools bucket being used the the released precise image stream being used12:50
wallyworldsay wot?12:50
jamarosales: right, so that bit looks to be working, but we want to get 1.13.3 into that container, and make sure to get 1.14 when we release it.12:50
jamwallyworld: arosales just tested that juju-1.13.3 finds 1.13.2 in our azure-local mirror of the tools.12:50
jamand that it picked Precise as the default image (for a while it was configured for Saucy)12:51
wallyworldok12:51
jamand 'daily' stream rather than Released12:51
arosalesjam, ack. bigjools has a tool to do that . . . I *think* I may have that command in some notes somewhere. I can get that to whom ever does the uploads12:51
jamarosales: "juju sync-tools" will do the right thing if you configure an environment with the right settings12:51
jamit is what I use for HP and Canonistack12:52
arosalesjam to confirm on "<jam> and 'daily' stream rather than Released"12:52
wallyworldwe may we be able to use the released stream now12:53
arosalesthe correct behavior and looks like the current default is to use the "released" stream rather than the "daily"12:53
arosalesto confirm my understanding of the defaults12:53
jamarosales: so the thing to check is what does "juju init --show" output12:54
jamarosales: for 1.13.3 it still says "saucy" and "daily"12:54
jambut for lp:juju-core/1.14 it should have those lines commented out12:54
jamand have:12:54
jam# image-stream: ""12:54
jam# default-series: precise12:54
arosalesjam, yes sorry I was saying with the appropriate lines commented out Juju has the correct defaults in 1.13.312:54
jamarosales: right, it did, but the bug we have for 1.14 is making sure we got the config right12:55
arosalesjam, gotcha12:55
jamarosales: very nice to know the tool lookup and all that was working properly in 1.13.3 (as long as we actually had the right tools and series available :)12:55
arosalesand I follow you in 1.13.3 not having this in the default yaml12:55
arosalesbut I was relived to see juju did the correct behavior when commenting out the image steam and tool bucket keys.12:56
jamarosales: yeah, the internal defaults were all correct for where we wanted to be, it was the config that was overriding it because they weren't properly available12:56
arosalesjam, cosmetically are you ok with the config having the following for the default12:58
arosales  # force-image-name: b39f27a8b8c64d52b05eac6a62ebad85__Ubuntu-13_10-amd64-server-DEVELOPMENT-20130713-Juju_ALPHA-en-us-30GB12:58
jamarosales: I don't think I'm particularly happy with it, but if it is an example image name it is what we have to live with, I think12:58
arosalesjam, ok. That image is probably deprecated, but is does have the correct format for an azure image name.13:00
dimiternfwereade_, updated https://codereview.appspot.com/13355046/13:08
rogpeppefwereade_: responded https://codereview.appspot.com/1335504413:22
fwereade_dimitern, rogpeppe, ack13:22
fwereade_dimitern, rogpeppe: bah, neither is obviously simpler to read than the other ;p13:26
rogpeppefwereade_: lol13:26
rogpeppefwereade_: what's the "Forbidden Design Pattern" ?13:29
fwereade_rogpeppe, sorry, Singleton ;p13:29
rogpeppefwereade_: yes, having a single shared config for all dataDir+tag combinations would work fine.13:35
rogpeppefwereade_: (probably)13:35
rogpeppes/for all/for each/ s/combinations/combination/13:36
dimiternfwereade, updated https://codereview.appspot.com/13604045/ as well - added tests13:40
rogpeppefwereade_: this was about as far as i got with the "set up mocking beneath jujud" thing BTW: http://paste.ubuntu.com/6097076/13:46
rogpeppefwereade_: the idea being to mock out (or interpose some call check on) the functions in stateWorkers and apiWorkers13:47
rogpeppefwereade_: but even doing that doesn't quite seem quite joined up enough for me to be convinced about the approach13:47
dimiternfwereade_, ping13:54
fwereade_dimitern, pong13:54
dimiternfwereade_, so first take a look at perhaps the smaller CL? https://codereview.appspot.com/13604045/13:55
fwereade_dimitern, ok, will do, I'm nearly done with rog's13:55
dimiternfwereade_, cheers13:56
fwereade_rogpeppe, reviewed14:00
rogpeppefwereade_: "14:01
rogpeppeAh, I see. In that case I question 0 having a special meaning... was it really14:01
rogpeppelike this before?14:01
rogpeppe"14:02
rogpeppefwereade_: yes - see the definition of Config.StatePort and Config.APIPort previously14:02
fwereade_rogpeppe, ah, I thought that was explicitly for dealing with them being missing14:03
rogpeppefwereade_: well, i think that was probably the intent14:03
fwereade_rogpeppe, we had to keep them missing to work with 1.10 iirc14:03
fwereade_rogpeppe, but it was asInt basically I guess?14:04
rogpeppefwereade_: yeah14:04
fwereade_rogpeppe, bleh14:04
fwereade_rogpeppe, ok, pre-existing behaviour, bug it and go :)14:04
rogpeppefwereade_: https://bugs.launchpad.net/juju-core/+bug/122449214:06
_mup_Bug #1224492: environs/config: zero-valued port settings are allowed but ignored <juju-core:New> <https://launchpad.net/bugs/1224492>14:06
fwereade_rogpeppe, thanks14:07
fwereade_dimitern, little one LGTM with a tweak14:08
dimiternfwereade_, thanks14:08
natefinchmgz: poke :)14:11
mgznatefinch: hey!14:13
natefinchmgz: so,... I tried to follow John's steps, but I think I was starting from something of a bad state, so I'm trying to get myself into a good state.14:14
TheMuefwereade_: ping14:14
fwereade_dimitern, LGTM14:15
fwereade_TheMue, pong14:15
dimiternfwereade_, sweet!14:15
natefinchmgz: namely, I think I had my trunk colo set up wrong, so I'm stating over (luckily I don't have anything local to lose right now)14:15
TheMuefwereade_: just stumbled over https://bugs.launchpad.net/juju-core/+bug/1199698 again14:15
_mup_Bug #1199698: intermittent test failure in BootstrapSuite <intermittent-failure> <juju-core:In Progress by themue> <https://launchpad.net/bugs/1199698>14:15
mgznatefinch: okay, so what I just did14:15
TheMuefwereade_: the bug still exists, as commented, with GOMAXPROCS > 114:15
TheMuefwereade_: with GMP=1 it's ok14:16
mgz`bzr log|less` then /Nate to find the change14:16
fwereade_TheMue, so is the problem that multiple tests run concurrently against the same dummy environment or something?14:16
mgzthen `bzr log -n0 -r1788` to see the commits on that merged branch... seems to just be one mistaken one, r1665.7.11 which makes life easy14:17
fwereade_TheMue, or can it be isolated within a single test method?14:17
natefinchyep14:17
mgzso, then it's just switch to a new branch, reverse that commit with `bzr merge -r1665.7.11..1665.7.10 .` (note dot on end), then you could commit that and lbox propose it14:18
mgzthen I'd probably eye the diff, self approve, and rv-submit14:18
TheMuefwereade_: it's always the same test method, a table driven test. but there in different tests.14:18
fwereade_TheMue, and it fails when run on its own too?14:18
mgznatefinch: one thing that might help with the confusion over coloworkspaces is a bash prompt hack14:19
mgzI don't use one myself, but jam does, and I think jelmer had one for colo stuff14:19
TheMuefwereade_: have to test again, one moment14:19
natefinchmgz: hmm yeah, that would help a lot14:19
mgzremember `bzr branches` will tell you what's around, and the star is the current active one14:20
natefinchmgz: my main problem right now is that I think I had my colo-trunk branch messed up. I couldn't ever pull changes from lp:juju-core, had to always merge them in, which seemed to confuse bzr (or at least confuse me).  I think I have to redo my colo setup14:20
mgzit is still a little easy to get yourself confused, but that's just the colo joy14:20
TheMuefwereade_: yep, same behavior14:20
TheMuefwereade_: it's the BootstrapSuite in cmd/juju14:21
mgznatefinch: it's easy enough to start from "scratch", move your old dir out of the way for the moment,14:21
fwereade_TheMue, ok so if any of this is new please do add it to the bug14:21
mgz`bzr branch lp:juju-core && cd juju-core && bzr switch -b trunk`14:21
TheMuefwereade_: ... Panic: interface conversion: interface is nil, not dummy.OpPutFile (PC=0x414321)14:21
mgzthen you can branch in any current feature branches from the old dir as well14:21
TheMuefwereade_: it's not new14:22
fwereade_TheMue, but unless it's really slowing you down I'm less inclined to worry about it... it's the dummy provider, which is a bit evil to begin with, and it only shows up when doing concurrent things with the known-grubby backend14:22
TheMuefwereade_: but i think to solve it the way of testing the bootstrapping would have to be changed14:22
fwereade_TheMue, that particular panic just implies we don't have our dummy resetting working quite right14:23
TheMuefwereade_: no, it's not slowing me down, i now use GMP=114:23
TheMuefwereade_: i only dislike this issue as open14:23
fwereade_TheMue, ok, good to confirm it still exists though, probably a good idea to note it14:23
TheMuefwereade_: ;)14:23
fwereade_TheMue, I'm afraid it's relatively low on my own mental list14:23
TheMuefwereade_: yep, it is14:24
TheMuefwereade_: but if it cares nobody it only fills the list. so i'll set the importance to low14:25
natefinchmgz: thanks, I think I'm all set now14:33
mgznatefinch: ace, do bug me if you need anything explaining or get frustrated with some issue or other14:34
natefinchmgz: thanks. I'm good 99% of the time, it's that pesky 1% :)14:35
mgzah, and john's emails have now come through to me, and he mentions the bash thing. let's bother him for the source later.14:36
natefinchmgz: definitely14:36
rogpeppedimitern: ah, dammit, your submit got in before mine and caused conflicts14:46
natefinchfwereade_, jam:  anything you guys want me to look at, once I get my prior mess cleaned up?  I have to do a little testing on the logrotate stuff to double check a concern Dave brought up, but  otherwise, I don't think I'll have enough work for the full day.14:46
fwereade_natefinch, definitely once I'm done with this call14:47
dimiternrogpeppe, oh, sorry about that14:47
natefinchfwereade_: cool, thanks14:47
rogpeppedimitern: i want to configure my machine so it shouts at me when the bot rejects a branch14:47
dimiternrogpeppe, after tinkering with the bot few days ago I realized tarmac is actually picking up branches to land from the approved list sorted alphabetically, so this might be the case14:48
natefinchrogpeppe: +114:48
rogpeppedimitern: it would be much nicer if it picked it up by approved time14:48
natefinchdimitern: so we'll start seeing a lot of branches starting with underscores? :)14:48
dimiternnatefinch, :)14:48
dimiternnatefinch, actually looking at the bot log there are quite a few branches that get picked up and then dropped, but are still in the review queue somehow - we need to clean up old stale code14:49
dimiternfwereade_, I've got another CL for you, sorry :) https://codereview.appspot.com/1330205414:50
dimiternfwereade_, and then 2 more until the uniter+api lands14:50
fwereade_dimitern, cool14:51
rogpeppefwereade_: this might be ready to land now - i've added tests for connectionIsFatal: https://codereview.appspot.com/1364004314:53
* rogpeppe crosses his fingers for another go14:57
rogpeppeis there anyone around that is easily able to run some live tests under non-ec2 environments?14:58
mgzrogpeppe: I probably can14:59
natefinchrogpeppe: lemme check my azure environment, it should be workable14:59
mgzthough, you should be able to canonistack just as easily :)14:59
rogpeppemgz: yes, i should really set that up15:00
fwereade_rogpeppe, dimitern: both LGTM15:00
rogpeppefwereade_: thanks15:00
dimiternfwereade_, cheers15:01
* rogpeppe gets in there first :-)15:01
natefinchrogpeppe: do you want my azure creds?  not sure if that would be easier or not for you15:02
rogpeppenatefinch: i did have some azure creds, but my initial free period expired. i'm not sure how much of the azure provider is meant to work actually. have they actually got any live unit tests?15:03
mgzrogpeppe: the live tests aren't run on azure, I'm pretty sure15:03
rogpeppemgz: so how much of azure actually works? does it bootstrap and deploy?15:04
mgzmanually doing the wordpress-mysql example is something instead I think15:04
natefinchrogpeppe: it works15:04
natefinchrogpeppe: bootstrap is slow as molasses, but it owrks15:04
* rogpeppe wonders why it doesn't have any of the live tests hooked in then15:04
mgzthe live tests aren't the easiest thing to adapt to non-ec2 environments15:04
rogpeppemgz: they were *supposed* to be provider-independent15:05
mgzI'm pretty sure only the openstack provider has made the effort15:05
rogpeppemgz: if they're not, i'd like to know why, so we can improve them15:05
natefinchrogpeppe: I have an MSDN account that gives me $150 worth of azure credit per month, which I'm not going to be using for anything personal, so might as well make it useful for work15:05
rogpeppenatefinch: ah, that would be useful. i hate claiming expenses.15:06
natefinchrogpeppe: ditto15:06
mgzwriting things to be generic when you have exactly one backend tends to not exactly work out. with some effort, I'm sure they could be made to do... things... on the other providers15:07
natefinchmgz: oh, I'm sure they do ...things... on the other providers right now :)15:07
dimiternjam, fwereade_, next one https://codereview.appspot.com/1367704315:08
natefinchmgz: just perhaps not the right things ;)15:08
mgzyeah, get skipped :P15:08
rogpeppemgz: that's why i think it's important that we try to make the live tests work on all the providers.15:17
rogpeppemgz: the idea is to exercise the Environ interface15:18
rogpeppemgz: which everything else relies on15:18
rogpeppei might try adding some of the live tests to azure at some point15:19
=== frankban_ is now known as frankban
* rogpeppe goes for a bit to eat15:21
rogpeppebite even15:21
dimiternnatefinch, perhaps you can review this small branch? https://codereview.appspot.com/1367704315:21
natefinchdimitern: sure15:27
natefinchdimitern: the halfway rename messes with my head.15:32
dimiternnatefinch, why half way?15:33
natefinchdimitern: we still refer to them as bundles when we're not talking about the API.  For example state.Charm.BundleURL()15:34
dimiternnatefinch, we're not renaming them in state, just whats "exposed" by the API15:34
ahasenackif, from a config-changed hook, I list all relations I have15:35
natefinchdimitern: that's the problem I have with it.  That's why it's halfway.  Over here we call them archives, over there we call them bundles. That's confusing15:35
ahasenackwill broken relations be in that list?15:35
ahasenacklike, relations that were attempted but are in a failed state?15:35
dimiternnatefinch, if you scroll up before the today's meeting jam explained why15:35
dimiternnatefinch, I don't want to mess with the state package for that15:35
dimiternnatefinch, and I don't like it myself - I raised the question several times in past sprints when the gui guys were talking about "bundles"15:36
TheMuedimitern: reviewd15:36
dimiternnatefinch, nobody gave a s*it :)15:36
dimiternTheMue, thanks15:36
TheMuedimitern: has been pretty clean ;)15:36
dimiternnatefinch, sorry, I need to land this so my other branches can get proposed, but thanks for the comments anyway15:37
natefinchI'll post my coments on there, but I don't want to stop you from continuing work15:38
natefinchdimitern: Frank can take the heat for the LGTM ;)15:38
dimiternnatefinch, cheers :)15:38
dimiternnatefinch, actually, jam should've but it's out for today already :)15:39
natefinchdimitern: there, my comments are official, feel free to land :)15:39
dimiternnatefinch, thanks again15:40
natefinchdimitern: other than that, yes, it LGTM too15:40
=== frankban_ is now known as frankban
TheMuedimitern, natefinch: a small one https://codereview.appspot.com/1344105215:54
natefinchTheMue: I can take it15:54
natefinchTheMue: done16:03
TheMuenatefinch: thx16:04
rogpeppedimitern, fwereade_: fairly trivial review (a drive-by change) https://codereview.appspot.com/13655044/17:05
natefinchrogpeppe: I can review17:06
rogpeppenatefinch: cool, thanks17:06
rogpeppenatefinch: actualy, i'm inclined to rename "SupportsCustomSources" to ImageSourcesGetter along with the other changes17:08
natefinchrogpeppe: I actually kinda hate "getter" :)17:09
rogpeppenatefinch: me too, but sometimes a simple "er" suffix is not sufficient17:09
rogpeppenatefinch: and "*Getter" is better than what's there, i think.17:09
rogpeppenatefinch: perhaps you have a better suggestion?17:09
rogpeppe"GetImageSourceser" :-)17:09
natefinchrogpeppe: ImageHost?17:10
rogpeppenatefinch: i'm not sure that's descriptive enough (and might be actively misleading - I don't know enough about the domain to tell)17:11
dimiternfwereade_, if still around - this is it https://codereview.appspot.com/13401050/  last bit of uniter api migration17:12
dimiternwow yet another record 14721 lines (+674/-12031)17:14
rogpeppedimitern: is that entirely mechanical?17:14
natefinchdimitern: -12k is awesome17:14
rogpeppedimitern: or are there some specific changes you've made in that particular CL?17:15
dimiternrogpeppe, not really, but most of it is17:16
TheMuedimitern: rm -vR juju-core > ex-juju-core.log ?17:16
rogpeppedimitern: i'd be happier if it was just 'cp apiuniter/* uniter'17:16
dimiternrogpeppe, a few integration changes, due to renaming of packages and import paths17:16
rogpeppedimitern: is that it?17:16
dimiternrogpeppe, no other changes17:16
rogpeppedimitern: ok, that's cool then17:16
rogpeppedimitern: i didn't want to review it in detail :-)17:17
natefinchrogpeppe: I almost think "ImageSource" might be an ok interface name.17:17
rogpeppenatefinch: i was just thinking that17:17
rogpeppenatefinch: or "ImageSourcer" ?17:17
dimiternrogpeppe, moreover, I live tested it with the local provider and --upload-tools, after making sure i go installed the cmd/juju and cmd/jujud stuff17:17
natefinchrogpeppe: ImageSourcerer? :)17:18
rogpeppedimitern: never again :-)17:18
natefinchrogpeppe: I think the -er is good when it works, but shouldn't be forced17:18
rogpeppenatefinch:  never again17:18
dimiternrogpeppe, ah, actually the only bit that's kinda new is in cmd/jujud/unit.go17:18
TheMuenatefinch: Forcerer?17:18
rogpeppenatefinch: "Sourcer" is a valid english word17:18
dimiternas is setterer17:19
rogpeppedimitern: nice!17:19
dimiternrogpeppe, come to think of it, perhaps I should've split the CL in two - deletions after the other changes, but hey..17:20
rogpeppedimitern: the deletions are easy to ignore17:20
* TheMue implements now a GoodByeerer17:21
natefinchrogpeppe: true.... just not one I heard used much.  I guess Sourcer is fine17:21
dimiternrogpeppe, yeah, I was thinking the same thing17:22
rogpeppenatefinch: actually, i just tried to look it up. "sourcer" doesn't appear to be a valid word, though given that "to source" is a valid verb, I'd've thought "sourcer", being "one who sources" would be ok17:24
dimiternrogpeppe, I have 2 branches planned to simplify some of the code dealing with api error codes: 1) make ErrCode() work with nil arg, 2) add params.IsCode*(err) bool helpers for most used (if not all) codes; 3) change the code accordingly to use them17:24
rogpeppedimitern: doesn't ErrCode already work with a nil arg?17:25
natefinchrogpeppe: google says people use it like recruiter... "one who ... "17:25
rogpeppedimitern: i certainly intended it to17:25
rogpeppedimitern: yeah, it does17:25
natefinchrogpeppe: can we make environs.ConfigGetter into environs.Configurable?17:25
dimiternrogpeppe, so nil.(something) is valid at runtime?17:26
rogpeppedimitern: of course17:26
rogpeppedimitern: erm, how long have you been using this language? :-)17:26
dimiternrogpeppe, I wasn't sure - well then, just the IsCode*() helpers17:26
dimiternrogpeppe, I'm still a bit unease around interfaces and reflection in general17:26
rogpeppedimitern: read the spec - it's quite straightforward17:27
dimiternrogpeppe, it seems so, but taking into account I did read it several times over the past months I keep forgetting how certain bits work, which is a bit of a "spec smell" for me :)17:28
rogpeppedimitern: but in general, nil is an ok value to do reflection or test type conversion on17:28
natefinchrogpeppe: oh, don't give him a hard time.... some of the more arcane stuff seems like it shouldn't work for reasons just because it doesn't in other languages17:28
rogpeppenatefinch: you're right. dimitern: i didn't wanna give you a hard time :-)17:28
dimiternrogpeppe, none taken :D17:28
dimiternok guys, I'll be off then, have a good night17:29
rogpeppedimitern: there are quite a few places where nil is ok to use without problem - nil maps and slices being another example17:29
rogpeppedimitern: g'night17:29
dimiternrogpeppe, I'll read it yet again tomorrow ;)17:29
natefinchrogpeppe: to be fair, nil is a lot more useful in Go than it is in other languages. Quite handy, really.17:30
TheMuedimitern, natefinch, rogpeppe: stepping out, have a nice evening17:31
natefinchTheMue: g'night17:31
rogpeppenatefinch: Configurable isn't great BTW, as it implies the config can be changed as well as retrieved17:40
natefinchrogpeppe: hmmm true.17:42
natefinchrogpeppe: it bugs me that ConfigGetter is only really used in one function (GetMetadataSources) and then only so you can get the ToolsURL off the config17:49
natefinch(like, that's the only method that actually calls the method Config())17:50
rogpeppenatefinch: two functions actually (both named GetMetadataSources)17:50
natefinchrogpeppe: ahh, I may have missed that there were two17:50
rogpeppenatefinch: i agree though - i'm not keen on the arrangement17:51
rogpeppenatefinch: but i don't want to change everything here, just improve things a little17:51
rogpeppenatefinch: the whole thing involving the secret SupportsCustomSources interface seems too much like magic for me17:52
natefinchrogpeppe: yeah17:52
rogpeppenatefinch: (especially when it wasn't documented in the slightest)17:52
natefinchrogpeppe: yeah, it's difficult to implement a method that really wants an interface with an optional method, without putting the burden on the caller to mock out the call17:54
rogpeppenatefinch: i'm not sure i understand that sentence :-)17:54
natefinchrogpeppe: sorry.... I mean, it sounds like what GetMetadataSources really wanted was one interface with both Config() and GetToolsSources()   and to have GetToolsSources to be optional for the caller to implement17:56
rogpeppenatefinch: yeah17:56
rogpeppenatefinch: i'd be happy if all providers just implemented both methods17:56
natefinchrogpeppe: yeah, really, it's not hard to implement a method that returns an empty slice17:56
rogpeppenatefinch: it's only three extra lines of code17:57
rogpeppenatefinch: exactly17:57
rogpeppenatefinch: and it makes the whole interface more transparent17:57
rogpeppenatefinch: anyway, eod for me17:58
rogpeppenatefinch: g'night - and a review of that branch appreciated if poss17:59
rogpeppeg'night all17:59
natefinchrogpeppe: cool, I'll LGTM that...17:59
thumpermorning folks21:10
thumpercan someone explain to me why we have a worker/apiuniter?21:18
thumperSpending the morning merging trunk in to my 8 branches and resolving conflicts :-|22:13
thumperfwereade_: ping22:44
thumperfwereade_: unping, can't remember what I was after22:59
bigjoolsthumper: old age23:21
thumperbigjools: must be23:21
thumperI'm happy I remember how to type23:21
thumperand I don't always get that right either23:22
bigjoolsdo you find yourself looking back at what you typed 5 minutes ago and only then notice all the typos?23:22
thumpersometimes23:25
thumperwallyworld: hello on call reviewer: Rietveld: https://codereview.appspot.com/1326905223:37
wallyworldhello23:37
wallyworldthumper: done. now i have to run a few errands. back in a bit23:48
thumperkk23:51
thumperI'm off to the gym shortly too23:51

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