| davecheney | yeah, the flags package isn't as flexible as some | 00:00 |
|---|---|---|
| davecheney | you have a usage() method which you can override, but from memory it doesn't have knowledge of the flags that have been defined so can't kind of convert the flag set into a usage document | 00:01 |
| davecheney | hmm | 00:01 |
| davecheney | actually that isn't true | 00:01 |
| davecheney | it does do that | 00:01 |
| davecheney | gimme two secs | 00:01 |
| davecheney | bigjools: you want flag.PrintDefaults() | 00:05 |
| bigjools | ok thanks | 00:06 |
| bigjools | I wasn't quite sure how the Usage() thing fits it, the docs don't make it obvious | 00:06 |
| davecheney | Usage is called when you pass a flag that doesn't parse or is not known | 00:06 |
| bigjools | how is it overridden? | 00:09 |
| bigjools | (that's the bit that wasn't obvious, sorry) | 00:09 |
| davecheney | flag.Usage = func() { ... ; flag.PrintDefaults } | 00:10 |
| * thumper heads out to get a floor for the new shed... | 00:10 | |
| davecheney | flag.Usage = func() { ... ; flag.PrintDefaults() } | 00:11 |
| bigjools | oh that was more simple than I'd expected | 00:12 |
| bigjools | thanks | 00:12 |
| davecheney | flag.Usage is a variable with type func() | 00:12 |
| davecheney | so you can call it, with flag.Usage() | 00:13 |
| davecheney | sort of like a javascript function | 00:13 |
| davecheney | in a slot | 00:13 |
| thumper | wow that took a lot longer than I thought it would | 01:39 |
| davecheney | wallyworld: 2013/03/27 01:43:40 ERROR JUJU:jujud:machine cmd/jujud: provisioning worker: failed to GET object provider-state from container ehmegerd-bhukett, caused by: https://region-a.geo-1.objects.hpcloudsvc.com/v1/17031369947864/ehmegerd-bhukett/provider-state%!(EXTRA string=failed executing the request), caused by: Get https://region-a.geo-1.objects.hpcloudsvc.com/v1/17031369947864/ehmegerd-bhukett/provider-state: lookup region-a.ge | 01:44 |
| davecheney | ^ get that pretty constantly inside hp | 01:44 |
| davecheney | can you confirm that, for you, deploying inside hp works ? | 01:44 |
| davecheney | if so, then i'll push out this damnable release | 01:44 |
| wallyworld | davecheney: i deployed yesterday, from tip. i don't recognise that error offhand. what rev are you looking to release? i'll do a deployment and check | 01:45 |
| davecheney | wallyworld: juju-core_1.9.12-3~1064~quantal1_amd64.deb | 01:46 |
| davecheney | it's the usual nonsense that hp cloud cannot resolve its own endpoing internally | 01:46 |
| wallyworld | davecheney: i deployed a bootstrap node and also a mysql node to check the debug log stuff. you saying that hp has known issues in this area? | 01:47 |
| davecheney | i have not raised an issue with hp | 01:47 |
| davecheney | i can assert that deploying into hp rarely works | 01:47 |
| davecheney | for me | 01:47 |
| davecheney | on my accont | 01:47 |
| wallyworld | well that sucks | 01:47 |
| davecheney | screw it | 01:47 |
| davecheney | if you've had some success | 01:47 |
| davecheney | that is a good enough test | 01:47 |
| davecheney | i'll push out the release | 01:48 |
| wallyworld | well, it did work for me from source yes | 01:48 |
| wallyworld | but it's not good it isn't reliable | 01:48 |
| davecheney | same here, have to --upload-tools for hp | 01:48 |
| wallyworld | davecheney: and cause i'm running raring, i need to hard code the version to precise in the code otherwise it's a pita | 01:49 |
| wallyworld | but upload-tools works fine if i do that | 01:49 |
| wallyworld | and deploy on a precise image | 01:49 |
| davecheney | i don't think this is related to series | 01:49 |
| davecheney | the error is related to dns name resolution | 01:50 |
| wallyworld | no, just mentioning for completeness | 01:50 |
| davecheney | ok | 01:50 |
| wallyworld | full disclosure and all that | 01:50 |
| davecheney | roger | 01:50 |
| wallyworld | davecheney: dumb question - the deb contains juju-core rev 1064. does it just pull in tip of goose trunk? what control is there over the rev nr of goose we bake in? what if we didn't want goose tip but an earlier revision? | 01:57 |
| davecheney | wallyworld: the recipe does a pull from tip | 02:06 |
| davecheney | there is no real control | 02:06 |
| wallyworld | :-( | 02:06 |
| wallyworld | ok | 02:06 |
| wallyworld | wcpgw | 02:06 |
| davecheney | https://code.launchpad.net/~dave-cheney/+recipe/juju-core | 02:06 |
| davecheney | ok, time to break for lunch | 02:07 |
| thumper | davecheney: ping | 03:26 |
| thumper | http://play.golang.org/p/ohvYEDuBbz, cc bigjools | 03:26 |
| davecheney | thumper: /me looks | 03:26 |
| bigjools | was about to do the same :) | 03:26 |
| davecheney | yup | 03:27 |
| davecheney | you've got two things there | 03:27 |
| davecheney | 1, a typed nil | 03:27 |
| davecheney | 2, you can have a method on a nil value | 03:27 |
| bigjools | !!!!!!!! | 03:27 |
| davecheney | err, not the right word, but you understand | 03:27 |
| bigjools | madness, but ok thanks | 03:28 |
| davecheney | bigjools: Go isn't like C++, there isn't a pointer to a virtual method table | 03:29 |
| davecheney | you anre't really dispatching to a method | 03:29 |
| davecheney | just calling a function with the receiver as the first argument | 03:29 |
| davecheney | it's just syntactic tomfoolery | 03:29 |
| sidnei | ohai folks, are things into a 'it now works with canonistack, no hacks involved' yet? | 03:29 |
| bigjools | it still confuses me that nil turns into something else | 03:29 |
| davecheney | sidnei: nope | 03:29 |
| * sidnei sets reminder for +7d | 03:30 | |
| davecheney | bigjools: so there are types | 03:30 |
| davecheney | pointer types | 03:30 |
| davecheney | and what those values point too | 03:30 |
| bigjools | nil magically turns into an object | 03:30 |
| davecheney | sidnei: set it for post 13.04 | 03:30 |
| sidnei | ah, thanks! | 03:30 |
| davecheney | sidnei: waaaaaaay past 13.04 | 03:30 |
| sidnei | more like 2014? :) | 03:31 |
| davecheney | bigjools: nil is just the zero value for a pointer | 03:31 |
| davecheney | if you have a pointer value | 03:31 |
| davecheney | *Error | 03:31 |
| davecheney | actually | 03:31 |
| davecheney | var e *Error | 03:31 |
| davecheney | it's value is nil, (well, 0, or close too) | 03:31 |
| davecheney | sidnei: probably not that bad, but the problems we have with canonistack aren't completely within the domain of Juju | 03:31 |
| davecheney | so calling e.Error() | 03:32 |
| davecheney | is actually doing | 03:32 |
| davecheney | func Error(e *Error) String | 03:32 |
| davecheney | and as nothing references the e inside that function call | 03:32 |
| davecheney | it's ok that e is nil | 03:32 |
| sidnei | davecheney: still the public ips thing? | 03:32 |
| davecheney | sidnei: yup | 03:32 |
| sidnei | :/ | 03:32 |
| davecheney | ipv4 isn't going to get any less scarece | 03:32 |
| thumper | davecheney: what is InjectMachine, and what is the instanceId? | 03:33 |
| bigjools | davecheney: thanks for the explanation. I have to get used to the way Go works here then. | 03:33 |
| sidnei | and why does it need public ips again? | 03:33 |
| davecheney | thumper: is this jeopardy ? | 03:33 |
| davecheney | bigjools: no worries | 03:33 |
| thumper | davecheney: no, I was hoping you'd know | 03:33 |
| davecheney | although I don't have much experience isn't this similar to how python does method dispatch ? | 03:34 |
| bigjools | davecheney: so how do I rearrange my code to DWIM? | 03:34 |
| davecheney | bigjools: let me loook again | 03:34 |
| bigjools | I want an "error" that has more info on it | 03:34 |
| * thumper shakes his head... | 03:35 | |
| thumper | InjectMachine is used in exactly one place | 03:35 |
| thumper | bootstrap | 03:35 |
| thumper | wtf | 03:35 |
| thumper | davecheney: what is the machine instance id? | 03:37 |
| thumper | davecheney: any idea? | 03:38 |
| thumper | oh... | 03:38 |
| thumper | the provider instance id | 03:38 |
| thumper | which when I'm wanting to start a new machine... | 03:38 |
| bigjools | davecheney: actually I can just change foo() to return an error I think, so no worries | 03:38 |
| thumper | it should be empty right | 03:38 |
| thumper | ? | 03:38 |
| thumper | so the provider picks it up and sets it... | 03:38 |
| * thumper answered his owne question | 03:39 | |
| davecheney | bigjools: http://play.golang.org/p/58xsXh9GMp | 03:39 |
| davecheney | maybe that wasn't what you were asking | 03:39 |
| davecheney | thumper: ok, sorry, now to your question | 03:39 |
| davecheney | thumper: this is bootstrapping isn't it ? | 03:40 |
| davecheney | it is a bit hairy | 03:40 |
| thumper | well, inject machine is | 03:40 |
| thumper | but I was more just wondering about the instance id for the machine | 03:40 |
| thumper | but I'm pretty sure that the provider sets it | 03:40 |
| thumper | when the machine has been started | 03:40 |
| bigjools | davecheney: that's pretty much it, thanbks | 03:41 |
| davecheney | bigjools: np | 03:41 |
| thumper | davecheney: I've almost forced this code into the shape I want | 04:12 |
| thumper | almost... | 04:12 |
| thumper | sometimes I think we are being too clever :) | 04:12 |
| thumper | davecheney: in what situations does the transaction runner return txn.ErrAborted ? | 04:14 |
| davecheney | thumper: i do not know the answer to that | 04:15 |
| thumper | davecheney: kk | 04:15 |
| davecheney | soz | 04:15 |
| thumper | davecheney: shouldn't really panic should I? | 04:20 |
| thumper | davecheney: perhaps just return an error that says "shouldn't really get here, oops" | 04:21 |
| davecheney | if it really is impossible, then panic sounds good | 04:23 |
| thumper | well, it could be an edge case I haven't thought of | 04:24 |
| davecheney | id would expect pushback in the review | 04:25 |
| thumper | lets see :) | 04:26 |
| thumper | I need tests anyway | 04:27 |
| thumper | ... | 04:27 |
| thumper | added state.Unit.AssignToNewMachine() | 04:27 |
| thumper | ok, tests later... | 04:31 |
| thumper | like.. tomorrow | 04:31 |
| thumper | ciao | 04:31 |
| === _mup__ is now known as _mup_ | ||
| dimitern | morning all | 07:49 |
| fwereade | dimitern, heyhey | 08:03 |
| thumper | evening | 08:06 |
| * thumper decided to get the tests done now, so first round of reviews can happen before I leave for easter | 08:07 | |
| thumper | rogpeppe: you around? | 08:09 |
| benonsoftware | sort | 08:11 |
| benonsoftware | Oops, sorry about that. | 08:16 |
| thumper | morning fwereade | 08:19 |
| fwereade | thumper, heyhey | 08:19 |
| thumper | fwereade: I'm just writing the tests for the assign to new machine in one transaction work | 08:19 |
| thumper | fwereade: which includes the AssignNew policy | 08:20 |
| fwereade | thumper, ok, cool | 08:20 |
| thumper | fwereade: I'm up now so I can get a first review of it before I stop for easter :) | 08:20 |
| fwereade | thumper, much appreciated :) | 08:20 |
| thumper | fwereade: actually I have a few questions... | 08:23 |
| fwereade | thumper, g+ in about 5 mins ok for you? | 08:23 |
| thumper | fwereade: let me poke this with a stick first :) | 08:23 |
| thumper | fwereade: given a state.Unit, how do you get the Machine it is running on? | 08:29 |
| thumper | fwereade: the closest I can see is DeployerName() | 08:30 |
| thumper | fwereade: which doesn't actually give me what I want | 08:30 |
| thumper | fwereade: ok to add methods? | 08:30 |
| fwereade | thumper, AssignedMachineId IIRC | 08:33 |
| thumper | fwereade: ah, yes | 08:33 |
| thumper | fwereade: so what is the normal way to get a machine given an ID? | 08:34 |
| thumper | from the outside? | 08:34 |
| thumper | ie, a test package | 08:34 |
| fwereade | thumper, State.Machine() takes an id | 08:34 |
| thumper | cool | 08:34 |
| thumper | fwereade: another dumb question, given a machine, how do I see what units are deployed on it? | 08:38 |
| thumper | Principles doesn't seem to be mentioned in any reasonable public method for access | 08:38 |
| fwereade | thumper, Units() returns all units including subordinates | 08:39 |
| thumper | ahh... it is using the doc "principal" name, not "Principal" | 08:40 |
| fwereade | thumper, if you need just the principals you could filter that by Unit.IsPrincipal() | 08:40 |
| thumper | and my search was case sensitive | 08:40 |
| thumper | fwereade: it is a test, there is only a princpal :) | 08:40 |
| fwereade | thumper, cool :) | 08:40 |
| thumper | fwereade: it is testing that the machine gets the principal set as well as the unit geting the machine id set | 08:40 |
| fwereade | thumper, cool | 08:40 |
| thumper | fwereade: is there an assert thingy for length of a slice? | 08:42 |
| fwereade | thumper, HasLen | 08:42 |
| * fwereade realises he wants to break compatibility with the released tools within 4 hours of their release; examines his conscience | 08:43 | |
| thumper | :) | 08:43 |
| * fwereade decides that the assignment policy change will do that anyway, as will a bunch of other things probably, decides not to worry about it | 08:44 | |
| thumper | fwereade: hmm... you can't get the Series for a unit... | 08:45 |
| thumper | fwereade: are you expected to go through the charm? | 08:45 |
| thumper | fwereade: sorry, service? | 08:45 |
| fwereade | thumper, you're not really expected to ask from outside at the unit level, describe use case a little more? | 08:45 |
| thumper | fwereade: inside the test, I want to create a machine with the correct series for the unit, to make sure it isn't claimed in AssignToNewMachine | 08:46 |
| fwereade | thumper, I'd just create one with the charm's series | 08:46 |
| thumper | fwereade: to do that, I need to know the series of the unit | 08:46 |
| fwereade | thumper, unit series is just a denormalised copy of the charm series | 08:47 |
| thumper | fwereade: I'm not liking the number of dots here... | 08:48 |
| thumper | let me show you. | 08:48 |
| rogpeppe | thumper: hiya | 08:48 |
| thumper | fwereade: nm... too many returns with value,err | 08:51 |
| thumper | rogpeppe: I had a question for you | 08:51 |
| rogpeppe | thumper: ask away | 08:51 |
| fwereade | thumper, haha, automatic trainwreck protection ;p | 08:51 |
| thumper | rogpeppe: perhaps best asked over a hangout if you have time | 08:51 |
| rogpeppe | thumper: sure | 08:51 |
| thumper | fwereade: verbose mode on | 08:51 |
| thumper | rogpeppe: shall I start one? | 08:51 |
| rogpeppe | thumper: do you wanna start... yeah | 08:51 |
| dimitern | fwereade: when updating RelationCount is it ok to also change the doc field or you're expected to call Refresh after SetCharm to get the updated value? | 09:12 |
| fwereade | dimitern, it's not visible externally, and it's not going to be accurate anyway in a multi-user context | 09:12 |
| fwereade | dimitern, just don't bother changing it IMO | 09:13 |
| dimitern | fwereade: ok, but in tests I used Refresh and added ServiceRelationCount(svc) -> int in export_test | 09:13 |
| rogpeppe | fwereade, dimitern: do any of the operations inspect it to decide what kind of transaction to start with? | 09:13 |
| thumper | fwereade: so testing charms always use the series "series" ? | 09:13 |
| dimitern | rogpeppe: no, but it was not updated after adding new peer rels | 09:14 |
| fwereade | thumper, they don;t *have* to but in parctice I think they do | 09:14 |
| thumper | ok | 09:14 |
| fwereade | rogpeppe, dimitern: actually they do, but the counts are assumed in general not to be accurate | 09:14 |
| rogpeppe | fwereade: if they're not accurate, why bother looking at them? | 09:14 |
| rogpeppe | fwereade: that would seem to me to be a reasonable argument for refreshing the count | 09:15 |
| rogpeppe | fwereade: personally, i think that assuming the current in-memory state is any kind of indicator of remote state is a bit bogus | 09:15 |
| fwereade | rogpeppe, it might be right, and probably will be if it's a fresh object, which in practice it always is except in tests | 09:16 |
| fwereade | rogpeppe, but trying to keep it accurate in a long-lived object seems pretty pointless | 09:17 |
| rogpeppe | fwereade: fair enough | 09:17 |
| fwereade | rogpeppe, the time to pay for the extra roundtrips is when we need to use them, I think | 09:18 |
| thumper | ah... seriously? no NotEqual ? | 09:18 |
| rogpeppe | thumper: Not(Equals) | 09:19 |
| * thumper sighs | 09:19 | |
| rogpeppe | thumper: 3 more characters doesn't seem too bad to me, and it's general functionality. | 09:20 |
| rogpeppe | if you want NotEqual: var NotEqual = Not(Equals) :-) | 09:21 |
| thumper | rogpeppe: for me it is more that it should be so commonly used, it should have its own | 09:21 |
| thumper | :) | 09:21 |
| thumper | suppose someone submits that to gocheck? | 09:21 |
| thumper | bags not | 09:21 |
| rogpeppe | thumper: it's just sugar | 09:21 |
| thumper | rogpeppe: sugar is good | 09:22 |
| thumper | rogpeppe: half of go is sugar | 09:22 |
| rogpeppe | thumper: and not that commonly used (20 out of 1621 uses of Equals in the code base) | 09:22 |
| thumper | for us maybe :) | 09:22 |
| rogpeppe | thumper: really? i beg to disagree. | 09:22 |
| thumper | ok | 09:22 |
| thumper | we can disagree | 09:23 |
| rogpeppe | thumper: there are one or two sugary bits in go (some automatic type promotions) but most is basic functionality, i think | 09:23 |
| rogpeppe | thumper: by "sugar" i mean "something that can easily be made from a combination of other existing parts" | 09:25 |
| thumper | fwereade: what is the best way to make a unit not alive for a test? | 09:29 |
| fwereade | thumper, Destroy makes it Dying | 09:29 |
| thumper | ta | 09:29 |
| fwereade | thumper, EnsureDead makes it Dead (and will probably work straight-off in a test context) | 09:29 |
| fwereade | thumper, Remove requires that EnsureDead has already been called and actually removes it from the db | 09:30 |
| thumper | destroy is enough, ta | 09:30 |
| dimitern | fwereade, rogpeppe: I think this should be ready now, would you take a look please? https://codereview.appspot.com/8034043/ | 09:33 |
| fwereade | dimitern, ack | 09:33 |
| * thumper is done | 09:47 | |
| thumper | tests written, all tests currently running prior to proposing | 09:47 |
| * thumper goes to put on the kettle, tea time | 09:47 | |
| TheMue | fwereade: ping, read your review, so we can talk about it now | 09:49 |
| thumper | w00t | 09:50 |
| thumper | all pass | 09:50 |
| * thumper does the lbox dance | 09:50 | |
| * fwereade cheers at thumper | 09:50 | |
| * fwereade applauds the dance | 09:50 | |
| * TheMue cheers too, because he knows that feeling | 09:50 | |
| fwereade | TheMue, would you start a hangout please? will join in <5m | 09:50 |
| TheMue | fwereade: will do | 09:51 |
| thumper | fwereade: https://codereview.appspot.com/7845048 | 09:53 |
| thumper | and I'm logging off now | 09:53 |
| thumper | see y'all tomorrow | 09:53 |
| dimitern | fwereade: i'm pinging you :) | 09:54 |
| dimitern | fwereade: wanna start a hangout? | 09:55 |
| fwereade | dimitern, sorry, need to chat to TheMue first | 09:55 |
| dimitern | fwereade: sure, np | 09:55 |
| fwereade | TheMue, here | 09:55 |
| TheMue | dimitern: i'll try to be fast ;) | 09:55 |
| dimitern | TheMue: no worries | 09:55 |
| TheMue | fwereade: ho is started, you should have an invitation | 09:56 |
| fwereade | TheMue, ah, sorry, was looking at my canonical account | 09:57 |
| fwereade | dimitern, ping | 10:18 |
| dimitern | fwereade: here | 10:18 |
| * fwereade starts hangout | 10:18 | |
| === teknico_ is now known as teknico | ||
| fwereade | TheMue, dimitern: I'm just WIPing your branches for now so I can see what I'm doing on +activereviews | 10:30 |
| dimitern | fwereade: sure, I noticed that :) it's a pity lbox propose doesn't do this somehow | 10:31 |
| fwereade | dimitern, lbox propose -wip | 10:31 |
| TheMue | fwereade: sure | 10:31 |
| rogpeppe | fwereade: is juju destroy-machine supposed to work? | 10:32 |
| fwereade | rogpeppe, depends... in what context? ;p | 10:32 |
| dimitern | fwereade: I don't think -wip sets the MP to WIP | 10:32 |
| rogpeppe | fwereade: i just tried to use it, and nothing seems to be destroyed | 10:32 |
| rogpeppe | fwereade: the instance stubbornly remains there | 10:32 |
| fwereade | dimitern, I don't think it shoudl destroy anything | 10:33 |
| fwereade | rogpeppe, ok, the machine is Dead but the provisioner doesn't pick it up? | 10:33 |
| rogpeppe | fwereade: i don't know for sure if the machine is Dead. | 10:33 |
| fwereade | rogpeppe, check the machine logs maybe? | 10:33 |
| rogpeppe | fwereade: am doing that right now | 10:34 |
| fwereade | rogpeppe, cheers | 10:34 |
| rogpeppe | fwereade: ha, i've been running this juju env for less than a day, with 3 units and the all-machines.log is already 24MB | 10:37 |
| fwereade | rogpeppe, how much of that is state/watcher logs? | 10:38 |
| rogpeppe | fwereade: first estimate: 83% | 10:39 |
| rogpeppe | fwereade: mostly "loading new events" | 10:40 |
| rogpeppe | fwereade: 'cos that happens every 5 seconds | 10:40 |
| fwereade | rogpeppe, yeah, thought so :/ | 10:41 |
| fwereade | rogpeppe, I don't want to switch off debug logging, but I don't want to see that stuff ATM, and I don't want to drop that logging either | 10:42 |
| rogpeppe | fwereade: we should just raise the log level a bit | 10:43 |
| rogpeppe | fwereade: well actually | 10:43 |
| fwereade | rogpeppe, no, because other stuff logged at debug level *is* handy | 10:43 |
| rogpeppe | fwereade: the "loading new events" messages could go | 10:43 |
| fwereade | rogpeppe, +1 | 10:43 |
| rogpeppe | fwereade: it's nice to see when new events actually come in | 10:43 |
| jam | fwereade: ian and I discussed the possibility of having DEBUG messages always logged locally, but rsyslogd would only sync INFO and higher | 10:44 |
| jam | or something like that. | 10:44 |
| fwereade | jam, yeah, that would be reasonable too | 10:45 |
| jam | fwereade: alternatively, you should be able to post-filter with "juju debug-log | grep -v DEBUG" or whatever, since all the tags should be well defined. But yeah, some of that was "tweaking as we find a good balance" | 10:46 |
| jam | we talked about DEBUG default off, until user requested, INFO only local, Notice and above global. | 10:46 |
| rogpeppe | fwereade: 82% of the size is from "loading new events" | 10:48 |
| fwereade | rogpeppe, ok, let's just drop that one | 10:49 |
| jam | rogpeppe: clearly that means when the logs are rotated it will trivially compress, right? :) | 10:49 |
| * rogpeppe prefers logs with greater signal to noise ration | 10:50 | |
| rogpeppe | fwereade: the log size reduces by 95% with grep -v 'loading new|synchronizing watcher' | 10:53 |
| rogpeppe | fwereade: so i'll remove the synchronizing watcher message too | 10:53 |
| fwereade | rogpeppe, jam: hmm, good point about compression actually | 10:54 |
| fwereade | rogpeppe, the reason I am nervous is that I can all too easily imagine us encountering surprising watcher behaviour at scale | 10:54 |
| fwereade | rogpeppe, ...but then, at scale, the sheer volume of messages may become crippling | 10:55 |
| * fwereade grumbles vaguely | 10:55 | |
| rogpeppe | fwereade: is it really worth printing log events for non-events ? | 10:55 |
| rogpeppe | fwereade: one thing: i see many more connect requests than i would expect | 10:56 |
| fwereade | rogpeppe, well, it allows us to distinguish between "nothing happened" and "the watcher is blocked" | 10:56 |
| fwereade | rogpeppe, oh yes? | 10:56 |
| rogpeppe | fwereade: like, over 4000 in the last day | 10:56 |
| fwereade | rogpeppe, what exactly are you referring to by connect requests? the machine agent reconnecting to state? | 10:57 |
| rogpeppe | fwereade: yeah. well, mgo dialling the state server, at any rate | 10:57 |
| rogpeppe | fwereade: looks like it happens every 3 minutes | 10:58 |
| fwereade | rogpeppe, that is... somewhat alarming | 10:58 |
| fwereade | rogpeppe, anything obvious in the logs preceding the attempts? | 10:59 |
| rogpeppe | fwereade: i wonder if there's a connection between that three minutes and this line in mgo: | 10:59 |
| rogpeppe | session.SetSyncTimeout(3 * time.Minute) | 10:59 |
| rogpeppe | hmm, no, that's in a test! | 10:59 |
| rogpeppe | fwereade: no not AFAICSS | 10:59 |
| rogpeppe | fwereade: ah, i think it must be const syncServersDelay = 3 * time.Minute | 11:05 |
| fwereade | rogpeppe, ha, I don't recognise that at all | 11:16 |
| dimitern | fwereade: indeed, when I commented out the RC++ block I started getting ErrExCont on destroy :) | 11:19 |
| fwereade | dimitern, cool, my mental model remains accurate :) | 11:19 |
| dimitern | fwereade: it's done and I like how the test got simplified, check it out | 11:26 |
| === teknico_ is now known as teknico | ||
| mattyw | rogpeppe, any idea why running go test in the juju-core/state/apiserver package might never terminate? | 11:39 |
| rogpeppe | mattyw: it can happen that a test fails and then things hang up trying to tear down. | 11:40 |
| rogpeppe | mattyw: (it's an issue that needs fixing) | 11:40 |
| rogpeppe | mattyw: try running go test -gocheck.vv | 11:40 |
| rogpeppe | mattyw: then you'll see all the test output as it happens | 11:40 |
| rogpeppe | mattyw: so you'll know where it's hanging | 11:40 |
| mattyw | rogpeppe, you're right http://pastebin.ubuntu.com/5652025/ | 11:43 |
| rogpeppe | mattyw: ah, i suspected that actually | 11:43 |
| rogpeppe | mattyw: you need a version of mongo that supports ssl | 11:44 |
| rogpeppe | mattyw: i think there might be an apt repo for it, but i'm not sure. | 11:44 |
| rogpeppe | mattyw: i got it from the s3 bucket that juju gets it from | 11:44 |
| mattyw | rogpeppe, I can try installing one by hand | 11:45 |
| mattyw | rogpeppe, good plan | 11:45 |
| fwereade | dimitern, LGTM with trivials | 11:45 |
| fwereade | everyone else, need to be hospitable for a bit until cath comes back | 11:45 |
| rogpeppe | mattyw: http://juju-dist.s3.amazonaws.com/tools/mongo-2.2.0-precise-amd64.tgz | 11:47 |
| mattyw | rogpeppe, just installing it from there | 11:47 |
| mattyw | rogpeppe, thanks very much | 11:47 |
| mattyw | rogpeppe, did I mention I like the api? | 11:47 |
| rogpeppe | mattyw: i think you did. that's nice to hear! | 11:48 |
| dimitern | fwereade: cheers | 11:48 |
| rogpeppe | mattyw: what do you like about it, BTW? | 11:48 |
| mattyw | rogpeppe, at the moment just the fact that a client doesn't really need to do anything to get data about units, I don't have to parse json or handle connections. I just sit on a channel | 11:49 |
| rogpeppe | mattyw: cool. we aim to please :-) | 11:50 |
| mattyw | rogpeppe, something that is confusing though is there doesn't seem to be a way for me to get ServiceInfo from a get. It would be nice to just watch the deltas for *params.UnitInfo, then for each one get the service details by doing a ServiceGet(u.Name) and get a params.ServiceInfo back | 11:51 |
| rogpeppe | mattyw: you should see the service details too, as they arrive | 11:52 |
| rogpeppe | mattyw: although the service will arrive in the same set of deltas as the unit of that service, there's currently no guarantee that the service will appear first in that slice. | 11:53 |
| mattyw | rogpeppe, but I'll have to keep track of them myself, it could be lasier by just watching for units then asking for the data each time (although now I think about it when a unit gets remove that would be messy) | 11:53 |
| rogpeppe | mattyw: (that was a policy decision taken because the GUI, the main consumer of the API currently, doesn't care) | 11:53 |
| mattyw | rogpeppe, in fact, the more I think about it, the more I think it's best I keep track of them myself | 11:54 |
| rogpeppe | mattyw: keeping track yourself should be easy, yeah | 11:54 |
| mattyw | rogpeppe, at the moment I watch for UnitInfo and ServiceInfo | 11:54 |
| rogpeppe | mattyw: all you need to do is keep a map[string]*params.ServiceInfo | 11:54 |
| rogpeppe | mattyw: and assign to it when things change | 11:54 |
| mattyw | rogpeppe, exactly what i'm doing | 11:54 |
| mattyw | rogpeppe, good to know I'm on the right track | 11:55 |
| * TheMue enjoys a short lunch break | 11:59 | |
| * dimitern lunch as well | 12:01 | |
| mattyw | rogpeppe, thanks, that seems to be working a treat now | 12:01 |
| rogpeppe | mattyw: great | 12:01 |
| mattyw | rogpeppe, something you might be able to answer without my trying it out. That sample you sent me yesterday. Would I be able to run that from within a deployed unit? Would it need any extra info to connect the the environment? | 12:03 |
| rogpeppe | mattyw: from within a deployed unit, you'd need slightly different code | 12:04 |
| rogpeppe | mattyw: this is an issue that the GUI team have too, in fact. | 12:04 |
| rogpeppe | fwereade: what do you think of the idea of making the API server address available in an environment variable to hooks ? | 12:04 |
| rogpeppe | fwereade: current the GUI charm is going delving in agent.conf files, which, while expedient, probably isn't a great long term solution | 12:05 |
| fwereade | rogpeppe, hmm, I need to think about that for a bit | 12:07 |
| fwereade | rogpeppe, definitely better than the current solution, but... :) | 12:08 |
| rogpeppe | fwereade: what do see as the potential problems with it? | 12:08 |
| === BradCrittenden is now known as bac | ||
| rogpeppe | fwereade: oh yes, i just looked at the life status of those machines i tried to destroy - they're all dying, but nothing's reacting | 12:28 |
| fwereade | rogpeppe, I'm not sure there are any problems, just hadn't really considered it | 12:30 |
| rogpeppe | fwereade: yeah, i wondered at first, then thought "why not?" | 12:30 |
| rogpeppe | fwereade: it's not going to be that uncommon for charms to want to talk to their juju environment in a proactive way | 12:31 |
| fwereade | rogpeppe, hmm, if you bounce the machine agents do they react then? | 12:31 |
| rogpeppe | fwereade: i'm trying to remember which part of the machine agent is responsible for setting the machine to Dead | 12:32 |
| fwereade | rogpeppe, I guess the core of my unease is that we give hooks a very specific environment to work with, and giving them direct API access kinda does an end run around that | 12:32 |
| fwereade | rogpeppe, the Machiner | 12:32 |
| rogpeppe | fwereade: they only get direct API access if someone specifically tells the charm the access key | 12:32 |
| fwereade | rogpeppe, true | 12:33 |
| fwereade | rogpeppe, I'm still wondering whether it makes more sense as a charm config param | 12:33 |
| fwereade | rogpeppe, probably not | 12:33 |
| rogpeppe | fwereade: looks like the Machiner isn't actually started by the machine agent. oops. | 12:34 |
| fwereade | rogpeppe, well, crap | 12:34 |
| fwereade | rogpeppe, that*would* explain hte trouble you've been having with the live tests though | 12:35 |
| rogpeppe | fwereade: ah! | 12:36 |
| rogpeppe | fwereade: https://codereview.appspot.com/6938072/diff/3001/cmd/jujud/machine.go | 13:02 |
| rogpeppe | fwereade: oops, i didn't spot it! | 13:02 |
| fwereade | rogpeppe, shit happens, my fault for crappy testing | 13:03 |
| fwereade | rogpeppe, IIRC we did discuss it and agreed that the complexities of the state-fuckery made it skippable | 13:03 |
| fwereade | rogpeppe, but that's clearly nonsense :) | 13:03 |
| fwereade | rogpeppe, except, hold on | 13:04 |
| fwereade | rogpeppe, we dropped machiner then | 13:04 |
| fwereade | rogpeppe, that was known | 13:04 |
| rogpeppe | fwereade: yeah, i'm just trying to to see when it came back | 13:04 |
| fwereade | rogpeppe, because machiner was trying to do deployer's job | 13:04 |
| fwereade | rogpeppe, I could have sworn I put it back at some point | 13:04 |
| fwereade | rogpeppe, but, maybe I just didn't | 13:05 |
| * fwereade heads off to lunch and maybe to find something to smack himself with | 13:05 | |
| === frankban_ is now known as frankban | ||
| mattyw | rogpeppe, ping? | 13:12 |
| rogpeppe | mattyw: pong? | 13:12 |
| mattyw | rogpeppe, hi, I'm trying to write some unit tests for my api querying thing, I'm using state/apiserver/api_test.go as a guide | 13:13 |
| rogpeppe | mattyw: ok | 13:14 |
| rogpeppe | mattyw: that may or may not be a good idea :-) | 13:14 |
| mattyw | rogpeppe, but there must be something I'm missing in the intitial setup because whenever I try to do anything in setUpScenario like AddUser, AddMachine or AddUnit I panic will a nil reference | 13:15 |
| rogpeppe | mattyw: could you paste your code? | 13:15 |
| mattyw | rogpeppe, just doing it... | 13:15 |
| mattyw | rogpeppe, http://paste.ubuntu.com/5652243 | 13:17 |
| mattyw | rogpeppe, I'm been just piling stuff into SetUpSuite to see what happens, each of these commands by themselves will blow up | 13:17 |
| rogpeppe | mattyw: i'll just see what happens when i run your code | 13:18 |
| mattyw | rogpeppe, ok thanks | 13:18 |
| rogpeppe | mattyw: ok, i've found your problem | 13:22 |
| rogpeppe | mattyw: try this: http://paste.ubuntu.com/5652259/ | 13:24 |
| rogpeppe | mattyw: problem 1) you were defining SetUpSuite but not calling the underlying JujuConnSuite SetUpSuite. | 13:25 |
| mattyw | rogpeppe, seems to run, thanks very much, I was looking for something like JujuConnSuite.SetUptest(c) but couldn't find anything | 13:26 |
| rogpeppe | mattyw: problem 2) JujuConnSuite only provides a State for individual tests, not for the whole suite. | 13:26 |
| mattyw | rogpeppe, thanks again for your help | 13:26 |
| rogpeppe | mattyw: np | 13:26 |
| mattyw | rogpeppe, ok | 13:26 |
| === andrewdeane is now known as awd | ||
| rogpeppe | fwereade: ping | 13:50 |
| fwereade | rogpeppe, pong | 13:54 |
| rogpeppe | fwereade: so... i've put Machiner back in the machine agent | 13:54 |
| rogpeppe | fwereade: and i have a test that fails before and passes now: http://paste.ubuntu.com/5652331/ | 13:54 |
| rogpeppe | fwereade: but... | 13:55 |
| rogpeppe | fwereade: i don't like the test, and i'm not sure of the best way to fix it | 13:55 |
| rogpeppe | fwereade: do you see why it's flaky? | 13:55 |
| fwereade | rogpeppe, is it the 5s matching watcher refreshes? | 13:56 |
| fwereade | rogpeppe, or something else? | 13:56 |
| rogpeppe | fwereade: yeah that's it | 13:56 |
| === wedgwood_away is now known as wedgwood | ||
| rogpeppe | fwereade: if the destroy happens a little bit later, then we really can take 5s to notice | 13:56 |
| rogpeppe | fwereade: but we can't call StartSync because we don't have access to the machiner's state | 13:57 |
| fwereade | rogpeppe, check out cmd/jujud/deployer_test.go | 13:57 |
| fwereade | rogpeppe, which does Bad Things to get around that problem | 13:57 |
| fwereade | rogpeppe, but to be fair Worse Things have happened | 13:57 |
| rogpeppe | fwereade: i'm wondering about a slightly more radical approach to StartSync | 13:57 |
| fwereade | rogpeppe, go on... | 13:58 |
| rogpeppe | fwereade: how about we make StartSync global? | 13:58 |
| rogpeppe | fwereade: so it syncs *all* watchers | 13:58 |
| fwereade | rogpeppe, I was afraid you were going to say that, but it is awfully tempting | 13:59 |
| fwereade | rogpeppe, that'd unfuck a number of other tests I think | 13:59 |
| rogpeppe | fwereade: this is for testing, right? | 13:59 |
| rogpeppe | fwereade: yeah | 13:59 |
| rogpeppe | fwereade: and i can't really see much of a down side | 13:59 |
| rogpeppe | fwereade: beyond the "global stuff is bad" thing | 14:00 |
| fwereade | rogpeppe, well, it's kinda papering over unhelpful factoring of the jujud stuff | 14:00 |
| rogpeppe | fwereade: there are lots of places where we've got several states and we change one and want another watcher to react | 14:01 |
| rogpeppe | fwereade: well, i say "lots"... i think that may be the case. | 14:01 |
| rogpeppe | fwereade: with the jujud stuff, i think it's ok to do black box testing, but that does mean that the command will open its own state. | 14:02 |
| fwereade | rogpeppe, I would prefer it if we could see a way to get the state out of jujud, but IIRC I looked into it and it was somewhat ard to tract | 14:03 |
| fwereade | rogpeppe, I do honestly favour black box testing wherever possible, I just prefer much smaller boxes :) | 14:04 |
| rogpeppe | fwereade: the problem is that the state in jujud is a changing thing | 14:04 |
| rogpeppe | fwereade: because it can reload | 14:04 |
| === wedgwood is now known as wedgwood_away | ||
| fwereade | rogpeppe, indeed so -- I do kinda think that mixing the state-opening up with the task-running is a bit of a code smell | 14:05 |
| === wedgwood_away is now known as wedgwood | ||
| rogpeppe | fwereade: well, it would be even worse if it wasn't | 14:06 |
| fwereade | rogpeppe, meeting btw | 14:06 |
| === wedgwood is now known as wedgwood_away | ||
| === wedgwood_away is now known as wedgwood | ||
| rogpeppe | lunch | 14:32 |
| === teknico is now known as teknico_mobile | ||
| === teknico_ is now known as teknico_mobile_ | ||
| dimitern | fwereade: wrt py-juju parity for upgrade-charm | 15:10 |
| fwereade | dimitern, oh yes | 15:10 |
| dimitern | fwereade: we still need those extra flags/args first, i think | 15:11 |
| fwereade | dimitern, ah, damn, --force | 15:11 |
| fwereade | dimitern, that's the only one that exists in python | 15:11 |
| fwereade | dimitern, IIRC | 15:11 |
| fwereade | dimitern, do double-check | 15:11 |
| dimitern | fwereade: which is different from --switch, right? | 15:11 |
| fwereade | dimitern, yeah | 15:14 |
| fwereade | dimitern, --force is just the second arg to SetCharm | 15:14 |
| dimitern | fwereade: your router acting funny again? :) | 15:14 |
| fwereade | dimitern, --switch is an awesome feature that it will at least be cheap and easy to add at some point in the future | 15:14 |
| fwereade | dimitern, no, I go distracted by some test failures | 15:15 |
| fwereade | dimitern, ==, !=, details details | 15:15 |
| dimitern | fwereade: then maybe I should do --force first before starting the other 2 cards (no card for --force though) | 15:15 |
| fwereade | dimitern, add a card for --force and please do no further work on upgrade-charm | 15:16 |
| === teknico__ is now known as teknico | ||
| fwereade | dimitern, we have parity and we have way too much else to do to go beyond at this point | 15:16 |
| dimitern | fwereade: no, i meant you're dropping out quite often today :) | 15:16 |
| fwereade | dimitern, ah, hadn't even noticed :) | 15:16 |
| dimitern | fwereade: you mean add a card for --force, but not do it now? | 15:16 |
| fwereade | dimitern, I mean do --force, because that's necessary for parity | 15:17 |
| dimitern | fwereade: ok, got you | 15:17 |
| fwereade | dimitern, but, with regret, don't do any of the other upgrade-charm stuff | 15:17 |
| dimitern | fwereade: fine by me :) so i'll move the card for bug 1032557 in todo | 15:18 |
| _mup_ | Bug #1032557: upgrade-charm should handle relation changes <upgrade-charm> <juju-core:In Progress by dimitern> < https://launchpad.net/bugs/1032557 > | 15:18 |
| fwereade | dimitern, cheers | 15:18 |
| rogpeppe | fwereade: what do you think about moving state.Endpoint into the charm package? | 15:19 |
| fwereade | rogpeppe, not *quite* right, because of ServiceName | 15:20 |
| rogpeppe | fwereade: we're pondering how api.Client.AddRelation should return details of the new relation | 15:20 |
| rogpeppe | fwereade: and i'm suggesting it should return the endpoints | 15:20 |
| rogpeppe | fwereade: but it can't use state.Endpoint | 15:20 |
| === teknico__ is now known as teknico_mobile | ||
| dimitern | fwereade: will you be off on friday? | 15:22 |
| fwereade | dimitern, doubt it :) | 15:22 |
| rogpeppe | fwereade: it could move into params, i suppose, but i don't like that | 15:22 |
| dimitern | fwereade: i'm swapping it | 15:22 |
| rogpeppe | fwereade: the other alternative is a duplicate definition | 15:22 |
| rogpeppe | fwereade: which i'm not greatly keen on either | 15:22 |
| fwereade | rogpeppe, how about everything *except* ServiceName as a charm.Endpoint, and embed a charm.Endpoint state.Endpoint? | 15:23 |
| fwereade | s/state./in state./ | 15:23 |
| fwereade | rogpeppe, then return... hmm... map[serviceName]charm.Endpoint? maybe? | 15:23 |
| rogpeppe | fwereade: i was just thinking the same | 15:23 |
| * fwereade peers into cloudy crystal ball for potential problems | 15:23 | |
| * fwereade can't see any obvious issues | 15:24 | |
| * fwereade wanted to do this a while ago, though, so he is biased | 15:25 | |
| rogpeppe | fwereade: did gustavo have issues with the idea? | 15:25 |
| fwereade | rogpeppe, (1) confusion over what "Endpoint" means (2) couldn't really see the point | 15:26 |
| fwereade | rogpeppe, so (2) is covered (and as a bonus state.RelationRole will move as well, yay); re (1) give it some thought, but I don't think it's a blocker | 15:27 |
| rogpeppe | fwereade: i *think* (1) is ok. | 15:28 |
| fwereade | rogpeppe, well, it is... no worse than the current situation re "relation" | 15:29 |
| fwereade | rogpeppe, in fact it maps perfectly | 15:29 |
| rogpeppe | fwereade: yeah. i think it's a useful ancillary data structure, even if nothing else in the charm package, erm, relates to it | 15:29 |
| fwereade | rogpeppe, [charm] relation, [charm] endpoint | 15:30 |
| fwereade | rogpeppe, well, hmm | 15:30 |
| fwereade | rogpeppe, it is actually *very* similar to a "charm relation" | 15:30 |
| fwereade | rogpeppe, maybe it actually is one | 15:31 |
| rogpeppe | fwereade: i'm just thinking that, yeah | 15:31 |
| rogpeppe | fwereade: although... it doesn't have the name | 15:31 |
| rogpeppe | fwereade: and Optional is a bit weird | 15:31 |
| fwereade | rogpeppe, but that already exists, and doesn't have the name because it lives in a map | 15:31 |
| fwereade | rogpeppe, or the role, similarly | 15:32 |
| rogpeppe | fwereade: Relation is really a specifier for a relation, and Endpoint represents an intantiated relation | 15:32 |
| rogpeppe | instantiated | 15:32 |
| fwereade | rogpeppe, interestingly, Optional and Limit probably *should* be in state.Endpoint | 15:33 |
| rogpeppe | fwereade: that's a good point actually | 15:33 |
| fwereade | rogpeppe, how would you feel about tacking name/role onto charm.Relation, and filling the fields when parsing a Meta? | 15:33 |
| fwereade | rogpeppe, I think that's the only place charm.Relations come from | 15:34 |
| rogpeppe | fwereade: that would mean implementing Meta.UnmarshalBSON and Meta.UnmarshalJSON i guess. but that's not a bad thing, probably. | 15:36 |
| fwereade | rogpeppe, I don't see a problem there but I may be myopic | 15:37 |
| rogpeppe | fwereade: it's just more code to write :-) | 15:37 |
| fwereade | rogpeppe, heh | 15:37 |
| fwereade | blast, sorry, bbiab | 15:38 |
| rogpeppe | fwereade: actually, i don't think we do need those Unmarshal methods | 15:41 |
| rogpeppe | fwereade: as long as we don't mind a bit of redundancy in the json | 15:42 |
| fwereade | rogpeppe, cool | 15:48 |
| rogpeppe | fwereade: and i'll add (in state): type Endpoint struct {ServiceName string; charm.Relation} | 15:49 |
| dimitern | there it is: upgrade-charm --force - https://codereview.appspot.com/7838054/ | 15:49 |
| fwereade | rogpeppe, sgtm | 15:50 |
| fwereade | dimitern, cheers | 15:50 |
| dimitern | fwereade: it's almost trivial, just the wording of the help text probably needs a closer look | 15:51 |
| fwereade | dimitern, yeah, that's the only thing that's exercising me ATM | 15:52 |
| dimitern | fwereade: :) I didn't know of that meaning of exercising - what's worrying you there? | 15:54 |
| fwereade | dimitern, trying to think of neater wording -- have some suggestions in the review | 15:56 |
| dimitern | fwereade: cool, thanks | 15:57 |
| dimitern | fwereade: surprisingly? probably unexpectedly is better | 15:58 |
| fwereade | dimitern, as you like, I had that to begin with and then changed it :) | 15:58 |
| dimitern | fwereade: well a surprise is usually associated with good things at first glance, while unexpected things are usually not good (i think) | 15:59 |
| dimitern | (while both can mean the opposite i agree) | 16:00 |
| fwereade | dimitern, I'm thinking of "surprising" as negative in a computery context | 16:00 |
| fwereade | dimitern, law of least surprise and all that | 16:00 |
| dimitern | fwereade: yeah, but you're more likely to see "unexpected error" than "surprising error" :) | 16:00 |
| fwereade | dimitern, I don't feel very strongly either way, go with unexpected | 16:01 |
| dimitern | fwereade: ok, thanks | 16:01 |
| dimitern | fwereade: are you feeling strongly against dropping the log.warning there? I think it's beneficial for analysing what went wrong with an upgrade through the logs more easily | 16:08 |
| dimitern | fwereade: s/dropping/leaving/ | 16:08 |
| dimitern | rogpeppe: if you have 5m for an easy review - https://codereview.appspot.com/7838054/ | 16:09 |
| rogpeppe | dimitern: quite busy at present trying to unblock teknico_mobile; will look in a bit | 16:10 |
| dimitern | rogpeppe: sure, no rush | 16:10 |
| * rogpeppe accumulated 7 things on the paper "to do" list this morning... | 16:10 | |
| fwereade | dimitern, logs in the CLI are generally just dropped | 16:12 |
| dimitern | fwereade: really? i was wondering why there are hardly any in the other commands | 16:12 |
| fwereade | dimitern, it's because we generally don't have any sane mechanism for getting output back to the user there | 16:13 |
| fwereade | dimitern, if you really wanted to, you could write something to ctx.Stderr | 16:13 |
| dimitern | fwereade: hmm.. it's a pity, but ok then - will drop it | 16:13 |
| fwereade | dimitern, but I think I'd need a bit more reason to actually do so | 16:13 |
| fwereade | dimitern, cheers | 16:14 |
| rogpeppe | fwereade: charm.Role rather than charm.RelationRole ? | 16:14 |
| fwereade | rogpeppe, let's stick with RelationRole, it's that little bit more explicit | 16:18 |
| rogpeppe | fwereade: ok | 16:18 |
| rogpeppe | fwereade: changing all those Endpoint literals is a right pain... | 16:53 |
| fwereade | rogpeppe, agreed | 16:53 |
| rogpeppe | fwereade: i think it looks nicer with field names though | 16:53 |
| fwereade | rogpeppe, almost certainly | 16:53 |
| fwereade | dimitern, I'm starting to feel more strongly about "surprisingly" | 16:57 |
| fwereade | dimitern, "unexpectedly" doesn't quite seem to cover the "lol I just changed the file you were editing" scenarios I worry about with --force | 16:58 |
| rogpeppe | fwereade: FWIW i almost never see "surprising" in an error message context | 16:59 |
| rogpeppe | fwereade: "unexpected" is usual, i think | 16:59 |
| fwereade | rogpeppe, it's a warning in documentation, not an error message | 17:01 |
| rogpeppe | fwereade: ah | 17:01 |
| rogpeppe | fwereade: forgive me for jumping totally without context! | 17:01 |
| fwereade | rogpeppe, no worries :) | 17:01 |
| === bac_ is now known as bac | ||
| dimitern | rogpeppe: exactly my point - unexpected is more common | 17:10 |
| dimitern | fwereade: but if you lean more towards surprisingly, I'll change it, np | 17:11 |
| fwereade | dimitern, I've come to peace with it, suggested a slight tweak in the review and LGTM | 17:11 |
| dimitern | fwereade: thanks | 17:12 |
| dimitern | i'll submit it once i have one more LGTM | 17:14 |
| fwereade | jam, fwiw I think sync-tools is awesome, but I haven't finished the review and I'm afraid I have to go for now | 17:15 |
| jam | fwereade: np, I think ian was going to look at it in his morning | 17:15 |
| jam | thanks | 17:15 |
| rogpeppe | fwereade: i'm wondering if i should go ahead with the global StartSync thing. i know it would knock 10s off the api tests for one. | 17:20 |
| fwereade | rogpeppe, alternative to global StartSync | 17:21 |
| fwereade | rogpeppe, testing method on Watcher that set the refresh period to 50ms? | 17:22 |
| fwereade | rogpeppe, most of all I'd like to just be able to sync the actual States we care about, but I can accept that that's probably too hard | 17:22 |
| rogpeppe | fwereade: there are various tests that fail when you do that, but i wanna fix those anyway | 17:23 |
| rogpeppe | fwereade: that is a reasonable alternative. | 17:23 |
| fwereade | rogpeppe, (the refresh period is package global, right?) | 17:24 |
| rogpeppe | fwereade: yes | 17:24 |
| fwereade | rogpeppe, just feels a touch less evil to me | 17:24 |
| rogpeppe | fwereade: it's certainly less code | 17:24 |
| fwereade | rogpeppe, that too ;) | 17:24 |
| rogpeppe | fwereade: it's quite amusing setting the sync period to 50ms and see how many tests fail. i recommend it for a laugh. | 17:25 |
| fwereade | rogpeppe, I'll save my bitter laughs for tomorrow, I think, I really must go :) | 17:25 |
| fwereade | gn all | 17:25 |
| rogpeppe | fwereade: night night | 17:25 |
| rogpeppe | dimitern: a large CL, but mostly mechanical: https://codereview.appspot.com/8055044/ | 17:28 |
| dimitern | rogpeppe: will take a look shortly | 17:35 |
| rogpeppe | dimitern: thanks! | 17:35 |
| rogpeppe | trivial deletion-only CL, anyone? https://codereview.appspot.com/7925045 | 17:43 |
| dimitern | rogpeppe: while i'm on the first one, would you have a look at my cl? | 17:47 |
| dimitern | https://codereview.appspot.com/7838054/ | 17:48 |
| rogpeppe | dimitern: on it | 17:48 |
| dimitern | rogpeppe: cheers | 17:49 |
| rogpeppe | dimitern: LGTM | 17:50 |
| dimitern | rogpeppe: awesome, i can submit it later then, tyvm | 17:50 |
| dimitern | rogpeppe: you got 2 reviews | 17:59 |
| rogpeppe | dimitern: thanks! | 17:59 |
| rogpeppe | i'm off for the day. g'night all. | 18:25 |
| thumper | morning | 19:39 |
| wallyworld | fwereade_: hi | 20:48 |
| thumper | morning wallyworld | 21:05 |
| wallyworld | gday | 21:05 |
| wallyworld | how's things? | 21:06 |
| thumper | wallyworld: good | 21:08 |
| thumper | wallyworld: waiting for dave to turn up, and deciding to learn about our upgrade methods while I wait | 21:08 |
| wallyworld | juju upgrade? | 21:09 |
| thumper | wallyworld: as they are the only areas I need to fix for my version.Current.die.die.die branch | 21:09 |
| thumper | aye | 21:09 |
| wallyworld | can't wait for that branch to land | 21:09 |
| thumper | wallyworld: feel like a review? | 21:10 |
| wallyworld | sure, why not | 21:11 |
| thumper | https://codereview.appspot.com/7845048/ | 21:11 |
| wallyworld | thumper: just read the cover letter - what happens to those existing machines that are not used anymore - do they get cleaned up eventually? | 21:13 |
| thumper | not automagically afaik | 21:13 |
| thumper | but I'm not sure | 21:13 |
| thumper | maybe | 21:13 |
| thumper | it certainly requires some thought | 21:13 |
| thumper | or looking into | 21:13 |
| thumper | but I'm not sure actually | 21:13 |
| thumper | there is always destroy-machine | 21:14 |
| thumper | I think we should have a timeout somewhere | 21:14 |
| wallyworld | you'd like to think that juju would "Do the right thing" and not leave stuff lying around | 21:14 |
| thumper | that says if a machine is lying idle for greater than X minutes | 21:14 |
| thumper | kill it | 21:14 |
| thumper | I agree, DTRT™ is good | 21:14 |
| thumper | wallyworld: but I have a feeling that is another hunk of work | 21:15 |
| wallyworld | i think we are at the stage where stuff works, but there's a lot of rough edges to get rid of to productise it | 21:15 |
| * thumper nods | 21:15 | |
| thumper | worth adding bug reports (good ones) | 21:15 |
| thumper | for those rough edges | 21:16 |
| thumper | as I don't think it is a huge amount of work to polish them off | 21:16 |
| wallyworld | still seems like a fair bit of must do stuff for 13.04 though | 21:16 |
| thumper | wallyworld: I don't think it will be polished for 13.04 | 21:23 |
| wallyworld | me either | 21:23 |
| thumper | wallyworld: mostly functional I think is what we are heading for | 21:23 |
| wallyworld | thumper: i don't fully grok the addMachine/addMachineOps logic, but the branch looks nice, and the tests are good | 21:26 |
| wallyworld | so i'll +1 it for what that's worth | 21:26 |
| thumper | :) | 21:26 |
| thumper | you get your name in the commit | 21:27 |
| thumper | I'll await dave :) | 21:27 |
| thumper | wallyworld: I've spent this week learning about the mongo transaction ops | 21:27 |
| wallyworld | yeah, best to get an informed opinion :-) | 21:27 |
| wallyworld | i really don't like how we write directly to mongo | 21:27 |
| wallyworld | and not go through a middle tier service | 21:28 |
| thumper | wallyworld: well, we are moving to that with the API | 21:29 |
| thumper | wallyworld: so, it is happening | 21:29 |
| wallyworld | yeah, that's good. better late than never i guess | 21:29 |
| thumper | morning davecheney | 22:15 |
| thumper | davecheney: looking for another +1 on https://codereview.appspot.com/7845048/ as wallyworld isn't entirely confident | 22:15 |
| davecheney | okie dokes | 22:16 |
| davecheney | thumper: still reviewing | 22:28 |
| thumper | davecheney: :) | 22:28 |
| davecheney | got sidetracked by some rage | 22:28 |
| thumper | davecheney: fingers crossed, we can get this landed today | 22:31 |
| davecheney | thumper: going to be one more round of changes I think | 22:44 |
| davecheney | just for the turd polishing | 22:44 |
| thumper | ok | 22:45 |
| * thumper is happy to learn | 22:45 | |
| davecheney | there is one thing in there that will make you throw a chair | 22:47 |
| davecheney | soz | 22:47 |
| thumper | oh? | 22:47 |
| thumper | why? | 22:47 |
| davecheney | https://codereview.appspot.com/7845048/#msg6 | 22:47 |
| thumper | davecheney: with regard to the FitsTypeOf... | 22:53 |
| thumper | davecheney: I think it is a flawed test | 22:53 |
| thumper | davecheney: it is potentially also possible for different providers to use a different policy | 22:53 |
| thumper | davecheney: so specifying a result there is wrong | 22:53 |
| thumper | davecheney: but the method returns a particular type | 22:54 |
| davecheney | thumper: the thing i saw was sometimes we use an Equals test, othertimes FitsTypeOf | 22:54 |
| thumper | davecheney: so saying "FitsTypeOf" is really just saying, does the language work | 22:54 |
| davecheney | i think both are valid, but is it possible to stick to one for all the cases ? | 22:54 |
| thumper | davecheney: I think the FitsTypeOf is wrong, and happy to remove it | 22:54 |
| davecheney | +1 | 22:54 |
| davecheney | i was just after consistency | 22:55 |
| thumper | and with respect to the typed constants, I was just following convention | 22:55 |
| thumper | I hold no strong opinions there | 22:55 |
| davecheney | but i'll take that argument as well | 22:55 |
| davecheney | fair enough re typed constants | 22:55 |
| davecheney | you don't need to get bogged down in that | 22:55 |
| davecheney | just something to remember | 22:55 |
| thumper | worst constant ever. state.MaxUnitsPerHost ? | 22:55 |
| thumper | I don't get that comment | 22:55 |
| davecheney | yeah, that current constant is terrible | 22:55 |
| davecheney | JobHostUnits ? | 22:55 |
| davecheney | something ? | 22:55 |
| davecheney | state.MaxUnitsPerHost is my suggestion | 22:56 |
| thumper | oh, no.. this already exists | 22:56 |
| thumper | this is a job type for the machine | 22:56 |
| thumper | saying it hosts units | 22:56 |
| davecheney | right, so still the worst constnt ever | 22:57 |
| thumper | anyway, I'm off to the gym now | 22:57 |
| davecheney | i had no idea what it meant | 22:57 |
| davecheney | roger | 22:57 |
| thumper | davecheney: I agree it is shitty, but I'm keeping it for now :) | 22:57 |
| davecheney | fair enough | 22:58 |
| davecheney | LGTM. submit and go to the gym | 22:58 |
| === wedgwood is now known as wedgwood_away | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!