[00:03] waigani: yep thanks. I changed parseTag during the meeting and those annotator tests started parsing [00:03] waigani: and I knew about the megawatcher change [00:03] menn0: sweet [00:03] waigani: from reviewing the services change [00:03] menn0: okay, i *think* they are the only spots that will need progressive updating as we migrate [00:04] other than dealing with collection specific quirks of course [00:06] waigani: that sounds right [00:06] waigani: and then there's all the queries that need to be updated to filter by env uuid [00:06] waigani: but we should get the collections in shape first [00:07] out of curiosity.. anyone using gooracle [00:07] davecheney, pong [00:07] menn0: largely mechanical [00:07] hazmat: I wrote a sublime text plugin: https://github.com/waigani/GoOracle [00:08] hazmat: I use it most days, has really helped :) [00:09] waigani, nice.. i've got sublime.. but emacs users myself.. currently resurrecting my go editor conf.. oracle seems pretty sweet [00:09] hazmat: menn0 uses emacs with oracle [00:10] I couldn't keep up with him, so a wrote the plugin for sublime ;) [00:12] waigani, solid.. i'm currently pimping my ride following along to http://yousefourabi.com/blog/2014/05/emacs-for-go/ [00:12] hazmat: are you coming to paris on friday ? [00:12] davecheney, unclear.. its not sold out it is it? [00:12] nope.. still avail [00:13] oh.. crap.. super late bird only now [00:13] hazmat: well, you are super late [00:13] davecheney, its like SO next month :-) [00:13] hazmat: its like so 15 days away [00:14] if i get through a day i'm on top of the world [00:14] davecheney, noted.. i'll have it sorted this week [00:15] hazmat: ok, if you get stuck I can ask the organisers [00:15] i won't be a free ticket [00:15] but we might be able to get you into the overflow [00:15] i gotta coordinate with sidnei he'll come out as well if i go [00:15] is sidnei still in zurich ? [00:15] yup [00:15] cool [00:15] well, let me know [00:16] will do [00:16] heading out latter this week to surgecon.. aka disaster porn [00:17] hah.. twitter feed on that topic.. "Bryan Cantrill @bcantrill · 8h Does systemd have you #illumos-curious? " [00:47] lol [00:47] systemd-make-me-a-sandwich [00:57] hazmat, waigani: the oracle emacs integration is pretty good. I use go-oracle-referrers and go-oracle-implements fairly often. [01:10] thumper, davecheney: is the first line of this method a little crazy? [01:10] http://paste.ubuntu.com/8414641/ [01:11] or is that intentional for some reason? [01:13] thumper, davecheney: actually I can understand why it might be making a copy, but assigning the copy to u seems a little sketchy [01:13] * thumper looks [01:14] menn0: where is the assigning to the copy? [01:14] oh [01:14] ew [01:14] first line [01:14] wha...? [01:15] I'm guessing the reciever is effectively a value type [01:15] assigning to it means nothing other than saving declaring another value? [01:15] seems a bit esoteric [01:16] it would be clearer if done like this: u2 := &Unit{st: u.st, doc: u.doc} [01:16] agreed [01:16] ok. [01:16] or... unitCopy [01:16] as long as it's not a bug or pure evil [01:16] or whatever [01:17] I don't think it is either of those [01:17] just a bit confusing [01:17] davecheney: do you agree with that assessment? [01:17] thumper: using a longer than 2 character variable name??? That's crazy talk! This is Go! [01:17] :P [01:18] :) [01:19] axw: you around? [01:20] axw: two things, I just noticed that we have a ci blocker, see topic ^^^ [01:20] axw: secondly I want to chat to you about the gridfs storage stuff [01:21] thumper: I am here now, sorry, bit late [01:22] * axw looks at topic [01:22] gah [01:22] ok, will get onto it [01:22] thumper: what about gridfs? [01:24] axw: I'm munch on lunch and then catch up [01:25] okey dokey [01:25] axw: mostly about how we store stuff in the paths and what impact multiple environments will have [01:25] if any [01:25] * thumper is hoping for no impact [01:25] mkay [01:25] paths already encode env UUID [01:26] we do need some modifications to pass in env-specific UUIDs though, atm it's just using State.Environment() to get the one and only UUID [01:37] axw: hangout? https://plus.google.com/hangouts/_/gupglu6np5t3logeqtcab2praua?hl=en [02:05] thumper menn0 davecheney: trivial review please: http://reviews.vapour.ws/r/91/ (fixes CI blocker) [02:06] * menn0 looks [02:09] axw: what happens if we run this test on arm or power? [02:09] axw: should it work? [02:10] or do we mock out the architecture elsewhere? [02:10] thumper: it's uploading fake tools, should work still [02:10] the UploadArches just tells the fake-tools-uploading code which arches to upload for [02:10] ok, but uploading fake tools for only i386 and amd64 [02:10] it's ec2, there is only i386 and amd64 [02:11] ah, ok [02:11] thumper, axw: i've already given it a Ship it (looks through the code to figure out the above) [02:11] axw: can I get you to add just a comment before the UploadArches? [02:11] sure [02:11] menn0: thanks [02:11] axw: just to say that if ec2 expands it architectures, we should add extra tools here? [02:12] thumper: will do [02:12] thanks [02:12] * thumper goes to rubber stamp menn0's review [02:13] * thumper goes to make a coffee [02:15] thumper: /me reads [02:15] hmm, that is some complex code [02:15] such side effect [02:15] much impurity [02:25] thumper, waigani: this units env UUID change has been tough, especially the unit watcher [02:25] 11 failing tests in state left [02:25] then the megawatcher change [02:25] and then the migration [02:25] menn0: yeah, I thought it would be pretty shitty [02:26] menn0: i *think* that line 2 on that paste does not update u [02:26] well, it updates the copy of the pointer passed to WatachSubUnits [02:26] but that is fine [02:26] davecheney: that's the conclusion that thumper and I came up with too [02:26] but it's confusing code and should be rewritten [02:26] but it's not especially clear [02:26] * menn0 nods [02:27] since finding that one I've found a few other methods that do the same thing [02:27] *u = &Unit would update the caller's copy [02:27] actually [02:27] *u = Unit [02:27] yep [02:27] u = &Unit changes the pointer value passed to you to point to something else [02:27] it's just a bit nuts [02:27] I'm pretty sure it is explicitly creating a copy as there will be a go-routine accessing it [02:28] and this way, they avoid extra mutexes [02:28] davecheney: I agree, unclear code [02:28] menn0: I suggest you create a new variable, not reusing 'u' :) [02:28] davecheney: as I said earlier something like u2 := &Unit{...} would have been much clearer [02:28] it updates u in the scope of that method [02:28] but to point to a new value [02:29] not the valye that u points to [02:29] what code are you lot talking about? [02:30] waigani: http://paste.ubuntu.com/8414641/ [02:30] thanks === Ursinha-afk is now known as Ursinha [02:35] * thumper makes a sad face [02:35] sel := bson.D{{"_id", bson.D{{"$regex", "^" + regexp.QuoteMeta(ensureActionMarker(prefix))}}}} [02:35] iter := actionsCollection.Find(sel).Iter() [02:35] bodie_: you around? [02:38] thumper: I may have been responsible for that code [02:38] jcw4: hey [02:38] hey :) [02:38] jcw4: I'm looking at the collections working out what is needed for the multi-environments [02:38] what makes up the id for an action? [02:38] also, what is the prefix normally in the above code? [02:39] as a general rule, I don't like encoding values into an id field that need to be decoded later [02:39] thumper: the prefix of an action id is the id of the unit to which it has been queued [02:39] thumper: agreed [02:39] jcw4: is an action always for a unit? [02:39] thumper: at this point yess [02:40] thumper: the intent is to allow prefixes of service id's too [02:40] jcw4: can you concieve of a time where it won't be? [02:40] hmm... [02:40] thumper: although even in the case that an action is queued for a service I think it will ultimately end up queued for a unit [02:40] thumper: so maybe not [02:41] jcw4: right, so the idea that the CLI says run this action for this service, and we create an action for each unit [02:41] thumper: the main rationale for the prefix is so that the watcher can easily filter [02:41] that exists at that time [02:41] thumper: right [02:42] jcw4: was there pushback from someone about having a Unit field explicitly and independently from the id? [02:42] thumper: long story - I think fwereade may have a strong opinion on it [02:42] * thumper chuckles [02:42] (but I don't want to put him on the spot) [02:42] :) [02:43] thumper: when I talk to you, my initial feeling that the id should not encode the unit id is re-inforced [02:43] this may be a situation where fwereade and I both hold strong and opposing points of view [02:43] thumper: however when I talk to fwereade he makes compelling case too [02:44] what is the case to keep it in the id, [02:44] apart from saving a few bytes? [02:44] thumper: I beleive the primary reason is to make the watchers efficient [02:44] ah fark [02:44] that's wright [02:45] most of the watchers only see the id field right? [02:45] thumper: exactly [02:45] wright? [02:45] right! [02:45] haha [02:45] fudge cake [02:46] thumper: that being said, I think env UUID prefix on the unit would transparently flow through to the action? [02:46] I hate designing our system around problems that we have made for ourselves [02:46] *cough* mongo [02:46] jcw4: no... [02:46] where is wallyworld when you need him [02:46] jcw4: not exactly [02:47] jcw4: the way we are dealing with the environment uuid is as follows [02:47] * jcw4 leans forward [02:47] jcw4: and we may want to tweak the actions collection too [02:47] jcw4: we read the full document [02:48] jcw4: and then prefix the existing _id field with ":" [02:48] jcw4: also adding "env-uuid": [02:48] jcw4: and changing the existing _id field to something meaningful [02:48] like "name" or "foo" [02:49] jcw4: I think it would be worthwhile also adding in "unit" [02:49] jcw4: when we do the schema migration for actions [02:49] thumper: prefixing the existing _id field and then adding something meaningful? [02:49] jcw4: the service document changes are now in master [02:49] jcw4: do you have an up to date master as of about 18 hours ago? [02:49] jcw4: I'll explain [02:49] thumper: yep [02:49] thumper: lemme look [02:49] hangout? [02:49] sure [02:50] jcw4: who do I add? [02:50] johnweldon4@gmail.com [02:50] kk [02:50] https://plus.google.com/hangouts/_/g424bdmxb7yrh4awwwarvkuw7ia?hl=en [03:10] jcw4: this commit: 59cfe5aff287b46de440e45dfc2aa25b34dac571 [03:11] jcw4: git diff 3c0b77b..59cfe5aff [03:16] thumper: git diff 59cfe5aff^..59cfe5aff [03:21] jcw4: ah, that works too? I'm assuming it takes the first parent if a merge? [03:49] thumper: yeah I think so [03:53] thumper: http://git-scm.com/book/ch6-1.html#Ancestry-References [03:54] jcw4: cheers [04:13] thumper, waigani: almost done with the units env uuid work [04:13] menn0: awesome [04:13] menn0: nice [04:13] thumper, waigani: just saw this in TestAddEnvUUIDToServicesIDIdempotent: [04:13] serviceResults[0].DocID = s.state.docID(serviceName) [04:13] } [04:14] that's supposed to be an assertion [04:14] thumper, waigani: we all missed it :( [04:14] yikes [04:14] haha [04:14] yep [04:14] * menn0 fixes [04:14] thank you [04:15] waigani, thumper: fixed. thankfully the tests pass with the assert in place [04:16] :) [04:16] few [04:16] phew? [04:17] true [04:29] * thumper whimpers [04:30] * thumper head desks [04:32] menn0: is the upgrades collection used solely by the state servers to synchronise updates? [04:33] thumper: yep [04:33] thumper: although Will and I were thinking of using it to report the status of the last upgrade in the juju status output [04:34] menn0: so, about env-uuid, should we add it? [04:34] thumper: probably not... since the upgrade isn't environment specific [04:34] well... it sis [04:34] is [04:34] thumper: the status thing isn't planned for any time soon [04:34] thumper: well it is sort of [04:34] yeah, sort ov [04:34] of [04:34] thumper: the state server upgrade aren't [04:34] ugh [04:35] thumper: the whole upgrades with MESS thing needs to serious thought. Maybe a good topic while we're in Brussels? [04:35] probably [04:38] thumper: for your current planning I would lean towards not including the env UUID in the upgrades doc [04:38] menn0: that is what I have decided too for now [04:39] thumper: k [04:45] thumper, waigani: the tests for adding env uuid to services and units are now each one line [04:45] the test implementation is generic and works for units and services [04:46] woot woot [04:46] it should now be much easier to add more env UUID migration steps [04:46] about to try a similar refactoring for the upgrade steps themselves [04:49] menn0: we also need to talk about what backup means for other environments [04:49] thumper: definitely === thumper is now known as thumper-cooking [04:58] thumper-cooking: http://reviews.vapour.ws/r/92/ (fwereade and TheMue too) [05:27] thumper-cooking: env UUID change for units here: http://reviews.vapour.ws/r/93/diff/ [05:46] waigani: if you're still working, can you please take a look at my response to your review? [05:50] axw: done [05:50] waigani: thanks! [05:51] davecheney: you too, if you don't mind - there's two issues you raised that I didn't resolve, but commented on [05:54] axw: /me looks [05:54] axw: which link ? === uru_ is now known as urulama [05:54] sec [05:55] davecheney: http://reviews.vapour.ws/r/82/ [05:56] axw: i can't see your comments [05:57] davecheney: nothing shows up next to your comments on the front page? [05:58] davecheney: there were two things unresolved: you asked if RB screwed up the formatting; looks fine to me on RB, and go fmt is happy anyway [05:58] the other was whether we should return an error from StartInstance if we can't record the instance-id [05:59] and I said no, because (a) we never recorded instance-ids for non-bootstrap machines before (known deficiency), and (b) if we fail to do it for bootstrap ,we'll have other issues anyway and bootstrap will tear down [06:08] axw: nope, can't see your comments [06:08] did you publish them ? [06:09] nah, RB is screwing up the indentation of some blocks [06:09] it's happened on other reviews as well [06:09] davecheney: yes, I did publish. weird, looks fine on my end [06:20] great, it's not even showing me my comments uinless i'm on the review screeen [06:20] it won't show them on the diff screen [06:20] what the f [06:28] davecheney: thanks. comments on the diff screen show up as a little box on the LHS, which you need to hover over [06:28] yup, couldn't see them [06:28] honestly given up [06:28] your change is good [06:28] no need to sweat the small stuff [06:29] hrm, yeah not showing up for me either. maybe because I uploaded a new changeset? I dunno === thumper-cooking is now known as thumper [06:29] woot, once more step till Environ.Storage() is no more [06:29] one* [06:30] \o/ [06:30] * thumper looks at the reviews [06:30] * thumper takes a deep breath [06:31] axw: i've no fucks left to give rb [06:32] heh :) I must say I'm not overwhelmed by it, I'd gotten used to GitHub reviews [06:34] * thumper headdesks [07:11] fwereade: have you started? [07:13] guess not [07:46] morning folks [07:49] morning mattyw [07:50] mattyw, you're OCR today, right? have a look at http://reviews.vapour.ws/r/95/ please? [07:51] dimitern, be happy to [08:35] does anyone know if it's ok to have a non-subordinate relation to juju-info ? [08:35] fwereade: ^ [08:37] dimitern: ^ [08:39] rogpeppe, why not? it's not bound to subordinates afaik [08:39] dimitern: i *thought* it's probably ok, but i'm just writing some relation-verification code in the charm package and wanted to check [08:40] dimitern: i'll assume it's ok for now. the code in the state package doesn't look as if it complains [08:40] rogpeppe, but in general, using juju-info is frowned upon [08:40] dimitern: frowning isn't relevant here :-) [08:41] rogpeppe, :) just saying [09:08] dimitern, LGTM [09:08] mattyw, cheers! [09:16] rogpeppe, sorry I missed that -- but yeah I think it's fine [09:16] fwereade: cool, thanks [09:16] rogpeppe, pretty sure that if it wasn't we would have scoped it differently [09:16] fwereade: yeah [09:18] rogpeppe, I think one of the possible use cases was a penetration testing charm :) [09:18] fwereade: nice idea [09:19] reviewboard seems slow today - anyone else seeing that? [10:03] axw: aren't you supposed to link your RB review back to the original branch somehow? (like via a github PR? ) [10:38] jam: could be helpful I guess - I didn't think anyone would care about the PR [10:38] I can do that tho [10:41] axw: so *I* occasionally like to actually download the code, and that is hard to do from just RB [10:42] fair enough [10:42] axw: in this case, I trolled github and found your PR and your branch, but it would be nice to have a direct link (though even just to the git branch would be fine for my purposes) [10:43] * axw nods [10:43] though I don't think github makes it easy to see what PR's a given branch is involved in [10:43] I can see what PRs are proposed to go into a branch, but is there an obvious "this branch is proposed in these PRs" ? [10:43] not that I'm aware of [10:45] mattyw: it seems intermittent, and I'm seeing slowdown elsewhere, so I'm wondering if it is "Internet" or RB [10:46] jam, it feels like it's just slower on the larger reviews [10:47] dimitern: standup ? [10:47] jam, omw [10:54] morning [11:16] fwereade, jam, mattyw, axw, please take a look http://reviews.vapour.ws/r/96/ [11:17] dimitern, looking [11:18] mattyw, ta! [11:29] dimitern, one thing from me, might be something we need to discuss [11:30] mattyw, I haven't changed the behavior what's allowed and what's not, I just converted the check already in place to use AuthFuncs [11:31] * mattyw looks again [11:32] dimitern, I see it now - that's a great point - and a potential bug - thanks very much [11:33] mattyw, sure, np [11:55] folks, just wanted to say I've landed some code that fails the go vet test because I forgot to enable the pre push hook - this is me saying I'm sorry, it won't happen again [11:56] mattyw: I'm thinking mgz should be aware that the bot should be running go vet if we want to require it [11:57] mattyw: we love you anyway [11:58] mattyw: I don't hold people to very high "you must be very careful doing X", but I do hold our *process* to that standard. So if something happened that shouldn't have, that is more a sign that we're missing a step. [11:58] especially something like 'go vet' which is highly automatable [11:58] perrito666, I have to assume you love me because of all this ;) [11:58] jam, in that case think of me as a part time - "problem in process finder" [11:59] fwereade: if you have any time today, I'd appreciate if you could weigh in on http://reviews.vapour.ws/r/94/ -- just to make sure I've not veered wildly off course [11:59] mattyw: everybody makes mistakes in process, which is why I don't feel bad. Anytime the design of something is "as long as we're careful" that's a sign of something brittle that should be done differently. [12:03] mattyw: we wouldn't need a test suite if we were just careful enough to not make mistakes :) [12:09] jam, sounds awesome - let's do it! [12:09] jam: what state is charm.v4 in? is it ok to move the charm store to use it? [12:09] rogpeppe: the only change there (to my knowledge) is updating the gocheck dependency, so it will still need to have v3 merged into it [12:09] jam: cool, thanks [12:10] jam: i have actually been applying v3 patches to v4 too, so all should be ok [12:50] jam: any idea why I would still be getting the, ERROR unknown object type "RunCommand", error from the rpc reflection, even though I've defined an init, registered a common facade 'RunCommand', and added that import to allfacades? [12:51] wwitzel3: not using upload tools? [12:53] using upload-tools [12:57] wwitzel3: facadeversions? [12:57] with capital V [12:58] perrito666: ? [12:58] perrito666: isn't facadeVersions built from registered facades? [12:58] I am checking Backups facade to see what other steps are required [12:59] wwitzel3: I see a map of strings and numbers [12:59] perrito666: I do too :/ [12:59] first time noticing it, le sigh [13:00] it would be nice to have a facade todo list :p [13:01] I even read through TheMue's API implementation doc, and I don't recall it mentioning facadeversions, but I probably just missed it. [13:03] wwitzel3: the current proposal? pushed it yesterday again. [13:04] wwitzel3: I'd have to see your code to give you any hints [13:05] TheMue: yeah, current version, I didn't see any mention of api/facadeversions [13:05] jam: well perrito666 pointed me at facadeversions which has me getting a different error now, but it is progress :) [13:05] wwitzel3: ah, ok, currently I only cover the server-side [13:06] wwitzel3: sure, but I'll help you with it if you point me to code. [13:06] TheMue: I'll write some notes so I can help fill out the client side [13:06] wwitzel3: backups is very cool for that, its a very uncommon word so it makes easy to find what must be done for a faade [13:07] wwitzel3: great, thanks [13:07] jam: https://github.com/wwitzel3/juju/blob/ww3-juju-run-with-context/apiserver/runcmd/runcmd.go [13:08] jam: and the client is https://github.com/wwitzel3/juju/blob/ww3-juju-run-with-context/api/runcmd/client.go [13:09] jam: the new error is ERROR no such request - method RunCommand(1).Run is not implemented .. that is after adding RunCommand, 1 to api/facadeversions.go [13:10] wwitzel3: your Run() function takes a slice [13:10] you're only allowed to put structs [13:10] so if you need a slice, then you need a struct that has a slice [13:11] wwitzel3: I *think* if you run at the right debug level the RPC code will tell you that it is discarding certain functions (and hopefully why) === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: None [13:11] wwitzel3: make sense? [13:12] jam: yep, makes sense [13:13] wwitzel3: so without setting facadeversions, it was probably using BestAPIVersion to then request version 0 of the facade, which didn't (and shouldn't) ever exist [13:13] wwitzel3: and Run() just wasn't being exposed because it was taking a slice. [13:14] jam: thank you [13:14] perrito666: thank you [13:14] wwitzel3: np [13:23] dimitern, http://reviews.vapour.ws/r/96/ ship it [13:28] can I get a quick review from someone? I'm OCR today but I don't trust myself: http://reviews.vapour.ws/r/97/ [13:29] mattyw: that's a decent excuse I suppose :P === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1373424 [14:02] fwereade, can you comment on bug 1372016. We either have a bug in the API or we need to document that clients cannot assume that the error information is in a consistent place [14:02] Bug #1372016: api errors coming back in response instead of envelope

[14:02] * fwereade looks [14:03] sinzui: fwereade: I think the statement there is "if your whole request is invalid, Error is on the overall scope", if one of the pieces of your request is invalid, then that piece gets the error. [14:03] since EA now takes a slice of machines, right? === kadams54_ is now known as kadams54 [14:04] (most bulk apis work this way, just because 1 entry of a slice is bad, we don't error on the whole request) [14:04] jam, fwereade, thank you. since we don't have a time machine to fix old envs, it might be true that all API clients need to check for the error in many locations [14:04] jam, sinzui: yes, the top-level errors are not the primary way of discovering problems at all -- if things are so bad that we can't give you the usual response, that's where the error will be [14:08] gsamfira: can you join #juju? someone is having difficulty deploying a windows charm [14:19] natefinch: joined [14:22] natefinch, can you ask someone to look into bug 1373424. I think the issue has been in the code for a long time [14:22] Bug #1373424: method Client.AddMachinesV2 is not implemented