bigjools | 2FA is properly getting on my tits lately | 00:34 |
---|---|---|
wallyworld | bigjools: go bot didn't want to land my gwacl branch after i marked the mp as approved | 00:49 |
wallyworld | retry_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 |
wallyworld | any clues what i need to do? | 00:49 |
bigjools | wallyworld: ummm looks like dependencies were missed when jam set it up, although he did mention that particular one | 00:50 |
bigjools | you have ssh access to the bot I think? | 00:52 |
wallyworld | yeah, connecting now | 00:52 |
bigjools | oh wait - that one ought to be in the source | 00:52 |
wallyworld | need to figure out how to fix | 00:52 |
bigjools | so I WTFing | 00:52 |
wallyworld | ah | 00:53 |
wallyworld | it looks in /home/tarmac/trees/gwacl-trees/src/launchpad.net/gwacl/fork/http | 00:53 |
wallyworld | but should be /home/tarmac/gwacl-trees/src/launchpad.net/gwacl/fork/http | 00:53 |
bigjools | ah | 00:53 |
wallyworld | so go path is wrong | 00:54 |
bigjools | this is odd since jam did a few test runs with the bot | 00:54 |
wallyworld | hmmm | 00:54 |
bigjools | wallyworld: congraulations | 01:17 |
bigjools | and congratulations too | 01:17 |
wallyworld | huh? | 01:17 |
bigjools | you landed it finally | 01:17 |
wallyworld | well, i hacked it | 01:18 |
bigjools | swoosh | 01:18 |
wallyworld | i sym linked the dir | 01:18 |
bigjools | haha | 01:18 |
wallyworld | and removed some .a files which were compiled with go 1.1.2 | 01:18 |
wallyworld | wtf happened i have no idea | 01:18 |
wallyworld | i'll followup with jam later | 01:18 |
bigjools | ok | 01:18 |
wallyworld | thumper: i have a little something for you :-D https://codereview.appspot.com/13302053/ | 01:26 |
davecheney | wallyworld: don't use symlinks | 01:31 |
davecheney | the go tool doesn't like them | 01:31 |
wallyworld | davecheney: it was just as an emergency to get the build to run on tarmac | 01:31 |
wallyworld | i'll fix when i talk to jam later cause i have nfi how the bot is set up and why gwacl was fooked | 01:31 |
davecheney | ok, but symlinking may not unfuck | 01:38 |
davecheney | ymmv | 01:38 |
wallyworld | davecheney: it all worked | 01:39 |
wallyworld | i've landed gwacl and juju-core branches after my "fix" | 01:39 |
davecheney | jolly good | 01:43 |
thumper | wallyworld: will look shortly | 01:47 |
wallyworld | np. thans | 01:47 |
wallyworld | thanks even | 01:47 |
thumper | wallyworld: hey | 02:25 |
thumper | wallyworld: your diff on rietveld is all fucked up | 02:25 |
thumper | error: old chunk mismatch | 02:25 |
* thumper opens the lp review | 02:26 | |
thumper | 1610 lines (+650/-371) 19 files modified | 02:26 |
thumper | hmm... | 02:26 |
thumper | good think I like you | 02:26 |
bigjools | davecheney: does any state get stored on the client that bootstrapped? | 02:26 |
wallyworld | thumper: otp to robbie | 02:26 |
thumper | ack | 02:27 |
thumper | bigjools: what do you mean? | 02:27 |
bigjools | IOW, can I replicate environments.yaml on another machine and expect it to work? | 02:27 |
davecheney | bigjools: yes | 02:29 |
davecheney | you basically need all of ~/.juju | 02:29 |
bigjools | davecheney: which question are you answering? | 02:29 |
bigjools | :) | 02:29 |
davecheney | 21:26 < bigjools> davecheney: does any state get stored on the client that bootstrapped? | 02:29 |
bigjools | ok so it's a NO to the second one | 02:29 |
bigjools | bugger | 02:29 |
davecheney | the truth is somewhere betwee all of ~/.juju and ~/.juju/environments.yaml | 02:30 |
davecheney | but the former is a good aproximation | 02:30 |
thumper | :( | 02:30 |
* thumper tried to do inline reviews on lp | 02:30 | |
thumper | kept clicking wondering why it wasn't working | 02:30 |
bigjools | oh dear | 02:31 |
bigjools | inline reviews always felt like a convenience for the reviewer at the expense of the reviewee | 02:31 |
wallyworld | thumper: so, do i re-submit? do you know how to resolve that diff issue? | 02:40 |
wallyworld | thumper: it's only a net 200 line addition - a lot of it is moving code, so it looks bigger than it is :-) | 02:41 |
thumper | wallyworld: NFI, currently just using LP | 02:41 |
* wallyworld can't wait to use use lp and not rietveld | 02:41 | |
* thumper sighs | 02:45 | |
thumper | wallyworld: there will be changes | 02:45 |
wallyworld | to my branch? | 02:46 |
thumper | review done | 02:54 |
wallyworld | thumper: thanks for review. saucy is needed because "defaultseries" is not a valid series. runs fine on raring etc | 03:05 |
thumper | where is saucy set? | 03:06 |
wallyworld | in cmd_test i think, i'll check | 03:07 |
wallyworld | no, actually just in the test itself | 03:08 |
wallyworld | because when we sync tools we now write metadata, all series names must be valid | 03:09 |
wallyworld | so "foo" etc won't work | 03:09 |
wallyworld | these tests just check output of things, they don't run anything as such | 03:09 |
jam | wallyworld, 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 |
wallyworld | jam: ok. so i'll go and and remove the symlink i added | 03:12 |
wallyworld | i wonder why son stuff had been compiled with Go 1.1.2 | 03:12 |
wallyworld | some | 03:12 |
wallyworld | and when I triggered the bot it used 1.1.1 | 03:12 |
jam | obviously I messed up that line | 03:14 |
wallyworld | ah, ok :-) | 03:14 |
jam | I'll clean that up now | 03:14 |
wallyworld | thumper: logging - i added loggo.GetLogger("").SetLogLevel(loggo.INFO) cause othewise infos weren't printed | 03:22 |
wallyworld | is there a better way to do it? | 03:22 |
thumper | don't look for the logging info | 03:22 |
thumper | and yes, use the LoggingSuite | 03:22 |
wallyworld | thumper: the implementation changed - it all used to be printed to stdout, now there's a combination of stdout and logging | 03:23 |
wallyworld | since the code which used to be in the command is now in a lib | 03:23 |
wallyworld | and the lib uses logging | 03:23 |
wallyworld | the cmd uses stdout | 03:23 |
wallyworld | and the tests check the output | 03:23 |
thumper | if the output should be writting out, don't use logging | 03:23 |
thumper | it's just wrong | 03:24 |
wallyworld | i don't want a lib to print to std out | 03:24 |
thumper | agreed | 03:24 |
thumper | test the results | 03:24 |
thumper | not the logging | 03:24 |
wallyworld | which i do - but jam likes to check logging output | 03:24 |
wallyworld | and the tests were written that way before my changes | 03:24 |
wallyworld | i'd be happy to kill that bit of the tests | 03:25 |
thumper | I vote to kill those bits | 03:25 |
wallyworld | \o/ | 03:25 |
thumper | as long as the results are tested | 03:25 |
thumper | don't check the logging | 03:25 |
wallyworld | they are | 03:25 |
thumper | seems like a waste | 03:25 |
jam | thumper: *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 |
wallyworld | to 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 work | 03:26 |
jam | How many of us default to "juju $SOMETHING -v" ? | 03:26 |
thumper | well that is about to break | 03:27 |
* thumper is writing a branch | 03:27 | |
thumper | jam: I'm not against writing it out in logs | 03:27 |
thumper | but we shouldn't be testing the logging | 03:27 |
wallyworld | jam: 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 checking | 03:27 |
* thumper taking another daughter to the doctor | 03:27 | |
jam | thumper: we've had multiple times where we've screwed up formatting in logs | 03:27 |
jam | why wouldn't we want to test it | 03:27 |
jam | wallyworld: isn't that "test what the user actually gets to see" which we probably want to keep? | 03:28 |
wallyworld | jam: it used to be all stdout, but it not now | 03:28 |
wallyworld | since core functionality was moved to a lib | 03:28 |
jam | wallyworld: If I remember the code, it originally went to ctx.stdout, but then the code got moved, and it accidentally got suppressed by default | 03:28 |
wallyworld | and the lib logs | 03:28 |
jam | which is why we wanted to test that the user actually gets feedback | 03:28 |
wallyworld | trouble is, libs should log, not print to std out | 03:29 |
jam | wallyworld: 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 |
wallyworld | jam: agreed. but here we are not blocking | 03:30 |
wallyworld | this is a command which writes some metadata | 03:31 |
jam | wallyworld: when I was writing synctools it takes several seconds to download something and upload something, and it was *really* nice to get feedback | 03:31 |
jam | I think users would appreciate it as well | 03:31 |
jam | hence I made it visible. | 03:31 |
jam | In IOM thumper fwereade_ and I agreed that we should default commands to having helpful user feedback | 03:31 |
jam | the exact definition of that is in the air, but it is a change from silent-by-default | 03:31 |
wallyworld | sure. so the loggig is done at INFO level | 03:31 |
jam | sync-tools was the first experiment in that direction | 03:31 |
wallyworld | which is not visible by default | 03:32 |
thumper | so... | 03:32 |
wallyworld | so we need to change that | 03:32 |
thumper | it should be passing through the context | 03:32 |
thumper | that controls stdout and stderr | 03:32 |
thumper | and write to those | 03:32 |
thumper | I'd like to have it so only developers care about --debug (or --show-log) | 03:33 |
thumper | which is what I'm writing | 03:33 |
thumper | --verbose should set a flag in the execution context | 03:33 |
thumper | as should --quiet | 03:33 |
thumper | squeezing time in to do this is tricky though | 03:33 |
wallyworld | i like using execution contexts, they work well. we have a shit load of code to change in that case | 03:33 |
thumper | wallyworld: there shouldn't be too much | 03:34 |
thumper | as most of the code is server side | 03:34 |
thumper | however yes, the client side code should accept a command context | 03:35 |
wallyworld | yep | 03:35 |
jam | thumper: we have a modest amount of shared code | 03:35 |
wallyworld | i'll change my current branch | 03:35 |
thumper | jam: less and less as the command line moves to the api | 03:35 |
jam | thumper: perhaps more and more? :) | 03:35 |
jam | actually shared execution of code, just not running in the client :) | 03:36 |
wallyworld | thumper: so how do we envision the api getting status messages back to the caller | 03:36 |
wallyworld | we need a return path | 03:36 |
thumper | wallyworld: pass through the cmd.Context from the Run method | 03:36 |
wallyworld | i haven't looked at the code - so long as that is supported it's all good | 03:36 |
thumper | I bet gnuflag doesn't like something like -vv | 03:37 |
thumper | calling the v flag twice | 03:37 |
wallyworld | or -vvvv even :-) | 03:37 |
jam | thumper: it likes it just fine, it just treats it as a single call, though | 03:38 |
thumper | of the dick who goes -vvqqvqv | 03:38 |
jam | thumper: it isn't too uncommon to have "last flag wins" | 03:38 |
jam | so you can have an alias and overwrite it | 03:38 |
jam | bzr used that | 03:38 |
* jam goes on a walk before it gets too hot outside | 03:40 | |
wallyworld | thumper: it seems other code uses a different pattern | 03:44 |
wallyworld | loggo.RegisterWriter("synctools", sync.NewSyncLogWriter(ctx.Stdout, ctx.Stderr), loggo.INFO) | 03:44 |
wallyworld | so the sync tools command calls a lib function without needing to pass in a context | 03:45 |
wallyworld | so the api stays clean | 03:45 |
wallyworld | and can sensibly be used easily by client and server callers | 03:45 |
thumper | yeah, that'd work | 03:45 |
wallyworld | i'll do that then | 03:45 |
thumper | however | 03:45 |
thumper | the synctools stuff will get sent to all the writers | 03:46 |
thumper | not just the synctools writer | 03:46 |
thumper | it'll appear in the logs too | 03:46 |
wallyworld | which is probably ok | 03:46 |
thumper | what does the NewSyncLogWriter do? | 03:46 |
wallyworld | return &syncLogWriter{out, err} | 03:46 |
wallyworld | type syncLogWriter struct { | 03:47 |
wallyworld | out io.Writer | 03:47 |
wallyworld | err io.Writer | 03:47 |
wallyworld | } | 03:47 |
wallyworld | syncLogWritter has a write method | 03:47 |
wallyworld | which prints to out or err | 03:47 |
wallyworld | depending on level | 03:47 |
wallyworld | < info goes to err | 03:47 |
thumper | ick | 03:48 |
thumper | that seems very messy | 03:48 |
thumper | in that it is trying to do too many things | 03:48 |
wallyworld | there's also this line: if name == "juju.environs.sync" { | 03:49 |
wallyworld | so it exits early if not the environs sync logger | 03:49 |
wallyworld | i'll see if i can do something sensible for the tools metadata command, so i don't need to mess with logging in the tests | 03:50 |
thumper | ok, cool | 03:57 |
bigjools | I'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 |
thumper | dafaq? | 03:58 |
* thumper got a weird test failure | 03:58 | |
thumper | DebugHooksServerSuite.TestRunHookExceptional | 03:58 |
thumper | :( | 03:59 |
axw | uh oh | 03:59 |
thumper | intermittent test failure | 03:59 |
axw | that's my code :) | 03:59 |
axw | what's the error? | 04:00 |
thumper | no, it is the apiserver | 04:00 |
axw | ermm | 04:00 |
thumper | no | 04:00 |
thumper | apiuniter | 04:00 |
axw | yeah it's the debug-hooks code | 04:00 |
thumper | axw: I'll paste it for you | 04:00 |
axw | some of those tests are time based, sadly... probably needs a tweak | 04:00 |
thumper | axw: http://pastebin.ubuntu.com/6095464/ | 04:00 |
axw | thumper: thanks | 04:01 |
thumper | axw: it succeeded when running alone | 04:01 |
thumper | twice | 04:01 |
axw | that error message is difficult to parse | 04:03 |
axw | "type must before start value" | 04:03 |
axw | what. | 04:03 |
thumper | that may have been me | 04:04 |
thumper | way back | 04:04 |
thumper | possibly | 04:04 |
thumper | if it is a checker | 04:04 |
thumper | wallyworld: while you are there | 04:06 |
thumper | the sync tools test is very noisy | 04:06 |
thumper | it writes to stdout | 04:06 |
thumper | plz fix | 04:06 |
axw | thumper: was your machine stressed when this failed? | 04:07 |
thumper | axw: no, not at all | 04:07 |
thumper | well, it was running the tests | 04:07 |
wallyworld | thumper: 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 later | 04:09 |
thumper | acj | 04:09 |
thumper | ack | 04:09 |
axw | thumper: 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 fix | 04:10 |
* thumper tries again | 04:10 | |
thumper | axw: my laptop is faster than average | 04:10 |
thumper | so I'd be real surpised | 04:10 |
axw | I can't get it to happen at all, and my laptop's probably slower than average... hrm. | 04:14 |
thumper | wallyworld: your turn: https://code.launchpad.net/~thumper/juju-core/show-log/+merge/185193 | 04:14 |
thumper | axw: no idea how it happened | 04:14 |
wallyworld | ok | 04:14 |
thumper | timing tests suck | 04:14 |
axw | thumper: I'll log a bug to make it not/less time sensitive | 04:15 |
thumper | kk | 04:15 |
thumper | wallyworld: Rietveld: https://codereview.appspot.com/13352052 if you'd prefer | 04:16 |
wallyworld | i can click trhough the mp you know :-) | 04:16 |
* thumper wants to add a base test suite that has "AddCleanup(cleanup func())" | 04:17 | |
axw | that would be nice. | 04:18 |
thumper | would mean we wouldn't need test tear downs | 04:18 |
thumper | sould have an AddSuiteCleanup(cleanup func()) too | 04:18 |
thumper | should | 04:18 |
thumper | fuck it | 04:21 |
thumper | doing it now | 04:21 |
thumper | it has been on my list for too long | 04:21 |
wallyworld | thumper: review done. quick, land it before anyone can object :-) | 04:24 |
thumper | wallyworld: ok | 04:24 |
thumper | davecheney, axw: what is the fastest/easiest way to iterate backwards over a slice? | 04:34 |
davecheney | for i := len(s) ; i != 0 ; i-- { ... } | 04:34 |
axw | for i := len(slice)-1; i >= 0; i-- {} ? | 04:34 |
axw | or that | 04:34 |
davecheney | ^ do what he said | 04:34 |
axw | or not, because of off by one | 04:35 |
axw | :P | 04:35 |
* thumper was hoping for a reverse builtin | 04:35 | |
axw | no reversed for you I'm afraid | 04:35 |
axw | :) | 04:35 |
thumper | can we use bound functions yet? | 04:38 |
thumper | are we 1.1? | 04:38 |
axw | I'm pretty certain I've seen changes from rogpeppe to use 1.1 syntax | 04:39 |
axw | davecheney: ? | 04:39 |
wallyworld | yeah, we are 1.1 | 04:41 |
wallyworld | thumper: 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 wrong | 04:42 |
wallyworld | loggo.RegisterWriter("toolsmetadata", cmd.NewCommandLogWriter("juju.environs.tools", context.Stdout, context.Stderr), loggo.INFO) | 04:42 |
thumper | wallyworld: the level specified there is the minimum level that the writer will write | 04:42 |
thumper | it doesn't specify any log levels | 04:42 |
thumper | so that writer won't write debug or trace | 04:42 |
thumper | you still need to explicitly specify the value for the logger | 04:43 |
thumper | or set the root level | 04:43 |
wallyworld | the logger is specified elsewhere | 04:43 |
wallyworld | ok, i'll set root level, via loggo.GetLogger("") ? | 04:43 |
axw | thumper: is there any good reason to allow --to=lxc:0 for the local provider? | 04:44 |
axw | context: https://codereview.appspot.com/13632046/patch/4001/5013 | 04:44 |
thumper | axw: no | 04:44 |
thumper | not at all | 04:44 |
thumper | not yet | 04:44 |
axw | not... yet? | 04:44 |
thumper | nested lxc doesn't work just now | 04:44 |
thumper | it works later | 04:44 |
axw | right, but 0 isn't a container | 04:45 |
axw | I meant specifically 0, sorry | 04:45 |
axw | I would say no, because you can just create a "machine"... | 04:45 |
thumper | oh, specificlly 0, no never | 04:46 |
thumper | machine 0 for the local provider doesn't actually have the job "manage units" | 04:47 |
thumper | so you can't | 04:47 |
thumper | or at least there should be a check for that :) | 04:47 |
axw | okey dokey | 04:47 |
thumper | damn | 05:02 |
thumper | just accidentally closed the emacs that opened for my lbox message | 05:03 |
thumper | second side-tracked branch for the day proposed | 05:04 |
thumper | wallyworld, axw: https://code.launchpad.net/~thumper/juju-core/cleanup-suite/+merge/185199 | 05:04 |
wallyworld | looking | 05:05 |
wallyworld | thumper: i've pushed my changes to the branch you reviewed | 05:05 |
thumper | wallyworld: want to lbox propose it to see if it fixes the diff? | 05:06 |
wallyworld | ok, doing now | 05:06 |
thumper | ta | 05:09 |
wallyworld | thumper: done, and looks like fixed | 05:09 |
thumper | cool | 05:09 |
axw | thumper: lgtm | 05:10 |
wallyworld | thumper: thanks :-) | 05:15 |
=== thumper is now known as thumper-afk | ||
thumper-afk | back later for meetings | 05:26 |
davecheney | jam: /me waves | 06:15 |
jam | hey davecheney | 06:15 |
davecheney | jam: what do I need to do to land that mgo revno:240 change | 06:15 |
davecheney | do I need to backport the other branches on that issue ? | 06:15 |
jam | davecheney: to 1.14 ? | 06:15 |
davecheney | y | 06:15 |
jam | davecheney: 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 |
jam | I updated the bot to monitor it | 06:17 |
jam | the particular branch should be just that change, and not the branch you landed to trunk | 06:17 |
davecheney | yup, that was the branch you approved yesterday | 06:17 |
davecheney | then there was some muttering about it not passing | 06:18 |
jam | davecheney: can you point me to it, I'm not seeing it right away | 06:18 |
jam | davecheney: 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 it | 06:19 |
jam | though it is marked approved, so let me look closer | 06:19 |
davecheney | https://bugs.launchpad.net/juju-core/+bug/1221705 | 06: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 |
davecheney | might need to flip it | 06:20 |
jam | no, that's a different one | 06:20 |
jam | davecheney: looks like the bot succeeded in landing it, but failed to push it back up, give me a sec | 06:21 |
jam | davecheney: fixed, and should be fixed for future patches to 1.14 sorry about that. | 06:21 |
davecheney | jam: sweet | 06:26 |
davecheney | i'll make as comitted | 06:26 |
davecheney | thumper-afk: does it say anyuwhere public that the local provider does not work with precise ? | 06:43 |
rogpeppe | mornin' all | 06:45 |
* davecheney waves | 06:46 | |
mgz | morning! | 07:29 |
TheMue | mgz: morning | 07:34 |
jam | hi TheMue and mgz | 07:39 |
fwereade_ | jam, rogpeppe: https://codereview.appspot.com/13355044/ | 08:36 |
rogpeppe | fwereade_: 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 LGTMing | 08:37 |
fwereade_ | jam, rogpeppe: just wanted to check with jam if there were any not-lgtmy concerns | 08:37 |
fwereade_ | jam, fwiw, I can't think of many cases in which one'd want to look in local .ssh for authorized_keys | 08:38 |
rogpeppe | fwereade_: jam doesn't seem to be around currently | 08:38 |
fwereade_ | rogpeppe, bah, so he doesn't | 08:39 |
fwereade_ | rogpeppe, I'll finish in case he scrolls back | 08:39 |
rogpeppe | fwereade_: 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 page | 08:40 |
rogpeppe | fwereade_: (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 up | 08:40 |
fwereade_ | rogpeppe, ie I wanted to be clear that local authorized_keys are not usually what you're looking for | 08:41 |
rogpeppe | fwereade_: i'm not sure i get that | 08:41 |
fwereade_ | rogpeppe, I do agree that we should indeed be getting the user's public key from .ssh | 08:41 |
fwereade_ | rogpeppe, but for a while we were reading authorized_keys from .ssh | 08:41 |
rogpeppe | fwereade_: ah! | 08:41 |
rogpeppe | fwereade_: yes, indeed | 08:42 |
rogpeppe | fwereade_: i'd forgotten the distinction | 08:42 |
fwereade_ | rogpeppe, and that's incoherent at worst and... surprising at best, even if there is a potentially valid use case | 08:42 |
fwereade_ | rogpeppe, ...which would itself be supported just fine with an explicit authorized-keys-path | 08: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 |
jam | fwereade_: 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 today | 08:44 |
fwereade_ | jam, ok, thanks, I will be particularly sensitive to that | 08:44 |
rogpeppe | fwereade_: AFAICS we don't read ~/.ssh/authorized_keys by default | 08:45 |
fwereade_ | rogpeppe, indeed we don't, I suspect I just misread something jam said that suggested to me I should mention it explicitly | 08:45 |
fwereade_ | rogpeppe, https://codereview.appspot.com/13355044/ reviewed | 09:20 |
rogpeppe | fwereade_: thanks! | 09:20 |
fwereade_ | rogpeppe, it's basically awesome, just not quite LGTM | 09:21 |
rogpeppe | fwereade_: 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 New | 09:21 |
rogpeppe | fwereade_: 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 merged | 09:22 |
fwereade_ | rogpeppe, ok, that makes sense if there's an easy followup coming | 09:23 |
mgz | fwereade_: 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 |
mgz | as 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 completes | 09:31 |
fwereade_ | mgz, I think that problem is much harder than it looks | 09:31 |
mgz | it 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 |
mgz | er... I don't, what did I mean... er, just how those symptoms happen at all | 09:33 |
fwereade_ | mgz, are we definitely talking about the same bug? the one you linked STM to be a feature request | 09:34 |
mgz | if a hook script takes 30mins to run, I don't see how `juju status` would report anything other than pending till 30mins was up | 09:34 |
mgz | but apparently it does | 09:35 |
fwereade_ | mgz, if "install" takes that long, I think you're stuck in pending | 09:35 |
fwereade_ | mgz, then "installed" will last for a config-changed and a start | 09:35 |
fwereade_ | mgz, after that it's just "started" independent of what's going on | 09: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 hope | 09:37 |
mgz | hm, and then relations are another thing again | 09:38 |
fwereade_ | mgz, right | 09:38 |
yolanda | hi, 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 helpfully | 09:40 |
fwereade_ | yolanda, you may be better off asking in #juju, nagios is not something I know | 09:40 |
yolanda | fwereade_ ok, thx | 09:41 |
mgz | fwereade_: thanks a lot, that made things much clearer for me | 09:42 |
=== frankban__ is now known as frankban | ||
jam | mgz: https://plus.google.com/hangouts/_/8aa005e26ea2bcdf23aaade34a5e64182dcd04c3 | 10:04 |
rogpeppe | davecheney: meeting | 10:04 |
rogpeppe | mgz: meeting? | 10:04 |
mgz | gah, rebooting | 10:06 |
mgz | will be 2 mins at least, start without me | 10:06 |
=== frankban__ is now known as frankban | ||
jam | dimitern: 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 totally | 11:48 | |
dimitern | jam, ok, will do | 11:48 |
dimitern | jam, that's kinda strange - charm bundles were there log before the gui started using it | 11:51 |
dimitern | jam, but we can rename it to archive i suppose - in a follow-up though | 11:51 |
jam | dimitern: well, "Bundle" wasn't a publicly exposed name, and I think juju-deployer called them that | 11:51 |
jam | anyway, the goal is to not have a stable release with Bundle in the API attributes | 11:52 |
jam | so we can reserve it for future use. | 11:52 |
dimitern | jam, what do you mean by "publicly exposed" ? it's in the source | 11:52 |
dimitern | jam, yeah, I got that | 11:52 |
jam | dimitern: you never referenced them as a bundle in command line syntax, etc | 11:53 |
jam | it is the name in the source file | 11:53 |
jam | but not in terminology someone not hacking on the source would have ever seen | 11:53 |
dimitern | jam, perhaps because bundles are only useful when deploying stuff from the store | 11:53 |
rogpeppe | weird 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 ok | 12:07 |
rogpeppe | s/is says/it says/ | 12:08 |
dimitern | rogpeppe, do you use a ubikey or the android app for 2fa? | 12:09 |
rogpeppe | dimitern: yubikey | 12:09 |
dimitern | rogpeppe, 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 form | 12:10 |
dimitern | rogpeppe, i had to remove and reauthorize it to get it working again - good that i had a backup device (my phone) | 12:10 |
rogpeppe | dimitern: if that happened, surely i wouldn't be able to then access the site successfully (unless our security is borked, i guess) | 12:10 |
rogpeppe | dimitern: yeah, i need to get my phone working as an auth device again | 12:11 |
arosales | jam, would you suggest trunk or 1.13.3 for testing Azure? | 12:12 |
jam | arosales: so we know today that there are 1-2 bugs for 1.13.3 in Azure. You could try lp:juju-core/1.14 | 12:12 |
arosales | jam, ok | 12:14 |
arosales | jam, and to confirm my understanding | 12:14 |
arosales | # public-storage-account-name: jujutools | 12:14 |
arosales | # public-storage-container-name: juju-tools | 12:14 |
jam | arosales: those are commented out because that is the hard-coded defaults internally | 12:15 |
arosales | if that is in your env.yaml you still get those be default, correct? | 12:15 |
arosales | ok | 12:15 |
jam | arosales: so the one thing to confirm with juju-1.13 is that we actually get the tools from the shared public bucket | 12:19 |
jam | since 1.14 hasn't been released you have to --upload-tools | 12:19 |
jam | but 1.13.3 should have tools in the bucket already | 12:19 |
arosales | jam I can give that a try | 12:20 |
fwereade_ | dimitern, jam: I just thought that we possibly *have* exposed the bundle terminology via the store, which sucks a bit | 12:28 |
jam | fwereade_: well, we can try to minimize existing terminology to migrate to new terminology | 12:28 |
fwereade_ | jam, dimitern: indeed, +1 | 12:29 |
arosales | jam, 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 |
arosales | http://pastebin.ubuntu.com/6096850/ | 12:39 |
arosales | jam, also did bootstrap the correct precise release image by default | 12:39 |
jam | arosales: that is by commenting out the image-stream, etc. Right? IIRC 1.13.3 still "juju init" with saucy in the config. | 12:40 |
arosales | correct I commented it out | 12:40 |
jam | wallyworld, 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 |
arosales | specifically image-stream and default-series I commented out | 12:41 |
arosales | ya 1.13.2 was the latest found | 12:41 |
wallyworld | jam: the release manager needs to upload them :-) | 12:42 |
jam | wallyworld: I don't think davecheney has the azure creds, I know *I* don't. Do we know who controls the "jujutools" account? | 12:43 |
jam | is it just bigjools and friends ? | 12:43 |
wallyworld | i *think* so | 12:43 |
arosales | jam, I do | 12:43 |
wallyworld | jam: when I say "needs to upload", that can include delegation | 12:44 |
arosales | jam, wallyworld, specifically the azure tools bucket | 12:44 |
wallyworld | we should have a socumented release process, preferrably automated | 12:44 |
wallyworld | hopefully orange squad can progress that | 12:44 |
jam | hp and ec2 both have 1.13.3, but it didn't get to azure | 12:48 |
arosales | jam, wallyworld, specifically the azure tools bucket tools juju-tools bucket being used the the released precise image stream being used | 12:50 |
wallyworld | say wot? | 12:50 |
jam | arosales: 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 |
jam | wallyworld: arosales just tested that juju-1.13.3 finds 1.13.2 in our azure-local mirror of the tools. | 12:50 |
jam | and that it picked Precise as the default image (for a while it was configured for Saucy) | 12:51 |
wallyworld | ok | 12:51 |
jam | and 'daily' stream rather than Released | 12:51 |
arosales | jam, 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 uploads | 12:51 |
jam | arosales: "juju sync-tools" will do the right thing if you configure an environment with the right settings | 12:51 |
jam | it is what I use for HP and Canonistack | 12:52 |
arosales | jam to confirm on "<jam> and 'daily' stream rather than Released" | 12:52 |
wallyworld | we may we be able to use the released stream now | 12:53 |
arosales | the correct behavior and looks like the current default is to use the "released" stream rather than the "daily" | 12:53 |
arosales | to confirm my understanding of the defaults | 12:53 |
jam | arosales: so the thing to check is what does "juju init --show" output | 12:54 |
jam | arosales: for 1.13.3 it still says "saucy" and "daily" | 12:54 |
jam | but for lp:juju-core/1.14 it should have those lines commented out | 12:54 |
jam | and have: | 12:54 |
jam | # image-stream: "" | 12:54 |
jam | # default-series: precise | 12:54 |
arosales | jam, yes sorry I was saying with the appropriate lines commented out Juju has the correct defaults in 1.13.3 | 12:54 |
jam | arosales: right, it did, but the bug we have for 1.14 is making sure we got the config right | 12:55 |
arosales | jam, gotcha | 12:55 |
jam | arosales: 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 |
arosales | and I follow you in 1.13.3 not having this in the default yaml | 12:55 |
arosales | but I was relived to see juju did the correct behavior when commenting out the image steam and tool bucket keys. | 12:56 |
jam | arosales: 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 available | 12:56 |
arosales | jam, cosmetically are you ok with the config having the following for the default | 12:58 |
arosales | # force-image-name: b39f27a8b8c64d52b05eac6a62ebad85__Ubuntu-13_10-amd64-server-DEVELOPMENT-20130713-Juju_ALPHA-en-us-30GB | 12:58 |
jam | arosales: 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 think | 12:58 |
arosales | jam, ok. That image is probably deprecated, but is does have the correct format for an azure image name. | 13:00 |
dimitern | fwereade_, updated https://codereview.appspot.com/13355046/ | 13:08 |
rogpeppe | fwereade_: responded https://codereview.appspot.com/13355044 | 13:22 |
fwereade_ | dimitern, rogpeppe, ack | 13:22 |
fwereade_ | dimitern, rogpeppe: bah, neither is obviously simpler to read than the other ;p | 13:26 |
rogpeppe | fwereade_: lol | 13:26 |
rogpeppe | fwereade_: what's the "Forbidden Design Pattern" ? | 13:29 |
fwereade_ | rogpeppe, sorry, Singleton ;p | 13:29 |
rogpeppe | fwereade_: yes, having a single shared config for all dataDir+tag combinations would work fine. | 13:35 |
rogpeppe | fwereade_: (probably) | 13:35 |
rogpeppe | s/for all/for each/ s/combinations/combination/ | 13:36 |
dimitern | fwereade, updated https://codereview.appspot.com/13604045/ as well - added tests | 13:40 |
rogpeppe | fwereade_: this was about as far as i got with the "set up mocking beneath jujud" thing BTW: http://paste.ubuntu.com/6097076/ | 13:46 |
rogpeppe | fwereade_: the idea being to mock out (or interpose some call check on) the functions in stateWorkers and apiWorkers | 13:47 |
rogpeppe | fwereade_: but even doing that doesn't quite seem quite joined up enough for me to be convinced about the approach | 13:47 |
dimitern | fwereade_, ping | 13:54 |
fwereade_ | dimitern, pong | 13:54 |
dimitern | fwereade_, 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's | 13:55 |
dimitern | fwereade_, cheers | 13:56 |
fwereade_ | rogpeppe, reviewed | 14:00 |
rogpeppe | fwereade_: " | 14:01 |
rogpeppe | Ah, I see. In that case I question 0 having a special meaning... was it really | 14:01 |
rogpeppe | like this before? | 14:01 |
rogpeppe | " | 14:02 |
rogpeppe | fwereade_: yes - see the definition of Config.StatePort and Config.APIPort previously | 14:02 |
fwereade_ | rogpeppe, ah, I thought that was explicitly for dealing with them being missing | 14:03 |
rogpeppe | fwereade_: well, i think that was probably the intent | 14:03 |
fwereade_ | rogpeppe, we had to keep them missing to work with 1.10 iirc | 14:03 |
fwereade_ | rogpeppe, but it was asInt basically I guess? | 14:04 |
rogpeppe | fwereade_: yeah | 14:04 |
fwereade_ | rogpeppe, bleh | 14:04 |
fwereade_ | rogpeppe, ok, pre-existing behaviour, bug it and go :) | 14:04 |
rogpeppe | fwereade_: https://bugs.launchpad.net/juju-core/+bug/1224492 | 14: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, thanks | 14:07 |
fwereade_ | dimitern, little one LGTM with a tweak | 14:08 |
dimitern | fwereade_, thanks | 14:08 |
natefinch | mgz: poke :) | 14:11 |
mgz | natefinch: hey! | 14:13 |
natefinch | mgz: 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 |
TheMue | fwereade_: ping | 14:14 |
fwereade_ | dimitern, LGTM | 14:15 |
fwereade_ | TheMue, pong | 14:15 |
dimitern | fwereade_, sweet! | 14:15 |
natefinch | mgz: 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 |
TheMue | fwereade_: just stumbled over https://bugs.launchpad.net/juju-core/+bug/1199698 again | 14:15 |
_mup_ | Bug #1199698: intermittent test failure in BootstrapSuite <intermittent-failure> <juju-core:In Progress by themue> <https://launchpad.net/bugs/1199698> | 14:15 |
mgz | natefinch: okay, so what I just did | 14:15 |
TheMue | fwereade_: the bug still exists, as commented, with GOMAXPROCS > 1 | 14:15 |
TheMue | fwereade_: with GMP=1 it's ok | 14:16 |
mgz | `bzr log|less` then /Nate to find the change | 14:16 |
fwereade_ | TheMue, so is the problem that multiple tests run concurrently against the same dummy environment or something? | 14:16 |
mgz | then `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 easy | 14:17 |
fwereade_ | TheMue, or can it be isolated within a single test method? | 14:17 |
natefinch | yep | 14:17 |
mgz | so, 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 it | 14:18 |
mgz | then I'd probably eye the diff, self approve, and rv-submit | 14:18 |
TheMue | fwereade_: 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 |
mgz | natefinch: one thing that might help with the confusion over coloworkspaces is a bash prompt hack | 14:19 |
mgz | I don't use one myself, but jam does, and I think jelmer had one for colo stuff | 14:19 |
TheMue | fwereade_: have to test again, one moment | 14:19 |
natefinch | mgz: hmm yeah, that would help a lot | 14:19 |
mgz | remember `bzr branches` will tell you what's around, and the star is the current active one | 14:20 |
natefinch | mgz: 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 setup | 14:20 |
mgz | it is still a little easy to get yourself confused, but that's just the colo joy | 14:20 |
TheMue | fwereade_: yep, same behavior | 14:20 |
TheMue | fwereade_: it's the BootstrapSuite in cmd/juju | 14:21 |
mgz | natefinch: 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 bug | 14:21 |
mgz | `bzr branch lp:juju-core && cd juju-core && bzr switch -b trunk` | 14:21 |
TheMue | fwereade_: ... Panic: interface conversion: interface is nil, not dummy.OpPutFile (PC=0x414321) | 14:21 |
mgz | then you can branch in any current feature branches from the old dir as well | 14:21 |
TheMue | fwereade_: it's not new | 14: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 backend | 14:22 |
TheMue | fwereade_: but i think to solve it the way of testing the bootstrapping would have to be changed | 14:22 |
fwereade_ | TheMue, that particular panic just implies we don't have our dummy resetting working quite right | 14:23 |
TheMue | fwereade_: no, it's not slowing me down, i now use GMP=1 | 14:23 |
TheMue | fwereade_: i only dislike this issue as open | 14:23 |
fwereade_ | TheMue, ok, good to confirm it still exists though, probably a good idea to note it | 14:23 |
TheMue | fwereade_: ;) | 14:23 |
fwereade_ | TheMue, I'm afraid it's relatively low on my own mental list | 14:23 |
TheMue | fwereade_: yep, it is | 14:24 |
TheMue | fwereade_: but if it cares nobody it only fills the list. so i'll set the importance to low | 14:25 |
natefinch | mgz: thanks, I think I'm all set now | 14:33 |
mgz | natefinch: ace, do bug me if you need anything explaining or get frustrated with some issue or other | 14:34 |
natefinch | mgz: thanks. I'm good 99% of the time, it's that pesky 1% :) | 14:35 |
mgz | ah, 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 |
natefinch | mgz: definitely | 14:36 |
rogpeppe | dimitern: ah, dammit, your submit got in before mine and caused conflicts | 14:46 |
natefinch | fwereade_, 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 call | 14:47 |
dimitern | rogpeppe, oh, sorry about that | 14:47 |
natefinch | fwereade_: cool, thanks | 14:47 |
rogpeppe | dimitern: i want to configure my machine so it shouts at me when the bot rejects a branch | 14:47 |
dimitern | rogpeppe, 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 case | 14:48 |
natefinch | rogpeppe: +1 | 14:48 |
rogpeppe | dimitern: it would be much nicer if it picked it up by approved time | 14:48 |
natefinch | dimitern: so we'll start seeing a lot of branches starting with underscores? :) | 14:48 |
dimitern | natefinch, :) | 14:48 |
dimitern | natefinch, 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 code | 14:49 |
dimitern | fwereade_, I've got another CL for you, sorry :) https://codereview.appspot.com/13302054 | 14:50 |
dimitern | fwereade_, and then 2 more until the uniter+api lands | 14:50 |
fwereade_ | dimitern, cool | 14:51 |
rogpeppe | fwereade_: this might be ready to land now - i've added tests for connectionIsFatal: https://codereview.appspot.com/13640043 | 14:53 |
* rogpeppe crosses his fingers for another go | 14:57 | |
rogpeppe | is there anyone around that is easily able to run some live tests under non-ec2 environments? | 14:58 |
mgz | rogpeppe: I probably can | 14:59 |
natefinch | rogpeppe: lemme check my azure environment, it should be workable | 14:59 |
mgz | though, you should be able to canonistack just as easily :) | 14:59 |
rogpeppe | mgz: yes, i should really set that up | 15:00 |
fwereade_ | rogpeppe, dimitern: both LGTM | 15:00 |
rogpeppe | fwereade_: thanks | 15:00 |
dimitern | fwereade_, cheers | 15:01 |
* rogpeppe gets in there first :-) | 15:01 | |
natefinch | rogpeppe: do you want my azure creds? not sure if that would be easier or not for you | 15:02 |
rogpeppe | natefinch: 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 |
mgz | rogpeppe: the live tests aren't run on azure, I'm pretty sure | 15:03 |
rogpeppe | mgz: so how much of azure actually works? does it bootstrap and deploy? | 15:04 |
mgz | manually doing the wordpress-mysql example is something instead I think | 15:04 |
natefinch | rogpeppe: it works | 15:04 |
natefinch | rogpeppe: bootstrap is slow as molasses, but it owrks | 15:04 |
* rogpeppe wonders why it doesn't have any of the live tests hooked in then | 15:04 | |
mgz | the live tests aren't the easiest thing to adapt to non-ec2 environments | 15:04 |
rogpeppe | mgz: they were *supposed* to be provider-independent | 15:05 |
mgz | I'm pretty sure only the openstack provider has made the effort | 15:05 |
rogpeppe | mgz: if they're not, i'd like to know why, so we can improve them | 15:05 |
natefinch | rogpeppe: 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 work | 15:05 |
rogpeppe | natefinch: ah, that would be useful. i hate claiming expenses. | 15:06 |
natefinch | rogpeppe: ditto | 15:06 |
mgz | writing 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 providers | 15:07 |
natefinch | mgz: oh, I'm sure they do ...things... on the other providers right now :) | 15:07 |
dimitern | jam, fwereade_, next one https://codereview.appspot.com/13677043 | 15:08 |
natefinch | mgz: just perhaps not the right things ;) | 15:08 |
mgz | yeah, get skipped :P | 15:08 |
rogpeppe | mgz: that's why i think it's important that we try to make the live tests work on all the providers. | 15:17 |
rogpeppe | mgz: the idea is to exercise the Environ interface | 15:18 |
rogpeppe | mgz: which everything else relies on | 15:18 |
rogpeppe | i might try adding some of the live tests to azure at some point | 15:19 |
=== frankban_ is now known as frankban | ||
* rogpeppe goes for a bit to eat | 15:21 | |
rogpeppe | bite even | 15:21 |
dimitern | natefinch, perhaps you can review this small branch? https://codereview.appspot.com/13677043 | 15:21 |
natefinch | dimitern: sure | 15:27 |
natefinch | dimitern: the halfway rename messes with my head. | 15:32 |
dimitern | natefinch, why half way? | 15:33 |
natefinch | dimitern: we still refer to them as bundles when we're not talking about the API. For example state.Charm.BundleURL() | 15:34 |
dimitern | natefinch, we're not renaming them in state, just whats "exposed" by the API | 15:34 |
ahasenack | if, from a config-changed hook, I list all relations I have | 15:35 |
natefinch | dimitern: 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 confusing | 15:35 |
ahasenack | will broken relations be in that list? | 15:35 |
ahasenack | like, relations that were attempted but are in a failed state? | 15:35 |
dimitern | natefinch, if you scroll up before the today's meeting jam explained why | 15:35 |
dimitern | natefinch, I don't want to mess with the state package for that | 15:35 |
dimitern | natefinch, 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 |
TheMue | dimitern: reviewd | 15:36 |
dimitern | natefinch, nobody gave a s*it :) | 15:36 |
dimitern | TheMue, thanks | 15:36 |
TheMue | dimitern: has been pretty clean ;) | 15:36 |
dimitern | natefinch, sorry, I need to land this so my other branches can get proposed, but thanks for the comments anyway | 15:37 |
natefinch | I'll post my coments on there, but I don't want to stop you from continuing work | 15:38 |
natefinch | dimitern: Frank can take the heat for the LGTM ;) | 15:38 |
dimitern | natefinch, cheers :) | 15:38 |
dimitern | natefinch, actually, jam should've but it's out for today already :) | 15:39 |
natefinch | dimitern: there, my comments are official, feel free to land :) | 15:39 |
dimitern | natefinch, thanks again | 15:40 |
natefinch | dimitern: other than that, yes, it LGTM too | 15:40 |
=== frankban_ is now known as frankban | ||
TheMue | dimitern, natefinch: a small one https://codereview.appspot.com/13441052 | 15:54 |
natefinch | TheMue: I can take it | 15:54 |
natefinch | TheMue: done | 16:03 |
TheMue | natefinch: thx | 16:04 |
rogpeppe | dimitern, fwereade_: fairly trivial review (a drive-by change) https://codereview.appspot.com/13655044/ | 17:05 |
natefinch | rogpeppe: I can review | 17:06 |
rogpeppe | natefinch: cool, thanks | 17:06 |
rogpeppe | natefinch: actualy, i'm inclined to rename "SupportsCustomSources" to ImageSourcesGetter along with the other changes | 17:08 |
natefinch | rogpeppe: I actually kinda hate "getter" :) | 17:09 |
rogpeppe | natefinch: me too, but sometimes a simple "er" suffix is not sufficient | 17:09 |
rogpeppe | natefinch: and "*Getter" is better than what's there, i think. | 17:09 |
rogpeppe | natefinch: perhaps you have a better suggestion? | 17:09 |
rogpeppe | "GetImageSourceser" :-) | 17:09 |
natefinch | rogpeppe: ImageHost? | 17:10 |
rogpeppe | natefinch: 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 |
dimitern | fwereade_, if still around - this is it https://codereview.appspot.com/13401050/ last bit of uniter api migration | 17:12 |
dimitern | wow yet another record 14721 lines (+674/-12031) | 17:14 |
rogpeppe | dimitern: is that entirely mechanical? | 17:14 |
natefinch | dimitern: -12k is awesome | 17:14 |
rogpeppe | dimitern: or are there some specific changes you've made in that particular CL? | 17:15 |
dimitern | rogpeppe, not really, but most of it is | 17:16 |
TheMue | dimitern: rm -vR juju-core > ex-juju-core.log ? | 17:16 |
rogpeppe | dimitern: i'd be happier if it was just 'cp apiuniter/* uniter' | 17:16 |
dimitern | rogpeppe, a few integration changes, due to renaming of packages and import paths | 17:16 |
rogpeppe | dimitern: is that it? | 17:16 |
dimitern | rogpeppe, no other changes | 17:16 |
rogpeppe | dimitern: ok, that's cool then | 17:16 |
rogpeppe | dimitern: i didn't want to review it in detail :-) | 17:17 |
natefinch | rogpeppe: I almost think "ImageSource" might be an ok interface name. | 17:17 |
rogpeppe | natefinch: i was just thinking that | 17:17 |
rogpeppe | natefinch: or "ImageSourcer" ? | 17:17 |
dimitern | rogpeppe, moreover, I live tested it with the local provider and --upload-tools, after making sure i go installed the cmd/juju and cmd/jujud stuff | 17:17 |
natefinch | rogpeppe: ImageSourcerer? :) | 17:18 |
rogpeppe | dimitern: never again :-) | 17:18 |
natefinch | rogpeppe: I think the -er is good when it works, but shouldn't be forced | 17:18 |
rogpeppe | natefinch: never again | 17:18 |
dimitern | rogpeppe, ah, actually the only bit that's kinda new is in cmd/jujud/unit.go | 17:18 |
TheMue | natefinch: Forcerer? | 17:18 |
rogpeppe | natefinch: "Sourcer" is a valid english word | 17:18 |
dimitern | as is setterer | 17:19 |
rogpeppe | dimitern: nice! | 17:19 |
dimitern | rogpeppe, come to think of it, perhaps I should've split the CL in two - deletions after the other changes, but hey.. | 17:20 |
rogpeppe | dimitern: the deletions are easy to ignore | 17:20 |
* TheMue implements now a GoodByeerer | 17:21 | |
natefinch | rogpeppe: true.... just not one I heard used much. I guess Sourcer is fine | 17:21 |
dimitern | rogpeppe, yeah, I was thinking the same thing | 17:22 |
rogpeppe | natefinch: 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 ok | 17:24 |
dimitern | rogpeppe, 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 them | 17:24 |
rogpeppe | dimitern: doesn't ErrCode already work with a nil arg? | 17:25 |
natefinch | rogpeppe: google says people use it like recruiter... "one who ... " | 17:25 |
rogpeppe | dimitern: i certainly intended it to | 17:25 |
rogpeppe | dimitern: yeah, it does | 17:25 |
natefinch | rogpeppe: can we make environs.ConfigGetter into environs.Configurable? | 17:25 |
dimitern | rogpeppe, so nil.(something) is valid at runtime? | 17:26 |
rogpeppe | dimitern: of course | 17:26 |
rogpeppe | dimitern: erm, how long have you been using this language? :-) | 17:26 |
dimitern | rogpeppe, I wasn't sure - well then, just the IsCode*() helpers | 17:26 |
dimitern | rogpeppe, I'm still a bit unease around interfaces and reflection in general | 17:26 |
rogpeppe | dimitern: read the spec - it's quite straightforward | 17:27 |
dimitern | rogpeppe, 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 |
rogpeppe | dimitern: but in general, nil is an ok value to do reflection or test type conversion on | 17:28 |
natefinch | rogpeppe: 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 languages | 17:28 |
rogpeppe | natefinch: you're right. dimitern: i didn't wanna give you a hard time :-) | 17:28 |
dimitern | rogpeppe, none taken :D | 17:28 |
dimitern | ok guys, I'll be off then, have a good night | 17:29 |
rogpeppe | dimitern: there are quite a few places where nil is ok to use without problem - nil maps and slices being another example | 17:29 |
rogpeppe | dimitern: g'night | 17:29 |
dimitern | rogpeppe, I'll read it yet again tomorrow ;) | 17:29 |
natefinch | rogpeppe: to be fair, nil is a lot more useful in Go than it is in other languages. Quite handy, really. | 17:30 |
TheMue | dimitern, natefinch, rogpeppe: stepping out, have a nice evening | 17:31 |
natefinch | TheMue: g'night | 17:31 |
rogpeppe | natefinch: Configurable isn't great BTW, as it implies the config can be changed as well as retrieved | 17:40 |
natefinch | rogpeppe: hmmm true. | 17:42 |
natefinch | rogpeppe: it bugs me that ConfigGetter is only really used in one function (GetMetadataSources) and then only so you can get the ToolsURL off the config | 17:49 |
natefinch | (like, that's the only method that actually calls the method Config()) | 17:50 |
rogpeppe | natefinch: two functions actually (both named GetMetadataSources) | 17:50 |
natefinch | rogpeppe: ahh, I may have missed that there were two | 17:50 |
rogpeppe | natefinch: i agree though - i'm not keen on the arrangement | 17:51 |
rogpeppe | natefinch: but i don't want to change everything here, just improve things a little | 17:51 |
rogpeppe | natefinch: the whole thing involving the secret SupportsCustomSources interface seems too much like magic for me | 17:52 |
natefinch | rogpeppe: yeah | 17:52 |
rogpeppe | natefinch: (especially when it wasn't documented in the slightest) | 17:52 |
natefinch | rogpeppe: 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 call | 17:54 |
rogpeppe | natefinch: i'm not sure i understand that sentence :-) | 17:54 |
natefinch | rogpeppe: 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 implement | 17:56 |
rogpeppe | natefinch: yeah | 17:56 |
rogpeppe | natefinch: i'd be happy if all providers just implemented both methods | 17:56 |
natefinch | rogpeppe: yeah, really, it's not hard to implement a method that returns an empty slice | 17:56 |
rogpeppe | natefinch: it's only three extra lines of code | 17:57 |
rogpeppe | natefinch: exactly | 17:57 |
rogpeppe | natefinch: and it makes the whole interface more transparent | 17:57 |
rogpeppe | natefinch: anyway, eod for me | 17:58 |
rogpeppe | natefinch: g'night - and a review of that branch appreciated if poss | 17:59 |
rogpeppe | g'night all | 17:59 |
natefinch | rogpeppe: cool, I'll LGTM that... | 17:59 |
thumper | morning folks | 21:10 |
thumper | can someone explain to me why we have a worker/apiuniter? | 21:18 |
thumper | Spending the morning merging trunk in to my 8 branches and resolving conflicts :-| | 22:13 |
thumper | fwereade_: ping | 22:44 |
thumper | fwereade_: unping, can't remember what I was after | 22:59 |
bigjools | thumper: old age | 23:21 |
thumper | bigjools: must be | 23:21 |
thumper | I'm happy I remember how to type | 23:21 |
thumper | and I don't always get that right either | 23:22 |
bigjools | do you find yourself looking back at what you typed 5 minutes ago and only then notice all the typos? | 23:22 |
thumper | sometimes | 23:25 |
thumper | wallyworld: hello on call reviewer: Rietveld: https://codereview.appspot.com/13269052 | 23:37 |
wallyworld | hello | 23:37 |
wallyworld | thumper: done. now i have to run a few errands. back in a bit | 23:48 |
thumper | kk | 23:51 |
thumper | I'm off to the gym shortly too | 23:51 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!