| rogpeppe | mornin! | 08:01 |
|---|---|---|
| dimitern | rogpeppe: ping | 08:34 |
| rogpeppe | dimitern: pong | 08:34 |
| dimitern | rogpeppe: hey, I wanted to ask you about your suggestion to implement a response which is also an error | 08:35 |
| rogpeppe | dimitern: ok | 08:35 |
| dimitern | rogpeppe: what do I need there? func (resp..) Error() string ? | 08:35 |
| rogpeppe | dimitern: exactly that | 08:35 |
| dimitern | rogpeppe: and how to verify the type? x.(respError) ? | 08:36 |
| rogpeppe | dimitern: i suggest you use *response throughout, as it's conventional to use pointer types for errors | 08:36 |
| dimitern | rogpeppe: ok | 08:36 |
| rogpeppe | dimitern: the type is already verified - it's the receiver | 08:36 |
| rogpeppe | dimitern: ah, you mean for actually sending the error? | 08:37 |
| dimitern | rogpeppe: a short example of your idea will be appreciated | 08:37 |
| rogpeppe | dimitern: ok, one mo | 08:37 |
| rogpeppe | dimitern: i'm thinking of something like this: http://paste.ubuntu.com/1449605/ | 09:08 |
| dimitern | rogpeppe: 10x! | 09:08 |
| rogpeppe | dimitern: there are quite a few places that might be different based on requirements, but i *think* the basic idea is sound | 09:10 |
| dimitern | rogpeppe: I'll try it out, looks good | 09:11 |
| dimitern | rogpeppe: couldn't I instead of having all the fields of the response object, just embed it, then I'll have send(w, r) already and just need Error() string | 09:13 |
| rogpeppe | dimitern: do you actually need the response type any more? | 09:14 |
| dimitern | rogpeppe: also, is there a particular reason to create *errorResponse vars instead of non-pointers? | 09:14 |
| dimitern | rogpeppe: ah, you mean replace it, ok | 09:14 |
| rogpeppe | dimitern: error types are conventionally pointers | 09:14 |
| dimitern | rogpeppe: but is it enough to have Error() on it to be treated like *error ? | 09:15 |
| rogpeppe | dimitern: it is, yes | 09:15 |
| rogpeppe | dimitern: but concrete error types are usually pointers | 09:15 |
| dimitern | rogpeppe: ok, cool, I though it was more complex :) | 09:15 |
| rogpeppe | dimitern: BTW s/*error/error/ above | 09:16 |
| rogpeppe | dimitern: the error interface is just interface{Error() string} | 09:16 |
| dimitern | rogpeppe: but then not all responses are errors, wouldn't it be misleading to call it errorResponse? | 09:16 |
| rogpeppe | dimitern: AFAICS you're only using the response type for errors currently | 09:17 |
| dimitern | rogpeppe: well, not exactly - noContentResponse is one example | 09:18 |
| dimitern | acceptedResponse is another | 09:18 |
| rogpeppe | dimitern: both of those seem like things that could be done inline or with a simple helper function | 09:19 |
| rogpeppe | dimitern: (neither include any significant nova-specific text) | 09:19 |
| dimitern | rogpeppe: well, they're exactly as nova sends them, but I'll think of a helper | 09:20 |
| rogpeppe | dimitern: both of them can be done by simply setting the http status code | 09:21 |
| rogpeppe | dimitern: as in, w.WriteHeader(http.StatusNoContent) | 09:21 |
| rogpeppe | dimitern: i don't think you need anything more | 09:21 |
| dimitern | rogpeppe: I need the body for accepted | 09:22 |
| dimitern | rogpeppe: but I'll create a simpleResponse or something for that | 09:22 |
| rogpeppe | dimitern: accepted has an empty body - don't you get that by default? | 09:23 |
| dimitern | rogpeppe: yes, you're right, actually for swift there was a body | 09:23 |
| rogpeppe | dimitern: anyway, if you do need a custom body, a helper like your sendJSON function would work fine | 09:24 |
| dimitern | rogpeppe: ok | 09:25 |
| jam | rogpeppe: I found the reason mongod is failing to start: error command line: unknown option sslOnNormalPorts | 10:12 |
| rogpeppe | jam: ah! | 10:13 |
| rogpeppe | jam: you need to use the mongodb that we're using in the cloud | 10:13 |
| rogpeppe | jam: doh, i should have thought of that | 10:13 |
| jam | dimitern: ^^ so the issue is that mongod that is available by default from Ubuntu doesn't have the new flags that juju-core wants. | 10:13 |
| jam | rogpeppe: where can you get that one? A ppa? | 10:14 |
| jam | build from scratch? | 10:14 |
| jam | rogpeppe: oddly enough, .Start() sees no failure, but we also don't set cmd.Stderr or cmd.Stdout to anything, so we never see that it actually says there is a problem. | 10:14 |
| rogpeppe | jam: fetch it from s3. you can find the URL in environs/cloudinit. | 10:14 |
| dimitern | jam: so that was the problem | 10:14 |
| jam | d | 10:14 |
| jam | dimitern: that was the first problem at least | 10:14 |
| rogpeppe | jam: start won't see a failure, because the binary started just fine | 10:15 |
| jam | rogpeppe: and then put it where? | 10:15 |
| rogpeppe | jam: in your path | 10:15 |
| rogpeppe | jam: http://juju-dist.s3.amazonaws.com/tools/mongo-2.2.0-$series-$arch.tgz | 10:16 |
| rogpeppe | jam: i think we've only built it for precise and quantal so far | 10:17 |
| rogpeppe | jam: and amd64 | 10:17 |
| jam | rogpeppe: I'm on quantal which is fine, but 32-bit, and http://juju-dist.s3.amazonaws.com/tools/http://juju-dist.s3.amazonaws.com/tools/mongo-2.2.0-quantal-amd64.tgz still gives me 404 | 10:17 |
| rogpeppe | jam: presumably you didn't double up the URL when you actually tried it? | 10:17 |
| jam | I did, I guess. | 10:18 |
| jam | i386 doesn't exist | 10:18 |
| jam | amd64 does | 10:18 |
| rogpeppe | jam: there's a builddb charm if you want to build your own | 10:18 |
| rogpeppe | jam: unfortunately there's a chicken/egg problem, i think | 10:19 |
| rogpeppe | jam: 'cos i'm not sure we can have units with a different architecture to the bootstrap node yet | 10:19 |
| rogpeppe | jam: you might be able to use the python juju to run the builddb charm on 386 | 10:21 |
| jam | rogpeppe: so to go back to a bit more basic, what is a good way to make sure the test suite doesn't just hang with no information in these circumstances? | 10:22 |
| jam | It seems to wait indefinitely to try to connect to a mongod that isn't running | 10:22 |
| rogpeppe | jam: it's a good question. the problem is that mongodb prints out shitloads of info, far too much to log by default | 10:23 |
| jam | (I may have to switch to 64-bit, but dimitern still has that problem without indication of what is wrong) | 10:23 |
| rogpeppe | jam: i'm not sure if it prints to stdout or stderr though | 10:23 |
| rogpeppe | jam: one possibility might be to wait for a little while after starting mongodb, to make sure it doesn't exit quickly with a usage or similar startup error | 10:24 |
| jam | rogpeppe: well my thought is that whatever bit is polling trying to connect to the port could timeout if it hasn't connected, then check if the child process is running, then try to connect again | 10:25 |
| jam | rather than to just hang trying to connect. | 10:25 |
| jam | not sure how the layering is. | 10:25 |
| jam | Unfortunately, the "bad command line" error message goes to Stdout, just like all the other spew when things are working correctly. | 10:25 |
| rogpeppe | jam: the code that is trying to connect is just the regular state open code - it has no idea that it's connecting to a local mongodb | 10:25 |
| jam | we could still put that into a bytes.Buffer in case we want to log it because the process died early. | 10:25 |
| jam | rogpeppe: but why would state.Open wait indefinitely either? | 10:26 |
| rogpeppe | jam: because we might be connecting to a mongodb on an instance that's starting up, which may take 10 minutes | 10:26 |
| rogpeppe | jam: i think that probably the code that starts mongodb should log something when mongodb dies. | 10:27 |
| jam | rogpeppe: so the code starting mongo is MgoTestPackage, which has a defer destroyMgoServer | 10:31 |
| rogpeppe | jam: yup | 10:31 |
| jam | would it be sensible to start a goroutine to monitor it for stopping early (not by destroy) and log something? | 10:31 |
| rogpeppe | jam: i think so. although there may be subtle issues around that. i'll have a look., | 10:32 |
| jam | rogpeppe: there doesn't seem to be a exec.Cmd.Poll() | 10:33 |
| jam | just Wait() | 10:33 |
| rogpeppe | jam: Wait should be sufficient | 10:33 |
| jam | rogpeppe: so how does it get layered if you want to Wait() to see if it exits early, but have a kill switch in destroy...() so that if destroy is stopping it, that the Wait goroutine doesn't print debug info. | 10:35 |
| jam | I'd also arguably like to grab log info, but preferrably not buffer all of it for the whole lifetime of the process. | 10:36 |
| rogpeppe | jam: it's not too hard - i'll knock something up | 10:36 |
| rogpeppe | jam: i think it's easier just to buffer the whole thing | 10:36 |
| rogpeppe | jam: otherwise you'll need to implement some kind of bounded buffer, which is probably unnecessary. | 10:37 |
| jam | well, its easier, but the total output from mongodb during the course of a full test suite (it is started once at the beginning, and not shut down until the test suite finishes) could be a bit much | 10:37 |
| rogpeppe | jam: i guess, but what's a few MB these days? :-) | 10:37 |
| jam | so dimitern, short answer for you is to download http://juju-dist.s3.amazonaws.com/tools/mongo-2.2.0-quantal-amd64.tgz and put the binary in your path, we'll try to sort out the rest. | 10:38 |
| jam | rogpeppe: as long as it is small MB, not a big deal. | 10:38 |
| dimitern | jam: yeah, pity I'm still on precise | 10:38 |
| jam | dimitern: there should be a precise-amd64 as well | 10:38 |
| rogpeppe | dimitern: precise works too | 10:38 |
| jam | there just isn't a i386 version. | 10:38 |
| dimitern | ah, ok | 10:39 |
| jam | rogpeppe: a thought. You could wait until you get 1 line of output from mongodb since you end up waiting for it to start regardless. | 10:44 |
| jam | if it then dies, you can log the output | 10:44 |
| jam | at least, if there is a commandline problem, you'll see it right away. | 10:45 |
| jam | though having a bigger "dump the mongodb output if the process dies" would be useful for lots of other sorts of failures | 10:45 |
| jam | so it is probably good to have something more generic anyway. | 10:45 |
| rogpeppe | jam: you'll see the line of output before you know if it's died with an error | 10:45 |
| rogpeppe | jam: so you'll still have to wait a while to see if you want to print the line | 10:46 |
| jam | rogpeppe: well the one line is "error command line: ..." | 10:46 |
| rogpeppe | jam: so we catch that one particular class of error, but there may be other kinds of error that don't fit that pattern | 10:47 |
| jam | yeah | 10:47 |
| jam | so if you had a way to short-term Wait() it would work | 10:47 |
| jam | but you don't, you have to put out a goroutine to wait and put it on a channel or whatever. | 10:47 |
| rogpeppe | jam: we can easily short-term wait if we want to | 10:47 |
| jam | rogpeppe: I don't see a way to interrupt Wait() | 10:47 |
| jam | so how do you make it 'short-term' ? | 10:47 |
| rogpeppe | jam: as you say, start a goroutine that does wait and sends the result on a channel | 10:48 |
| jam | rogpeppe: which means it is around forever? | 10:48 |
| rogpeppe | jam: sure, why not? | 10:48 |
| jam | rogpeppe: I don't see a "builddb" charm in the charmworld "precise" list that I downloaded, I do see mongodb, but no mention of builddb. Is there a doc I should be looking at? | 12:14 |
| jam | BTW, the 'README' should be updated with how to install mongodb, since "apt-get install mongodb" isn't sufficient anymore. | 12:15 |
| rogpeppe | jam: builddb is a command in the source tree. tbh, i think it should just be a charm, but it was an interesting experiment. | 12:34 |
| rogpeppe | jam: BTW it seems that mongodb always does exit(0) even when it gets a usage error | 12:35 |
| rogpeppe | jam: marvellous | 12:35 |
| jam | rogpeppe: how very nice of it. But at least you could detect that it had exited early, I guess. | 12:36 |
| jam | so just running "builddb" it tries to bootstrap my environment and says it is already bootstrapped... | 12:36 |
| rogpeppe | jam: that's true. i think it should never exit unless killed, and that's easy to detect | 12:36 |
| rogpeppe | jam: yeah, it shouldn't bootstrap. have you actually got a bootstrapped environment, or is that just a hangover from a previous bootstrap? | 12:37 |
| jam | rogpeppe: I probably ran something in the past | 12:37 |
| rogpeppe | jam: if the latter, just do juju destroy-environment first | 12:37 |
| rogpeppe | jam: https://codereview.appspot.com/6966045 | 13:07 |
| dimitern | rogpeppe, fwereade_: PTAL https://codereview.appspot.com/6940073/ | 13:11 |
| rogpeppe | dimitern: looking | 13:11 |
| === bcsaller1 is now known as bcsaller | ||
| rogpeppe | dimitern: you've got another review | 14:05 |
| dimitern | rogpeppe: cheers! | 14:05 |
| dimitern | rogpeppe: ok, I'm a bit confused why you think sendError(err, w, r) is redundant - how can I send errors which are not errorResponse then? | 14:19 |
| rogpeppe | dimitern: novaHandler.ServeHTTP handles this case | 14:20 |
| rogpeppe | dimitern: that's why you're testing for err.(http.Handler) | 14:20 |
| dimitern | rogpeppe: you mean after the internal handler is called? | 14:20 |
| dimitern | rogpeppe: I see, ok | 14:21 |
| rogpeppe | dimitern: basically, we allow some errors to know how to marshal themselves as an http response. | 14:21 |
| rogpeppe | dimitern: others we use the default internal error for | 14:22 |
| dimitern | rogpeppe: and others we wrap, yeah, nice! | 14:22 |
| dimitern | rogpeppe: I wanted something like that initially, but wasn't sure how's best | 14:22 |
| rogpeppe | dimitern: i think it works quite nicely :-) | 14:22 |
| rogpeppe | dimitern: it's a pity that we need to match all the real server's errors so exactly. it really clutters the code. | 14:24 |
| dimitern | rogpeppe: yeah, but I was thinking of implementing like a generic subhandler for a path + method | 14:25 |
| dimitern | rogpeppe: then, I can have several of these which are just doing the same (like put/post or get/delete pairs - lots of these can be trimmed) | 14:25 |
| rogpeppe | dimitern: that probably wouldn't be too hard. | 14:26 |
| dimitern | rogpeppe: assuming we define them similarly in a map like setupHTTP | 14:26 |
| dimitern | rogpeppe: or, maybe even better - implement them an get a helper to be called for a list of methods, path and handler | 14:27 |
| dimitern | rogpeppe: come to think of it, not even the path is needed | 14:30 |
| dimitern | rogpeppe: can I have a handlersMap := map[[]string][]*handler | 14:33 |
| rogpeppe | dimitern: no | 14:33 |
| rogpeppe | dimitern: but there's actually no particular reason for handlersMap to be a map | 14:33 |
| rogpeppe | dimitern: tbh, i'm not sure how much you'd win | 14:34 |
| dimitern | rogpeppe: yeah, probably shouldn't bother anyway - this is just a testing service | 14:35 |
| dimitern | rogpeppe: i should just work | 14:35 |
| rogpeppe | dimitern: i think it's quite nice to have all the irregularity of the server's interface laid out so it's easy to see. like errNotFound vs errNotFoundJSON, wtf?! | 14:36 |
| dimitern | rogpeppe: yep, it's like that - probably as a result of stitching different backends together :) | 14:38 |
| dimitern | rogpeppe: or more likely smart inner and quick and dumb outer API handlers - the inner taking into account I said Accept: json | 14:41 |
| dimitern | rogpeppe: and the outer just returning 404 plain early for unexpected URLs | 14:42 |
| hazmat | rogpeppe, do you happen to recall which version of golang has the broken http client.. one of our lbox users is still hitting panics.. even after a manual compile of lbox with 1.0.3 ... https://pastebin.canonical.com/80719/ | 15:39 |
| rogpeppe | hazmat: 1.0.3 | 15:40 |
| rogpeppe | hazmat: the most recent version, unfortunately | 15:40 |
| hazmat | eek | 15:40 |
| rogpeppe | hazmat: they could use tip or 1.0.2 | 15:40 |
| hazmat | rogpeppe, so 1.0.2 which is the default in the quantal archive should work ok then | 15:40 |
| hazmat | cool | 15:40 |
| rogpeppe | hazmat: it should | 15:41 |
| hazmat | rogpeppe, thanks | 15:41 |
| rogpeppe | hazmat: np | 15:41 |
| hazmat | rogpeppe, i think he tried it with 1.0.2 as well, but i'll verify next year (he's on holiday) | 15:41 |
| dimitern | rogpeppe: ping | 16:18 |
| rogpeppe | dimitern: pong | 16:18 |
| dimitern | rogpeppe: I'm almost done, fixing a few tests, but I'm having trouble debugging "multiple WriteHeader calls" message - how can I plug into the log that gocheck uses? | 16:19 |
| rogpeppe | dimitern: is that a panic, or is WriteHeader returning an error? | 16:20 |
| dimitern | rogpeppe: "2012/12/19 17:20:12 http: multiple response.WriteHeader calls" - think it's more like a warning | 16:21 |
| rogpeppe | dimitern: i recommend just finding that log message in the http package and turning into a panic, then running the test again | 16:21 |
| rogpeppe | dimitern: (remember to change it back afterwards...) | 16:22 |
| rogpeppe | dimitern: there are other ways, but that's the easiest | 16:22 |
| rogpeppe | dimitern: alternatively, change all occurrences of WriteHeader in your code to call a local function that writes a log message before calling WriteHeader | 16:23 |
| rogpeppe | dimitern: (that should be easy enough to do with a global substitution) | 16:23 |
| dimitern | rogpeppe: ha, ok.. wouldn't think it's that easy :) | 16:24 |
| dimitern | rogpeppe: ok, all done, please take a look, I'd really like to land this today https://codereview.appspot.com/6940073/ | 16:49 |
| rogpeppe | dimitern: looking (thanks for bearing with me - i hope you think it's worth it!) | 16:49 |
| dimitern | rogpeppe: sure, I appreciate your comments and see how to do things simpler in the future | 16:50 |
| rogpeppe | dimitern: reviewed | 17:11 |
| dimitern | rogpeppe: thanks! | 17:12 |
| dimitern | rogpeppe: ready with all suggestions - do you want to look at it or I should just submit it? | 17:35 |
| rogpeppe | dimitern: i'll just have a quick look | 17:36 |
| rogpeppe | dimitern: have you proposed it? | 17:36 |
| dimitern | rogpeppe: it's under way | 17:36 |
| dimitern | https://codereview.appspot.com/6940073 | 17:36 |
| rogpeppe | dimitern: reviewed. looks great. two small things still to do, but it looks great in general. | 17:44 |
| dimitern | rogpeppe: thanks again | 17:45 |
| dimitern | rogpeppe: with these, I'm submitting | 17:50 |
| rogpeppe | dimitern: great | 17:50 |
| dimitern | I'm off then, 'night guys! | 17:52 |
| * rogpeppe is off too. | 18:22 | |
| rogpeppe | g'night all | 18:22 |
| fwereade__ | rogpeppe, heyhey, I'm very much not working -- but if you are later this week, I'd love a final look at https://codereview.appspot.com/6944058/ | 23:09 |
| rogpeppe | fwereade__: me neither currently (just playing with new camera) but will look tomorrow | 23:10 |
| fwereade__ | rogpeppe, cheers :) | 23:11 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!