[00:19] davecheney: ping [00:21] thumper: ack [00:21] davecheney: hangout to talk about sigquit and logs? [00:21] sure [00:21] davecheney: 1:1 hangout? [00:21] sure === thumper is now known as thumper-afk [01:14] thumper-afk: the power test failed. My hack to run subpackages is wrong. I need to try again. [01:22] thumper-afk: got the basics working [01:29] thumper-afk: lucky(~/src/github.com/juju/juju/state) % ./state.test [01:29] ^\exiting with signal: quit [01:29] lucky(~/src/github.com/juju/juju/state) % ./state.test [01:29] ^Cexiting with signal: interrupt [01:32] menn0, thumper-afk https://github.com/juju/juju/pull/2675 === thumper-afk is now known as thumper [01:56] davecheney: ta [01:56] * thumper looks [01:57] let me know if it looks sane [01:57] i did a bit of testing with the sample function supplied [01:57] i can hook and unhook functions [01:57] s/functions/signals [02:19] davecheney: hangout? [02:24] thumper: two secs [02:25] gotta change computers [02:27] thumper: i'm in the 1:1 [03:12] http://reviews.vapour.ws/r/2053/ [03:13] davecheney: ^^ [03:13] * thumper is running out [03:13] if you are happy, please add $$merge$$ bits [03:13] cheers === thumper is now known as thumper-afk [03:16] Good god, I just wrote 300 lines of ridiculously tedious code just so we have copies of our business logic structs in the API. And now I have to go write tests for this crap. [03:24] fatal error: runtime: stack split at bad time [03:24] i think i have annoyed the go gods [03:24] (this is on arm64 though) [03:36] mwhudson: excuse me, is not a good time for a stack split ? [03:37] thumper-afk: LGTM [03:37] probably will end up dumping more than we except [03:37] but we can always disable ti again [03:44] davecheney: https://github.com/golang/go/issues/11482 [04:41] <_thumper_> hmm... [04:41] <_thumper_> for some reason the nick thumper is temporarily unavailable [04:41] * _thumper_ needs to work out who to ping [04:42] <_thumper_> sinzui: I've landed a tweak for the upgrade tests so if they timeout and get killed, we dump the logs [04:42] <_thumper_> sinzui: so we can debug the ci failures [04:43] <_thumper_> sinzui: how soon is the next ci run for 1.24? === _thumper_ is now known as thumper [05:30] and now we wait [05:30] o/ dimitern [05:30] thumper: looks like your change landed [05:30] davecheney: yep, on 1.24 and master [05:31] hopefully the next ci test run will give us more logging [05:31] davecheney: actually I wonder if I can reproduce now... [05:31] davecheney: as the timing is different between -check.vv and what we have done [05:31] thumper, hey [05:31] .vv causes lots of I/O sync [05:32] * thumper goes to poke the power machine [05:35] thumper: all that io flushing to disk will generate a lot of scheduler churn [05:35] agreed [05:35] what was straight line code will weave through the scheduler [05:35] \o/ butterfly wings [05:36] with that said [05:36] it is rare that making a program _more_ concurrent increases it's stability [05:41] hmm... it appears to be passing individually [05:42] * thumper tries with the whole suite [05:44] * thumper sighs [05:44] still seems to be passing [05:44] bah humbug [05:44] davecheney: I love yours stress test script [05:44] that thing broke so many people's dreams [05:44] davecheney: I think we should probably commit that into the tree somewhere [05:44] :) [05:45] i've been using it for years [05:45] for some reason GOMAXPROCS=42 (no shit) [05:45] so much better that a bash for loop [05:45] always caused a set of tests to segfacult [05:45] haha [05:45] that is freaking hilarious [05:46] 42 insn't a prime [05:46] but 7 is one of it's factors [05:46] and that is a prime [05:46] so that's something [05:48] pffft [05:50] the suite has passed heaps, then failed with mongo not coming up [05:50] * thumper retries [05:50] davecheney: just noticed that the stress script stops when a test fails [05:50] nice [05:50] it's designed for tests that take seconds (cough) [05:50] so if it didn't stop, you may not notice the failure [07:11] ffs [07:11] * thumper walks away again [07:11] dinner time [07:24] ashipika, ping [07:25] ashipika, was wondering if you needed any clarification re http://reviews.vapour.ws/r/1933/ -- in particular my suggestion that it feels like an operation? [07:44] fwereade: hey.. yes.. some clarification would help.. :) [07:45] ashipika, ok, so, we've got to the point where pretty much everythiong the uniter *does* is encapsulated in an operation [07:45] ashipika, see uniter/operation [07:45] fwereade: *looking* [07:46] ashipika, the idea being that the modes can decide what needs to be done [07:46] ashipika, but *how* those things are done is determined elsewhere [07:47] ashipika, it's not 100% clean, particularly the entanglement of operation state with operation definition [07:48] ashipika, but at first blush it looked like a plausible way to keep the flushing details out of the modes [07:48] fwereade: ok.. i'll have a look at operations and if i have any questions i'll come back to you.. but from what you've said i can see how this could be a good fit for an operation [07:48] ashipika, (...and I hold out hopes that we'll be able to change the modes into something more isolated and comprehensible, but every little bit of extra complexity in there works against that) [07:48] ashipika, ok, cool [07:50] fwereade: hey, I'd like a chat if you have a few minutes [07:50] thumper, sure [08:22] fwereade, jam if one of you has 2 minutes I have a hopefully trivial request [08:50] mattyw, oops sorry! [08:50] mattyw, what can I do for you? [10:06] axw, btw, if you're on: I'm missing a bit of context around the session-handling in state/tools.go (and a little bit in state/images.go, but that's simpler) [10:07] axw, possibly what I'm confused about is the underlying reasons for the differences between the two? [10:07] fwereade: I'll need to refresh state, one minute [10:11] fwereade: I think I just realised I could do it a better way when I did the second one (image metadata), and never went back to clean up tools storage [10:11] fwereade: AFAIR, the uses of ToolsStorage only keep ahold of it for a brief time, so the session won't become stale [10:11] fwereade: but it would be better to just Copy and dispose inside the method call [10:46] axw: hiya [10:47] rogpeppe: ahoy [10:47] axw: have you had anything to do with the juju user support, by any chance? [10:47] rogpeppe: none [10:47] axw: ah, ok. [10:47] axw: darn [10:47] axw: :) [10:47] rogpeppe: I think thumper [10:47] axw: he's never online when i am :-( [10:48] yeah :/ [10:48] axw: i *think* this is meant to work: http://paste.ubuntu.com/11798402/ [10:50] rogpeppe: "juju switch" doesn't seem right [10:50] rogpeppe: that's always been for switching environments... [10:51] axw: i thought that a jenv file represented an environment [10:51] axw: (and some creds to access that environment) [10:51] rogpeppe: ah, I see. hrm. I guess it's validating against environments.yaml? not really sure [10:52] axw: looks like configstore list is broken somehow [10:53] axw: ha [10:54] axw: "juju user add" writes the .jenv file to the current directory, it seems [10:54] rogpeppe: heh :) [11:51] hmpf, three disconnects in a few minutes [13:01] hangout [13:02] omw === tvansteenburgh1 is now known as tvansteenburgh [13:24] axw, still around? [13:25] fwereade: hey, what's up? [13:25] axw, was wondering about Unit.findCleanMachineQuery and its returned closer [13:26] axw, it's seeming like a good idea to pass the closee in, and keep that all happening at one level [13:26] axw, any objections? [13:26] * axw looks [13:28] fwereade: pass the closee in? pass what closee in to what? how about change findCleanMachineQuery to take a *machineDoc, and execute the query directly? [13:29] axw, one client wants .One and one wants .All, I think [13:30] fwereade: ah, didn't see hte other one. [13:30] axw, but if the two clients each did their own newDB, passed that in, and closed it themselves, it might be sane? [13:31] fwereade: oh I see. that sounds fine to me [13:31] axw, cool, thanks, sorry mangled english in original question :) [13:31] nps [13:34] dammit, I'm not sure that makes that much sense actually, at least not without inappropriate application of secret knowledge :/ [13:37] axw, hmm. how would it be if we just made it return terms, and made the client get the machines collection itself? what's the worst that could happen re copied sessions there? we already know it's potenntially inaccurate; coudl it get apppreciably worse? [13:56] fwereade: sounds reasonable, but how would you do the container check bit? [13:58] fwereade: or the instance-data bit for that matter [13:59] sounds like it'd get a bit messy [13:59] seems like [14:00] axw, I *think* it's less messy to just copy the session inside fCMQ for the container and instanceData checks [14:00] axw, and defer those closes in a sane scope [14:01] axw, and then, separately, let the clients use their own session copies to actually grab the machine, and defer *those* closes in a sane scope [14:02] axw, given that the transactions are themselves probably running in their *own* session I don't think things will get worse [14:02] fwereade: thanks for the reviews [14:04] fwereade: yeah, I think that'd be fine. [14:04] axw, cool [14:05] ericsnow, hope they help :) [14:08] axw, ok, looking back at tools.go [14:09] axw, does tools storage depend on any per-environment features? or can I just create a plain txn runner? [14:10] fwereade: it doesn't have env-specific metadata, though it possibly should [14:10] axw, that said I probably ought to give it an env-aware runner anyway [14:11] axw, if it doesn't touch env bits it makes no difference having it [14:11] axw, if it does, *not* having it makes a big difference [14:11] fwereade: yep. I think it'd be best to pass one in if possible, to avoid later surprise [14:14] fwereade: what are you doing anyway? [14:15] w.r.t. tools storage [14:15] axw, getting angry about raw DB access all over the place and trying to make it impossible [14:15] axw, or at least *harder* to do without knowing you're being bad [14:16] fwereade: heh :) sounds good [14:23] fwereade: you have some time to talk about state? [14:28] * perrito666 is suddenly held in a very strange position in front of his computer by a back contracture [14:29] ericsnow, sure, but probably best on irc right now [15:44] ericsnow: where were you thinking the APIClient implementation would live? [15:45] ericsnow: process/api/client I assume [15:45] natefinch: right [15:46] ericsnow: hmm.. problematic to write the client code without the endpoint args available [15:46] natefinch: why wouldn't they be available? [15:47] ericsnow: can you or wwitzel3 do a quick review on the server code I PR'd? It's super simple code. It will probably need some tweaks once your state branch lands, but only super minimal things (like status is a string instead of a struct, etc) [15:47] natefinch: already did :) [15:47] Bug #1470150 opened: CentOS fails to bootstrap using juju 1.24 [15:48] ericsnow: huh, weird, wonder where that email went. Maybe I archived it by accident. Anyway, thanks! I'll work on that,. [15:48] natefinch: cool [15:50] ericsnow: LOL "TestUnregisterTransaction" .... this is what I get for writing tests during standup ;) [15:50] natefinch: :) [16:23] I hate it when I delete code and it makes my coverage percentage go down :/ [16:23] that was well-tested useless code [16:24] ericsnow: natefinch katco : I need help with the go test command line. I want to omit githubm.com/juju/juju/cmd/jujud from the test run (just that packge and below). I tried -run, but is don’t match on path [16:25] github.com/juju/juju/cmd/jujud I mean [16:27] sinzui: i don't think go test or gocheck have an exclusion flag [16:27] sinzui: omission is not really supported, AFAIK [16:27] sinzui: you just have to test only the things you care about (i.e. all the import paths) [16:28] sinzui: you'll have to do some CLI hacking. You can do go list github.com/juju/juju to get a list of all the packages and then just run go test manually over each one except jujud [16:28] katco: natefinch I was afraid of that. I can contrive a way to test subpackages, but is gets ugly [16:29] it's not actually horrible, since you're still using the go tool to get the list of packages [16:29] natefinch: yeah, that is how I disccovered that juju/juju/cmd will fail, but running the subpackages by itself themselves will pass [16:30] sinzui: I remember seeing something like that a while back. probably a cleanup issue or something [16:31] natefinch: oh is there a way to ask go test to give me a list of all the packages it finds? I could re-order such as list to that the problem ppc package it tested first [16:32] sinzui: go list github.com/juju/juju/... |grep -v github.com/juju/juju/cmd/jujud |xargs go test [16:32] sinzui: go list creates the same list that go test would [16:33] :) === kadams54 is now known as kadams54-away [16:35] natefinch: thank you. This looks promising [16:36] ^^ katco too [16:36] sinzui or jog: I have two branches for review, one a prerequisite of the other: https://code.launchpad.net/~abentley/workspace-runner/s3-script/+merge/263383 https://code.launchpad.net/~abentley/workspace-runner/s3-artifacts/+merge/263384 [16:38] how can I test an unexported implementation of an interface? currently I'm using export_test and for a function I instantiate the type and call the method. Is there a better way? [16:38] abentley, I'll take a look [16:39] jog: Thanks. === kadams54-away is now known as kadams54 [17:01] axw, if you're awake you shouldn't be, but in case you are: why is image metadata in the images DB, but tools metadata in the juju db? [17:19] fwereade: do we require comments on every exported field on an exported struct? === kadams54 is now known as kadams54-away [18:37] ericsnow: hey, sorry, finally rolling back onto api server abstraction. [18:37] ericsnow: i took a look at what other apiserver facades were doing to see if we could extract some commonality, but it doesn't look like there is any. [18:37] katco: np [18:38] ericsnow: given we're just going to be using the standard RegisterFacade, and composing an adapter in components/all/process.go... what is the abstraction bit? [18:38] katco: I do think we should add RegisterHookContextFacade [18:38] jog, sinzui: So I get two reviews for the first branch and no reviews for the second? Is that how it is? [18:38] ericsnow: i haven't found anything that would make that unique [18:38] katco: it takes care of auth [18:38] abentley, sorry on a call [18:39] abentley: I am doing the second. I am just distracted [18:39] ericsnow: well, different facades use auth in different ways [18:39] jog, sinzui: No worries. [18:39] katco: context hook facades (i.e. uniter) only use auth in 1 way [18:40] ericsnow: so this entire card turns out to be creating a RegisterHookContextFacade, calling AuthUnitAgent, and that's it? [18:40] katco: basically RegisterHookContextFacade would take care of most of the stuff that is in newUniterBaseAPI (in apiserver/uniter/uniter_base.go) [18:41] ericsnow: there's actually not much in there aside from constructing some closures for the uniter === kadams54 is now known as kadams54-away [19:08] natefinch, ...not *necessarily*, but I have been finding that forcing myself to document every field has generally led only to good things [19:09] fwereade: kk... I find that usually the field is pretty obvious and the comment is redundant, and just makes the struct harder to read. Howeever, occasionally it has made a better name for the field more obvious. [19:09] fwereade: and of course, sometimes a field name can't encompass all the info you need about the field [19:09] natefinch, yeah [19:10] natefinch: nonsense, unles your attribute has a very java like long descriptive name, a doc is always a good thing [19:11] natefinch, I do still kinda believe in my heart that every comment is a sign the code is insufficiently clear, but I don't think that's *actually* as helpful as I wish it were :) [19:14] natefinch, perrito666: and, to be fair, I would say that while java-style naming has its squickiness, I think it's more a consequence of trying to retain clarity on large projects with lots of people working on them; and that juju has, for a while, been big enough that every little bit of bludgeoning obviousness helps [19:14] perrito666: I'm with fwereade - if the name isn't clear enough, that's a problem with the code. Comments are nice, but they shouldn't be everything... and I think that if you need to call out something about a specific field, it's actually more appropriate to do it on the comment for the struct itself. === kadams54-away is now known as kadams54 [19:16] perrito666, fwereade - it's like comments on a function - you don't comment every arg... if an argument is non-obvious, spell it out in the comment for the function. Most of the time, most arguments and fields should be obvious, and the comments on them reflect that: "ID is the unique id of the foo" [19:16] natefinch: data structures cannot be all that expressive by themselves [19:16] perrito666, natefinch: the strongest argument against comments, ofc, is that they lie, and we're not immune to that [19:16] perrito666, I dunno, what's that quote [19:16] * fwereade goes hunting [19:17] fwereade: poor translation from spanish [19:17] fwereade: actually, I think that the problem with comments on fields and arguments is that they're so often useless, that if one is actually USEFUL, no one will read it because they'll assume it's useless. [19:17] natefinch, perrito666: "Show me your flowcharts and conceal your tables, and I shall continue to be mystified. Show me your tables, and I won’t usually need your flowcharts; they’ll be obvious." [19:18] perrito666: I agree that comments aren't bad themselves.... but by requiring them on the vast majority of fields that *are* obvious..... you then can't see the forest for the trees [19:18] natefinch: many times our structures have fields that represent something that makes sense to someone that knows a lot about juju only, and that doesnt mean that the name is unclear, only that the struct has some baggage, and given the size of the project, a comment helps there [19:18] natefinch, perrito666: again, more usually true in theory than in practice, because people write run=bbish data structures just as much as they write rubbish code [19:18] type thisStructContainsInformationAboutFilesMatchingAGivenCriteria struct { thisIsTheNameOfTheFileThatMatchedTheCriteria string ... } [19:19] TheMue: so you've written java? [19:19] natefinch: one, in my dark past hours. but even then not so long identifiers [19:20] natefinch, perrito666, TheMue: the trouble is that there often *is* more meaning than can be packed into a name [19:20] natefinch, perrito666, TheMue: consider the Life fields in state [19:21] natefinch, perrito666, TheMue: they're *loaded* with subtle meaning, but everything has its own Life field [19:21] Well, for that, I think that either a comment on the struct itself, or perhaps an exception to the no-comments-on-fields rule is fine. But for the vast majority of cases, the comment on a field is just noise. [19:21] fwereade: yes, where I still like comments is (a) in describing behavior, not meaning, and (b) as a kind of optical separator. with highlighting quickly blocks are identifyable [19:21] for example: https://github.com/juju/juju/blob/master/apiserver/params/actions.go#L54 [19:22] natefinch, perrito666, TheMue: where do we document them? I write quite a lot of useful stuff in the doc directory, and I've tried to make a point of telling everyone to read them when they join, but... [19:22] sorry, s/write/wrote/ -- I have suffered some degree of disillusionment [19:22] no comments on fields... aside from the difference between Message and Status, all the fields are pretty damn obvious, and a comment is not going to help anyone. [19:22] I am not all that clear, how does he know that the action is going to be completed? :p [19:23] perrito666: that information should be on the struct itself or the function that uses it... not on the field. [19:24] ok ok, I give up, there is a level of obviousness [19:24] it also comes down to how it looks in godoc. Field comments are in plaintext, struct and function comments are able to be styled and are therefore a lot easier to read [19:25] for example: https://godoc.org/github.com/juju/deputy#Deputy [19:25] This might be the exception to the rule, since the fields are basically 100% of what the package is about. but even so, it's kind of hard to read. [19:26] natefinch, yeah, like so many things it's a matter of taste and judgment [19:26] yep [19:26] natefinch: Errors is a poor name for that field :p [19:27] natefinch, fwiw, I'm trying to factor some stuff into one place for state, and I've ended up with this type: http://paste.ubuntu.com/11800862/ [19:27] natefinch, overcommented? perhaps [19:27] natefinch, but *useless* comments? I hope not [19:27] natefinch, and I don't know where to put them if not the struct iitself [19:28] perrito666: not when used in the code: d := deputy.Deputy{ Errors: deputy.FromStderr } [19:30] fwereade: I think this is like my deputy example, where there's a lot of subtlety in the fields. Maybe this is a good use for field comments... but then again, it's really hard for me to even see what fields exist in the struct because of all the comments. The comments might do just as well living in the struct comment, and leave the body of the struct easier to read. I dunno. [19:31] natefinch, yeah -- it's not great, but AFAICS documenting the structs is less awful than any other approach === kadams54 is now known as kadams54-away [19:33] fwereade: maybe for your struct, short comments per field and longer comments in the struct comment is warranted. [19:35] natefinch: this is close to derive in an editor war [19:37] perrito666: is derive an editor? [19:37] Bug #1455623 opened: TestPingCalledOnceOnlyForSeveralWorkers fails [19:37] lets suppose I want to do something for command line like juju deploy --flag foo=afoo bar=abar beh=beh, what would be the better syntax? (or does gnuflag support multiple --flag? [19:38] natefinch: was that a joke? I am unsure [19:38] perrito666: not a joke. I could google for it, I suppose [19:39] fwereade: thanks for email. I replied but I'm not sure if it sent, my client is playing up (can't see it in sent box), did you get it? [19:39] lool [19:39] natefinch: apparently some of my english is outdated [19:39] says the dict [19:39] archaic : bring [19:40] heh [19:40] perrito666: sorry, the way it was used, it sounded like derive was an editor.... I never know what wacky editors people use. if sublime can be an editor, why not derive? :) Sorry for the misunderstanding. [19:42] derive was an old math sofware for windows 3.1 [19:42] perrito666: make flag work like constraints --constraints "foo=bar baz=bat" [19:42] ah, that is nice, thx [19:58] Bug #1470220 opened: Juju-deployer incorrectly reporting errors in juju 1.24.0 environment [20:07] waigani, yeah, I saw it, thanks for the quick response [20:10] fwereade: ah phew. np. [20:27] ericsnow: you mentioned I should also add the code for registering this in component/all/process.go ... but I'm not really sure what that code should look like. [20:28] natefinch: take a look at what's there already; it should be similar [20:29] natefinch: http://pastebin.ubuntu.com/11801171/ [20:29] natefinch: basically a call to RegisterStandardFacade (or whatever) in registerHookContext [20:29] katco, ericsnow: thanks :) [20:33] natefinch: could you take another look at http://reviews.vapour.ws/r/1996/? [20:33] natefinch: I'd really like to get that landed [20:36] natefinch: looks like your conversion helpers (API2Proc, etc.) did not make it into your latest patch [20:44] ericsnow: doh, yeah, new file. Stupid git [20:44] natefinch: I figured :) [20:54] ericsnow: http://reviews.vapour.ws/r/2061/diff/2-3/ [20:56] natefinch: doc comments on API helpers... === kadams54 is now known as kadams54-away [20:57] natefinch: otherwise looks good :) [20:58] ericsnow: ahh yeah, had to export them so now have to comment them, damn [20:58] natefinch: yeah, sorry [21:55] thumper: this is the only issue unique to wily and vivid. fixing https://bugs.launchpad.net/juju-core/+bug/1468349 will grealy increase chances of a pass [21:55] Bug #1468349: discoverySuite.TestDiscoverServiceLocalHost: invalid series for wily and vivid [22:10] sinzui: cheers [22:32] sinzui: I can replicate the singular intermittent failure locally [22:32] * thumper is on it [23:04] sinzui: I have a fix for the singular worker [23:04] \o/ [23:04] thanks to davecheney's stress script [23:04] weird time.Timer behaviour, but fix works [23:07] waigani_, cherylj: http://reviews.vapour.ws/r/2066/diff/# [23:07] as on call reviewers :) [23:08] note: the interval := PingInterval call is probably not strictly necessary, but I did it initially when the calls were separated, and I wanted to make sure the value wasn't changing outside that go routine [23:08] so it only used the local value [23:08] it wasn't changing, but the isolation is still there [23:08] * thumper thinks [23:09] * thumper removes it [23:10] menn0: http://reviews.vapour.ws/r/2066/diff/# [23:11] thumper: looking [23:11] cheers [23:11] * menn0 is glad to have something to distract him from the woeful situation he is looking at [23:22] thumper: I'm assuming you manually tested the fix and the intermittent failures are gone? [23:22] waigani_: sure as eggs [23:22] thumper: lgtm [23:41] davecheney: hey, have you tried to build/run juju with the go arm64 port yet?