| menn0 | cmars: done with the review | 00:06 |
|---|---|---|
| cmars | menn0, thanks! | 00:06 |
| menn0 | davecheney: you have a ship it from me although I see you already had Tim's approval on GH | 00:09 |
| === Spads_ is now known as Spads | ||
| === kadams54 is now known as kadams54-away | ||
| thumper | davecheney: I edited that message before it emailed... | 00:39 |
| thumper | I missed out a word | 00:39 |
| thumper | davecheney: what I mean is "all good, land it!" | 00:40 |
| davecheney | thumper: cool | 00:46 |
| davecheney | i'll have to do a big branch to fix the callers of this type | 00:46 |
| davecheney | there are a lot | 00:46 |
| davecheney | and a lot of tests panic | 00:46 |
| davecheney | now | 00:46 |
| davecheney | that doesn't mean they are wrong | 00:46 |
| davecheney | but they could be wrong | 00:47 |
| davecheney | it depens on the state of the set passed in | 00:47 |
| davecheney | for the record, this showed up doing a code review with ericsnow | 00:47 |
| davecheney | i suspect he'd figured this out | 00:47 |
| davecheney | and had worked around the problem | 00:47 |
| davecheney | which prompted me to look a little closer | 00:47 |
| * thumper nods | 00:54 | |
| === fuzzy_ is now known as Ponyo | ||
| davecheney | menn0: https://github.com/juju/juju/pull/1108 | 01:17 |
| davecheney | ^ this is the kind of cleanup that suggests this refactor is on the right track | 01:17 |
| menn0 | davecheney: just finishing another review | 01:18 |
| davecheney | s'ok | 01:18 |
| davecheney | no hurry | 01:18 |
| davecheney | this will be the last one I do on this today | 01:18 |
| davecheney | need to do cleanup from the set.Strings change | 01:18 |
| menn0 | anastasiamac_: just reviewed 407 for you | 01:35 |
| anastasiamac_ | menn0: thx!!! | 01:36 |
| LinStatSDR | Hello :D | 01:37 |
| menn0 | davecheney: that change looks good and paves the way for allowing the watcher to work in a multi-env state server | 01:44 |
| menn0 | davecheney: how do we feel though about types defined in state being used as part of the API? | 01:44 |
| thumper | davecheney: was the top level juju file there just to allow moving things around? | 01:46 |
| thumper | davecheney: my first thought was "WTH is that doing there?" | 01:46 |
| * thumper is +1 on the fix | 01:46 | |
| menn0 | davecheney: with your changes, Delta is defined in state/multiwatcher and is used by the multiwatcher internally, but is also used for the allwatcher API | 01:47 |
| menn0 | davecheney: ultimately there might need to be a related struct defined in the API that returns Deltas | 01:48 |
| menn0 | davecheney: that might happen when we do the multi-env support for the multiwatcher but I think we might want to change state/megawatcher/Delta anyway | 01:49 |
| davecheney | menn0: api/ or apiserver/ ? | 01:56 |
| davecheney | apiserver, i'm fine with | 01:56 |
| davecheney | api, not so much | 01:57 |
| davecheney | that's at the end of the list | 01:57 |
| davecheney | it will mean params has to translate between state/multiwatcher types and apiserver/params | 01:57 |
| menn0 | well right now Delta is used in api/ | 01:58 |
| davecheney | yeah | 01:58 |
| davecheney | it's not awesome | 01:58 |
| menn0 | api/allwatcher.go | 01:58 |
| davecheney | but it is what it is | 01:58 |
| menn0 | I think ultimately we want apiserver to have its own type for this with some translation done there | 01:58 |
| davecheney | yup | 01:59 |
| davecheney | it's on the list | 01:59 |
| menn0 | especially if Delta in allwatcher is going to grow some extra (internal-only) bits, which i suspect it will with multi-env suport | 01:59 |
| menn0 | ok, as long as it's on the roadmap | 01:59 |
| menn0 | davecheney: I think you could have easily moved MachineInfo, ServiceInfo etc etc in this PR as well | 02:00 |
| thumper | ugh | 02:01 |
| * thumper spotted shitty code he wrote some time ago | 02:01 | |
| menn0 | oh FFS, will you people stop getting shit done :) | 02:04 |
| menn0 | more things to review | 02:04 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| === kadams54 is now known as kadams54-away | ||
| waigani | menn0: talking of which http://reviews.vapour.ws/r/409/ ;) | 02:13 |
| menn0 | waigani: that's what prompted my outburst :) | 02:15 |
| waigani | menn0: LOL, your welcome :) | 02:15 |
| menn0 | waigani: I have been reviewing non-stop all day | 02:15 |
| waigani | menn0: can you see the matrix yet? | 02:17 |
| thumper | menn0: https://github.com/juju/testing/pull/37/files | 02:39 |
| thumper | :) | 02:39 |
| menn0 | thumper: looking in a sec | 02:45 |
| menn0 | waigani: any upgrade steps should now be targeted to 1.22. I just realised you've put some against 1.21. That needs to get fixed. | 02:46 |
| waigani | menn0: ah yeah right | 02:47 |
| menn0 | waigani: have any changed merged already where this isn't right? | 02:47 |
| waigani | menn0: so settings was the last upgrade step I merged, that went in on Monday | 02:48 |
| menn0 | yeah, they need to be fixed too | 02:50 |
| menn0 | that was after the 1.21 cut | 02:50 |
| menn0 | settings and settingRefs | 02:50 |
| menn0 | waigani: ^ | 02:50 |
| waigani | menn0: yep, I'll move them | 02:50 |
| menn0 | thumper: done. | 02:55 |
| thumper | ta | 02:56 |
| thumper | geez | 02:56 |
| thumper | nit pick much | 02:56 |
| thumper | but... | 02:57 |
| thumper | good review | 02:57 |
| * thumper will tweak | 02:57 | |
| thumper | menn0: here is another https://github.com/juju/errors/pull/12 | 02:58 |
| menn0 | thumper: that errors change looks fine although it appears to be used in a number of places in Juju. Are you going to change those? | 03:10 |
| thumper | menn0: yup | 03:10 |
| thumper | right now | 03:10 |
| menn0 | thumper: "it" being LoggedErrorf of course | 03:10 |
| thumper | yeah, I knew what you ment | 03:10 |
| thumper | meant | 03:10 |
| thumper | which rhymes with bent | 03:10 |
| thumper | got I hate the english language | 03:10 |
| waigani | menn0: done. | 03:12 |
| thumper | menn0: although I can't change juju until the branch has landed (well, make it work anyway) | 03:14 |
| menn0 | thumper: doesn't it make slightly more sense to change juju first? | 03:14 |
| thumper | menn0: umm... sure | 03:15 |
| menn0 | wallyworld_: do we need to poke someone to get either https://github.com/juju/juju/pull/993 or https://github.com/juju/juju/pull/689 in? | 03:56 |
| menn0 | wallyworld_: seems like an important fix to get in that hasn't due to poor handling of the review process | 03:56 |
| wallyworld_ | menn0: oops, yeah, i'll take a look | 03:57 |
| menn0 | wallyworld_: cheers | 03:59 |
| wallyworld_ | menn0: only trouble is that there's a dependent goose mp on lp that needs fixing, so i'll have to follow up with the author of that mp | 04:02 |
| menn0 | wallyworld_: right. just wanted to point the PRs out since they didn't seem to be moving. | 04:04 |
| wallyworld_ | yep, np, thanks, they had been forgotten about for sure | 04:04 |
| thumper | fuckity fuck fuck | 04:16 |
| * thumper is fixing an intermittent error in the actions time checking | 04:18 | |
| thumper | menn0: https://github.com/juju/juju/pull/1110 | 04:47 |
| thumper | which is http://reviews.vapour.ws/r/410/ | 04:49 |
| === liam_ is now known as Guest46061 | ||
| === urulama___ is now known as urulama | ||
| === Spads_ is now known as Spads | ||
| mattyw | morning all | 09:00 |
| jam | TheMue: standup ? | 10:02 |
| perrito666 | morning-ish | 10:06 |
| voidspace | jam: it occurs to me that 404 might be different | 10:39 |
| jam | k | 10:39 |
| jam | voidspace: how so | 10:39 |
| voidspace | jam: I think if we get a 404 maas might bomb out and not stop the other instances | 10:39 |
| voidspace | jam: with a 409 it collects all the errors | 10:39 |
| voidspace | jam: let me check | 10:39 |
| jam | voidspace: so if you ask to stop 10 things, and *one* is a 404, it bombs out early.... :( | 10:40 |
| voidspace | jam: yep, confirmed | 10:40 |
| voidspace | jam: damn, my branch was ready | 10:40 |
| voidspace | https://github.com/voidspace/juju/compare/maas-404 | 10:40 |
| voidspace | :-( | 10:40 |
| voidspace | but in the release bulk call it first checks that the number of found nodes matches the number requested | 10:41 |
| jam | voidspace: k... do we have a way to know which item was causing the 404 ? | 10:41 |
| voidspace | and raises a PermissionDenied | 10:41 |
| voidspace | jam: parsing the message... | 10:41 |
| jam | voidspace: my concern is that if you got your environment into that situation, how do you get out of it? | 10:41 |
| jam | voidspace: I guess if this is "destroy-environment --force" then its ok because we're only listng the MaaS server anyway | 10:41 |
| voidspace | jam: http://pastebin.ubuntu.com/8961592/ | 10:42 |
| jam | and it won't have it | 10:42 |
| jam | voidspace: so is that 404 or is that PermissionDenied ? | 10:42 |
| voidspace | jam: what do you mean by "only listing MaaS server anyway" | 10:42 |
| jam | maybe they translate PermDenied to 404 | 10:42 |
| voidspace | maybe, but any missing for any reason will show up like that | 10:43 |
| jam | voidspace: it *should* be 403 Perm | 10:43 |
| voidspace | I think in that code they're assuming that if a node is not found it's because of permission | 10:43 |
| jam | voidspace: its a fair point to not distiguish from 403 and 404 | 10:43 |
| voidspace | the release single call (api) does an object_or_404 | 10:43 |
| jam | since otherwise you can leak the existence (or not) of something | 10:43 |
| jam | that you don't otherwise have permission to see | 10:43 |
| voidspace | right | 10:43 |
| voidspace | it's an incompatibility between those two calls though | 10:44 |
| voidspace | so handling 404 in StopInstances is pointless | 10:44 |
| jam | so there is still a small concern on the "destroy-environment --no-force" case | 10:44 |
| voidspace | The single call /api/node/<system-id>/op=release can 404 | 10:45 |
| jam | which is that the API server still wants to stop an instance that it knew about | 10:45 |
| voidspace | if one node has been deleted but the rest are present, then the call to StopInstances will fail without releasing any | 10:45 |
| jam | voidspace: it sounds bug worthy that "single node release == 404" but "bulk node release == 403" | 10:46 |
| voidspace | jam: I'll raise it | 10:46 |
| voidspace | so, for a 403 we could loop over the system ids and stop the nodes individually - or we could parse the error message and retry without the missing nodes | 10:47 |
| voidspace | parsing the error message sounds fragile | 10:47 |
| jam | voidspace: it does... my concern is that we are asking the API server to tear down nodes it knows about, and not giving any way for it to handle nodes that are already gone. | 10:48 |
| voidspace | jam: for a 403 I can retry removing the instances individually and ignore 403/404/409 | 10:49 |
| voidspace | just logging as a Warning | 10:50 |
| jam | voidspace: so, not being able to "destroy-environment" because a machine was decommisioned in maas does sound like a poor user experiience failure mode, but it could be considered a different bug | 10:51 |
| voidspace | so either fix it now, or right this discussion up as a new bug | 10:51 |
| voidspace | it doesn't seem to have hit anyone yet | 10:51 |
| voidspace | and it's easy enough to fix if we need to | 10:51 |
| voidspace | the sympton will be destroy environment failing with a 403 error and not releasing any nodes | 10:52 |
| jam | voidspace: I feel like "we know this is an easy to fix bug, but we won't address it now" isn't great. I'm wondering if we need to involve more people or if we've discussed it sufficiently to be confident in the fix | 10:52 |
| voidspace | jam: well, I'm pretty confident of my reading of the maas code - it seems straightforward | 10:54 |
| voidspace | ah, wait... | 10:54 |
| voidspace | hah | 10:54 |
| voidspace | jam: in fact... there's a call to self._check_system_ids_exist first | 10:54 |
| voidspace | jam: that raises MAASAPIBadRequest if nodes don't exist | 10:54 |
| voidspace | jam: so the PermissionDenied really does mean that the nodes failed to be found because of the permissions problem | 10:55 |
| jam | voidspace: right, but we still need to handle MAASAPIBadRequest | 10:55 |
| voidspace | and BadRequest isn't a 404 either | 10:55 |
| voidspace | 400 I assume | 10:55 |
| voidspace | so same problem, different number | 10:56 |
| voidspace | and should we handle 403 | 10:56 |
| voidspace | the node was released and given to someone else | 10:56 |
| jam | voidspace: it sounds like we should test to see if we can actually make it happen, and then do as you say "kill in bulk, then go 1-by-1" | 10:56 |
| voidspace | same problem as with a missing node - now we can't destroy environment | 10:56 |
| voidspace | jam: ok, I'll try deleting the node and check that we fail to destroy environment with a 400 | 10:57 |
| voidspace | and treat 400/403/404 in the same way | 10:57 |
| jam | voidspace: note that I would expect "juju destroy-environment" to maybe complain on the API server side, but the client-side cleanup to be fine because it just lists the maas provider | 10:57 |
| jam | (though there is still potentially a race) | 10:57 |
| voidspace | jam: so you would want the --no-force to fail - or just warn? | 10:58 |
| jam | voidspace: if the failure is that the node is already gone, destroy-environment should succeed | 10:58 |
| voidspace | right | 10:58 |
| voidspace | so what do you mean by "complain"? | 10:59 |
| jam | voidspace: I mean that today | 10:59 |
| jam | the "soft shutdown" is likely to fail | 10:59 |
| voidspace | ah, I see | 10:59 |
| voidspace | right | 10:59 |
| voidspace | it did for 409 | 10:59 |
| jam | but the hard shutdown doesn't know anything about the missing node | 10:59 |
| voidspace | hence the bug - and even with today's code it should die with a 400 | 10:59 |
| voidspace | right, missing nodes shouldn't be a problem for --force | 11:00 |
| voidspace | understood | 11:00 |
| voidspace | ok, I'll try it and see what happens | 11:00 |
| TheMue | morning and sorry for missing standup, had to wait longer at the dentist as planned. thankfully e'thing OK as usual. | 11:16 |
| TheMue | but at least my new setup maas controller used this time to download 135 boot images *phew* | 11:16 |
| TheMue | dimitern: the described configuration of the interfaces worked btw | 11:19 |
| dimitern | TheMue, great, so your VM networking is all set up correctly now? | 11:20 |
| TheMue | dimitern: it looks so. I will now create the second instance for PXE on the private network of the controller | 11:21 |
| jam | TheMue: 135?... | 11:31 |
| TheMue | jam: yeah, impressive number, even if the size of my virtual disk lets expect a lower number | 11:32 |
| TheMue | jam: but thats what my list her on the screen shows | 11:33 |
| TheMue | jam: they are always with the purposes "comissioning", "install", "xinstall". so maybe they share a lot of resources? | 11:34 |
| perrito666 | and now, a bit more awake, hello everybody | 11:34 |
| TheMue | perrito666: hello | 11:34 |
| TheMue | jam: also several sub-architectures per arch | 11:35 |
| gnuoy | Hi, I'm trying to build juju on windows. "go get -d -v github.com/juju/juju/..." seems happy enough but "go install -v github.com/juju/juju/..." results in ... watcher.go:121: undefined: "github.com/juju/errors".LoggedErrorf. Is this an issue with missing dependancies? | 11:35 |
| perrito666 | gnuoy: take a look at GOPATH/src/github.com/juju/errors/errortypes.go | 11:42 |
| perrito666 | that should be defined there | 11:42 |
| perrito666 | mm or try running godeps -u dependencies.tsv on the juju/juju folder | 11:42 |
| fwereade | wallyworld, hey, don't suppose you're around? wanted to chat quickly about the changes we need to make to distinguish between waiting/blocked | 11:43 |
| wallyworld | fwereade: hey | 11:43 |
| wallyworld | hangout? | 11:43 |
| gnuoy | perrito666, this godeps you speak of, should it be bundled with go for windows ? | 11:43 |
| perrito666 | gnuoy: nope, go get launchpad.net/godeps/... | 11:44 |
| fwereade | wallyworld, joining ian-william | 11:45 |
| gnuoy | perrito666, fantastic, I think there may be light at the end of the tunnel | 11:46 |
| perrito666 | :D | 11:46 |
| TheMue | jam: digged into /var/lib/maas/boot-resources, the files in the subarchs are hardlinked | 11:47 |
| voidspace | how are we merging to 1.21 - git cherry-pick? | 12:47 |
| fwereade | dimitern, would you pop into #jujuskunkworks please? | 14:21 |
| dimitern | fwereade, ok, just a sec | 14:25 |
| wwitzel3 | fwereade: ping, need to speak to you about charm level constraints at some point | 14:28 |
| wwitzel3 | rick_h_: ping ^ | 14:28 |
| rick_h_ | wwitzel3: howdy | 14:28 |
| rick_h_ | wwitzel3: what's up? | 14:29 |
| wwitzel3 | rick_h_: hey, I'm going to be doing the work around min juju version | 14:29 |
| wwitzel3 | rick_h_: was told you might be a good person to sync up with before I start | 14:29 |
| rick_h_ | wwitzel3: ok, what's the latest plan? | 14:29 |
| wwitzel3 | rick_h_: well .. I think I'm making right now ;) | 14:29 |
| rick_h_ | wwitzel3: heh, I'm a chief complainer so happy to help :) | 14:30 |
| rick_h_ | wwitzel3: if you can deal with coffee shop background I'm free to chat for the next hour | 14:30 |
| rick_h_ | wwitzel3: or if you've got a doc/notes you'd like looked over I'm happy to | 14:30 |
| wwitzel3 | rick_h_: awesome, lets do it .. nope, no notes yet, some might exist, but I haven't seen any yet. | 14:31 |
| wwitzel3 | rick_h_: https://plus.google.com/hangouts/_/canonical.com/moonstone?authuser=1 | 14:31 |
| rick_h_ | wwitzel3: rgr | 14:32 |
| rick_h_ | wwitzel3: in there | 14:32 |
| natefinch | fwereade: ping | 14:33 |
| fwereade | natefinch, sorry, omw | 14:34 |
| perrito666 | hey, does anyone know what are the specs of the nucs marco used for this? http://marcoceppi.com/2014/06/deploying-openstack-with-just-two-machines/ | 14:53 |
| rick_h_ | perrito666: just ordered two more the other day to expand our maas, I'll get you a link | 14:54 |
| rick_h_ | perrito666: http://www.newegg.com/Product/Product.aspx?Item=N82E16856102035 | 14:54 |
| perrito666 | rick_h_: tx, I am trying to make a very similar experiment by mounting 4 physical nodes with 600U$D or less (obviously i will not be using nucs) | 14:55 |
| rick_h_ | perrito666: gotcha, yea we setup http://maas.jujugui.org/MAAS using 3 (will be 5 now later today) nucs on a switch and such. | 14:56 |
| rick_h_ | perrito666: look forward to seeing what you find to use. | 14:56 |
| perrito666 | rick_h_: hoy much ram are you actually packing into those? says up to 16 but I am not sure you are actually putting all that in | 14:56 |
| rick_h_ | I used 8 and 128gb HD | 14:56 |
| rick_h_ | because we lxc/container with them | 14:56 |
| rick_h_ | http://www.newegg.com/Product/Product.aspx?Item=N82E16820191523 and http://www.newegg.com/Product/Product.aspx?Item=N82E16820148697 | 14:56 |
| rick_h_ | perrito666: I think the orange boxes use the full 16 | 14:57 |
| TheMue | yeeehaw, found the correct config to boot my vms in my vmaas | 14:57 |
| rick_h_ | perrito666: but for our needs the 8's been ok mostly QA'ing stuff and doing some small testing. | 14:57 |
| perrito666 | rick_h_: I am trying to go with http://www.asrock.com/mb/Intel/H61M-VG3/ + http://ark.intel.com/products/71073/Intel-Celeron-Processor-G1620-2M-Cache-2_70-GHz | 14:58 |
| rick_h_ | perrito666: does it have any sort of ipmi/amt control? | 14:58 |
| perrito666 | rick_h_: I am trying to figure out that :) I am pretty sure one of the boards of that family support ipmi | 14:59 |
| wwitzel3 | perrito666: you will hate life the moment you try to put any load on that Celeron | 14:59 |
| perrito666 | wwitzel3: I hate life in general | 14:59 |
| wwitzel3 | lol | 14:59 |
| perrito666 | wwitzel3: what kind of load are you talking about? | 15:00 |
| rick_h_ | perrito666: good luck, I'm not seeing any sort of power control on that one but yea might be others. | 15:00 |
| perrito666 | rick_h_: true, that is the first model available here, but I am told I can get pretty much any model from that company | 15:00 |
| perrito666 | and they have decent ones | 15:00 |
| perrito666 | wwitzel3: I was honestly thinking on nothing more than have a maas with 4 of those and use it to test my juju on maas | 15:01 |
| perrito666 | locally | 15:01 |
| rick_h_ | perrito666: gotcha, cool. Well if you get something let me know. We're heading to 4 nodes in our maas but I'd like to have more down the road | 15:01 |
| rick_h_ | and if I could get them for less than $500 each that'd be nice | 15:01 |
| perrito666 | rick_h_: well I will most likely get a howto of this, the only thing stopping me is that hardware here has, from scratch a 50% import tax :p which kills my under 600 mark pretty easily | 15:02 |
| rick_h_ | perrito666: ah time for a US sprint :) | 15:02 |
| perrito666 | rick_h_: well 600/4 is a good price :p | 15:02 |
| rick_h_ | perrito666: definitely | 15:03 |
| perrito666 | rick_h_: I am pretty sure that if I pack 4 pcs with me I will have hard time convincing customs that it is for personal use :p | 15:03 |
| rick_h_ | psh, you should see the stuff these guys travel around with. | 15:03 |
| rick_h_ | I know one guy went to a sprint with 5 laptops for folks he got them for | 15:04 |
| ericsnow | natefinch, perrito666, wwitzel3: standup? | 15:04 |
| wwitzel3 | ericsnow: tosca call atm | 15:04 |
| perrito666 | oh my cal says its in one hour | 15:04 |
| ericsnow | natefinch, perrito666, wwitzel3: duh, see ya | 15:04 |
| perrito666 | rick_h_: our customs people are a bit sensitive these days :p | 15:04 |
| perrito666 | rick_h_: http://www.newegg.com/Product/Product.aspx?Item=N82E16813157419 | 15:06 |
| perrito666 | beats the nuc by quite far | 15:06 |
| perrito666 | :p | 15:06 |
| rick_h_ | perrito666: so $130 and needs a cpu | 15:07 |
| rick_h_ | perrito666: so not sure on 'quite far' not sure how much a cpu/cooler for that goes for | 15:07 |
| rick_h_ | perrito666: but cool | 15:07 |
| perrito666 | I understood cpu was included | 15:07 |
| perrito666 | rick_h_: anyway yest, that is the server line :) I am looking for one in the consumer end | 15:08 |
| rick_h_ | oh is it? cool guess it is http://ark.intel.com/products/77982/Intel-Atom-Processor-C2550-2M-Cache-2_40-GHz | 15:09 |
| rick_h_ | perrito666: cool, if maas supports that ipmi tempted to try it out. | 15:10 |
| perrito666 | rick_h_: yup, cpu included supports a lot of ram, packed with 3 eths, it is quite production ready for the price | 15:10 |
| perrito666 | also 8 cores, pretty nice machine | 15:11 |
| rick_h_ | perrito666: yea, might be worth an email to the maas list to see if anyone knows anything about it | 15:11 |
| perrito666 | rick_h_: yup, at least for testing purposes they seem quite useful given the price tag | 15:11 |
| perrito666 | meh, they have a lot of ipmi enabled motherboards, none consumer level | 15:23 |
| voidspace | katco`: ping | 15:55 |
| voidspace | anyone want an easy-ish review for 1.21? | 15:56 |
| voidspace | http://reviews.vapour.ws/r/413/ | 15:56 |
| perrito666 | natefinch: ericsnow wwitzel3 can we push the standup an hour more, I need to rush to look for my wife that is a bit ill at her job | 15:56 |
| jw4 | fwereade, dimitern, thumper, et. al. state.nowToTheSecond() is used in a few places to reduce the precision of time.Now() to the level of Seconds | 15:57 |
| ericsnow | perrito666: fine with me | 15:57 |
| wwitzel3 | perrito666: sure, hope she feels better | 15:57 |
| jw4 | however that function uses time.Round(Second), and I think it would be more reliable to use time.Truncate(Second) | 15:57 |
| fwereade | jw4, +1 | 15:58 |
| jw4 | fwereade: ta | 15:58 |
| natefinch | perrito666: sure thing | 15:59 |
| natefinch | perrito666, ericsnow, wwitzel3: I'm going to push it 2 hours because 1 hour is no good for me. | 16:00 |
| ericsnow | natefinch: k | 16:00 |
| bodie_ | hey ericsnow, I'm trying to understand some of the cmd/juju/backups code, since I'm also implementing a new supercommand | 16:04 |
| ericsnow | bodie_: sure | 16:04 |
| bodie_ | ericsnow, so I'm getting "client.Close undefined (type APIClient has no field or method Close)" | 16:04 |
| ericsnow | bodie_: FWIW, I mostly followed the example of the new User super command :) | 16:05 |
| bodie_ | hmm, okay | 16:05 |
| bodie_ | I think my Client type actually is composed from two interfaces which both require Close() | 16:05 |
| bodie_ | so that might be the issue | 16:05 |
| ericsnow | bodie_: what is APIClient in this case? | 16:05 |
| bodie_ | I'll take a look at the cmd/juju/user code | 16:05 |
| ericsnow | bodie_: backups has its own API client type | 16:06 |
| bodie_ | ericsnow, it's like your backups APIClient interface requiring the methods from the api type | 16:06 |
| bodie_ | yeah, we have an Actions api client type | 16:06 |
| ericsnow | bodie_: nice | 16:06 |
| bodie_ | (api/actions/client.go) | 16:06 |
| bodie_ | the implementation is a little different though, since we don't require SendHTTPRequest | 16:07 |
| voidspace | sinzui: I'm one of the culprits, sorry | 16:07 |
| voidspace | currently waiting for a review of the backport of the fix | 16:07 |
| voidspace | sinzui: (marking bugs as fix committed but not backporting I mean) | 16:07 |
| voidspace | dimitern: you still around? | 16:07 |
| voidspace | dimitern: this is a 1.21 bug fix that could do with a review: http://reviews.vapour.ws/r/413/ | 16:08 |
| ericsnow | bodie_: I don't see a Close method on the actions Client | 16:08 |
| ericsnow | bodie_: backups gets it from api.State | 16:08 |
| dimitern | voidspace, looking | 16:08 |
| ericsnow | bodie_: so I expect you don't need to call Close on your client | 16:08 |
| voidspace | dimitern: thanks | 16:09 |
| sinzui | voidspace, all is well when we spot the problem early | 16:10 |
| voidspace | good | 16:10 |
| bodie_ | ericsnow, I think that api.State bit is the missing link I'm not seeing | 16:10 |
| bodie_ | either way, I think the user cmd matches what we need a bit better, thanks! | 16:10 |
| ericsnow | bodie_: yeah, backups keeps it around for the direct HTTP requests so it has to close it later rather than in the New func | 16:11 |
| ericsnow | bodie_: cool | 16:11 |
| bac | marcoceppi: can you try to look at this soon? https://github.com/marcoceppi/amulet/pull/48 | 16:13 |
| dimitern | voidspace, LGTM +small comment | 16:17 |
| voidspace | dimitern: where's the comment? | 16:17 |
| voidspace | dimitern: did you publish it? | 16:17 |
| voidspace | heh, reviewboard can't display the diff | 16:18 |
| voidspace | ericsnow: http://reviews.vapour.ws/r/413/diff/# | 16:18 |
| * ericsnow checks | 16:19 | |
| dimitern | voidspace, yeah, it's on the PR itself | 16:21 |
| voidspace | dimitern: ah, cool - thanks | 16:22 |
| voidspace | dimitern: I can't quite - as a deleted node will still fail (400 error) | 16:22 |
| voidspace | dimitern: I'm working on that now | 16:22 |
| voidspace | dimitern: (close that bug you suggest) | 16:22 |
| ericsnow | voidspace: I expect the webhook code is trying to apply the diff to master rather than 1.21 | 16:23 |
| ericsnow | voidspace: I'll fix that | 16:23 |
| voidspace | ericsnow: thanks old boy | 16:24 |
| ericsnow | :) | 16:24 |
| * ericsnow doffs hat | 16:24 | |
| bac | tvansteenburgh: can you look at an amulet pull request for me? | 16:25 |
| dimitern | voidspace, sure, thanks | 16:26 |
| tvansteenburgh | bac, sure | 16:26 |
| bac | tvansteenburgh: https://github.com/marcoceppi/amulet/pull/48 | 16:26 |
| bac | thanks | 16:26 |
| voidspace | dimitern: do you know off hand if the errors package has a way to wrap multiple errors as a single error? | 16:27 |
| * voidspace is looking now | 16:27 | |
| dimitern | voidspace, I don't believe so, but it should be relatively easy to add a wrapper-for-two | 16:28 |
| voidspace | dimitern: no it doesn't | 16:28 |
| voidspace | I'd like a MultiError | 16:29 |
| voidspace | dimitern: actually, philosophical question | 16:29 |
| voidspace | dimitern: if we get an error 400/403/404 from maas release operation then at least one of the nodes is already gone and *none* will have been released | 16:29 |
| voidspace | dimitern: so I'm going to loop and release them individually - ignoring ones with 400/403/404 errors | 16:30 |
| voidspace | dimitern: so the question is, if in the loop one fails with a *different error* | 16:30 |
| voidspace | dimitern: should I stop - or should I complete the loop and collect (and return) all the errors | 16:30 |
| dimitern | voidspace, I think the loop should make the best effort to release all nodes, which means probably logging any errors and continuing | 16:33 |
| voidspace | dimitern: ok | 16:33 |
| voidspace | dimitern: we *should* return an error if there are unrecognised errors | 16:33 |
| voidspace | dimitern: so I'll collect errors | 16:33 |
| voidspace | dimitern: MultiError would be a pain, because it would be in compatible with Cause | 16:34 |
| voidspace | dimitern: I'll construct a compound error | 16:34 |
| dimitern | voidspace, there are a few cases around connecting to the api server code which strike me as very similar | 16:35 |
| dimitern | voidspace, with the "moreImportant" filtering of encapsulated errors | 16:35 |
| voidspace | dimitern: "moreImportant"? | 16:35 |
| voidspace | dimitern: ah, interesting | 16:36 |
| dimitern | voidspace, yeah, it might be what you need | 16:37 |
| tvansteenburgh | bac, i can't merge into marcoceppi | 16:38 |
| tvansteenburgh | 's repo, but i left a LGTM | 16:38 |
| marcoceppi | bac: tvansteenburgh merged | 16:40 |
| tvansteenburgh | marcoceppi: thanks! | 16:41 |
| bac | thanks tvansteenburgh and marcoceppi | 16:41 |
| * marcoceppi is in conference lag mode | 16:41 | |
| voidspace | dimitern: moreImportant drops errors | 16:42 |
| voidspace | dimitern: I don't think we want to do that | 16:42 |
| voidspace | dimitern: unless we log all the errors and just return the last one | 16:42 |
| voidspace | dimitern: the actual errors will be in the log | 16:43 |
| dimitern | voidspace, I was thinking something along those lines, yes | 16:43 |
| voidspace | dimitern: that's easy enough :-) | 16:43 |
| dimitern | voidspace, we definitely need to log them, but only return unexpected ones perhaps? | 16:43 |
| voidspace | dimitern: the question is what to do about multiple unexpected ones | 16:43 |
| voidspace | dimitern: the only "expected" ones we're just ignoring | 16:44 |
| voidspace | so I'll return the last unexpected error and log *all* errors - expected or not | 16:44 |
| dimitern | voidspace, the last? | 16:44 |
| dimitern | voidspace, yeah, ok | 16:44 |
| voidspace | dimitern: well, I'm looping setting err as I go | 16:45 |
| voidspace | dimitern: so the easiest implementation leaves err set to the last one... | 16:45 |
| voidspace | well, I'll have to copy it actually | 16:45 |
| voidspace | but you get the idea | 16:45 |
| dimitern | voidspace, but for expected errors let's use Debugf or something (maybe Warningf with a bit more context?) | 16:45 |
| voidspace | dimitern: I think Warning | 16:45 |
| dimitern | voidspace, I mean they logged errors shouldn't look like something bad happened and we didn't handle it | 16:45 |
| voidspace | dimitern: something unexpected has happened, it just isn't a problem (we couldn't stop the instance because it's already dead) | 16:46 |
| voidspace | dimitern: ok | 16:46 |
| dimitern | voidspace, +1, cheers | 16:46 |
| rogpeppe | is there an easy way of finding the reviewboard review of a PR from the PR on github? | 16:51 |
| rogpeppe | dimitern: ^ | 16:52 |
| natefinch | in theory, whoever makes the PR is supposed to paste a link to the review on github | 16:52 |
| natefinch | it seems the search capability of reviewboard is either exceedingly terrible, or simply not enabled | 16:52 |
| rogpeppe | the CONTRIBUTING page doesn't seem to have a link to the reviewboard page | 16:53 |
| rogpeppe | i can't even find the site currently, let alone the individual review :) | 16:53 |
| perrito666 | back | 16:53 |
| rogpeppe | it would be really nice if a link to the review made it into the final commit | 16:53 |
| natefinch | http://reviews.vapour.ws/ | 16:53 |
| rogpeppe | because a review is really useful context for a commit | 16:54 |
| rogpeppe | i still find the old codereview.com links in the old juju commit messages to be invaluable sometimes | 16:55 |
| natefinch | rogpeppe: yes absolutely. We put some time into getting links automatically added to the PR, but there were some complicating problems, and so it was put on the back burner for now. EVeryone is supposed to just manually make a comment on the PR with the link to the review. If someone's not doing that, you can give them flack for it :) | 16:55 |
| natefinch | oh, you mean in the commit message itself, I see. Yes, I agree | 16:55 |
| rogpeppe | natefinch: none of the juju PRs i've looked at recently have done that | 16:55 |
| rogpeppe | natefinch: yes, the commit message itself too | 16:55 |
| rogpeppe | natefinch: basically, i want to be able to do a "git blame", find the responsible commit and see if there were any comments on it | 16:56 |
| natefinch | rogpeppe: I'll send out a reminder to put the review link in the PR. It's pretty essential information to have. | 16:56 |
| natefinch | rogpeppe: and definitely putting the review link in the commit as well is a great idea, that I think we just didn't think of. | 16:57 |
| perrito666 | natefinch: so standup in 50 mins right? | 17:10 |
| natefinch | perrito666: correct | 17:11 |
| katco` | voidspace: sorry for the delayed response -- pong | 17:28 |
| === katco` is now known as katco | ||
| voidspace | katco: no problem, I wanted a review | 17:28 |
| voidspace | katco: 'tis done | 17:29 |
| katco | voidspace: sorry about that, having a day | 17:29 |
| voidspace | katco: I hope it improves | 17:29 |
| voidspace | katco: mine is nearly over | 17:29 |
| voidspace | well, the work part of it | 17:30 |
| katco | voidspace: :) | 17:30 |
| rogpeppe | ha, i can't believe it - if you try to add another comment in reviewboard when you've still got one open, the one you've got open is just deleted, even if it's got lots of text in it | 17:37 |
| rogpeppe | anyone know of a way of getting my deleted comments back? | 17:38 |
| rogpeppe | natefinch: ^ | 17:39 |
| rogpeppe | and it seems to be doing something weird with the forms too - lazarus doesn't work on them | 17:39 |
| natefinch | rogpeppe: don't think so, I think that's all held locally in javascript | 17:39 |
| rogpeppe | natefinch: :-( | 17:40 |
| rogpeppe | natefinch: i've just lost about 5 comments, i think | 17:40 |
| katco | natefinch: is that so? i swear i've come back on different machines to my reviews | 17:40 |
| rogpeppe | katco: that'll be after you've saved the comments | 17:40 |
| natefinch | well, I just mean, if you're in the middle of typing and the window closes | 17:40 |
| * katco leaves the possibility open that she's misremembering | 17:40 | |
| rogpeppe | katco: i hadn't saved them | 17:40 |
| katco | ah | 17:40 |
| katco | rogpeppe: i see | 17:40 |
| rogpeppe | katco: i'd scrolled down the page and double-clicked another line | 17:41 |
| natefinch | rogpeppe: save often, sir :) | 17:41 |
| rogpeppe | katco: the original comment just gets deleted | 17:41 |
| rogpeppe | natefinch: yeah, but if i save, the comment gets hidden | 17:41 |
| rogpeppe | natefinch: apart from that tiny number on the left | 17:41 |
| natefinch | rogpeppe: I agree... there are several UI nitpicks that seem suboptimal. I'd love to have a way to toggle all comments open or closed | 17:42 |
| rogpeppe | natefinch: deleting your in-progress comment without warning isn't just suboptimal - it's actively shit | 17:42 |
| * rogpeppe still misses codereview | 17:43 | |
| perrito666 | ericsnow: natefinch wwitzel3 standup is now right? | 18:04 |
| ericsnow | perrito666: yep | 18:04 |
| * perrito666 's calendar is a bit off bc he forgot to re-set the timezone | 18:04 | |
| wwitzel3 | natefinch: ping, standup | 18:07 |
| voidspace | right folks | 18:12 |
| voidspace | g'night | 18:12 |
| drbidwell | I have 6 machines behind a maas server that are commissioned and ready. "juju bootstrap -e maas" picks 1 or 2 machines to bootstrap, runs for 30 minutes and fails. The "-v and --debug" options tell me "2014-11-12 18:02:01 ERROR juju.provider.common bootstrap.go:122 bootstrap failed: waited for 30m0s without being able to connect: /var/lib/juju/nonce.txt does not exist". I can ssh to ubuntu@host and sudo. I can see that juju has successfully aut | 18:22 |
| LinStatSDR | sucessfully what | 18:33 |
| LinStatSDR | max char limit? | 18:33 |
| drbidwell | LinStatDR: /var/log/auth.log shows me that ubuntu@host successfuly authenticated via ssh, but I have no idea what it was trying to do or if it succeeded or not. | 18:41 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| jw4 | katco: I think we're using nowToTheSecond elsewhere too, not just tests... Agree about the cognitive overhead. | 19:25 |
| katco | jw4: good to know | 19:25 |
| jw4 | katco: I'd like to know if we should be using nowToTheSecond elsewhere too | 19:25 |
| katco | jw4: yeah, just seems strange to lose resolution | 19:25 |
| katco | jw4: and as i said, the whole "what is this thing? should i be doing this?" | 19:25 |
| jw4 | katco: I understand not capturing precision that's misleading | 19:25 |
| jw4 | katco: but it would be nice to have a documented rationale | 19:26 |
| bodie_ | *refrains from jokes about readability* | 19:26 |
| bodie_ | my problem with second granularity is that sub-second diffs are now "the same" | 19:26 |
| katco | bodie_: as a nerd, i would enjoy a good readability joke :) | 19:26 |
| bodie_ | it's not really funny... heh. let's just say there are lots of things going on in lots of places that aren't immediately obvious despite go's readability ;) | 19:27 |
| bodie_ | I also favor reducing those cases rather than increasing | 19:28 |
| katco | here here | 19:28 |
| katco | well, now that i've met thumper, i now know he can take an opinionated review with good humor :) | 19:29 |
| katco | what are people's opinions on table-driven tests? | 19:35 |
| natefinch | katco: generally good, but can be overused and overcomplicated | 19:35 |
| katco | natefinch: ty (witholding comment to give others a chance to weigh in) | 19:35 |
| katco | natefinch: btw, not sure if you saw this. might be helpful for your mud server: https://github.com/katco-/cmdtree | 19:36 |
| katco | natefinch: actually, https://plus.google.com/u/0/+KatherineCoxBuday/posts/JUaiV9Maf4v | 19:37 |
| katco | discussion; i haven't had time to do a readme or anything, so this is the documentation for now ;) | 19:37 |
| natefinch | haha | 19:38 |
| bodie_ | katco, I personally love using a slice of test structs (I guess this is what you mean) and do it for almost every nontrivial test | 19:38 |
| katco | bodie_: that is indeed what i mean | 19:38 |
| katco | would an equivalent be splitting common functionality into a helper function, and then passing the struct in? | 19:39 |
| natefinch | I like them when they are testing exactly one thing with different inputs.... too often people glom on more and more tests so that half the fields in the table are only used in half the tests | 19:40 |
| bodie_ | typically I would use the struct to define the arguments to the helper | 19:40 |
| bodie_ | in a table that is | 19:40 |
| natefinch | yep | 19:40 |
| katco | bodie_: i'm typing up this in a review comment which i'll link | 19:40 |
| bodie_ | I've been told it's better to keep the test struct in the specific testing function however | 19:41 |
| bodie_ | as opposed to using a single giant struct with multiple different tests working against it | 19:41 |
| natefinch | bodie_: absolutely | 19:41 |
| bodie_ | also prevents other files in the package from manipulating it (anti-pattern) | 19:41 |
| natefinch | bodie_: unless they are canonical datasets that multiple tests might work against (which is possible, but often results in harder-to-read tests) | 19:42 |
| bodie_ | in Go, we'd typically put that as a member on the test type extended for each test file, I would imagine, and initialize it in the SetUpTest for the file's suite | 19:42 |
| bodie_ | s/test type/suite/ | 19:42 |
| katco | http://reviews.vapour.ws/r/407/#comment3988 | 19:43 |
| bodie_ | er, the unified bit would be in the parent type's SetUpTest | 19:43 |
| natefinch | bodie_: gocheck is not super typical go testing, btw ;) | 19:43 |
| katco | please don't take this as a criticism against this particular change | 19:43 |
| bodie_ | mm, true | 19:43 |
| katco | more a vehicle for the discussion | 19:43 |
| jw4 | katco: I think I agree fully; I'd be interested in some examples | 19:44 |
| katco | jw4: i'll go ahead and flush out that comment with how i would write that test | 19:44 |
| jw4 | katco: cool! | 19:45 |
| natefinch | katco: while I like the idea of not subverting the structure of tests.... I also don't really like having the whole logic of a test in a helper function and then just one line in the test function itself calling the helper | 19:45 |
| natefinch | katco: we've done that in some packages, and it makes it very very hard to understand what test failed and why it failed | 19:45 |
| katco | natefinch: can you expound on why? how is that different from one line in a struct, and the helper "method" being the body of the loop? | 19:45 |
| === kadams54 is now known as kadams54-away | ||
| katco | natefinch: really? i have found the opposite! it's so hard for me to find out which row in the table failed, but if a test function fails, i know exactly where to look | 19:46 |
| natefinch | katco: it's basically the same thing, honestly.... | 19:46 |
| bodie_ | c.Logf("test %d: %#v", i, t.args) perhaps? | 19:47 |
| perrito666 | katco: if you find hard to know which row in the table failed someone forgot to add info to that table | 19:47 |
| bodie_ | (I like a description field, too. something like a "should") | 19:47 |
| katco | natefinch: except if each test is in the container your host platform (Go) expects it to be in, you get all the benefits of your entire stack of test runners | 19:47 |
| natefinch | katco: yes, I really like that part of your proposal... but here's the problem with helpers. When the test reports a failure at this line, what test failed: https://github.com/juju/juju/blob/master/worker/uniter/uniter_test.go#L2078 | 19:48 |
| katco | if you define a new "type" of test-runner (i.e. a loop), you've now done exactly that: "here's a new way to run tests". and it's definitely less functional than what Go provides us | 19:48 |
| katco | natefinch: it will tell you what function failed when it fails | 19:49 |
| katco | ^what test function | 19:49 |
| natefinch | katco: the output isn't sycnronous, and if you have more than one failure, it'll be hard to understand which test is failing, and why. I've had this exact problem with the uniter tests. Now... that can be avoided by being careful in how you write your assertions, and make sure you print out context etc. | 19:50 |
| katco | perrito666: i guess i spend time jumping between the row and the table-runner trying to figure out what's getting used where | 19:50 |
| perrito666 | katco: agreed, that can be a pain | 19:50 |
| perrito666 | katco: unless... | 19:50 |
| perrito666 | you editor can vsplit :p | 19:50 |
| perrito666 | sorry, had to | 19:50 |
| jw4 | perrito666: you bad boy | 19:50 |
| natefinch | haha | 19:50 |
| * jw4 out for lunch | 19:50 | |
| katco | perrito666: lol of course which emacs can do :) | 19:50 |
| natefinch | yes, I also hate jumping between the table and the test | 19:51 |
| bodie_ | description field helps | 19:51 |
| katco | natefinch: i don't understand. under the covers when it does t.Fail() or t.Errorf(), it's going to report the test function, yes? not the stack-frame you were in? | 19:51 |
| perrito666 | really? you guys have like twice the resolution I have, I always have definition/code open | 19:51 |
| katco | natefinch: i haven't ever had a problem debugging tests written like that. i may be misremembering. | 19:52 |
| bodie_ | perrito666, ... tmux makes me happy | 19:52 |
| perrito666 | bodie_: regardless of the tool | 19:52 |
| bodie_ | heh, just saying. at any given moment I have between 2 and 5 panes per window, per session... :D | 19:52 |
| * bodie_ has the crazy | 19:53 | |
| natefinch | katco: maybe it was something simple I was missing, but I know I was about ready to throw my computer across the room the last time I debugged the uniter tests (granted that was like 7 months ago) | 19:54 |
| katco | haha | 19:54 |
| katco | that is a challenging suite | 19:54 |
| thumper | grr | 19:55 |
| thumper | how do I reply to these dumb issues on RB | 19:55 |
| natefinch | honestly, my preferred way to write table driven tests would be to generate the code for them, so I don't have to piece together WTF is going on... but I'm sure no one else in the world agrees with me there ;) | 19:55 |
| thumper | do I have to go to the diff to do it? | 19:56 |
| perrito666 | natefinch: I doubt it, your computer seems too heavy | 19:56 |
| katco | natefinch: code-generating-code is underutilized i think | 19:56 |
| perrito666 | katco: beware, if that gets too efficient we all get out of jobs, and then terminator | 19:57 |
| bodie_ | natefinch, those goddamn uniter tests. LOL | 19:57 |
| katco | perrito666: (rhythmic drums) | 19:59 |
| thumper | katco: replied | 20:00 |
| katco | jw4: (et. al) example up: http://reviews.vapour.ws/r/407/#comment3988 | 20:00 |
| katco | thumper: ty looking | 20:00 |
| katco | thumper: i think i still disagree, but thank you for explaining. | 20:02 |
| katco | thumper: ship it from me :) | 20:02 |
| thumper | :) | 20:02 |
| === liam_ is now known as Guest39243 | ||
| === kadams54-away is now known as kadams54 | ||
| katco | wwitzel3: natefinch: do i want to jump into wwitzel3's review, or have fwereade and others mostly covered it? | 20:09 |
| katco | for juju-run | 20:09 |
| drbidwell | How do I find out what "juju bootstrap" is trying to do? How do I tell what it is actually doing? I have tried "-v --debug", but need more data | 20:15 |
| perrito666 | drbidwell: which data exactly? | 20:16 |
| katco | drbidwell: juju debug-log aggregates data from all nodes | 20:16 |
| katco | drbidwell: to my knowledge --debug is just client-side | 20:16 |
| natefinch | katco: ummm... I think it's all set | 20:17 |
| katco | natefinch: cool, ty... was not looking forward to that :) | 20:18 |
| natefinch | haha... yeah, it is a problem when you're on call, and there's a bunch of gigantic reviews that are halfway done (or more or less, who can tell? | 20:19 |
| drbidwell | I have 6 machines provisioned with maas. I can ssh to them as ubuntu@host and sudo. juju bootstrap just times out after 30 minutes and complains "waited for 30m0s without being able to connect: /var/lib/juju/nonce.txt does not exist" | 20:19 |
| drbidwell | I don't know what is wrong or how to fix it. | 20:19 |
| drbidwell | katco: "juju debug-log" returns "ERROR environment is not bootstrapped" | 20:21 |
| katco | drbidwell: ah right, sorry. | 20:21 |
| drbidwell | How do I get the remove server side logs? | 20:23 |
| waigani | thumper, menn0, davecheney, mwhudson: sorry I missed standup, here now | 20:23 |
| mwhudson | waigani: good timing, we finished like 30s ago | 20:23 |
| waigani | ugh | 20:23 |
| katco | drbidwell: i would do just what you have been, ssh in and poke around var | 20:23 |
| mwhudson | :) | 20:24 |
| katco | drbidwell: if there's nothing there that's juju specific, i would start looking at general OS logs to see what's going on | 20:24 |
| natefinch | I gotta run early today. See you all tomorrow | 20:25 |
| perrito666 | cu | 20:25 |
| katco | natefinch: ta | 20:25 |
| whit | cmars, rick_h_ suggested I bother you about collect-metrics in 1.21beta1. mainly curious what I can do with it | 20:27 |
| === kadams54 is now known as kadams54-away | ||
| === menn0_ is now known as menn0 | ||
| === kadams54-away is now known as kadams54 | ||
| cmars | hi whit, this feature is pretty early-stage and still somewhat being worked out. not quite fit for general consumption yet. | 21:07 |
| whit | cmars, that's alright. I was interested in trying it out for some metric consumption stuff we are doing in cloudfoundry | 21:08 |
| whit | cmars, I'm guessing I would need to query the statedb directly? | 21:08 |
| whit | (to get my aggregated stats) | 21:09 |
| cmars | whit, we'd like to avoid direct state access.. i'd like to set up a hangout to discuss your requirements. we'd prefer this sort of stuff to go through proper APIs -- and we're still working out what those look like ;) | 21:10 |
| whit | cmars, sure | 21:11 |
| whit | cmars, obviously we would rather use the ws api too | 21:12 |
| whit | but we are also fine evolving with the system | 21:12 |
| whit | having a way to subscribe to metrics based on annotations would be ideal | 21:14 |
| whit | for our usecase | 21:14 |
| whit | cmars, thanks for the invite | 21:15 |
| cmars | whit, sure | 21:15 |
| rick_h_ | wallyworld: is provider storage going away in 1.21 so manual provider doesn't need it any more? | 21:57 |
| perrito666 | ericsnow: I answered to one of your issues I am really interested in sorting that out so if you can follow up ill be very thankful | 22:10 |
| wallyworld | rick_h_: no, it won't | 22:10 |
| rick_h_ | wallyworld: k, ty | 22:10 |
| ericsnow | perrito666: thanks. I'll have a look | 22:14 |
| perrito666 | ericsnow: btw, where are we on with upload and metadata.version? | 22:31 |
| perrito666 | I am under the impression that there was something missing for me from metadata but I cannot recall what it was | 22:32 |
| ericsnow | perrito666: see http://reviews.vapour.ws/r/398/ | 22:32 |
| ericsnow | perrito666: JSON -> metadata (added in http://reviews.vapour.ws/r/346/) | 22:32 |
| perrito666 | excelent, when do you foresee those landing? | 22:32 |
| ericsnow | perrito666: allows getting the metadata out without unpacking the archive: http://reviews.vapour.ws/r/356/ | 22:33 |
| ericsnow | perrito666: as soon as I address the reviews | 22:33 |
| perrito666 | ok with that I can address a couple of your comments | 22:34 |
| ericsnow | perrito666: sweet | 22:35 |
| ericsnow | perrito666: I responded to your comment | 22:35 |
| ericsnow | perrito666: basically, why can't we just use the existing code in the agent package (like ReadConfig)? | 22:36 |
| perrito666 | because we are not using the agentconfig type (I think some of this predates the existing of some of those things) | 22:36 |
| perrito666 | I can de-duplicate that code gladly, I will nevertheless maintain my possition on not making that method public. | 22:37 |
| * perrito666 looks at eric's pointer | 22:37 | |
| ericsnow | perrito666: sure | 22:40 |
| ericsnow | perrito666: it just looks like the code already exists elsewhere | 22:40 |
| perrito666 | ericsnow: I see, yes some of that predates restore in its new version | 22:41 |
| === menn0_ is now known as menn0 | ||
| === kadams54 is now known as kadams54-away | ||
| perrito666 | mm, how much I hate when a function takes a file path instead of a reader | 22:46 |
| ericsnow | perrito666: you should totally fix it | 22:50 |
| ericsnow | perrito666: :) | 22:50 |
| perrito666 | ericsnow: I was about, yet its not trivial | 22:51 |
| ericsnow | perrito666: it's only used in cmd/jujud/agent.go (and in tests), right? | 22:53 |
| perrito666 | ericsnow: dunno, just reading what it does to see how to change it to not use paths but id might not be so wise in terms of clarity | 22:53 |
| ericsnow | perrito666: change "configData, err := ioutil.ReadFile(configFilePath)" to "configData, err := ioutil.ReadAll(configReader)", no? | 22:55 |
| perrito666 | ericsnow: context | 22:56 |
| ericsnow | perrito666: in ReadConfig (maybe you were talking about something else) | 22:56 |
| perrito666 | ericsnow: if you read down the code it seems to both use the path to set something that I need to make sure is not used all the way and it also tries to open another file looking for legacy format | 22:58 |
| ericsnow | perrito666: ah, got it | 22:58 |
| perrito666 | just being careful, I dont think it cant be done | 22:59 |
| perrito666 | I most likely can split it in a couple of funcs factoring out whatever I need but this is a rather sensitive area | 22:59 |
| ericsnow | perrito666: agreed | 23:00 |
| davecheney | thumper: natch | 23:12 |
| davecheney | action_test.go:90: c.Check(action.Enqueued(), jc.TimeBetween(before, later)) | 23:12 |
| davecheney | ... obtained time.Time = time.Time{sec:63551430607, nsec:842000000, loc:(*time.Location)(0x13b5c80)} ("2014-11-13 10:10:07.842 +1100 AEDT") | 23:12 |
| davecheney | ... obtained value time.Time{sec:63551430607, nsec:842000000, loc:(*time.Location)(0x13b5c80)} type must before start value of time.Time{sec:63551430607, nsec:842300369, loc:(*time.Location)(0x13b5c80)} | 23:12 |
| davecheney | just hit that bug | 23:12 |
| jw4 | davecheney: I've got a fix I'm gonna push up soon | 23:16 |
| davecheney | jw4: thumpers has a fix already | 23:17 |
| jw4 | davecheney: a second fix? The one thumper put in yesterday (or monday) is related to this... | 23:17 |
| jw4 | davecheney: this is the nexus of two changes | 23:18 |
| jw4 | davecheney: thumper introduced nowToTheSecond instead of time.Now() (+1) | 23:18 |
| jw4 | davecheney: and bodie_ switched to using jc.TimeBetween | 23:19 |
| jw4 | davecheney: (thumper, et. al) my fix involves 1) not using TimeBetween (which excludes boundaries) | 23:19 |
| jw4 | and 2) modifying nowToTheSecond to use time.Truncate(Seconds) instead of time.Round(Seconds) | 23:20 |
| davecheney | jw4: ok | 23:30 |
| davecheney | i haven't been paying close attention | 23:30 |
| davecheney | thumper was talking about some fixes he had proposed which sounded related | 23:30 |
| jw4 | davecheney: I'm just re-reviewing git and it looks like the fix I was thinking of only landed 3 hours ago | 23:30 |
| jw4 | davecheney: thumper had proposed it a couple days ago... | 23:31 |
| jw4 | davecheney: thanks for the update | 23:31 |
| davecheney | menn0: part two https://github.com/juju/juju/pull/1116 | 23:35 |
| menn0 | davecheney: i've already just had a look. one question on the review. | 23:36 |
| davecheney | yup | 23:36 |
| davecheney | that is cruft | 23:36 |
| davecheney | i'll belete it | 23:36 |
| menn0 | davecheney: so that means the temp juju.go goes right? | 23:48 |
| menn0 | davecheney: I just noticed another issue too. see review. | 23:48 |
| menn0 | davecheney: otherwise is looking good I think | 23:48 |
| waigani | not sure if this sent, resending: thumper, menn0: any idea why the AllWatcher id is a pointer to a string? | 23:50 |
| davecheney | https://github.com/juju/utils/pull/86 | 23:51 |
| davecheney | waigani: the id is supposed to be opaque | 23:51 |
| davecheney | well, when i say supposed | 23:51 |
| davecheney | i mean | 23:51 |
| davecheney | it looks like it's just a random identifier | 23:51 |
| davecheney | at places in the multiwatcher, the id is stuffed into a interface{} | 23:52 |
| davecheney | so it's used for comparison only, afaik | 23:52 |
| waigani | okay, but why a pointer? | 23:53 |
| jw4 | fwereade: (dimitern) http://reviews.vapour.ws/r/417/ idPrefixWatcher bug we discussed earlier | 23:53 |
| waigani | so we just compare the pointers, not the values | 23:53 |
| waigani | got it | 23:53 |
| davecheney | waigani: it's just an opaque identifier | 23:54 |
| davecheney | i don't konw why it's a *string specifically | 23:54 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!