[00:00] * thumper looks [00:02] sinzui: no. sorry [00:03] sinzui: but I have felt that our upgrade story is too flakey [00:03] I have plans (in my head) [00:03] and half written down [00:03] yeah. Sometimes I need to wait forever for it to happen. I cannot predict if it is slow or will fail [00:04] but I think the 14 -> 16 upgrades are better than 12 -> 14 [00:05] :) === bjf is now known as bjf[afk] === gary_poster|away is now known as gary_poster === gary_poster is now known as gary_poster|away [03:52] thumper: I'm updating the API to support pushing secrets now. I was chatting with rogpeppe yesterday, and we discussed having a negotiation thing during login. Then fwereade and jam reminded us that you were keen on making bootstrap blocking, and secrets-pushing can go there [03:53] so... I'm just going to add an API call to push secrets. sound ok? [03:53] we'll unconditionally call it when making an API connection for now, and later just during bootstrap [03:53] sure [03:53] and yes we want bootstrap synchronous [03:53] it would solve all manner of bootstrap issues [03:54] sure [04:04] wallyworld: FYI - https://codereview.appspot.com/22320043 [04:05] thumper: looing [04:05] looking even [04:05] * thumper goes to help with dinner, then swim [04:05] I'll be back later this evening === thumper is now known as thumper-afk [04:06] +1000 to synchronous bootstrap. a lot of us have been wanting that for ever [04:06] so long as there's feedback as it executes [04:15] WHOOOOOOOOOOOOT! [04:16] ooh [04:16] yay for output :) [06:13] ping ? [06:21] what is the standard way of including charmhelpers in a charm ? [06:22] i.e. should I branch charmhelpers and check it in together with the rest of my charm? [06:23] sodre: thats what I've done in my charms, but I don't know if its considered best practice [06:28] bradm: thanks. [06:35] sodre: from everything I've seen you copy charmhelpers into your code [06:36] jam: alright :) That's how I'll do it then. [06:38] axw: I don't think we want to call it uncondionally during api.Open [06:40] jam: why not? [06:40] axw: we only need to pass secrets 1 time, rather than the 100 times you ever call status/etc8 [06:41] jam: sure, but detecting that requires feedback from the server - seems like a lot of work for something that's going to go away soon [06:41] we currently go to state each time we connect to see if we need to do it [06:48] oops, race dector'd my machine [06:48] detector'd even [06:48] race decor'd sounds more interesting [06:49] jam: if you replied, I missed it. last message I have is "we currently..." [06:49] no, I didn't say anything [06:49] ok [06:50] jam: I realise doing it every connection sucks, so if there's a non sucky way, I'm open to suggestions [06:50] axw: is it that hard to add an extra field to Login that says now that you've logged in, can you send the credentials [06:50] ? [06:52] jam: the Login code knows nothing about environments or secrets. It's probably not hard, just felt a bit wrong. I don't know that area of the code well, though [06:52] well, it knows the state object of course [06:53] axw: so I think we need a state/api/client level interface that supports passing secrets in, no doubt about that. [06:55] jam: ah, you're suggesting a result from Login that says the next call should be PushSecrets? [06:55] axw: looking at Login today, all it can return is an err [06:55] and if we changed *that* behavior [06:55] it would break things like the GUI [06:55] yup [06:55] I suppose that might be ok, since the GUI can't ever send secrets? [06:55] one idea that came up was returning a special error [06:55] right [06:55] so you have to always connect via another means 1st time anyway [06:56] I'd really like to run it by gary_poster|away or someone else on his team [06:56] I think they're all mostly US time based [06:57] ok [06:57] axw: so the Client api for pushing up secrets is needed regardless, we'll likely use it in bootstrap, etc [06:57] axw: do you know about "directory.canonical.com" ? [06:57] jam: yeah, I was thinking you were talking about the idea of adding secrets into the Login call itself, which I think is too much work [06:57] jam: sure do [06:58] so frankban is the only one in EU timezone. Huw is in AU, but he's more Web/UI focused. [06:59] axw: so I wouldn't put the secrets *into* Login, because I would avoid putting them on the wire as much as possible [06:59] I was hoping Login had a nice way of telling you something after you login [06:59] rather than only returning an err [06:59] and that would give us a way to extend it with "please upload secrets" without telling clients to shove off [06:59] err => most clients will probably just disconnect right away [07:00] jam: yeah. I think GUI requires it to be there already, but I can check with gary_poster|away [07:00] axw: I actually don't think it strictly does, though to install the GUI you would have had to pass the secrets [07:00] because, you have to "juju deploy" the gui [07:00] so actually, I think we're ok there [07:00] well, sorry, that's what I meant [07:01] implicitly requires :) [07:01] sorry I'm mostly catching up to something that you've probably thought more about [07:01] so I think short term putting the err return in Login is ok [07:01] that's cool [07:02] because we don't have much juju client stuff that connects via the api in older versions [07:02] just add-unit and get [07:02] and you are very unlikely to call either of those [07:02] until you've run some other command [07:02] which is why we could get away with it :) [07:02] hehe [07:02] axw: it *was* why I picked those. That and they were pretty straightforward. [07:02] axw: if you don't mind investigating a bitd [07:02] bit [07:03] can you see if we can add a return parameter to Login that isn't an err? [07:03] and see if old clients complain about it [07:03] jam: sure, will do [07:03] axw: because if we can do that compatibly, I can see a use case for doing other things at Login time [07:03] giving hints about compatibility, etc [07:03] I'll get the new API call working first, then go onto the Login stuff and look at that first [07:03] so adding a struct to the return will be generally beneficial [07:04] but if we need to go the err route, I think that's ok [07:04] sounds good [07:04] axw: I would even go as far as propose the API call so that we can land it [07:04] so you don't have to have that pending in a branch [07:04] unless you think it might need tweaking based on your other results [07:05] we'll see, tho I expect the only thing that'll change is where it's called [07:05] (one thought, bootstrap doesn't ever connect as a Client today, so it might just push the secrets direct into the DB, though the fact that we want bootstrap to hang around until the API is up, maybe it is sane to have it do the Client login as well...) [07:05] axw: in which case, it would just be the first thing that logged in [07:06] axw: I don't really like it being an err as a general quality thing [07:06] yeah that did occur to me - we could just leave that bit alone once it's implemented for the general case [07:06] but I could live with it [07:06] yup, fair enough [07:06] it's not an error, after all [07:07] axw: so looking at state/api/Login today, it just passes "nil" for the response struct [07:08] jam: yeah, but does adding one after the fact break the clients? that's what you're asking me to investigate, right? [07:09] axw: right. I'm poking around right now :). If resp == nil ; resp = &struct{}{} and then we still call conn.codec.ReadBody(resp, isRequest) [07:09] so it looks like [07:09] current clients will still consume a body response, and just throw it away [07:09] which is fine [07:09] cool [07:09] axw: but we should probably test that in practice :) [07:10] and clients ignore extraneous fields, right? [07:10] axw: generally, yes [07:10] JSON.Unmarshal loves throwing away unrecognized things [07:10] heh [07:10] axw: AFAICT there's no way to pass it a Strict flag [07:13] * jam => away for lunch and groceries [07:14] later [07:15] axw: to summarize before I go. It looks like putting it as an extra parameter is going to be just as much work and just as compatible with the existing clients, rather than changing the Err, so I'd rather do that. [07:15] but, as always, testing with 1.16 against a running patched 1.17 is probably worthwhile [07:15] jam: agreed, I will go with that. [07:15] (and test) [07:48] morning all === thumper-afk is now known as thumper [07:54] fwereade, jam: do you guys have time for a catch up hangout? [07:58] thumper, sure [08:01] fwereade, jam: https://plus.google.com/hangouts/_/76cpjgbfmbsvbg46v349h6l5ug?hl=en [08:32] dimitern: hi there. how close are you to finishing the get-environment migration to the api? [08:35] wallyworld, get-env is done, finishing set-env now and proposing both [08:35] dimitern: great :-) cause i need the ability to get environ config via api and i figure your work will include that [08:36] wallyworld, yeah [08:37] wallyworld, basically, you can use client.api.state.EnvironConfig to get it, where client is the "c" receiver of the method [08:38] ok, thanks [08:39] i need EnvironConfig() exposed in state/api/client [08:39] wallyworld: is that for add-machine? [08:39] yeah [08:39] the manual provisioning [08:39] ah [08:41] * wallyworld -> dinner, bbiab [08:54] fwereade: I take it you and tim already finished up? [09:05] wallyworld, dimitern: I think we need to think reasonably carefully about exposing all of EnvironConfig via the Client API. Because it ends up having environ credentials which we don't want to expose to something like the GUI (I think). [09:06] jam: i need it to create an environs.Environ instance. we could audit the usage of that to see what's really needed. it will be messy [09:06] wallyworld: what do you need for Environ given you are doing manual provisioning? [09:07] (I have the feeling it is accidentally using something it shouldn't) [09:07] just so it can do something like get AuthorizedKeys [09:07] jam: it's used in the manual provisioning code in a few places [09:07] which we ran into for the Provisioner [09:07] setting up auth is one such usage i think [09:07] finding tools is another [09:07] i can't recall the third offhand [09:08] wallyworld: given what you've been saying about avoiding provider storage at all, is that something we are trying to get away from? [09:08] anyway, I can see it being necessary today, though I'd like us to understand what we really need, because we've run into several cases where we really didn't want it [09:09] possibly. i'm initially looking to just do a straight port [09:09] agreed about seeing what we really need [09:09] will get messy, so looking to do it in stages [09:09] wallyworld: right, the main problem with just the straight port is that we can expose secrets that we don't want to commit to supporting [09:10] well, we use State now to get a full env in the current code [09:10] so the cmd has secrets today doesn't it? [09:10] wallyworld: yes, but it has straight DB access today. The issue is that this isn't the only point that uses Client [09:10] GUI uses it directly, for example [09:11] mornin' all [09:11] and we are trying to support juju -the-commandline that doesn't have raw access to EC2 creds [09:11] jam: ok, and you are saying as soon as we add that to the api the gui could misuse it [09:11] so that you can share your env in a limited fashion [09:11] wallyworld: I'm not saying the GUI would, because i trust Gary et al, but it is *an* api that is leaking security in a place where we weren't before [09:11] jam: what about the case where a GUI client actually needs to change cloud credentials? [09:12] EnvironConfig specifically [09:12] jam: yes, that's what i meant [09:12] is something we've been really careful about sharing [09:12] jam: or in fact, and client [09:12] when it came to the agents [09:12] s/and/any/ [09:12] wallyworld: I think the specific security hole is some *other* code running in the same machine where the GUI (etc) are running and getting access to secrets. [09:12] certainly that was the Uniter issue [09:12] * wallyworld nods [09:12] we didn't want any charm to be able to get access to EnvironConfig [09:13] jam: ok, so we may need to avoid the EnvironConfig via the api and stick to current method and refactor [09:13] jam: but get-environment uses it afaik [09:13] rogpeppe1: that may be a use case that we need to support. *My* point is that we need to actually discuss it and decide from a security perspective rather than just saying "oh just expose this, no problem" [09:13] jam, wallyworld, we already have a way to mask out secret attributes in environ config - take a look at the provisioner api's EnvironConfig [09:13] jam, wallyworld: i think that for the time being at any rate (pending user roles/permissions) we should not take away the ability for a client to see and change the environ configuration [09:14] jam: if we don't, we're breaking backwards compatibility [09:14] wallyworld: from what I've understood from fwereade, get-environment and set-environment do far too much. [09:14] i can believe that [09:14] and that is partially because we've sort of just shoved everything in EnvironConfig [09:14] jam: i guess it's possible we want to do that, but we should at least audit current users to check how much they rely on get-environment [09:14] let's discuss after standup with dimitern et al [09:15] jam, wallyworld: well, set-environment is terribly flaky by nature, but I think that's separate [09:15] wallyworld: like you can trigger an upgrade via set-environment [09:15] jam, wallyworld: yeah, that's something I would like to be very careful to avoid [09:15] jam, wallyworld: we may or may not have code that tries to prevent that [09:15] rogpeppe1, wallyworld: I would *guess* the #1 use case for it is to handle setting constraints, because we don't expose another way to set whole-environment values [09:15] i totally agree that there are somethings in the environ config that really should not be [09:16] and you set whole-env values at bootstarp time [09:16] jam, wallyworld: but the state method does basically depend on people calling it "right" [09:16] yeah [09:16] jam, there are no constraints in environ config actually [09:16] jam: you can set constraints with it? [09:16] fwereade: isn't that set in "juju set-environment" ? [09:16] jam, nope, set-constraints [09:16] it may not be EnvironConfig but part of that command? [09:16] ah, k [09:17] jam: if your environment credentials have been compromised and you need to change them, set-environment would seem like the right thing to use [09:18] fwereade, wallyworld, jam, rogpeppe1: https://codereview.appspot.com/22210044/ set/get-environment [09:18] jam: and for debugging, i've certainly relied on get-environment before, for sanity checking (otherwise there's no way of actually finding out what environ config the agents are really using) [09:19] jam: but we should certainly consider (at least) erasing secrets from the return of get-environment [09:22] dimitern: tests? [09:22] rogpeppe1, jam: I *think* that's dependent on user roles/permissions, isn't it? [09:22] fwereade: yeah, that's my thought [09:23] fwereade: and currently we treat the client as admin, so they get everything [09:23] fwereade: so I don't have a particular stake in the matter, I just wanted to make sure it got discussed [09:23] fwereade: it would seem a strong end-run around sharing a .jenv that didn't have env creds [09:23] if the first thing you could do [09:23] is run "juju get-env" [09:23] and have access to them [09:24] fwereade: given they can deploy to machine 0 and read the database, there's not really any security hole that we're avoiding by making the environ config available [09:24] jam: i think that having less privileged users is something we are aiming for, but we're not there yet [09:25] rogpeppe1, jam: true, but I long-term expect people to create relatively restricted users and share .jenvs authing those users as themselves, rather than having anyone actually share their jenv files directly [09:25] rogpeppe1: so I think with your work about api.Open we could actually support it for many commands very soon [09:25] wallyworld, there are tests already that pass without change [09:25] fwereade: sure, but if we don't actually think about security at get-environment, then it is a pretty clear whole [09:26] hol [09:26] hole [09:26] anyway, meh. as I said, I wanted us to be aware of it more than anything [09:26] dimitern: for the new apis? EnvironmentGet/Set? [09:26] wallyworld, oh, right [09:26] add-machine sounds like something that should explicitly *not* have an Environ since we aren't provisioning from the environ [09:26] wallyworld, will add, thanks [09:26] dimitern: we've been adding new tests for those new things we add [09:26] np [09:26] but I can see where it just used it so far, so we can keep doing so [09:27] jam: right, so we need to understand how env is used in manual provisioning, cause that's why add-machine needs it [09:27] add-machine per se doesn't need it [09:28] but add machine calls into the manual provisioning api [09:28] wallyworld: right, LXC Provisioner was asking for it too, and the *only* thing it pulled out of it was authorized keys [09:28] which thumper had to work to fix [09:28] manual does that too [09:28] plus some other stuff [09:28] because the MaaS environ can't be created if you strip secrets out of it [09:28] so we'll run into this again with not being able to manually provision with MaaS if we just did the same thing. [09:29] sounds so [09:29] i've just read the manual provisioning code today for the firt time [09:31] wallyworld, jam: a couple of things spring to mind: state/api.Info (could be gotten another way), and finding tools [09:31] wallyworld: sure. So I would encourage you to see if you can do add-machine without a full EnvironConfig because it shouldn't really be using it. If it isn't worth the time, then we can hack it in [09:32] wallyworld: so we do already have a Tools api for agents, I wonder how hard it would be to expose that for an "I want a machine over here". [09:32] is it call the null provider [09:32] the manual provider [09:32] jam, wallyworld: don't we need the full env config for the provisioned machine's machine config though...? [09:32] or the ssh provider [09:32] please help [09:33] davecheney: I'm *pretty* sure we settled on Manual provisioner. However, this is still about manually registering a machine to an env that could have a regular provider. [09:33] axw: I'm pretty sure we don't [09:33] jam: what is SABDFL decided to call it ? [09:33] i understand there was a ruling [09:33] axw: Uniter and Provisioner today don't have access to all of EnvironConfig [09:34] davecheney: he liked the phrase "manual registration" [09:34] well, that isn't helpful [09:34] and not having the word "Provider" in there [09:34] because nothing is *providing* machines [09:34] +1 for semantic correctness [09:34] davecheney: so the last bit I remember was "not having provider: in the environ config and using the term manually register new machines" [09:34] -1,000 for usefullness :) [09:36] axw, we should in general never need any of the stuff in environ config [09:36] axw, authorized-keys is the only generally useful bit of it that springs to mind [09:36] fwereade, jam: yeah I was just revising, that's only used for manual bootstrap [09:36] axw, and *that* is pretty dumb and broken regardless :( [09:37] axw: right. manual bootstrap is pretty different [09:37] axw, because *surely* the *useful* concept is "authorized users" [09:37] dimitern, commented [09:42] fwereade, thanks [09:42] fwereade, not sure I get your comment about Key/Keys - it's what the command is supposed to do, no? [09:43] dimitern, yeah, but I'm not sure it's actually very useful at an API level [09:43] dimitern, I would prefer to keep the api small and orthogonal as much as possible [09:43] dimitern, baking into it "and this is a use case that's sometimes helpful for the CLI doesn't seem like much of a win, when the cli can filter just fine" [09:44] dimitern, it's kinda like `juju get` which I have a horrible feeling returns the crazy nonsense that the command outputs [09:44] dimitern, the api for `juju get` [09:45] dimitern, which is no use to man nor beast, and even if it is it should be constructed for the specific context it's used in, out of clear sensible atoms, rather than forcing that interpretation on every api user [09:46] dimitern, anyway I could be convinced that Keys was sensible [09:47] dimitern, not so much Key [09:48] fwereade, but there's no need for Keys [09:48] fwereade, it's either one key or all of them [09:49] fwereade, are you saying to have Keys and use only one or zero keys when calling the command and have the rest of the logic moved to the command itself (key == "" -> all keys, else just that key) ? [09:50] fwereade, I thought we want to make the client API as close as possible to the CLI, logic included, so the GUI can provide the same features? [09:51] dimitern, at the moment there's no need for keys because we have a set-env that was hacked together without much thought and it only uses one [09:51] dimitern: so I would guess the point is to think about what is actually useful in an API. Today the CLI is actually deficient in that area. If I want 3 settings, I have to do 3 round trips, when you can easily implement the API to support multiple in one pass. [09:51] fwereade, set-env accepts multiple key=value args [09:52] dimitern, then all the more reason to allow us to ask for multiple values in get, surely? [09:52] jam, exactly [09:53] fwereade, ok, I'll change EnvironmentGet to accept zero or more keys [09:53] jam, dimitern: we're trying to write a sane api that happens to allow for the CLI functionality [09:53] jam, dimitern: rather than just writing an API for the CLI [09:53] fwereade, and the CLI get-env will use it like that, except there'll be the bit that if key == "" -> return the map, otherwise, return just the value of the given key [09:55] dimitern, sgtm, I'm not *that* much against Keys ;) [09:55] fwereade, ok, because frankly I can't see the alternative [09:56] dimitern, just ask for the whole env config? [09:56] fwereade, always? [09:56] fwereade, it seems unnecessary trafficwise [09:56] dimitern: as a general guide, if it is less than 64kB it isn't worth filtering [09:56] dimitern, gut feeling says that's simplest and always adequate, but maybe all those extra bytes add up to an annoying degree [09:56] as the bandwidth cost == latency of a round tirp [09:57] if its big, then we can do somethnig [09:57] ok, I'll get rid of Key/Keys and return the whole lot always then [09:57] dimitern, cheers [09:57] that's at least the numbers I've found in the past [09:58] 100ms at 1M/s = 100 [09:58] 100kb I guess is 12kB [09:58] though I've got 16MB and I've seen 300ms ping === rogpeppe1 is now known as rogpeppe [10:04] jam: just saw your conversation with axw about client secret-pushing. this is what i suggested yesterday - it seems to me that this is more-or-less where you ended up, is that right? http://paste.ubuntu.com/6363731/ === BradCrittenden is now known as ba === ba is now known as bac [10:05] rogpeppe: except not putting it into Login [10:05] I started down that path this morning, it got a bit messy [10:05] Login will (after my next CL) respond with a flag saying that secrets need to be pushed [10:05] but it won't require it there and then [10:06] axw: ah, ok [10:06] axw: where did the messiness come from? [10:07] rogpeppe: possibly just my misunderstanding of the API stuff, but it seemed wrong to add knowledge of environments in there. Now that I think of it though, it's going to have to anyway... [10:07] axw: yeah, that's what i was thinking [10:07] well, either approach will not be much of a change from what I've got now anyway [10:08] axw: cool [10:08] rogpeppe: the other thing I wondered was about validating config that soon [10:08] axw: I'm wondering if we want it in api.Open or if we could do it one layer higher [10:09] NewAPIConn is what the current CLI code ends up using, and it has the ENviron there [10:09] is it possible the config is wrong at that point? then how do we fix it without being able to login? [10:09] axw: my gut instinct at https://code.launchpad.net/~axwalk/juju-core/api-push-env-secrets/+merge/194083 was that it looks like the wrong level [10:09] axw: if the config is wrong, you shouldn't be able to push it into the environment [10:10] jam: I originally had it higher, in fact. It seemed better to me to keep the Login-results-stuff inside api.Open [10:10] axw: you can fix it by pushing a correct config [10:10] rogpeppe: true, unless the correctness changes over time :p [10:10] axw: but having state/api know anything about environs secrets is ... dissapointing [10:10] rogpeppe: ah, so I was just planning to push the secrets, not the entire config [10:10] axw: well, having a bad config doesn't make the environment inaccessible [10:11] jam: true, but the apiserver is going to have to anyway, right? [10:11] rogpeppe: I'm saying if we require login to successfully Validate config before letting the API client through [10:12] (back to our discussion yesterday) [10:12] jam: i think that the api will need to know about environ secrets even when bootstrap is synchronous [10:12] axw: is that problematic? [10:12] rogpeppe: axw: so it feels very strange to push "people connecting to the API should have environ and secrets" when we *really* want to get the environ state from the api [10:12] rogpeppe: only if it's feasible that the first time you connect, config is bad. I think it's probably not, like you said. [10:13] and it is only because of the handoff that we need it for the *very first* login. [10:13] jam: only the first client connecting needs environ and secrets [10:13] rogpeppe: needs, but everything else looking at the function will go "where do I get these bits" [10:13] jam: we allow nil to be passed [10:13] rogpeppe: "allow nil to be passed" is not very good API design. [10:14] jam: and when bootstrap is synchronous, we can have a separate API call [10:14] jam: rather, a separate entry point in juju [10:14] jam: sometimes i think it's ok [10:15] jam: when the thing really is optional [10:15] jam: so we'd provide it as an argument only if we find a BootstrapConfig in the .jenv. [10:16] jam: hmm, actually there's perhaps another possibility [10:17] jam: we can actually know if we're the first to connect, because the .jenv file has no cached API address (well - that will be the case when we do actually cache API addresses...) [10:18] jam: what's your suggestion for the (juju) package API? [10:19] rogpeppe: so we could just do a "Do you need secrets" call in NewAPIConn, and if it returns true, then call Client.SetSecrets() [10:19] though even there, NewAPIConn is used by agents [10:19] I think [10:19] jam: the agents use api.Open directly [10:20] axw: k, that would be pretty clear that the 99% case is not wanting to pass in the extra data, so we probably don't want to put it there. [10:20] axw: AFAICT the only thing we gain is 1 round trip which we are going to get rid of with doing it at bootstrap [10:21] jam: it looks to me as if NewAPIConn isn't used by anything except NewAPIClientFromName [10:22] rogpeppe: it is a point where we already have the data we need, and it is called by NewAPIConnFromName which most (all?) CLI cmds will be using to connect. [10:22] jam: sorry, confused. are you suggesting that I go back to doing the unconditional API call for the client? [10:22] axw: a thought, api.Open() could cache some status onto the api object itself, so that a follow up "api.DoIneedSecrets" doesn't have to do a round trip [10:22] but we don't expose it directly in the return of api.Open [10:23] axw: well it was a "CheckIfWeNeedSecrets" unconditionally. [10:25] though the putting it in a result from Login and then caching that on the api.State object. [10:26] if it's going to go away, i don't really mind an extra round trip for every API connection [10:26] jam: what's the big difference between unconditionally calling "PushEnvironmentSecrets" instead of "CheckIfWeNeedSecrets" and t hen "PushEnvironmentSecrets" [10:26] the former does nothing if it's not needed [10:26] rogpeppe: well today we get to remove about 5 round trips by caching the API address :) [10:26] but after that it shows up more [10:27] axw: passing secrets over the wire when they aren't neededx [10:27] jam: we're talking about something which we're going to fix anyway, right? [10:27] rogpeppe: right, I'm saying we add 1 round trip here, but then we take away 5 over there pretty soon, and eventually get back to this 1 [10:27] jam: so cluttering the API as little as possible seems like a reasonable approach [10:29] rogpeppe, should DeepEquals work and return 2 for 2 identical map[string]interface{} values? [10:29] sorry guys, gotta go help get my daughter ready for bed. I may be on a bit later [10:29] jam: we can probably use exactly the same logic that the mongo connection uses currently - no need to change the API at all [10:29] dimitern: yes [10:29] rogpeppe, s/return 2/return true/ [10:29] rogpeppe, well it doesn't for some reason [10:29] dimitern: are you sure about int types [10:29] int 2 != int64 2 [10:29] dimitern: they're not identical then :-) [10:29] interface{} is really bad about this [10:29] and the output in the log looks like [10:30] {a: b} != {a: b} which is very WTF [10:30] rogpeppe, I checked them key by key http://paste.ubuntu.com/6369724/ [10:30] dimitern: ah, there's one other thing: it won't return true if some values are not comparable ISTR [10:30] rogpeppe, perhaps like ca-cert? [10:30] dimitern: gospew is useful for diagnosing stuff like this [10:30] dimitern: I would bet api-port is an 'int' on one side and something like 'int32' or 'int64' somewhere else [10:31] or if this is data you got out of JSON, even float74 [10:31] dimitern: i bet it's a JSON issue [10:31] float64 [10:31] which writes out as int if it doesn't have a decimal [10:31] dimitern: remember that any numbers in json arrive as float64, as jam says [10:31] both ports look like ints [10:31] I've hit that in the past [10:31] printf(float64(1)) == 1 [10:31] not 1.0 [10:31] damn! [10:31] which is sort of a shame [10:31]