| === gary_poster|away is now known as gary_poster | ||
| niemeyer | thumper: Yo | 01:38 |
|---|---|---|
| thumper | niemeyer: oh hai | 01:39 |
| thumper | I had a go question... | 01:39 |
| thumper | if you have a select and each of the cases are waiting on a channel | 01:40 |
| thumper | what is happening under the covers? | 01:40 |
| thumper | and that select is inside a for { } | 01:40 |
| thumper | like the provisioner | 01:40 |
| niemeyer | thumper: An epoll | 01:40 |
| niemeyer | thumper: Not sure if that's your question, though? | 01:41 |
| thumper | niemeyer: I was just wanting to make sure it wasn't doing stupid looping when nothing is happening | 01:41 |
| thumper | I considered wrting a test app to check, but got busy thinking of something real | 01:42 |
| niemeyer | thumper: Ah, no, it will block | 01:42 |
| niemeyer | thumper: Unless you have a default in the select loop.. then it would spin like crazy | 01:42 |
| * thumper nods | 01:42 | |
| niemeyer | s/select loop/select clause/ | 01:43 |
| wallyworld_ | thumper: if you have a moment at some point, https://code.launchpad.net/~wallyworld/juju-core/create-machine-command/+merge/165518 | 02:19 |
| thumper | wallyworld_: sure | 02:19 |
| wallyworld_ | thumper: i didn't update machine state to include a Clean attribute. still not sure if we want that or not | 02:20 |
| thumper | wallyworld_: I'm about to email the group about my proposed machine state changes as there are a lot of them | 02:20 |
| wallyworld_ | i'm thinking if we do add it, we can change the default assignment policy | 02:21 |
| wallyworld_ | from AssignNew to AssignUnused | 02:21 |
| wallyworld_ | since we can look for a "clean" machine | 02:22 |
| thumper | right... | 02:22 |
| thumper | there is more to it though :) | 02:22 |
| thumper | which I'll be putting in the email | 02:22 |
| wallyworld_ | sure, containers complicate things | 02:22 |
| wallyworld_ | what i say above gives us the same thing as we have now | 02:23 |
| wallyworld_ | since without it, create-machine just wastes resources | 02:23 |
| thumper | wallyworld_: well, you sould be able to use --force-machine | 02:23 |
| thumper | wallyworld_: does that work? | 02:23 |
| wallyworld_ | ah,true | 02:23 |
| wallyworld_ | should work | 02:24 |
| wallyworld_ | the machine is created, spun up, and is just sitting there | 02:24 |
| wallyworld_ | with no principal units or anything running | 02:24 |
| wallyworld_ | i'll check it out | 02:24 |
| wallyworld_ | thumper: all works as expected :-) | 02:34 |
| thumper | wallyworld_: good | 02:46 |
| wallyworld_ | thumper: i think juju status should print out the constraints for the machine | 02:48 |
| wallyworld_ | right now it doesn't seem to do that | 02:48 |
| thumper | perhaps a flag | 02:48 |
| thumper | we are getting too much info by default | 02:48 |
| thumper | much more than is usually useful | 02:49 |
| wallyworld_ | you think? | 02:49 |
| wallyworld_ | i sorta like what's there now | 02:49 |
| wallyworld_ | there's only 5 lines for eacdfh machine | 02:50 |
| wallyworld_ | adding one more for constraints doesn't seem too bad | 02:50 |
| === wedgwood is now known as wedgwood_away | ||
| thumper | well, that is me about done for the day | 05:25 |
| rogpeppe1 | mornin' all | 06:32 |
| kyhwana | evening | 06:34 |
| dimitern | morning | 06:51 |
| rogpeppe1 | kyhwana: :-) | 06:51 |
| rogpeppe1 | dimitern: hiya | 06:51 |
| dimitern | rogpeppe1: hey | 06:53 |
| dimitern | rogpeppe1: i couldn't finish yesterday because just as i was nearing the end and only a few more test cases were needed, i started getting test failures or panics when running the apiserver tests; but not all the time, like 1 in 10 cases | 06:54 |
| rogpeppe1 | dimitern: hmm, that's not entirely surprising for that kind of logic. | 06:54 |
| rogpeppe1 | dimitern: have you run it with race detection on? | 06:55 |
| dimitern | rogpeppe1: yes, multiple times - no races | 06:55 |
| rogpeppe1 | dimitern: well, not a memory race anyway, eh? :-) | 06:56 |
| dimitern | rogpeppe1: so there are 2 kinds of failures - timeouts for the watchers (always the same - next() returns a stopped error before stopping either the server or the watcher - and in the logs i can see it is indeed getting stopped somehow) | 06:56 |
| rogpeppe1 | dimitern: is that one kind of failure or two? | 06:58 |
| dimitern | rogpeppe1: another issue is the pretty long and somehow not very legible panic, usually mentioning PutCharm some 50 frames down and having mgo stuff close to the top | 06:58 |
| rogpeppe1 | dimitern: could you paste the panic here? | 06:58 |
| dimitern | rogpeppe1: running them again | 06:59 |
| dimitern | rogpeppe1: http://paste.ubuntu.com/5696111/ | 07:01 |
| rogpeppe1 | dimitern: interesting, looks like a runtime bug | 07:01 |
| rogpeppe1 | dimitern: you can reproduce this ok? | 07:01 |
| dimitern | rogpeppe1: trying more times | 07:02 |
| dimitern | rogpeppe1: how can you tell is likely a runtime bug? | 07:03 |
| rogpeppe1 | dimitern: it's dying during garbage collection | 07:03 |
| dimitern | rogpeppe1: wow :) these kind of things always happen to me :D | 07:04 |
| rogpeppe1 | dimitern: so it seems. | 07:04 |
| rogpeppe1 | dimitern: please could you try and see if you can reproduce the behaviour using gustavo's new Go-only yaml implementation | 07:05 |
| dimitern | rogpeppe1: it happened again - almost the same, but trace order is changed a bit | 07:05 |
| dimitern | rogpeppe1: http://paste.ubuntu.com/5696122/ | 07:06 |
| dimitern | rogpeppe1: so how to get it? | 07:06 |
| rogpeppe1 | dimitern: i'm just looking for it | 07:08 |
| dimitern | rogpeppe1: hmm now it happens on every run - yesterday happened less often.. also it doesn't happen if I run individual tests | 07:10 |
| rogpeppe1 | dimitern: https://code.launchpad.net/~niemeyer/goyaml/go-port | 07:10 |
| dimitern | rogpeppe1: so what - delete the goyaml in gopath and get this instead? | 07:11 |
| rogpeppe1 | dimitern: yeah - although you'll need to manually branch it into launchpad.net/goyaml | 07:12 |
| dimitern | rogpeppe1: or just remove the files and replace them | 07:12 |
| rogpeppe1 | dimitern: yeah, that's probably easier | 07:12 |
| TheMue | morning | 07:14 |
| rogpeppe1 | dimitern: fairly trivial CL: https://codereview.appspot.com/9681045 | 07:14 |
| rogpeppe1 | TheMue: hiya | 07:14 |
| dimitern | TheMue: hey | 07:15 |
| dimitern | rogpeppe1: looking | 07:15 |
| dimitern | rogpeppe1: LGTM | 07:17 |
| rogpeppe1 | dimitern: ta | 07:17 |
| === rogpeppe1 is now known as rogpeppe | ||
| dimitern | rogpeppe: ha! i happened at first with goyaml-go, but wasn't sure it's clean, so I went in pkg/linux_*/launchpad.net/ and deleted goyaml.a | 07:18 |
| dimitern | rogpeppe: now there's no panic, just some failures | 07:18 |
| rogpeppe | dimitern: hmm. keep trying | 07:19 |
| dimitern | rogpeppe: http://paste.ubuntu.com/5696153/ | 07:19 |
| rogpeppe | dimitern: it may still be a runtime bug | 07:20 |
| rogpeppe | dimitern: just that you're not triggering it any more for some reason | 07:20 |
| dimitern | rogpeppe: oops there is it again: http://paste.ubuntu.com/5696157/ | 07:20 |
| rogpeppe | dimitern: because i'm not sure the code path you're testing uses yaml much | 07:20 |
| rogpeppe | dimitern: ah, good. (well, kinda :-]) | 07:21 |
| rogpeppe | dimitern: could you push the branch - i'll see if i can reproduce | 07:21 |
| dimitern | rogpeppe: sure | 07:22 |
| dimitern | rogpeppe: ha! now all passed | 07:22 |
| rogpeppe | dimitern: run it in a loop | 07:22 |
| TheMue | *: any probs with the current goyaml? | 07:23 |
| dimitern | rogpeppe: lp:~dimitern/juju-core/041-provisioner-api-calls | 07:23 |
| dimitern | rogpeppe: how do you mean in a loop? | 07:23 |
| dimitern | rogpeppe: it seems it happens in the TestOperationPerm ... except now it happened later + a failure: http://paste.ubuntu.com/5696171/ | 07:25 |
| rogpeppe | dimitern: while true; do go test; done | 07:26 |
| dimitern | rogpeppe: running | 07:28 |
| rogpeppe | dimitern: i've just reproduced it | 07:31 |
| dimitern | rogpeppe: cool | 07:31 |
| dimitern | rogpeppe: w/o goyaml-go? | 07:32 |
| rogpeppe | dimitern: no - i'll just try with non-cgo goyaml | 07:33 |
| dimitern | rogpeppe: ok, if it's indeed a runtime bug now begins the tedious hunting for a minimal case to reproduce i guess.. | 07:34 |
| rogpeppe | dimitern: yeah. | 07:34 |
| dimitern | rogpeppe: while you're looking at this, i'll finish the rest of the tests, so hopefully if there's a way around the panic it'll be ready for proposing | 07:41 |
| dimitern | rogpeppe: if you don't mind | 07:41 |
| rogpeppe | dimitern: you could work against 1.0.3 for the time being | 07:42 |
| dimitern | rogpeppe: does it work like that? | 07:42 |
| rogpeppe | dimitern: it certainly should do | 07:42 |
| rogpeppe | dimitern: we're trying to keep 1.0.3 compatibility after all | 07:43 |
| rogpeppe | dimitern: if it doesn't, it's a bug | 07:43 |
| dimitern | rogpeppe: i'm getting 1.0.3 again to try | 07:43 |
| rogpeppe | dimitern: if you're using go from source, you can just do hg update go1.0.3 | 07:43 |
| rogpeppe | dimitern: then run all.bash | 07:44 |
| dimitern | rogpeppe: I'll put it in a separate dir and just wipe out the gopath/pkg/*, i messed it up last time i tried to setup multiple go versions + gopaths so i can switch between them easily, i'll be more careful this time | 07:45 |
| rogpeppe | dimitern: i have an entirely separate tree for working with separate go versions | 07:45 |
| rogpeppe | dimitern: (which is actually very useful because i can be testing against one branch while developing another) | 07:46 |
| dimitern | rogpeppe: well, I tried having ~/go/<ver> for goroot, and ~/work/go/<ver> for gopath, but then it didn't work with emacs somehow after the raring upgrade | 07:46 |
| dimitern | rogpeppe: (come to think about it, it still doesn't work - i need to run emacs from a terminal rather than from the launcher, otherwise the gopath/root i set in .bashrc are not visible | 07:47 |
| rogpeppe | dimitern: that's not surprising. emacs won't run .bashrc | 07:47 |
| rogpeppe | dimitern: did you try putting the settings in your .profile ? | 07:48 |
| dimitern | rogpeppe: but it worked in quantal! now it cannot find gofmt etc. unless run from the shell | 07:48 |
| dimitern | rogpeppe: i tried .profile, .Xsessionrc, /etc/profile, everything.. no joy | 07:48 |
| rogpeppe | dimitern: try removing it from your .bashrc, adding it to your .profile, logging out and logging in again | 07:49 |
| rogpeppe | dimitern: .bash_profile might work too | 07:50 |
| dimitern | rogpeppe: tried that as well, just emacs somehow defies all known ways it should cooperate with my environment :) | 07:50 |
| rogpeppe | dimitern: i'd be surprised. this is an environment-variable/shell issue, not emacs, i think | 07:50 |
| dimitern | rogpeppe: well, that very well maybe, but haven't seen a problem with another app so far | 07:51 |
| rogpeppe | dimitern: that's probably because no other app needs $GOPATH and $GOROOT | 07:51 |
| rogpeppe | dimitern: or relies on any setting in your .bashrc, i suspect | 07:52 |
| dimitern | rogpeppe: btw - if i just rm -fr $GOPATH/pkg/ and then run should work, or I should leave pkg empty? | 07:52 |
| rogpeppe | dimitern: that should work fine. | 07:52 |
| rogpeppe | dimitern: directories are created on demand | 07:52 |
| dimitern | rogpeppe: hmm apparently goyaml-go uses go1.1 features (function ends without a return statement) | 07:55 |
| dimitern | rogpeppe: switching to goyaml-c | 07:55 |
| rogpeppe | dimitern: ha | 07:55 |
| dimitern | wallyworld_: ping | 07:59 |
| wallyworld_ | hi | 07:59 |
| dimitern | wallyworld_: hey, looking at create-machine | 08:00 |
| wallyworld_ | thanjs | 08:00 |
| dimitern | wallyworld_: why do we need this at all? why not have an argument to deploy instead? don't we want it to be atomic? | 08:00 |
| wallyworld_ | it's part of the containerisation work | 08:00 |
| wallyworld_ | maybe read tim's email | 08:01 |
| dimitern | wallyworld_: i know; i read it | 08:01 |
| dimitern | wallyworld_: but still now convinced it's a good idea to have a separate command | 08:01 |
| wallyworld_ | the next step is to allow containers to be created inside a machine | 08:01 |
| dimitern | wallyworld_: i see that, ok, but still - using 2 steps to deploy inside a container seems wrong | 08:02 |
| wallyworld_ | ultimately the common operation ie deploy will be atomic. this is a step along the way | 08:04 |
| dimitern | rogpeppe: with go 1.0.3 i see no panics so far, but still some tests failures with the watchers (again, not on every run) | 08:04 |
| wallyworld_ | just constructing the jigsaw peices | 08:04 |
| dimitern | wallyworld_: so you're saying create-machine isn't there to stay? | 08:04 |
| wallyworld_ | not sure yet | 08:04 |
| wallyworld_ | all up for prototyping and discussion | 08:05 |
| wallyworld_ | will aid in testing during the development etc | 08:05 |
| wallyworld_ | plus it ma turn out to be useful longer term, not sure yet | 08:05 |
| dimitern | wallyworld_: yeah, that's just the thing - adding stuff we're not sure we need right now will surely bend the direction in the future to finding ways to use them | 08:06 |
| dimitern | wallyworld_: anyway, just thinking out loud - will take my concerns to tim's email :) | 08:07 |
| wallyworld_ | fair questions. i started this work before the email. it's all very much in a state of flux | 08:08 |
| wallyworld_ | but as a developer, the ability of create machines/containers will be very usrful | 08:08 |
| dimitern | wallyworld_: sure when starting on a big new feature there's always uncertainty where to begin | 08:08 |
| wallyworld_ | eg may want to spin up a few containers ahead of time | 08:09 |
| dimitern | wallyworld_: i agree, but i doubt a command is the necessary for that | 08:09 |
| wallyworld_ | and then scale out later | 08:09 |
| wallyworld_ | how else would it be done if there were no command to so it | 08:09 |
| dimitern | wallyworld_: you see "juju's way" of doing things afaiui is telling it what you need and letting it do it for you | 08:10 |
| dimitern | wallyworld_: what mark (sabdfl) said scaling out a service + containers could be automated | 08:10 |
| wallyworld_ | sure. but having some manual control would be nice too | 08:11 |
| dimitern | wallyworld_: having a service deployed on 1 machine + some containers and this is 1 unit, adding a unit could replicate that and the containers | 08:11 |
| dimitern | wallyworld_: could be, not sure yet, i think it's worth some careful consideration | 08:12 |
| wallyworld_ | think of it like being able to add an existing running machine/vm to the env | 08:12 |
| wallyworld_ | it's just another tool in the kit | 08:13 |
| dimitern | wallyworld_: you could do that with manual provisioning (eventually), but this is just a special case - why complicate things adding more special cases like that? | 08:13 |
| rogpeppe | dimitern: https://code.google.com/p/go/issues/detail?id=5554 | 08:14 |
| wallyworld_ | dimitern: for me it's a useful tool. ymmv. the command bit is a small portion of the logic. i like having manual control | 08:16 |
| dimitern | rogpeppe: cool! thanks for filing a bug | 08:17 |
| rogpeppe | dimitern: i'm currently testing against tip. it's possible the problem is already fixed. | 08:18 |
| dimitern | rogpeppe: so with go 1.0.3 more often than not tests pass (1 in 5 failure so far) | 08:18 |
| rogpeppe | dimitern: i'm sure there's a problem you need to fix too :-) | 08:19 |
| rogpeppe | dimitern: i'm not looking into it any further for the moment, because there's lots of stuff i want to get proposed today before i go away next week | 08:19 |
| dimitern | rogpeppe: it'll be helpful if i manage somehow to reproduce it consistently | 08:19 |
| dimitern | rogpeppe: sure | 08:19 |
| dimitern | rogpeppe: but I want to propose a split of apiserver.go into multiple submodules | 08:20 |
| rogpeppe | dimitern: different packages? | 08:20 |
| rogpeppe | dimitern: if you're just talking about splitting the file, +1 | 08:21 |
| dimitern | rogpeppe: the same package state/apiserver/ but having things like root.go, client.go, machine.go, etc. | 08:21 |
| dimitern | rogpeppe: ok, i'll do it after i finish this | 08:21 |
| rogpeppe | dimitern: i'd name all the ones implementing the actual api to start with "api" | 08:21 |
| rogpeppe | dimitern: e.g. apiroot.go, apiclient.go, apimachine.go | 08:22 |
| rogpeppe | dimitern: so it's easy to see which files implement the core logic | 08:22 |
| dimitern | rogpeppe: also i'd like to split api_test.go into perm_test.go, machine_test.go, client_test.go, etc. | 08:22 |
| rogpeppe | dimitern: seems reasonable | 08:23 |
| dimitern | rogpeppe: i'm fine with api*.go | 08:23 |
| dimitern | rogpeppe: cool | 08:23 |
| rogpeppe | dimitern: if you do it, make it a branch which is entirely mechanical - no code changes at all, please. | 08:23 |
| rogpeppe | dimitern: i'm not seeing any panics against tip, BTW | 08:24 |
| dimitern | rogpeppe: sure, but after i'm done with this; don't want to make it harder for me to merge later | 08:24 |
| rogpeppe | dimitern: yeah | 08:24 |
| dimitern | rogpeppe: will it impede the run time too much if we split agent and client perm tests into 2 files and 2 tables? | 08:25 |
| rogpeppe | dimitern: i'm not sure i see the point | 08:26 |
| rogpeppe | dimitern: the point of those tests is to test permissions against the entire API surface area | 08:27 |
| rogpeppe | dimitern: you could split the table, i suppose | 08:28 |
| dimitern | rogpeppe: hmm, ok - but only perm checks should be in there; error conversion tests, bad logins, etc should be separate | 08:28 |
| rogpeppe | dimitern: agreed | 08:28 |
| rogpeppe | dimitern: i have some vaguely formed thoughts about how we can make the perm checks much smaller and faster | 08:28 |
| dimitern | rogpeppe: oh? | 08:30 |
| dimitern | rogpeppe: (while i'm at splitting stuff i might as well split apiclient to multiple files as well) | 08:30 |
| rogpeppe | dimitern: that's a fair bit smaller - i wonder if it might be better just as two files - one for the client api and one for the agent api | 08:32 |
| rogpeppe | dimitern: or actually | 08:32 |
| rogpeppe | dimitern: it's perhaps good to have a 1-1 file mapping between apiserver and api | 08:32 |
| dimitern | rogpeppe: exactly my point | 08:32 |
| rogpeppe | dimitern: for the pieces implementing the api | 08:32 |
| dimitern | rogpeppe: good, will do then | 08:32 |
| rogpeppe | dimitern: i've run the apiserver tests 17 times so far against tip and no panic | 08:35 |
| dimitern | rogpeppe: good! so it seems fixed | 08:36 |
| rogpeppe | dimitern: or the bug just isn't being tickled any more | 08:36 |
| dimitern | rogpeppe: well, it might also mean someone will recognize it and say it's fixed | 08:37 |
| rogpeppe | dimitern: yeah. i'm hoping someone will say "ah, we fixed it then", and then they'll issue a new point release | 08:38 |
| rogpeppe | wallyworld_: ping | 08:38 |
| wallyworld_ | rogpeppe: hi, i'm just about to leave for soccer | 08:39 |
| rogpeppe | wallyworld_: v quick question | 08:39 |
| wallyworld_ | ok | 08:39 |
| rogpeppe | wallyworld_: in environs/cloudinit, you did syslogConfigRenderer, right? | 08:39 |
| wallyworld_ | i think so, let me check | 08:39 |
| rogpeppe | wallyworld_: is there a particular reason why the config is rendered after bootstrap-state is called? | 08:40 |
| rogpeppe | wallyworld_: because i'm thinking about moving it earlier in the script | 08:40 |
| wallyworld_ | rogpeppe: i think it will be ok to move. i think it was put where it is because that's where some previous logging stuff was initialised | 08:41 |
| wallyworld_ | but i'm not entirely sure | 08:41 |
| rogpeppe | wallyworld_: ok, cool | 08:41 |
| wallyworld_ | all it does anyway is write a conf file | 08:41 |
| wallyworld_ | so that can go anywhere really | 08:41 |
| rogpeppe | wallyworld_: that's what i thought; just wanted to check there wasn't anything subtle going on. | 08:42 |
| rogpeppe | wallyworld_: thanks | 08:42 |
| wallyworld_ | np. | 08:42 |
| === ehg_ is now known as ehg | ||
| dimitern | rogpeppe: | 09:12 |
| rogpeppe | dimitern: | 09:12 |
| dimitern | rogpeppe: oops, I have a question :) | 09:12 |
| rogpeppe | dimitern: go on | 09:13 |
| dimitern | rogpeppe: in this log: http://paste.ubuntu.com/5696381/ | 09:13 |
| dimitern | rogpeppe: is it normal to have an Error response to the Stop request? | 09:14 |
| rogpeppe | dimitern: i don't see that you are | 09:15 |
| rogpeppe | dimitern: which line are you thinking is an error response to the stop request? | 09:15 |
| rogpeppe | dimitern: i am seeing that i need to sort out the rpc logging - it's logging each message twice | 09:16 |
| dimitern | rogpeppe: ah, right - [LOG] 94.86663 DEBUG rpc/jsoncodec: -> {"RequestId":4,"Error":"watcher has been stopped","ErrorCode":"stopped","Response":{}} | 09:16 |
| dimitern | this is the Next response | 09:16 |
| rogpeppe | dimitern: (i know why) | 09:16 |
| rogpeppe | dimitern: that's a response to a Next | 09:17 |
| rogpeppe | dimitern: note the RequestId | 09:17 |
| dimitern | rogpeppe: yeah | 09:17 |
| dimitern | rogpeppe: i think we have a problem with the current watcher implementation in the client, but only the environ config one fails because it needs to do extra work on marshaling/unmarshaling the AllAttrs when receiving a response | 09:18 |
| dimitern | rogpeppe: it seems next should quit as soon as stop was called | 09:18 |
| dimitern | rogpeppe: (the next goroutine in the client, that is) | 09:18 |
| rogpeppe | dimitern: on the server side? | 09:18 |
| rogpeppe | dimitern: why's that? | 09:19 |
| dimitern | rogpeppe: because otherwise we have this issue | 09:19 |
| dimitern | rogpeppe: and arguably it's an error to try reading a stopped watcher anyway | 09:19 |
| rogpeppe | dimitern: perhaps you could try describing the issue and why it's happening | 09:19 |
| dimitern | rogpeppe: well, i *think* as soon as stop is called the next goroutine does not exit immediately and tries to read, getting the error, and then exits, but the error is dispatcher to the client outside | 09:20 |
| dimitern | rogpeppe: but really it shouldn't report that error if it knows for sure the server-side watcher was stopped | 09:21 |
| rogpeppe | dimitern: isn't that what the ErrStillAlive check is about? | 09:22 |
| dimitern | rogpeppe: if it worked i shouldn't have received anything on the in channel, right? | 09:23 |
| dimitern | rogpeppe: when next encounters a stopped state | 09:23 |
| rogpeppe | dimitern: ha, yes | 09:23 |
| rogpeppe | dimitern: it should return after the Kill | 09:24 |
| dimitern | rogpeppe: ah, I think I see the error - after w.tomb.Kill(err) i should return rather than trying the select | 09:24 |
| dimitern | :) | 09:24 |
| dimitern | yeah | 09:24 |
| dimitern | rogpeppe: that fixed it | 09:26 |
| rogpeppe | dimitern: cool | 09:26 |
| dimitern | rogpeppe: only one things bothers me - i think it's maybe worth panicking when i cannot unmarshal the environ config i got on the in channel, rather than logging an error | 09:28 |
| rogpeppe | dimitern: i think the watcher should die if that happens | 09:28 |
| dimitern | rogpeppe: it does, but if we got that far something is very wrong, hence a panic is in order i think | 09:29 |
| rogpeppe | dimitern: can't that happen if the server sends some dubious data back? | 09:29 |
| dimitern | rogpeppe: and where did it get it from? state - so it think it's still worth panicking | 09:30 |
| rogpeppe | dimitern: definitely not | 09:30 |
| dimitern | rogpeppe: explain? | 09:30 |
| rogpeppe | dimitern: in general we don't panic when an external data source produces something unexpected | 09:30 |
| dimitern | rogpeppe: hmm | 09:30 |
| rogpeppe | dimitern: it might be talking to a version of the API with a subtly different interface | 09:30 |
| dimitern | rogpeppe: ok then, i'll leave just the error log | 09:31 |
| dimitern | rogpeppe: fair point | 09:31 |
| rogpeppe | dimitern: you don't need to log the error - you can kill the watcher with an appropriate error | 09:31 |
| rogpeppe | dimitern: then it will be picked up as usual by the rest of the logic | 09:31 |
| rogpeppe | dimitern: causing the agent to die, or whatever | 09:31 |
| dimitern | rogpeppe: i'm killing it, and logging the error | 09:31 |
| rogpeppe | dimitern: i'm not sure that the log is necessary, but i guess | 09:32 |
| dimitern | rogpeppe: it won't hurt when debugging incompatible api server/client connections, i think | 09:32 |
| dimitern | rogpeppe: btw you copied my branch when you filed the golang issue, right? | 09:33 |
| rogpeppe | dimitern: yes | 09:36 |
| dimitern | rogpeppe: cool, i'm proposing it soon | 09:36 |
| dimitern | rogpeppe: so with the newly gained understanding of the api internals hopefully, i'll be able to help the other guys next week while you're gone | 09:46 |
| rogpeppe | dimitern: that would be great, thanks | 09:46 |
| rogpeppe | dimitern: another small CL: https://codereview.appspot.com/9710044 | 09:47 |
| * dimitern looking | 09:47 | |
| dimitern | rogpeppe: what's "bootstrap-state" ? | 09:50 |
| rogpeppe | dimitern: jujud bootstrap-state | 09:50 |
| rogpeppe | dimitern: the thing that initialises the mongo state | 09:50 |
| rogpeppe | dimitern: and sets the initial password for the machine agent | 09:50 |
| dimitern | rogpeppe: i see (never heard of it before) | 09:50 |
| dimitern | rogpeppe: LGTM | 09:52 |
| dimitern | rogpeppe: do you think you'd manage adding the agent API connection stuff today? | 09:59 |
| rogpeppe | dimitern: that's the aim | 09:59 |
| rogpeppe | dimitern: i have a branch that does it | 10:00 |
| rogpeppe | dimitern: but there are one or two branches that need to go in first | 10:00 |
| dimitern | rogpeppe: that will be awesome | 10:00 |
| rogpeppe | dimitern: well, let's sat i have a branch that does it *in theory* :-) | 10:00 |
| dimitern | rogpeppe: i was thinking once i started the api stuff i might as well do most (if not all of it), so others can concentrate on replacing state calls with api calls next week, once we have the connection | 10:01 |
| rogpeppe | dimitern: that sounds like a good plan | 10:01 |
| rogpeppe | dimitern: BTW the API connection stuff involved refactoring the machine agent, and it's turned out really quite nice | 10:01 |
| dimitern | rogpeppe: good to hear | 10:02 |
| rogpeppe | dimitern: did you look at the Runner CL? | 10:05 |
| rogpeppe | dimitern: i don't think you reviewed it, which was a bit surprising :-) | 10:06 |
| dimitern | rogpeppe: this one? https://codereview.appspot.com/9643043/ | 10:17 |
| rogpeppe | dimitern: yeah | 10:18 |
| dimitern | rogpeppe: I looked at it briefly, but by the time I got the time to review it it already landed :) | 10:18 |
| dimitern | rogpeppe: https://codereview.appspot.com/9721043/ is ready | 10:18 |
| dimitern | rogpeppe: will look at yours once more more carefully | 10:19 |
| rogpeppe | dimitern: np. more eyes always appreciated - it's a moderately complex piece of code. | 10:19 |
| rogpeppe | dimitern: i'm slightly surprised you've done all those api calls in one CL. i thought they might work well as many separate ones. | 10:20 |
| rogpeppe | dimitern: it's not a particular problem, but i might not get it reviewed today | 10:20 |
| dimitern | rogpeppe: they're closely related - all needed by the provisioner and really most are trivial | 10:21 |
| dimitern | rogpeppe: np, go as far as you can | 10:21 |
| rogpeppe | dimitern: i guess i just prefer separable changes to be separate :-) | 10:22 |
| dimitern | rogpeppe: i guess watchers and machine calls can be separated | 10:22 |
| rogpeppe | dimitern: that might be good, if you don't mind too much. | 10:22 |
| dimitern | rogpeppe: uhh.. you're right actually - just now glanced at the diff - it's in excess of 1000 lines | 10:23 |
| rogpeppe | dimitern: yeah, it's pretty big | 10:23 |
| dimitern | rogpeppe: didn't look like that when i was doing it :) anyway - will try bzr pipes to see if i can split it easily | 10:24 |
| rogpeppe | dimitern: you might not need pipes too much | 10:24 |
| rogpeppe | dimitern: most of the changes are orthogonal | 10:24 |
| rogpeppe | dimitern: if you do the commonWatcher thing first (making the EntityWatcher use it only) then all the rest are likely to be independent | 10:25 |
| dimitern | rogpeppe: good point, will do | 10:26 |
| dimitern | rogpeppe: ok, disregard that CL then, I'll propose it after splitting | 10:27 |
| rogpeppe | dimitern: thanks | 10:28 |
| * dimitern hates bzr diff!!!! | 10:34 | |
| mgz | ? | 10:35 |
| mgz | you know it takes arguments, right? | 10:35 |
| dimitern | mgz: so i've been in this situation before | 10:35 |
| dimitern | mgz: i have trunk and a branch off it | 10:35 |
| dimitern | mgz: i need to see changes in a particular path between the two branches | 10:36 |
| dimitern | mgz: but none of the suggested args help | 10:36 |
| dimitern | mgz: tried --old trunk while on the new branch; tried -r-2, but i need a path as well, not the changes in the whole tree | 10:36 |
| dimitern | mgz: tried --old trunk --new feature-branch - still frustratingly no results! | 10:37 |
| dimitern | mgz: please help :) | 10:38 |
| dimitern | mgz: (if it makes a difference - both branches are clean - no uncommitted changes) | 10:39 |
| mgz | hm, I'd expect --old to work | 10:39 |
| mgz | have you got qbzr intalled? | 10:40 |
| dimitern | mgz: well it doesn't; yes I have qbzr | 10:40 |
| dimitern | mgz: with qdiff is still the same (No changes) | 10:41 |
| dimitern | mgz: all i need here is diff trunk:tip and feature:tip and show me the changes in state/api/ only | 10:41 |
| mgz | so, it works for me with standalone branches | 10:42 |
| dimitern | mgz: so i can cherry pick some of them and split the feature into multiple smaller ones | 10:42 |
| mgz | but something seems to be unhappy with native colo | 10:42 |
| mgz | but, I'd expect if you cd ~ then do `bzr diff --old .../src/real/branch/location/trunk --new .../src/.../feature state/api` it'll work | 10:43 |
| dimitern | mgz: here's what bzr reports: http://paste.ubuntu.com/5696558/ | 10:43 |
| dimitern | mgz: ah, i'll try that | 10:43 |
| mgz | right, so try --old ~/src/juju-core/trunk --new ~//src/juju-core/041-provisioner-api-calls | 10:44 |
| mgz | there might be an issue with checkouts, I'll see if we have a bug filed | 10:45 |
| dimitern | mgz: hallelujah! it works like that | 10:45 |
| dimitern | mgz: although i'd expect it to work in a LW checkout as well.. | 10:46 |
| mgz | looks like bug 596785 plus bad location resolution code in the diff command | 10:47 |
| _mup_ | Bug #596785: 'bzr diff --old|new PATH' does silently exists if arg does not exist <diff> <patch-needswork> <Bazaar:Confirmed> <https://launchpad.net/bugs/596785> | 10:47 |
| mgz | can you file a new bug with your symptoms please dimitern? | 10:48 |
| dimitern | mgz: a new one or amend this | 10:48 |
| mgz | a new one | 10:48 |
| dimitern | mgz: sure | 10:48 |
| mgz | the location code being wrong I don't see any entry for, can do both issues in one merge possibly, but might make sense to do them seperately | 10:49 |
| dimitern | mgz: bzr diff --old/--new does not report and changes with a lightweight checkout of a native repo? | 10:51 |
| dimitern | mgz: as summary | 10:51 |
| mgz | s/and/any/ and seems reasonable | 10:51 |
| mgz | and just end after "checkout" | 10:52 |
| dimitern | mgz: not sure i get you - where's the "and" ? | 10:52 |
| dimitern | mgz: ah | 10:52 |
| mgz | :) | 10:53 |
| dimitern | mgz: bug 1183776 | 11:01 |
| _mup_ | Bug #1183776: bzr diff --old/--new does not report and changes with a lightweight checkout of a native repo <amd64> <apport-bug> <bzr> <diff> <raring> <bzr (Ubuntu):New> <https://launchpad.net/bugs/1183776> | 11:01 |
| mgz | thanks! | 11:01 |
| dimitern | mgz: any idea how to make my life easier and cherry pick interactively what to merge? rather than having to manually edit the diff and run patch with it | 11:04 |
| danilos | dimitern, mgz: I need to step out to sign some documentation in the court, likely to not make it for the stand-up | 11:09 |
| dimitern | danilos: ok | 11:09 |
| rogpeppe | dimitern, TheMue: another small CL: https://codereview.appspot.com/9723043 | 11:19 |
| mgz | dimitern: use shelve | 11:20 |
| dimitern | rogpeppe: LGTM | 11:21 |
| rogpeppe | dimitern: ta! | 11:21 |
| dimitern | mgz: i'll try | 11:21 |
| dimitern | mgz: how will shelve help with committed trees? | 11:22 |
| mgz | cherrypick the feature branch across to your new integration branch | 11:22 |
| mgz | then set EDITOR and use shelve interactive to edit out the bits you don't want | 11:23 |
| dimitern | mgz: it seems way too complicated.. i'll just branch the old feature and vistually merge into the new one with directory comparison | 11:24 |
| mgz | it's two commands... | 11:24 |
| mgz | but depending on how hard the changes are to unpick, interactive mode may be useful or not | 11:25 |
| mgz | oh, and I'm not actually sure what `bzr merge -i` does | 11:26 |
| mgz | what you want, it seems | 11:26 |
| mgz | so, you have feature, which you want to split out | 11:27 |
| mgz | dimitern: so, in your checkout, on trunk, `bzr switch -b prefeature` then `bzr merge -i .../feature` | 11:27 |
| dimitern | mgz: i'll try next time, thanks; spent too much time already fiddling around with tools, so i'm back to good old manual methods + meld for visual merging | 11:29 |
| TheMue | rogpeppe: done | 11:29 |
| dimitern | mgz, wallyworld_: are we doing the standup? | 11:34 |
| mgz | lets | 11:34 |
| mgz | can just say hi at least :) | 11:34 |
| dimitern | rogpeppe, mgz: https://codereview.appspot.com/9667048 first part | 11:48 |
| rogpeppe | dimitern: that's more like it :-) :-) | 11:48 |
| dimitern | TheMue: ^^ reviews appreciated | 11:52 |
| dimitern | there are 3 more CLs coming in this pipeline | 11:52 |
| TheMue | dimitern: done | 11:58 |
| dimitern | TheMue: cheers | 11:58 |
| dimitern | TheMue: actually making them arguments to init() will make the code less readable | 11:59 |
| dimitern | TheMue: and I don't see an advantage in doing that | 12:00 |
| dimitern | TheMue: could you explain your reasoning? | 12:00 |
| TheMue | dimitern: having them as arguments would let you check if they are set. that's the only reason. | 12:04 |
| dimitern | TheMue: it won't compile anyway if they're not set | 12:04 |
| dimitern | TheMue: i'll add a check in init() to ensure they are set, but not as arguments | 12:07 |
| TheMue | dimitern: it won't compile? | 12:07 |
| dimitern | TheMue: no, you're right - it will compile, but panic at runtime | 12:09 |
| dimitern | TheMue: i'll add a check, thanks | 12:09 |
| TheMue | dimitern: you set them after init(), so a check there would fail. | 12:09 |
| dimitern | TheMue: i moved init after setting the, | 12:12 |
| TheMue | dimitern: so please also comment that those values have to be set before calling init(). | 12:13 |
| dimitern | TheMue: sure | 12:13 |
| TheMue | dimitern: great, thx | 12:14 |
| dimitern | thanks guys, submitting with the changes | 12:19 |
| dimitern | rogpeppe, mgz, TheMue: next in line: https://codereview.appspot.com/9714044 | 12:51 |
| dimitern | danilos: you as well? ^^ | 13:12 |
| rogpeppe | dimitern: you have a review | 13:35 |
| dimitern | rogpeppe: tyvm | 13:35 |
| rogpeppe | dimitern: much better split into bits like this! | 13:35 |
| dimitern | TheMue: calling init() in commonLoop() is not quite the same, because it's needs to be called before the go routine spins up | 13:36 |
| TheMue | dimitern: due to the created channel? | 13:38 |
| dimitern | TheMue: yeah, that as well | 13:39 |
| TheMue | dimitern: ok, could lead to a race otherwise. what else? | 13:41 |
| dimitern | TheMue: actually I think this is it - couldn't find other reason | 13:42 |
| TheMue | dimitern: ok, fine | 13:46 |
| === wedgwood_away is now known as wedgwood | ||
| === BradCrittenden is now known as bac | ||
| dimitern | thanks for the reviews, submitting soon; 2 more to go | 14:00 |
| dimitern | rogpeppe: kanban | 14:05 |
| dimitern | rogpeppe, TheMue: last one about the provisioner: https://codereview.appspot.com/9708044/ | 14:51 |
| TheMue | *click* | 14:54 |
| TheMue | dimitern: done | 15:05 |
| dimitern | TheMue: thanks! | 15:05 |
| rogpeppe | dimitern: reviewed | 15:18 |
| dimitern | rogpeppe: cheers | 15:18 |
| dimitern | rogpeppe: we you're saying the agent shouldn't use SetMongoPassword at all, and use SetPassword instead? | 15:19 |
| rogpeppe | dimitern: yes | 15:20 |
| dimitern | rogpeppe: ok, even better | 15:20 |
| rogpeppe | dimitern: the API server knows when SetMongoPassword is appropriate, and does it | 15:20 |
| dimitern | rogpeppe: it's getting smarter then :) | 15:21 |
| rogpeppe | dimitern: that's kinda the idea of the API :-) | 15:21 |
| dimitern | rogpeppe: indeed | 15:22 |
| dimitern | rogpeppe: does the same apply for unit.SetMongoPassword? | 15:37 |
| rogpeppe | dimitern: yes. hmm, that's an interesting point actually | 15:37 |
| rogpeppe | dimitern: currently SetPassword should be doing SetMongoPassword for all agents | 15:37 |
| dimitern | rogpeppe: so for unit.SetMP i'll use the same logic as for a machine | 15:38 |
| rogpeppe | dimitern: yeah - i.e. it should always be set. | 15:39 |
| rogpeppe | dimitern: presumably you've merge trunk? | 15:39 |
| mramm | anybody know how sync-tools works | 15:39 |
| dimitern | rogpeppe: ok | 15:39 |
| rogpeppe | merged? | 15:39 |
| dimitern | yeah | 15:39 |
| rogpeppe | mramm: i've never used it | 15:39 |
| dimitern | rogpeppe: it's used to sync ec2 public tools to your private bucket | 15:39 |
| mramm | and have a minute to walk danwest through getting tools into MAAS? | 15:39 |
| rogpeppe | dimitern: yeah i know roughly what it does | 15:40 |
| rogpeppe | dimitern: but i've never actually *used* it | 15:41 |
| dimitern | rogpeppe: ops, sorry - that was for mramm actually | 15:41 |
| dimitern | rogpeppe: got too used to addressing you lately :) | 15:41 |
| rogpeppe | dimitern: np - i do that too sometimes | 15:41 |
| mramm | can you use sync-tools as an argument to bootstrap | 15:43 |
| mramm | and IIRC you need to put your EC2 credentials somewhere to make it work | 15:44 |
| mramm | is this documented anywhere that we can just point danwest too? | 15:44 |
| dimitern | mramm: no | 15:44 |
| mramm | :( | 15:44 |
| dimitern | ramit's a separate command | 15:44 |
| mramm | ok | 15:44 |
| dimitern | mramm: there was a mail from jam about how it works | 15:45 |
| mramm | so bootstrap and then sync tools? | 15:45 |
| mramm | ok, will search for that | 15:45 |
| dimitern | rogpeppe: do you think unit tests for agents/workers in general should also only use the api? | 15:46 |
| rogpeppe | dimitern: that's an interesting question. | 15:46 |
| rogpeppe | dimitern: i'm not entirely sure it's possible | 15:46 |
| rogpeppe | dimitern: you'd need to juggle at least two API connections and send the right commands to the right one | 15:47 |
| dimitern | rogpeppe: yeah | 15:47 |
| dimitern | rogpeppe: and if we leave them using state directly how bad will it be? | 15:47 |
| rogpeppe | dimitern: i'm not sure that's bad at all | 15:47 |
| rogpeppe | dimitern: the actual code being tested will need to use the API | 15:47 |
| rogpeppe | dimitern: it's just the setup code which will talk directly to mongo (and probably some verification code too) | 15:48 |
| dimitern | rogpeppe: yeah, exactly, the tests can still access it directly | 15:48 |
| rogpeppe | dimitern: yup | 15:48 |
| dimitern | rogpeppe: and btw this will save a *huge* amount of work | 15:48 |
| rogpeppe | dimitern: yeah | 15:48 |
| rogpeppe | i hadn't seen this error before: cannot make S3 control bucket: A conflicting conditional operation is currently in progress against this resource. Please try again. | 15:54 |
| dimitern | rogpeppe: it seems like an ec2 error | 15:54 |
| rogpeppe | dimitern: yeah it is. just another one i hadn't seen before | 15:55 |
| rogpeppe | dimitern: to join the collection of sporadic live test failures. | 15:55 |
| rogpeppe | dimitern: getting live tests to pass reliably is gonna be really hard | 15:55 |
| dimitern | rogpeppe: yup, unfortunately | 15:55 |
| danwest | dimitern: mramm pointed me your way for juju-core/MAAS bootstrap issues | 15:56 |
| dimitern | danwest: hm, ok - what do you need? | 15:58 |
| danwest | dimitern: new raring install with apt-get juju-core | 15:59 |
| danwest | trying to bootstrap against MAAS provider and get the following: | 16:00 |
| danwest | ~$ juju -v bootstrap | 16:00 |
| danwest | 2013/05/24 11:27:45 INFO environs: reading tools with major version 1 | 16:00 |
| danwest | 2013/05/24 11:27:45 INFO environs: falling back to public bucket | 16:00 |
| danwest | 2013/05/24 11:27:45 ERROR command failed: no tools available | 16:00 |
| danwest | error: no tools available | 16:00 |
| dimitern | danwest: can you try juju-core from source and see if it helps? | 16:01 |
| mramm | I expect that nobody has tested the sync tools path with MAAS | 16:02 |
| mramm | it landed at the very last second for the raring cycle | 16:02 |
| mramm | I know I have not tested it myself :/ | 16:02 |
| dimitern | mramm: me too, and maas landed at the last moment as well, so it might not work from the released version | 16:02 |
| danwest | dimitern: mramm: I can try from source but that might take me a couple to get setup | 16:03 |
| mramm | dimitern: dan is working on the "cloud installer" and any help we can give him would be good | 16:03 |
| danwest | thanks mramm | 16:03 |
| mramm | dimitern: we want to keep him unblocked ;) | 16:03 |
| dimitern | mramm: sure, i'll help if i can, but have never tried the maas provider | 16:04 |
| mramm | yea, but william is out | 16:04 |
| dimitern | danwest: we fixed a bunch of issues with tools and bootstrapping after the raring release, hence my suggestion to run from source | 16:05 |
| danwest | dimitern: k | 16:05 |
| mramm | danwest: once you have a working source install | 16:05 |
| mramm | the upload tools thing you were pointed at a while ago will probably work | 16:05 |
| danwest | dimitern: mramm: grabbing source now | 16:07 |
| mramm | danwest: sorry this is not a better path yet | 16:07 |
| mramm | I have to run out and get some food, back in a few | 16:07 |
| danwest | same here - just grabbing source first | 16:08 |
| mramm | william will also be back on monday, and I know he has tested this | 16:08 |
| danwest | k | 16:09 |
| Makyo | I'm seeing http://pastebin.ubuntu.com/5697229/ in trunk, running in a precise lxc. Is this a known thing? | 16:37 |
| dimitern | Makyo: don't tell me you never updated from lucid? :) | 16:37 |
| Makyo | dimitern, Haha, nah, running raring, but tests were failing there, thus the precise lxc dev setup | 16:38 |
| dimitern | Makyo: well, lucid is definitely not among the expected series iirc | 16:38 |
| dimitern | Makyo: can you reproduce it consistently? | 16:39 |
| Makyo | dimitern, Every time. | 16:39 |
| Makyo | Let me run with -gocheck.v | 16:39 |
| Makyo | No more info. | 16:40 |
| dimitern | Makyo: running with -gocheck.v in environs/imagemetadata/ | 16:41 |
| Makyo | dimitern, Yeah, no extra info on the failure, others pass. | 16:43 |
| Makyo | dimitern, lucid is specified in the test, environs/imagemetadata/simplestreams_test.go:463 | 16:43 |
| dimitern | Makyo: file a bug; maybe they decided to drop lucid from the cloud archive just today | 16:44 |
| Makyo | dimitern, got it. Thanks. | 16:44 |
| dimitern | Makyo: thanks | 16:44 |
| dimitern | rogpeppe: ping | 16:45 |
| rogpeppe | dimitern: pong | 16:45 |
| dimitern | rogpeppe: actually why do you insist on having api prefix to filenames when split? | 16:45 |
| rogpeppe | dimitern: because i like having error.go and server.go stand out as files related to core api server functionality, not the actual API methods | 16:46 |
| dimitern | rogpeppe: hmm | 16:47 |
| dimitern | rogpeppe: well, ok | 16:47 |
| dimitern | rogpeppe: so apiserver.go will be gone, to be replaced by apiroot.go, apistate, apiclient, apimachine, etc, right? | 16:48 |
| rogpeppe | dimitern: there is actually a possibility that the api* files could be moved into their own package actually (in the api server anyway) | 16:48 |
| rogpeppe | dimitern: yeah | 16:48 |
| dimitern | rogpeppe: that sounds better - so apiserver/api/* | 16:48 |
| rogpeppe | dimitern: yeah, but it's only a possibility - i'd prefer not to do it for now | 16:49 |
| dimitern | rogpeppe: what are your concerns about it? | 16:50 |
| rogpeppe | dimitern: it feels unnecessary | 16:50 |
| dimitern | rogpeppe: why not rename server to apiserver, errors to apierrors and leave the rest uprefixed? | 16:51 |
| dimitern | rogpeppe: they's stand out even more | 16:51 |
| rogpeppe | dimitern: that's not a bad idea | 16:51 |
| dimitern | rogpeppe: cool, i'll make it so then | 16:52 |
| rogpeppe | dimitern: sgtm | 16:52 |
| dimitern | rogpeppe: and the same for the client? | 16:52 |
| rogpeppe | dimitern: yeah | 16:52 |
| dimitern | rogpeppe: nice | 16:52 |
| danwest | dimitern: any tricks to building juju-core from source? | 17:50 |
| dimitern | danwest: so if you do go get -v -u launchpad.net/juju-core/... (note the ...) | 17:51 |
| dimitern | danwest: once done, you'll just need to do go install launchpad.net/juju-core/cmd/juju | 17:51 |
| danwest | k | 17:52 |
| dimitern | danwest: and then use juju <command> as usual | 17:52 |
| dimitern | rogpeppe: still there? | 18:16 |
| rogpeppe | dimitern: aye | 18:17 |
| dimitern | rogpeppe: i'm done with the splitting, proposing in 5m, and i'd like if you take a look | 18:18 |
| rogpeppe | dimitern: ok. i'm gonna be gone quite soon though, and i'm really trying to get this branch proposed | 18:18 |
| dimitern | rogpeppe: if you can; it's mechanical, but mostly if it seems ok as an arrangement https://codereview.appspot.com/9746043 | 18:21 |
| rogpeppe | dimitern: well remembered about the copyright notices :-) | 18:23 |
| dimitern | rogpeppe: oh yeah :) | 18:23 |
| rogpeppe | dimitern: i get "upload in progress" at https://codereview.appspot.com/9746043/diff/1/state/apiserver/apiserver.go?column_width=90 | 18:24 |
| rogpeppe | dimitern: i think that's because codereview is confused though. | 18:24 |
| dimitern | rogpeppe: hmm.. well it's just what server.go used to be | 18:25 |
| dimitern | rogpeppe: probably the rename + modify somehow confuses rietveld | 18:25 |
| rogpeppe | dimitern: utils.go rather than helpers.go ? | 18:25 |
| dimitern | rogpeppe: np | 18:26 |
| rogpeppe | dimitern: i'm wondering if each test file should have its own suite | 18:26 |
| dimitern | rogpeppe: we can do that, but let it be a follow up | 18:27 |
| rogpeppe | dimitern: but i don't mind that much | 18:27 |
| rogpeppe | dimitern: yeah, definitely | 18:27 |
| dimitern | rogpeppe: i think it should be considered a trivial CL | 18:27 |
| dimitern | rogpeppe: and would really want to land it today, to save me some trouble tracking moving trunk | 18:28 |
| rogpeppe | dimitern: where have the auth* functions ended up? | 18:29 |
| dimitern | rogpeppe: in root.go | 18:29 |
| rogpeppe | dimitern: ah, that seems goo | 18:30 |
| rogpeppe | d | 18:30 |
| rogpeppe | dimitern: it all seems great actually | 18:30 |
| dimitern | rogpeppe: cool enough to land? | 18:30 |
| rogpeppe | dimitern: LGTM trivial | 18:31 |
| dimitern | rogpeppe: great, tyvm! | 18:31 |
| rogpeppe | dimitern: i think that's the largest "trivial" change ever :-) | 18:31 |
| dimitern | rogpeppe: yeah :) | 18:31 |
| === wedgwood is now known as wedgwood_away | ||
| === wedgwood_away is now known as wedgwood | ||
| rogpeppe | dimitern: in case you're still around, you might want to take a look at this: https://codereview.appspot.com/9738043 | 19:14 |
| dimitern | rogpeppe: will do | 19:16 |
| dimitern | rogpeppe: LGTM | 19:30 |
| rogpeppe | dimitern: thanks. i won't submit until the next one's ready though | 19:30 |
| dimitern | rogpeppe: ok | 19:30 |
| dimitern | rogpeppe: btw "biggest diff ever": Diff against target:7640 lines (+3825/-3612) 26 files modified | 19:30 |
| rogpeppe | dimitern: what's that in? | 19:32 |
| dimitern | rogpeppe: the one I just landed | 19:32 |
| rogpeppe | dimitern: :-) | 19:32 |
| rogpeppe | dimitern: not as big as the error one which may happen sometime, probably | 19:33 |
| dimitern | rogpeppe: in lp - doesn't show the full diff, just the first 5000 lines, but the summary at the top shows the info above | 19:33 |
| dimitern | rogpeppe: oh, yeah - we should do that | 19:33 |
| rogpeppe | dimitern: i didn't get it working yet - i was spending too much time on it, so put it on hold | 19:34 |
| dimitern | rogpeppe: yeah, it'll come to that | 19:35 |
| * dimitern is still not quite sure i didn't screw up trunk with that rename | 19:35 | |
| dimitern | I pulled and run all tests pass | 19:36 |
| dimitern | now i'm doing the same in a fresh clone of trunk, just to be shure | 19:36 |
| dimitern | *sure | 19:36 |
| * dimitern is relieved.. all tests pass on trunk.. *whew* | 19:44 | |
| dimitern | time to go | 19:46 |
| dimitern | rogpeppe: good night and have a nice holiday! | 19:46 |
| === wedgwood is now known as wedgwood_away | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!