| davecheney | ping thumper | 00:02 |
|---|---|---|
| fwereade__ | davecheney, thumper, whoops, I should sleep | 00:15 |
| thumper | fwereade__: I guess so :) | 00:15 |
| davecheney | thumper: are your tests still broken ? | 00:15 |
| fwereade__ | davecheney, thumper: if either of you is inclined to go on a trawl through my +activereviews I have a whole bunch and they should all be relatively easy | 00:16 |
| davecheney | i have some suggestions that you can apply from my experiences making the tests work under Q | 00:16 |
| thumper | davecheney: I'm running a quantal virtual box | 00:16 |
| thumper | I've not tried today with raring | 00:16 |
| fwereade__ | davecheney, thumper: they're all in a pipeline because they're all things I need to do to connect constraints up to environs | 00:16 |
| * fwereade__ waves and departs | 00:16 | |
| thumper | fwereade__: ok | 00:16 |
| thumper | davecheney: do you know where lbox stores the google auth token? | 00:17 |
| davecheney | ~/.lpad_oauth | 00:18 |
| thumper | ta | 00:18 |
| thumper | davecheney: it seems that a whole bunch of ubuntu pastebins were deleted over the weekend | 00:21 |
| thumper | no idea why | 00:21 |
| thumper | davecheney: also, ~/.lpad_oauth seems to only be the launchpad credentials | 00:22 |
| thumper | davecheney: I deleted it and tried lbox propose | 00:22 |
| davecheney | thumper: good point | 00:23 |
| thumper | davecheney: but it used the google credentials I was trying to change | 00:23 |
| davecheney | let me look again | 00:23 |
| davecheney | thumper: ~/.goetveld_* | 00:26 |
| thumper | davecheney: ta | 00:26 |
| davecheney | one per host | 00:27 |
| davecheney | should only be one | 00:27 |
| * thumper nods | 00:27 | |
| * thumper deletes it | 00:27 | |
| thumper | and tries again | 00:27 |
| thumper | wow: error: Failed to load data for project "juju-core": use of closed network connection | 00:28 |
| thumper | tried again... and it does something | 00:28 |
| davecheney | yup, nobody is sure if that is LP, or something weird in the go http client | 00:29 |
| davecheney | 2013/02/25 11:28:41 JUJU environs: searching for tools compatible with version: 1.9.10-quantal-amd64 | 00:29 |
| davecheney | 2013/02/25 11:28:45 JUJU juju bootstrap command failed: cannot start bootstrap instance: cannot allocate a public IP as needed | 00:29 |
| davecheney | and the public IP shortage continues to make it impossible to test canonistack | 00:29 |
| thumper | davecheney: I'm tempted to just leave the raring test issues until atlanta, as I expect the round-tripping will be a lot faster | 00:43 |
| davecheney | thumper: i'll propse a branch | 00:43 |
| davecheney | i know most of the places that are broke | 00:43 |
| thumper | oh, ok | 00:43 |
| thumper | I am happy to test | 00:43 |
| davecheney | from memory it is the test mocks | 00:44 |
| davecheney | /s/mock/fixtures | 00:44 |
| davecheney | <h1>Please try again</h1> | 00:47 |
| davecheney | <p> | 00:47 |
| davecheney | Sorry, there was a problem connecting to the Launchpad server. | 00:47 |
| davecheney | </p> | 00:47 |
| davecheney | <p> | 00:47 |
| davecheney | Try reloading this page in a minute or two. | 00:47 |
| davecheney | If the problem persists, let us know in | 00:47 |
| davecheney | <a href="irc://chat.freenode.net/#launchpad" | 00:47 |
| davecheney | >the #launchpad IRC channel on Freenode</a>. | 00:47 |
| davecheney | </p> | 00:47 |
| davecheney | <p>Thanks for your patience.</p> | 00:47 |
| davecheney | love this | 00:47 |
| davecheney | thumper: Proposal: https://code.launchpad.net/~dave-cheney/juju-core/097-environs-raring-fixtures/+merge/150254 | 00:49 |
| davecheney | Rietveld: https://codereview.appspot.com/7380047 | 00:49 |
| thumper | davecheney: what's with the 097 branch prefix? | 00:52 |
| davecheney | dunno, just something we all started doing | 00:52 |
| davecheney | personally, i got lost in all the open branchs I had | 00:52 |
| davecheney | numbering them at least let me know where they were temporarlly | 00:52 |
| thumper | davecheney: hmm, still fails a lot, I'll pastebin the errors | 00:56 |
| davecheney | ta | 00:57 |
| davecheney | gonna have to do this blind, i'm not upgrading to raring | 00:57 |
| thumper | :) | 00:58 |
| davecheney | sorry, i need my machine to work | 00:59 |
| thumper | davecheney: http://paste.ubuntu.com/5563524/ | 01:01 |
| thumper | davecheney: I've managed to keep working using a quantal VM I had lying around :) | 01:01 |
| davecheney | you need to do the git dance | 01:01 |
| davecheney | arguably we should pass a faux gitconfig | 01:02 |
| thumper | I have done that... | 01:02 |
| davecheney | dance harder, monkey boy | 01:02 |
| thumper | how to I get git to tell me what it has configured for user and email? | 01:03 |
| davecheney | cat ~/.gotconfig | 01:03 |
| davecheney | cat ~/.gitconfig | 01:03 |
| thumper | $ cat ~/.gitconfig | 01:04 |
| thumper | [user] | 01:04 |
| thumper | email = tim@penhey.net | 01:04 |
| thumper | name = Tim Penhey | 01:04 |
| thumper | and it still fails | 01:04 |
| davecheney | le sigh | 01:04 |
| thumper | so I'm wondering what else has changed | 01:04 |
| davecheney | either way, the test shouldn't assumed you've done the git dance | 01:04 |
| thumper | agree there 100% | 01:04 |
| davecheney | thumper: am fixing now | 01:05 |
| davecheney | can you be a sport and raise an issue | 01:05 |
| thumper | davecheney: for juju-core? | 01:06 |
| davecheney | sure | 01:06 |
| thumper | for the entire test failure, or just the git dance? | 01:06 |
| davecheney | either | 01:06 |
| thumper | sure | 01:06 |
| davecheney | raise as many or as few as you like | 01:06 |
| thumper | the tests pass with quantal | 01:06 |
| thumper | and I hadn't done the git dance there (at least I don't think I had) | 01:07 |
| thumper | or if I had, something else has changed between quantal and raring about where git looks | 01:07 |
| davecheney | yeah, they didn't used too, which is why i'm wary of upgrading to raring | 01:07 |
| * thumper files a bug about tests failing on raring | 01:07 | |
| davecheney | ok, raise an issue about the git parts failing | 01:07 |
| davecheney | that is the biggest part of the failure atm | 01:07 |
| thumper | hmm... | 01:09 |
| thumper | filed bug 1132585 just now | 01:09 |
| _mup_ | Bug #1132585: Tests fail on raring <juju-core:New> < https://launchpad.net/bugs/1132585 > | 01:09 |
| davecheney | ta muchly | 01:09 |
| thumper | davecheney: boo... | 01:19 |
| thumper | I thought I was going to be so clever | 01:19 |
| thumper | but go didn't like it | 01:19 |
| thumper | I was thinking I'd be able to inject a couple of methods into the cmd.Context function set from a testing module | 01:20 |
| thumper | but it ddn't work | 01:20 |
| thumper | was wanting something like: func (c *cmd.Context) StdoutString() string | 01:20 |
| thumper | but go said no | 01:20 |
| * thumper decides to use testing.stdout(c *cmd.Context) instead | 01:21 | |
| thumper | with a capital S | 01:21 |
| thumper | I think that is nicer than bufferString(c.Stdout) | 01:21 |
| thumper | testing.Stdout(c) instead | 01:22 |
| thumper | davecheney: what do you think? | 01:22 |
| * davecheney doesn't really follow | 01:22 | |
| davecheney | paste ? | 01:22 |
| thumper | davecheney: the guts of it is: we have two identical bufferString implementations | 01:22 |
| thumper | and I was wanting to not | 01:22 |
| thumper | also considered changing the names somewhat | 01:23 |
| thumper | as I thought it read better | 01:23 |
| thumper | am I being too anal? | 01:23 |
| * davecheney doesn't really follow, has three people talking to him at the moment | 01:23 | |
| thumper | :) | 01:23 |
| * davecheney is confused, i've hacked the uniter tests to supply a completely impossible value for GIT_CONFIG, and the tests still pass | 01:24 | |
| * thumper shelves the change for an anal day | 01:24 | |
| davecheney | u gonna submit your cmd aliases branch, or what ? | 01:24 |
| thumper | I did | 01:31 |
| thumper | I thought I did it last week | 01:31 |
| thumper | but for some reason it didn't go through | 01:31 |
| thumper | fixed and resubmitted today | 01:31 |
| thumper | ugh | 01:35 |
| thumper | gnuflag FTL | 01:36 |
| * thumper realises that a bunch of refactoring wasn't needed | 01:36 | |
| * thumper relocates | 01:38 | |
| davecheney | thumper: i can't figure it out, i've removed my own .gitconfig and the tests still pass | 04:59 |
| davecheney | thumper: def a version change for git in raring | 05:05 |
| davecheney | https://launchpad.net/ubuntu/+source/git | 05:05 |
| davecheney | NFI what has changed | 05:05 |
| davecheney | can you post the source for goroutine 10 [select]: | 05:43 |
| davecheney | github.com/wharf/tuplespace.(*TupleSpaceImpl).run(0xc20015f000) /Users/alec/.go/src/github.com/wharf/tuplespace/tuplespace.go:164 +0xb88 | 05:43 |
| davecheney | created by github.com/wharf/tuplespace.NewTupleSpaceImpl /Users/alec/.go/src/github.com/wharf/tuplespace/tuplespace.go:58 +0x142 ? | 05:43 |
| jam | wallyworld_: poke | 06:44 |
| wallyworld_ | hi | 06:44 |
| wallyworld_ | mumble? | 06:44 |
| jam | yep | 06:44 |
| jam | just got on | 06:44 |
| dimitern | fwereade__: ping | 08:09 |
| rogpeppe | dimitern, fwereade__: morning! | 08:11 |
| dimitern | rogpeppe: morning! | 08:11 |
| dimitern | rogpeppe: how was your holiday? | 08:11 |
| rogpeppe | dimitern: very good, thanks. nice weather, made it to the top of a few hills, had some nice food. | 08:12 |
| dimitern | rogpeppe: cool :) | 08:13 |
| jam | fwereade__, rogpeppe: Maybe one of you guys know. How do you compile just the test binary? I want to profile it, and I can run "go test -cpuprofile" but "go tool pprof" needs the build binary as a reference to interpret the profile output | 08:18 |
| rogpeppe | jam: hiya | 08:18 |
| jam | But I can't find a flag for "build just the test binary" | 08:18 |
| rogpeppe | jam: go test -c | 08:18 |
| jam | rogpeppe: anyway to actually find the documentation for that? | 08:18 |
| rogpeppe | jam: go help test | 08:18 |
| jam | go help testflag doesn't seem to have it | 08:18 |
| rogpeppe | jam: testflag gives help on the flags for the test binary itself | 08:18 |
| jam | rogpeppe: there are 4-different ways to get help, and I missed that one... :( | 08:18 |
| jam | go test -h no good | 08:18 |
| jam | go help | 08:19 |
| jam | go help testflag | 08:19 |
| jam | grep around online for a while | 08:19 |
| jam | rogpeppe: thanks for at least pointing it to me. I really expected to find it somewhere else | 08:19 |
| rogpeppe | jam: go help <command> should be your first stop | 08:19 |
| rogpeppe | jam: the testflag documentation is unusual | 08:19 |
| rogpeppe | jam: because it's not really documentation on a go subcommand | 08:20 |
| rogpeppe | jam: but it is referred to by the go test help | 08:20 |
| rogpeppe | jam: (how else did you know about testflag?) | 08:20 |
| jam | rogpeppe: well 'go <command> -h' is my natural first stop, but that ==> 'go help' which is, unhelpful IMO | 08:20 |
| jam | why would 'go help <subcommand>' not match 'go <subcommand> -h' | 08:20 |
| jam | rogpeppe: note that "go tool command -h" *is* the way to get help for individual commands as part of tools. | 08:21 |
| rogpeppe | jam: i dunno. but to be fair that (and go help) both say "Use "go help [command]" for more information about a command." | 08:21 |
| jam | Though "go help tool" *doesn't* show you what tools are available. | 08:21 |
| rogpeppe | jam: they might well accept a patch making go command -h work | 08:22 |
| jam | rogpeppe: and "go tool -h" also == "go help tool" which is a bit inconsistent. | 08:22 |
| jam | rogpeppe: as is 'go build -h' | 08:23 |
| jam | (I'm not planning on trying all of them, though) | 08:23 |
| rogpeppe | jam: oh yes, so it is | 08:23 |
| rogpeppe | jam: that's definitely an inconsistency i'd never noticed | 08:24 |
| rogpeppe | jam: (because i've always used "go help x" - as i do in bzr and hg etc) | 08:24 |
| jam | rogpeppe: also, there is no way to get the flags for the test command, you have to do some weird invocation of "go test -invalidflag" and then the binary fails and reports its options | 08:24 |
| rogpeppe | jam: (but not juju!) | 08:24 |
| jam | though those *don't* include the ones to 'go test' itself, because it is a different binary that interprets them. | 08:24 |
| rogpeppe | jam: go help testflag, no? | 08:25 |
| jam | rogpeppe: And I've generally always used "bzr command -h" :) | 08:25 |
| rogpeppe | jam: ah, except not quite | 08:25 |
| jam | rogpeppe: go help testflag doesn't do the -gocheck.* etc. | 08:25 |
| jam | I tried reporting an issue on it | 08:25 |
| jam | that there was no way to get the flags for the test suite | 08:25 |
| jam | and it was sort of punted. | 08:25 |
| rogpeppe | jam: you might be able to do "go test . -help" | 08:26 |
| jam | rogpeppe: nope, that '-help' gets interpreted by 'go test' and just get 'go help' output | 08:26 |
| rogpeppe | jam: yeah, that seems wrong | 08:28 |
| jam | rogpeppe: anyway thanks, -c helped me get what I wanted, and I can use the 'go help foo' style | 08:28 |
| rogpeppe | jam: np | 08:28 |
| rogpeppe | jam: it'd be worth raising an issue i think. now's the time if we want it fixed by go 1.1 | 08:29 |
| jam | rogpeppe: care to raise it? Might get more attention from you or dave | 08:30 |
| rogpeppe | jam: i'd appreciate a review of https://codereview.appspot.com/7390043 if you have some time at some point, BTW. | 08:30 |
| rogpeppe | jam: ok | 08:31 |
| rogpeppe | jam: FWIW, go test is the *only* subcommand for which -h doesn't work | 08:39 |
| rogpeppe | jam: it's probably something to do with the fact that it mixes flags between top level and compiled-binary level | 08:39 |
| TheMue | Morning all. | 08:41 |
| dimitern | TheMue: morning | 08:42 |
| fwereade__ | TheMue, dimitern, rogpeppe, jam: mornings | 08:42 |
| rogpeppe | fwereade__: yo! | 08:42 |
| rogpeppe | fwereade__: i'm angling for reviews of https://codereview.appspot.com/7390043, but you're probably aware of that :-) | 08:43 |
| dimitern | fwereade__, jam: morning | 08:51 |
| dimitern | mgz: hey | 08:51 |
| mgz | hey dimitern | 08:55 |
| fwereade__ | rogpeppe, just sent a slightly bewildered one :) | 09:04 |
| rogpeppe | fwereade__: ok | 09:04 |
| fwereade__ | rogpeppe, can you think of any reason for the ec2 tests to depend on the dummy provider? | 09:04 |
| rogpeppe | fwereade__: i don't *think* so | 09:04 |
| rogpeppe | fwereade__: do they? | 09:05 |
| fwereade__ | rogpeppe, can't run them without fixing the dummy provider, it seems, but I can't quite figure out the connection | 09:05 |
| rogpeppe | fwereade__: ah, i know | 09:06 |
| rogpeppe | fwereade__: the tests build the tools when testing PutTools | 09:06 |
| fwereade__ | rogpeppe, oh, ffs, ofc | 09:07 |
| rogpeppe | fwereade__: and i guess the tools depend... hmm, they shouldn't | 09:07 |
| fwereade__ | rogpeppe, ah! | 09:09 |
| fwereade__ | rogpeppe, juju/testing uses the dummy provider | 09:09 |
| rogpeppe | fwereade__: ah, of course | 09:09 |
| fwereade__ | rogpeppe, btw: why does StartInstance take apiinfo/stateinfo? | 09:09 |
| * rogpeppe tries to remember | 09:10 | |
| fwereade__ | rogpeppe, fwiw I'm also planning to drop the tools param | 09:10 |
| rogpeppe | fwereade__: oh yes, the idea was that the new instance needs to know how to contact the state server | 09:11 |
| fwereade__ | rogpeppe, yeah, but the environment already knows that | 09:11 |
| fwereade__ | rogpeppe, the only thing that's actually needed is the password | 09:12 |
| fwereade__ | rogpeppe, (btw are passwords for stateinfos or just for apiinfos? it's not clear why we save them for both when bootstrapping and only for state when startinstanceing) | 09:12 |
| === negronjl` is now known as negronjl | ||
| rogpeppe | fwereade__: i'm not sure what you're referring to by that last sentence | 09:14 |
| fwereade__ | rogpeppe, when you start a machine with a api info, what uses that API info? | 09:16 |
| rogpeppe | fwereade__: i think i was trying to avoid making the assumption that the environment always knows where the state server is. | 09:16 |
| fwereade__ | rogpeppe, why? | 09:16 |
| fwereade__ | rogpeppe, that's the one core thing we depend on the environment being able to do for us ;p | 09:16 |
| rogpeppe | fwereade__: we might want to store the current set of state server addresses in the state itself. | 09:17 |
| rogpeppe | fwereade__: in fact for the state server (as opposed to the API server) i think that's going to be the case | 09:18 |
| rogpeppe | fwereade__: hmm, but actually | 09:18 |
| rogpeppe | fwereade__: we're going to be able to drop state.Info eventually | 09:19 |
| rogpeppe | fwereade__: when you start a machine with an api info, the api info is used by the agents on that machine to connect to the API server. | 09:20 |
| rogpeppe | fwereade__: one advantage of passing a state info into StartInstance is that the environ doesn't have to query the bucket and resolve the DNS name each time. but perhaps that's a trivial cost. | 09:22 |
| rogpeppe | fwereade__: and it could be cached. | 09:22 |
| jam | rogpeppe: so I'd like to ask a bit about possibly caching the 'bundleTools' output during the test suite. Right now it takes about 2.5s for it to compile jujud and then tarball.gz it, and it happens far more times than I would expect. | 09:26 |
| jam | (eg, "BootstrapMultiple" test actually rebuilds the tools at least 2 times for that one test, but it also happens between tests.) | 09:27 |
| rogpeppe | jam: yeah, it would be nice to get rid of that overhead | 09:27 |
| dimitern | fwereade__: I think waitHooks{} is not working correctly | 09:27 |
| dimitern | fwereade__: or maybe I'm not using it right | 09:27 |
| rogpeppe | jam: i could never think of a sufficiently clean way of doing it, but i didn't think about it that hard. | 09:28 |
| fwereade__ | rogpeppe, ok, long-term, I agree that the state info will probably be stored elsewhere and distributed separately, but for now it seems that the state info is the one we use and the api info is just kinda hanging on alongside without any agent actually using it yet -- is this correct? | 09:28 |
| fwereade__ | dimitern, hmm, ok... would you paste me your output again and I'll see if any inspiration strikes? | 09:28 |
| rogpeppe | fwereade__: agents will use it very soon. they do actually connect to it, i believe, currently, even if they don't actually use it. | 09:28 |
| rogpeppe | fwereade__: actually... maybe they won't use it so soon. | 09:29 |
| rogpeppe | fwereade__: hmm. | 09:29 |
| dimitern | fwereade__: I'm augmenting the logging to see what's going on, just a moment | 09:29 |
| rogpeppe | fwereade__: yes, the API info isn't currently used. but it will be - it's a placeholder for what's to come. | 09:34 |
| dimitern | fwereade__: I found it! | 09:34 |
| * fwereade__ cheers at dimitern | 09:34 | |
| fwereade__ | dimitern, go on | 09:34 |
| dimitern | fwereade__: matchLogHooks does not accept u/0 db2:0 | 09:34 |
| fwereade__ | dimitern, grar, sorry | 09:35 |
| dimitern | fwereade__: `u/0(| [a-z-]+:[0-9]+)` that's the hook pattern | 09:35 |
| fwereade__ | dimitern, yeah, all makes sense | 09:35 |
| dimitern | fwereade__: I'll change it to `u/0(| [a-z0-9-]+:[0-9]+)` | 09:35 |
| fwereade__ | dimitern, sgtm | 09:37 |
| dimitern | fwereade__: now the db2-relation-joined mysql/0 db2:0 is matched, but still somehow ignored | 09:38 |
| fwereade__ | dimitern, discraded by test id badge checking? | 09:38 |
| dimitern | fwereade__: still checking.. | 09:39 |
| dimitern | fwereade__: :) the other regexp - donePattern needs changing as well | 09:41 |
| fwereade__ | dimitern, ah, ok | 09:41 |
| dimitern | fwereade__: now it works :) yay! | 09:44 |
| fwereade__ | dimitern, w00t! | 09:44 |
| dimitern | fwereade__: well, it fails, but for a different reason | 09:44 |
| dimitern | fwereade__: matching seems to pick up all hooks now | 09:45 |
| dimitern | fwereade__: this is what I got before the failure: [LOG] 0.53984 JUJU actual: []string{"install", "config-changed", "start", "upgrade-charm", "config-changed", "db2-relation-joined mysql/0 db2:0"}, ctx.hooks: []string{"install", "config-changed", "start", "db2-relation-joined mysql/0 db2:0"} | 09:45 |
| dimitern | [LOG] 0.54002 JUJU expected []string{"install", "config-changed", "start", "db2-relation-joined mysql/0 db2:0"}; never got anything | 09:45 |
| dimitern | fwereade__: upgrade-charm hook is missingm hence the failure | 09:46 |
| fwereade__ | dimitern, yeah, I think you should specify the exact sequence of hooks | 09:46 |
| fwereade__ | dimitern, I think if anything it makes the test clearer actually | 09:47 |
| fwereade__ | dimitern, "do these things as close together as possible; check everything happens in the correct order" | 09:47 |
| dimitern | fwereade__: yeah, I commented out the waitHooks{"upgrade-charm", "config-changed"} before the db2-r-j one, now I uncommented that and run it again | 09:47 |
| dimitern | fwereade__: and guess what - OK: 1 passed | 09:48 |
| dimitern | fwereade__: cool! I'll propose it now | 09:48 |
| dimitern | fwereade__: I could debug it effectively by replacing c.Logf() with log.Printf() - we should think about this probably (otherwise these are 2 separate logs and are out of sync) | 09:51 |
| fwereade__ | dimitern, expand please... I'm pretty sure that log.Printf is the one that produces apparent sequence breaks | 09:51 |
| dimitern | fwereade__: well juju uses log.Printf/Debugf and all lines are in a nice, expected order when printed | 09:53 |
| fwereade__ | dimitern, gaah sorry I totally misread you | 09:54 |
| dimitern | fwereade__: c.Logf on the other hand is a separate log, so the combined output is not sequential | 09:54 |
| fwereade__ | dimitern, they're happening on different goroutines anyway, though... | 09:55 |
| dimitern | fwereade__: yes, and we you look at the output it's not helpful (smth happened here and before or after is happened this other thing in the other log) | 09:56 |
| dimitern | s/we// | 09:56 |
| dimitern | s/is// | 09:56 |
| dimitern | :) I'm too fast to be readable today | 09:56 |
| fwereade__ | :) | 09:57 |
| dimitern | fwereade__: running a couple of times without the log hacks, to make sure the timing wasn't screwed somehow | 09:57 |
| fwereade__ | dimitern, well, you can see the two streams of independent events going on, right, they're just interspersed -- how would that change if they used the same log? | 09:58 |
| dimitern | fwereade__: well, you can tell that all lines after X happened after X happened, while now some things before X also happened after X | 09:59 |
| dimitern | fwereade__: X = juju log | 09:59 |
| dimitern | fwereade__: it'll be really nice if we separated parts of the uniter tests in smaller chunks (they can still use the same table format) - maybe on logical boundaries ("// XYZ tests" + NL) | 10:02 |
| fwereade__ | dimitern, yeah, I think that'd be a definite win | 10:03 |
| dimitern | fwereade__: there it is https://codereview.appspot.com/7385049 | 10:04 |
| fwereade__ | dimitern, cool, will take a look shortly | 10:04 |
| fwereade__ | dimitern, thanks :) | 10:05 |
| dimitern | fwereade__: cheers, I left some questions in comments | 10:06 |
| fwereade__ | dimitern, reviewed, LGTM, some comments | 10:32 |
| dimitern | fwereade__: thanks! | 10:32 |
| fwereade__ | rogpeppe, dimitern, jam: I would very much appreciate your thoughts on https://codereview.appspot.com/7394048 which is still WIP | 10:42 |
| dimitern | fwereade__: I'll take a look in 2m | 10:42 |
| rogpeppe | fwereade__: chunk mismatch. please reupload. | 10:42 |
| fwereade__ | rogpeppe, what does that complaint actually imply? what will change if I repropose? | 10:43 |
| rogpeppe | fwereade__: i've no idea, but reproposing usually seems to fix the problem. | 10:43 |
| fwereade__ | rogpeppe, haha, ok :) | 10:44 |
| fwereade__ | rogpeppe, better? | 10:45 |
| rogpeppe | fwereade__: yup | 10:45 |
| rogpeppe | fwereade__: thanks | 10:45 |
| fwereade__ | rogpeppe, cool, wish I understood what was up there | 10:45 |
| rogpeppe | fwereade__: me too. let me know if you find out! | 10:46 |
| rogpeppe | fwereade__, dimitern: i've responded to your comments on this, and made some changes: https://codereview.appspot.com/7390043 | 10:50 |
| rogpeppe | fwereade__: (i've also identified one significant omission, which i will be addressing shortly) | 10:51 |
| fwereade__ | rogpeppe, sorry, it was clearly monday morning... I think I read the goroutine as a defer or something | 10:52 |
| rogpeppe | fwereade__: ah, i wondered where the comments were coming from :-) | 10:53 |
| dimitern | rogpeppe: cheers, I'll take a look | 10:53 |
| dimitern | fwereade__: can we assume my CL closes bug 1101140 ? | 10:56 |
| _mup_ | Bug #1101140: uniter does not skip unimplemented relations <upgrade-charm> <juju-core:In Progress by dimitern> < https://launchpad.net/bugs/1101140 > | 10:56 |
| fwereade__ | dimitern, yep | 10:56 |
| dimitern | fwereade__: sweet! | 10:57 |
| fwereade__ | :D | 10:57 |
| dimitern | fwereade__: one down, 7 to go (wrt upgrade-charm bugs) | 10:58 |
| fwereade__ | dimitern, yay :) | 10:58 |
| fwereade__ | dimitern, SetCharm is the next goal, but I'm not sure whether you should be handling the config changes first or the sanity checks first | 10:59 |
| dimitern | fwereade__: I'll revisit the notes and we may need to have a g+ later perhaps? | 11:00 |
| fwereade__ | dimitern, I suspect doing the config changes is your best bet... maybe go with that, not worrying about the refcounting for now | 11:00 |
| fwereade__ | dimitern, but, yeah, reread the notes and let me know | 11:00 |
| fwereade__ | dimitern, I'd really appreciate thoughts on the Environ change first though :) | 11:01 |
| dimitern | fwereade__: I'm on it now | 11:01 |
| fwereade__ | dimitern, <3 | 11:01 |
| dimitern | btw i'll need one more LGTM on this guys https://codereview.appspot.com/7385049/ - anyone willing? | 11:02 |
| fwereade__ | rogpeppe, what's the other oversight? can't spot it, so LGTM, but you've made me nervous ;p | 11:07 |
| rogpeppe | fwereade__: i never tear down the watchers when the connection is dropped | 11:07 |
| fwereade__ | rogpeppe, ha! I remember that thought crossing my mind and then flitting away before I captured it in the first read | 11:08 |
| fwereade__ | rogpeppe, ty, good catch | 11:08 |
| rogpeppe | fwereade__: it requires a little more more mechanism, and i'm not quite sure of the best method yet | 11:08 |
| rogpeppe | fwereade__: am reviewing your bootstrap refactoring branch BTW | 11:09 |
| fwereade__ | rogpeppe, comments always welcome but it's far from complete | 11:09 |
| fwereade__ | rogpeppe, it's totally focused on getting ec2 working, nothing else | 11:10 |
| rogpeppe | fwereade__: i'm trying to think about implications for future providers | 11:10 |
| fwereade__ | rogpeppe, I have made an effort to consider that but am glad for the backup scrutiny | 11:12 |
| fwereade__ | rogpeppe, LaunchInstance might actually be something more like RunMachineAgentOnInstance if you're bothered by the local provider | 11:13 |
| rogpeppe | fwereade__: how does the new environs.Bootstrap square with the local provider? | 11:15 |
| fwereade__ | rogpeppe, what does it use that any provider doesn't provide? | 11:15 |
| rogpeppe | fwereade__: i'm not sure that the local provider provides a working StartInstance, does it? | 11:16 |
| fwereade__ | rogpeppe, no reason it can't do it, though, is there? the "instance" is always the local machine, but it gets "launched" when the machine agent is started | 11:16 |
| fwereade__ | rogpeppe, same deal as the SSH provider | 11:17 |
| rogpeppe | fwereade__: and what about future environments that have no bootstrap instance? | 11:17 |
| fwereade__ | rogpeppe, I don't see that you'd use bootstrap at all in that case | 11:18 |
| fwereade__ | rogpeppe, there's no actual bootstrapping going on there, is there? | 11:19 |
| rogpeppe | fwereade__: yeah - you'd tell some other machine to create an environment for you. | 11:19 |
| rogpeppe | fwereade__: that's still "bootstrapping" even though it doesn't require a new machine | 11:19 |
| rogpeppe | fwereade__: in general, i like the way that the refactoring looks (and it's necessary) but i'm not sure about its exact positioning | 11:20 |
| fwereade__ | rogpeppe, well, it performs tasks that have *some* degree of overlap with those in Bootstrap | 11:20 |
| TheMue | dimitern: You've got a review. | 11:21 |
| dimitern | TheMue: thanks! | 11:21 |
| fwereade__ | rogpeppe, go on | 11:21 |
| TheMue | dimitern: yw | 11:21 |
| rogpeppe | fwereade__: i'm still mulling it over | 11:21 |
| dimitern | fwereade__: reviewed | 11:25 |
| fwereade__ | dimitern, cheers | 11:25 |
| rogpeppe | fwereade__: i'm wondering about leaving environs.Bootstrap as it is, but providing another function, say environs.BootstrapInstance, that contains the bulk of what ec2.Bootstrap does now (the tools uploading and the instance starting). then ec2.Bootstrap (and other similar providers) would just call that, but stranger providers would be free to do as they wished. | 11:29 |
| fwereade__ | rogpeppe, is your concern specifically about starting instances? because that feels to me like the only behaviour that should differ | 11:30 |
| rogpeppe | fwereade__: it's not just about starting instances. it's also about the way that your proposal makes Bootstrap into a fundamentally racy thing when called concurrently. | 11:31 |
| rogpeppe | fwereade__: perhaps i should publish my current comments on the CL | 11:31 |
| fwereade__ | rogpeppe, was it not so beforehand? | 11:31 |
| fwereade__ | rogpeppe, sgtm | 11:31 |
| rogpeppe | fwereade__: (they're superceded by the above thought, but still pertinent to the CL) | 11:32 |
| dimitern | mgz: standup? | 11:32 |
| mgz | dimitern: yup | 11:32 |
| rogpeppe | fwereade__: no, Bootstrap was only racy because its implementations were racy, not because the operation is fundamentally racy, i think. | 11:33 |
| rogpeppe | fwereade__: as a trivial example, dummy.Bootstrap isn't racy | 11:34 |
| rogpeppe | fwereade__: i published my comments BTW | 11:35 |
| allenap | Hi. In Python Juju, how does Juju get onto a machine that I'm deploying to? It is fetched from Launchpad? I'll put it another way. I make some code changes to a machine provider in my local branch of lp:juju. I then deploy a charm using this code. Does the allocated machine use my updated code? Does a machine need the machine provider code? (MAAS in this instance.) | 11:35 |
| * TheMue enjoys lunch, biab | 11:35 | |
| fwereade__ | rogpeppe, cheers | 11:35 |
| fwereade__ | allenap, depends on juju-origin in the config | 11:35 |
| fwereade__ | allenap, IIRC you can set it to your own lp: branch | 11:35 |
| rvba | juju-origin: ppa | 11:36 |
| rvba | allenap: ^ | 11:36 |
| fwereade__ | allenap, you'll need to push before it'll use it though | 11:36 |
| fwereade__ | allenap, btw, I have a maas question | 11:36 |
| fwereade__ | allenap, can a process on a maas node easily find out the maas instance id? | 11:36 |
| allenap | fwereade__: Cool, thank you, I think that'll be useful. | 11:36 |
| allenap | fwereade__: rvba is checking that for you. | 11:38 |
| fwereade__ | allenap, rvba, cheers | 11:38 |
| allenap | fwereade__: Doesn't look like it. What's the use case? | 11:39 |
| fwereade__ | allenap, there's this foul InstanceIdAccessor hack that's persisted all the way back since orchestra and is uglying up every provider | 11:40 |
| * allenap looks it up | 11:40 | |
| rvba | fwereade__: I think the only thing you have is the 'name' of the node. | 11:41 |
| fwereade__ | allenap, the bootstrap node needs to know its own instance id and set it before its provisioning agent starts trying to provision an instance for itself | 11:41 |
| fwereade__ | rvba, and that's not the same as the instance id, I guess? | 11:41 |
| rvba | No | 11:41 |
| rvba | The 'name' is just the node's hostname. | 11:42 |
| fwereade__ | rvba, ok, thanks, no worries | 11:42 |
| fwereade__ | allenap, it's not a big deal, I can certainly make it less evil regardless | 11:43 |
| allenap | fwereade__: The MAAS provider (Python) does call `cloud_init.set_instance_id_accessor(instance_uri)` so I assume it's getting over to the machine somehow. | 11:44 |
| rvba | I'll check that it's there as soon as my node comes up… | 11:45 |
| rvba | (I've just destroyed my environment in the lab) | 11:45 |
| rvba | fwereade__: actually, allenap was right, the instance id is in /var/lib/cloud/instances/ | 11:52 |
| rvba | $ ls lib/cloud/instances/ | 11:52 |
| fwereade__ | allenap, rvba, wow, really? cool | 11:52 |
| rvba | node-74c10d5c-7f34-11e2-9a9a-525400123456 | 11:52 |
| allenap | Right, so the directory names is the instance ID? | 11:53 |
| rvba | Yes | 11:53 |
| allenap | s/names/name/ | 11:53 |
| rvba | Contains cloud-init stuff. | 11:53 |
| allenap | Is it also in a file within? | 11:53 |
| fwereade__ | rvba, allenap: in that case I think we can just straight-up drop InstanceIdAccessor and ask providers to implement InstanceId just as they do PrivateAddress/PublicAddress | 11:54 |
| fwereade__ | rvba, allenap: we can always hack it in via provider-specific machine config for functionality-challenged clouds in the future | 11:54 |
| mgz | fwereade__: that would make more sense than the chunk of bash method used currently | 11:55 |
| fwereade__ | mgz, yeah, it's total crack | 11:55 |
| fwereade__ | mgz, it's only there for orchestra | 11:55 |
| allenap | fwereade__: I haven't got my head around what InstanceIdAccessor does yet, so I'm not confident to say, but if all we need is the instance ID on the node then I guess we're okay. | 11:56 |
| fwereade__ | allenap, that is all it does | 11:56 |
| allenap | Cool. | 11:57 |
| fwereade__ | rogpeppe, repsonded | 12:15 |
| rogpeppe | fwereade__: looking | 12:20 |
| rogpeppe | fwereade__: there's something about the new structure that doesn't feel quite right to me. a combination of a few things, i think | 12:25 |
| rogpeppe | fwereade__: for instance, why does LaunchInstance now depend on a cloudinit data structure, when cloudinit is really just an artifact of one particular way of starting instances? | 12:26 |
| fwereade__ | rogpeppe, that data structure *defines* how we configure an instance -- AFAICT it's only in cloudinit because we don't have a more general place for it yet | 12:27 |
| fwereade__ | rogpeppe, the important bit is the "MachineConfig", not the "cloudinit.", I think | 12:28 |
| rogpeppe | fwereade__: i don't think so. it defines the information that the generated cloudinit script needs to start stuff up, but that's a slightly different thing. | 12:28 |
| fwereade__ | rogpeppe, please expand upon the distinction | 12:28 |
| fwereade__ | rogpeppe, AFAICT MachineConfig is a representation of what needs to be done to get a machine agent running, and the other stuff in cloudinit is resposible for transforming that into cloudinit data | 12:30 |
| fwereade__ | rogpeppe, I anticipate that other providers will maybe have different ways of transforming fundamentally the same data | 12:31 |
| rogpeppe | fwereade__: it's got other stuff too. AuthorizedKeys isn't about getting a machine agent running, for example. | 12:32 |
| jam | mgz: poke | 12:32 |
| rogpeppe | fwereade__: i think that using cloudinit.MachineConfig is just fine as part of the bootstrap process, but i don't see it as fundamental to the Environ interface | 12:33 |
| fwereade__ | rogpeppe, surely that's only there because nobody bothered to do it properly? :) | 12:33 |
| rogpeppe | fwereade__: which i still think is a reasonable abstraction | 12:33 |
| fwereade__ | rogpeppe, what if MachineConfig were to be defined somewhere else? | 12:35 |
| rogpeppe | fwereade__: and i think that taking the guts of ec2.environ.Bootstrap and putting into a new function in environs, specialised for starting a cloudinit-oriented environ, is a reasonable way forward | 12:35 |
| rogpeppe | fwereade__: rather than making Environ.Bootstrap more specific and meaning that providers that don't fit the mould will need to simulate stuff in order to "pretend" to start instances etc | 12:36 |
| mgz | jam: hey, you're back? | 12:37 |
| jam | mgz: yep | 12:37 |
| jam | care to do a 1:1 mumbling? | 12:37 |
| fwereade__ | rogpeppe, why do you think MachineConfig is connected with cloudinit? is there anything other than the simple accident of placement? | 12:38 |
| rogpeppe | fwereade__: because it was fashioned entirely to hold the things that cloudinit needs to do its job. | 12:38 |
| fwereade__ | rogpeppe, that job being..? | 12:38 |
| rogpeppe | fwereade__: there's no point in passing half of that stuff into the Environ, because it already knows it | 12:39 |
| rogpeppe | fwereade__: the fact that the data dir is in /usr/lib/juju is something that the Environ should decide, not the caller of the environ. | 12:40 |
| rogpeppe | fwereade__: (for example) | 12:40 |
| fwereade__ | rogpeppe, yes... some fields are filled in by provider-specific code, and some by non-provider-specific code | 12:40 |
| dimitern | fwereade__: just to double check - in order to handle config stuff properly in SetCharm, these are all related: http://paste.ubuntu.com/5564588/ | 12:41 |
| rogpeppe | fwereade__: i would expect every parameter to Environ methods to be filled in a non-provider-specific code | 12:41 |
| rogpeppe | s/in a non/in by non/ | 12:41 |
| fwereade__ | rogpeppe, like yuo did with stateinfo and tools, eh? | 12:41 |
| rogpeppe | fwereade__: those types *are* filled in by code external to the environ, no? | 12:42 |
| fwereade__ | rogpeppe, they're a crazy frankenstein patchwork of could-come-from-anywhere AFAICT | 12:43 |
| rogpeppe | fwereade__: at least they each represent a coherent resource (the state we need to connect to; the tools we want to use) | 12:43 |
| fwereade__ | rogpeppe, both of which are totally meaningless and inappropriate from the caller's POV | 12:44 |
| fwereade__ | rogpeppe, how did tools get into the interface in the first place? | 12:44 |
| rogpeppe | fwereade__: i thought the process of finding the tools could be portable. | 12:45 |
| rogpeppe | fwereade__: but i think you're right that it should probably be internal to Environ | 12:46 |
| rogpeppe | fwereade__: and perhaps the same with state.Info as passed to Environ.StartInstance | 12:46 |
| fwereade__ | rogpeppe, it could never be portable | 12:47 |
| rogpeppe | fwereade__: no? | 12:47 |
| rogpeppe | fwereade__: perhaps not, but it seemed not too far from a possibility to me | 12:47 |
| dimitern | fwereade__: have a look pls when you have time | 12:47 |
| * dimitern lunch | 12:47 | |
| fwereade__ | rogpeppe, it always depended on the arch, which could only ever be determined after contrsint handling | 12:47 |
| fwereade__ | dimitern, will do, cheers | 12:48 |
| rogpeppe | fwereade__: anyway, all of that is making the Environ interface less, not more, specific. but your proposal makes it much *more* specific. | 12:48 |
| rogpeppe | fwereade__: yeah, all of this stuff was always going to change with the advent of constraints. | 12:48 |
| fwereade__ | rogpeppe, it makes explicit the fact that you need to run a machine agent on an instance, and it takes the non-provider-related job of configuring that machine agent out of the hands of the environ | 12:50 |
| rogpeppe | fwereade__: there's nothing explicitly mentioned about a machine agent in MachineConfig, is there? | 12:51 |
| fwereade__ | rogpeppe, ISTM pretty clear that a MachineConfig holds exactly the information necessary to get a machine agent running | 12:52 |
| fwereade__ | rogpeppe, please explain again the distinction you are seeing between a machine config and a ...MachineConfig ;) | 12:52 |
| rogpeppe | fwereade__: there's lots more in there - StateServerCert/Key, StateServer, MongoPort, APIPort, ProviderType, MongoURL, AuthorizedKeys, Config | 12:54 |
| fwereade__ | rogpeppe, how do you propose to run a machine agent without state to run it against? | 12:54 |
| rogpeppe | fwereade__: maybe the state is available elsewhere | 12:55 |
| rogpeppe | fwereade__: that's up to the provider, surely? | 12:55 |
| fwereade__ | rogpeppe, in that case it doesn't need to be specified... and indeed it is not specified | 12:55 |
| rogpeppe | fwereade__: what if someone passes a MachineConfig to the local provider's LaunchInstance with StateServer=true ? | 12:56 |
| fwereade__ | rogpeppe, what it they do? | 12:57 |
| rogpeppe | fwereade__: i'm interested though: what objection do you have to factoring out the ec2-like bootstrap stuff in a similar way to how the cloudinit stuff was factored out before? | 12:58 |
| fwereade__ | rogpeppe, who's going to be doing that in the first place? | 12:58 |
| rogpeppe | fwereade__: the (new) Environ interface looks like it's a viable possibility | 12:58 |
| rogpeppe | fwereade__: i'd like to keep environs.Environ as simple as possible, and i think we can do that without loss of generality | 12:59 |
| rogpeppe | fwereade__: and as a bonus that means that simpler providers don't need to mess with any of that stuff. | 13:00 |
| fwereade__ | rogpeppe, so what of the stuff that I do in bootstrap do you believe to be optional? | 13:00 |
| rogpeppe | fwereade__: LaunchInstance, for one | 13:01 |
| fwereade__ | rogpeppe, it's basically RunMachineAgent, though, right? | 13:01 |
| rogpeppe | fwereade__: upload tools also | 13:01 |
| rogpeppe | fwereade__: no, it's more than that. running a machine agent is considerably more simple. | 13:02 |
| fwereade__ | rogpeppe, er, you made upload-tools a command line param, and part of the interface of environ in the first place | 13:02 |
| fwereade__ | rogpeppe, how do you imagine that to be optional? | 13:02 |
| rogpeppe | fwereade__: there's nothing that states that the tools need to be uploaded to the environ's Storage AFAICS. the local provider could simply copy the binaries to the new container, for example. | 13:03 |
| fwereade__ | rogpeppe, running a machine agents *once you have all the requirements in place* is simple | 13:03 |
| fwereade__ | rogpeppe, why would we fuck up the local provider like that? because more code paths are a Good Thing? | 13:04 |
| fwereade__ | rogpeppe, because it's Fun to implement providers that randomly break when you want to upgrade? | 13:05 |
| rogpeppe | fwereade__: because copying something to the storage just to copy it out again in the next line isn't very useful? | 13:06 |
| rogpeppe | fwereade__: the only code path that depends on the tools being available in the storage is the shell script in cloudinit | 13:07 |
| rogpeppe | fwereade__: apart from later when we upgrade | 13:07 |
| rogpeppe | fwereade__: which is different. | 13:08 |
| fwereade__ | rogpeppe, oh really? | 13:08 |
| rogpeppe | fwereade__: yeah, i think so | 13:08 |
| === gary_poster|away is now known as gary_poster | ||
| rogpeppe | fwereade__: istm that this might be better in G+ | 13:08 |
| fwereade__ | rogpeppe, yeah, maybe, just a mo, not sure if lunch is coming | 13:09 |
| jam | mgz: apparently hdmi really is needed :) | 13:11 |
| fwereade__ | rogpeppe, https://plus.google.com/hangouts/_/f13861a7f65b5a679aa07c1d94088d17c4e8330b?authuser=0&hl=en | 13:13 |
| benji | bac: let me know when you have a minute to sync up | 13:26 |
| bac | benji: ok, how about in 5 minutes? | 13:26 |
| benji | bac: sounds great | 13:26 |
| bac | benji: i'll make a hangout | 13:37 |
| benji | rogpeppe: I am getting test failures running "go test ./state/api/" on trunk; is that a know issue? | 14:09 |
| benji | in fact, trunk doesn't look like it even builds: worker/firewaller/firewaller.go:319: undefined: state.IsNotAssigned | 14:13 |
| rogpeppe | benji: on a call, sorry, will be with you in a bit | 14:24 |
| benji | rogpeppe: thanks | 14:24 |
| === wedgwood_away is now known as wedgwood | ||
| rogpeppe | benji: out of call finally. trunk at least compiles all tests for me currently. just checking that all tests run ok. | 15:22 |
| benji | rogpeppe: I blew away my entire launchpad.net tree and re-got it and it works now <shrug> | 15:23 |
| benji | perhaps I am missing some part of the dev process; is there some sort of re-fetch-and-build-all-dependencies command that people use? | 15:23 |
| rogpeppe | benji: hmm, i wonder what the problem was. in future, you might try blowing away just $GOPATH/pkg instead and see if that makes a difference | 15:23 |
| benji | k | 15:23 |
| rogpeppe | benji: not really. the only thing i usually have to update is launchpad.net/goose | 15:24 |
| benji | how do you go about doing that? | 15:24 |
| rogpeppe | benji: other than that, all dependency management *should* be done by the go tool | 15:24 |
| rogpeppe | benji: go get -u launchpad.net/goose | 15:24 |
| benji | k, thanks | 15:24 |
| rogpeppe | benji: i update it only when some openstack stuff starts failing | 15:25 |
| benji | k | 15:25 |
| fwereade__ | dimitern, ping | 15:26 |
| dimitern | fwereade__: pong | 15:33 |
| dimitern | fwereade__: I decided to propose what we discussed in the mean time - split the uniter tests | 15:33 |
| dimitern | fwereade__: will be ready shortly | 15:35 |
| fwereade__ | dimitern, awesome, just going t have a really quick piece of toast | 15:40 |
| fwereade__ | dimitern, hey, sorry, actually here now | 15:55 |
| dimitern | fwereade__: np | 15:55 |
| fwereade__ | dimitern, starting a hangout | 15:56 |
| dimitern | fwereade__: cool | 15:56 |
| fwereade__ | dimitern, https://plus.google.com/hangouts/_/4e9d42088fdb90f3cdce77923346ef9c1a62a89e?authuser=0&hl=en | 15:56 |
| fwereade__ | rogpeppe, don;t suppose I can interest you in a quick review of https://codereview.appspot.com/7396056/ ? | 16:50 |
| rogpeppe | fwereade__: looking | 16:51 |
| dimitern | fwereade__: there it is https://codereview.appspot.com/7378066/ | 16:55 |
| dimitern | :) | 16:55 |
| fwereade__ | dimitern, cheers | 16:55 |
| fwereade__ | dimitern, that's awesome, LGTM | 16:56 |
| fwereade__ | dimitern, do they get any faster? :) | 16:56 |
| dimitern | fwereade__: thanks! | 16:56 |
| dimitern | fwereade__: haven't check, but if they did is not much noticeable | 16:57 |
| dimitern | fwereade__: I'll run them now to compare with older times | 16:57 |
| fwereade__ | dimitern, I think it ought to save a couple of seconds at least, but maybe that's all eaten up in extra test setup/teardown | 16:58 |
| rogpeppe | fwereade__: reviewed | 16:59 |
| dimitern | fwereade__: nah.. probably they're *slightly* slower due to extra call overhead | 16:59 |
| dimitern | fwereade__: WOW they're actually faster! | 16:59 |
| fwereade__ | dimitern, wanna know why? | 17:00 |
| dimitern | fwereade__: old time: 149.123 s, new time: 102.571s | 17:00 |
| fwereade__ | dimitern, waitHooks is O(len(testlog)) | 17:00 |
| dimitern | fwereade__: yes! | 17:00 |
| dimitern | fwereade__: oh, I see | 17:00 |
| fwereade__ | dimitern, my money's on that, anyway | 17:00 |
| dimitern | :) | 17:00 |
| dimitern | fwereade__: can we consider this trivial with one LGTM | 17:03 |
| fwereade__ | dimitern, if they're all passing, yeah, I think so -- no logic changes, so go for it :) | 17:05 |
| dimitern | fwereade__: yeah, all passing | 17:12 |
| dimitern | fwereade__: cheers | 17:12 |
| rogpeppe | fwereade__: if len(testlog) is a significant contributor to the test runtime, we should fix it. it surely wouldn't be hard. | 17:18 |
| dimitern | TheMue: ty | 17:19 |
| fwereade__ | rogpeppe, it only became so gradually as the test table got seriously long | 17:19 |
| rogpeppe | fwereade__: yeah, maybe it's insignifant again. | 17:19 |
| rogpeppe | icant | 17:19 |
| fwereade__ | rogpeppe, I'm not sayng it *shouldn't* be fixed, but I think the bulk of its contribution has been wiped out | 17:20 |
| fwereade__ | rogpeppe, btw, good catch on init-time version.Current; ty | 17:20 |
| rogpeppe | fwereade__: np | 17:20 |
| rogpeppe | fwereade__: i thought uniter spent all its time making changes to mongo and waiting for changes. | 17:21 |
| rogpeppe | fwereade__: (the uniter tests, that is) | 17:21 |
| fwereade__ | rogpeppe, I had a closer look the other day and only then spotted that the testlog-scanning had become significant | 17:22 |
| fwereade__ | rogpeppe, and was starting to consider rewriting the whole lot as separate test cases but didn't quite have the energy ;p | 17:22 |
| rogpeppe | fwereade__: you could easily avoid using the whole test log by pre-filtering for particular log messages, no? | 17:22 |
| rogpeppe | fwereade__: (i remember doing something similar once) | 17:23 |
| fwereade__ | rogpeppe, a target that collects the ones I care about and stashes them somewhere? yeah, might be nice | 17:24 |
| rogpeppe | fwereade__: yeah. a simple prefix regexp worked ok for me. | 17:24 |
| fwereade__ | rogpeppe, so long as I don;t disturb the rest of the log, which does actually contain useful stuff sometimes | 17:24 |
| rogpeppe | fwereade__: yeah - i logged as well as stashing | 17:25 |
| fwereade__ | rogpeppe, cool, didn't think of that at the time | 17:25 |
| rogpeppe | fwereade__: for our copious spare time :-) | 17:25 |
| fwereade__ | rogpeppe, indeed :) | 17:26 |
| dimitern | i'm off guys | 17:29 |
| dimitern | have a good evening | 17:30 |
| fwereade__ | dimitern, gn, take care | 17:30 |
| rogpeppe | fwereade__: a small CL that enables cleaning up of watchers in the other one: https://codereview.appspot.com/7398050 | 17:46 |
| rogpeppe | fwereade__: perhaps "Abort" might be better than Kill, but i thought it's similar enough to tomb's Kill that the same name is probably appropriate. | 17:47 |
| rogpeppe | right, that's me for the day. see y'all tomorrow. | 18:42 |
| === wedgwood is now known as wedgwood_away | ||
| === wedgwood_away is now known as wedgwood | ||
| * thumper stabs go in the face | 20:56 | |
| thumper | go not having any form of real inheritance sucks monkey balls | 20:57 |
| === _thumper_ is now known as thumper | ||
| thumper | morning davecheney | 22:21 |
| thumper | davecheney: why does go make it so hard to do something like virtual dispatch? | 22:21 |
| davecheney | thumper: do you want the long answer or the short answer | 22:26 |
| thumper | davecheney: I want to know how to do what I want to do | 22:26 |
| thumper | as each thing I try, I get stuck by go's limitations | 22:27 |
| davecheney | thumper: not limitations, design decisions | 22:27 |
| thumper | :) | 22:27 |
| thumper | davecheney: so... lets see if you can help me out | 22:27 |
| thumper | right now in the cmd package, there is a help method on the Info object | 22:28 |
| thumper | s/object/struct | 22:28 |
| davecheney | mmm | 22:28 |
| thumper | and a hard coded function call when needing help that takes (c *Command) -> c.Info().Help(f) where f aresome flags | 22:28 |
| thumper | what I want however is to override that behaviour when c is a SuperCommand | 22:28 |
| thumper | and I feel that looking at the underlying type is a horrible crutch | 22:29 |
| thumper | and was trying to find a better way | 22:29 |
| thumper | including having a "Base struct member" like CommandBase | 22:29 |
| thumper | however that doesn't work | 22:29 |
| thumper | as it can't call something it doesn't know about | 22:29 |
| davecheney | i think the problem is we're trying to make SuperCommand a subclass of Command | 22:30 |
| thumper | hmm... | 22:30 |
| thumper | perhaps | 22:30 |
| davecheney | the first thing that comes to mind is makeing Command an interface that both | 22:30 |
| davecheney | subcommand and supercommand satisfy | 22:31 |
| thumper | right... I started down that path | 22:31 |
| thumper | actually, that is how it works right now | 22:32 |
| thumper | but there is no concept of a "subcommand" as an entity in its own right | 22:32 |
| thumper | and I don't want to have to implement default behaviour in every subcommand | 22:33 |
| thumper | what I really want is some equivalent to the "pure virtual method" issue with an intermediate default implementation | 22:34 |
| thumper | surely go has some answer to that design problem | 22:34 |
| thumper | I just don't know what it is | 22:34 |
| davecheney | embedding | 22:36 |
| davecheney | it is also that go does not have an answer for this | 22:36 |
| davecheney | the authors made some deliberate decisions to not include type inheritence or function polymorphism | 22:36 |
| davecheney | so sometimes it isn't possible to directly transpose from one language to another | 22:37 |
| thumper | davecheney: I tried embedding, but it fails in one critical aspect | 22:44 |
| thumper | davecheney: there isn't a way to define a method that doesn't yet exist | 22:45 |
| thumper | davecheney: where you can get the result from a more-derived class | 22:45 |
| thumper | davecheney: and you can't have a method on an interface, nor can you have function overloading | 22:45 |
| davecheney | yeah, there is only composition, not virtual dispatch | 22:45 |
| thumper | do I'm stuck trying to work out a solution for the problem in go | 22:46 |
| davecheney | wut "you can't have a method on an interface" ? | 22:46 |
| thumper | that is trivial in several other languages | 22:46 |
| thumper | and I'm venting | 22:46 |
| thumper | as the target of the method | 22:46 |
| thumper | like | 22:46 |
| * davecheney pats thumper on the shoulder | 22:46 | |
| thumper | func (i *SomeInterface) Foo() {} | 22:46 |
| thumper | which would work fine if it worked | 22:46 |
| davecheney | nup, can't do that, intefaces only define the contract, not satisfy them | 22:46 |
| thumper | right | 22:47 |
| thumper | hmm... time to collect my daughter for lunch | 22:48 |
| thumper | bbl | 22:48 |
| davecheney | thumper: or, you could use the type assertion and move on with your life :) | 22:48 |
| thumper | I may just do that, bit it feels horrible! | 22:49 |
| thumper | goes against a whole lot of my professional life | 22:49 |
| thumper | it is like saying, "just shoot that person, and get on with your life" | 22:49 |
| thumper | I've spent 20 years learning not to do that | 22:49 |
| davecheney | sure | 22:49 |
| davecheney | then lets fix it properly | 22:49 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!