[00:34] <thumper> davecheney: hmm, went to the gym before the LGTM
[00:36] <davecheney> shit happens
[00:44] <thumper> davecheney: changes being proposed now if you want to LGTM the actual review...
[00:44] <thumper> davecheney: just give it a minute
[00:45] <davecheney> changes look good, i trust ya
[00:45] <davecheney> it was only nit picking stuff
[00:45] <thumper> davecheney: so submitting now should pick the right reviewers?
[00:45] <thumper> davecheney: one time before it failed to do so
[00:45] <thumper> and had "R=" in the commit message
[00:45] <davecheney> thumper: looks like that thing is unreliable
[00:45] <davecheney> i don't think anyone cares
[00:45] <davecheney> or looks
[00:45] <davecheney> or both
[00:45]  * thumper cares
[00:45] <thumper> kinda
[00:45] <davecheney> that makes one then
[00:46]  * thumper does the switch to trunk, merge and run tests prior to submitting
[00:46] <thumper> :)
[00:47] <davecheney> thumper: so you switch to trunk then merge your branch before pushing ?
[00:47] <davecheney> i do the other way around
[00:47] <davecheney> i merge trunk into my branch, test, then push if it passes
[00:48] <thumper> if I switch to trunk then merge it, and it works, it is all good
[00:48] <thumper> then I revert, and switch and submit
[00:48] <thumper> kinda the same
[00:48] <davecheney> horses for courses
[00:48] <thumper> but my way avoids bringing in the extra merge
[00:48] <davecheney> meh, that isn't the slow part
[00:48] <thumper> heh, no
[00:49] <thumper> davecheney: if you have time, i'd love to chat about the upgrade process
[00:49] <thumper> as in 'juju upgrade'
[00:49] <thumper> do you know much about it?
[00:49] <davecheney> a little, dating back to august last year (2nd lisbon sprint)
[00:49] <davecheney> i don't think much has changed since then
[00:50] <davecheney> want me to grab moi headset ?
[00:51] <thumper> sure, 5 min?
[00:51] <thumper> as in, in 5 min?
[00:51] <davecheney> kk
[00:54] <thumper> hangout?
[00:58] <davecheney> sure
[00:58] <davecheney> lemmie try
[02:25] <wallyworld> thumper: davecheney: want the easiest review ever? +1/0 https://codereview.appspot.com/8064043
[03:44] <jam> wallyworld: for your patch, the idea is that we should retry only for IsNotFound errors, but exit on everything else?
[03:44] <wallyworld> jam: yes
[03:44] <wallyworld> that's how ec2 does it too
[03:44] <jam> wallyworld: I would probably find it easier to read with inverted if logic
[03:45] <jam> if !errors.IsNotFound(err) { return err}
[03:45] <jam> otherwise LGTM
[03:45] <wallyworld> ok, i copied the logic style used by ec2 for consistency
[03:45] <jam> wallyworld: yeah, but consistency with *cough* is not necessarily worth keeping.
[03:45] <wallyworld> sure :-)
[03:46] <jam> i would be happy if you wanted to tweak the ec2 side as well
[03:46] <wallyworld> will do
[03:46] <wallyworld> jam: i reworked the debug log branch also, as per william's review, less code now
[03:49] <jam> wallyworld: to make sure I understand: https://codereview.appspot.com/7749048/patch/6001/7004
[03:49] <jam> that change is because you can't use the nil constructor
[03:49] <jam> because 'sshCmd' is an interface.
[03:49] <jam> so it doesn't know which one to do.
[03:49] <jam> The only thing I might say is "could we just check for nil?"
[03:49] <wallyworld> jam: go doesn't allow default values in struct definitions sadly
[03:50] <jam> so Init does something like "if s.sshCmd == nil {sshCmd = XXX}"
[03:50] <jam> wallyworld: right, but you set it in Init if it isn't set yet.
[03:50] <wallyworld> jam: i did that, but SetFlags() is called before init, so the nil check needed to go there, hence i aborted that way of doing it because it wasn't obvious to the reader where the nil check needed to go
[03:50] <jam> anyway, I'm not to unhappy with just specifying it at register.
[03:51] <jam> wallyworld: yeah, I wondered about that. SetFlags vs Init... good enough for me.
[03:51] <wallyworld> ie the expected place Init() wasn't the right place at all
[03:51] <wallyworld> so best to be explicit
[03:51] <jam> you could hide it under a method call
[03:51] <jam> DebugLogCommand.sshCmd() which would do it.
[03:51] <wallyworld> i could, but then debug log would be the only command with a New()
[03:51] <jam> but yeah
[03:52] <wallyworld> hmm, i could have used a method like you suggest
[03:52] <jam> wallyworld: I was meaning a method on DebugLogCommand. But it is a fair amount of *stuff* just to avoid setting it for registration.
[03:52] <jam> wallyworld: so certainly, think about it, if you like it more than what you have you can change it, but it seems very much 6-of-one half-dozen of another.
[03:53] <jam> Neither is clearly better.
[03:53] <wallyworld> yeah, agreed
[03:53] <wallyworld> it's landed now, so not really worth changing unless really needed
[03:55] <jam> wallyworld: anything left on your plate that hasn't landed?
[03:55] <jam> wallyworld: and did you get a chance to look at sync-log?
[03:56] <wallyworld> jam: writing tests for the invalid region stuff. will propose soon
[03:56] <wallyworld> remind me, what was sync-log again?
[04:02] <jam> wallyworld: sorry not sync-log, sync-tools
[04:02] <jam> I was mixing my work with yours
[04:02] <jam> it is "get the official posted binaries, upload them to my local space"
[04:04] <wallyworld> jam: ah, balls, i haven't looked yet. sorry. will do it now. i was going to do it before you came on line and clearly i didn't expect you to log on till later :-)
[04:04] <jam> wallyworld: I'm not officially here for another hour at least
[04:05] <wallyworld> that's what i thought :-)
[05:32] <wallyworld> jam: i made the change to that branch without first seeing your comment. you happy with my version?
[05:32] <jam> wallyworld: change to which branch?
[05:32] <wallyworld> sorry, the retries one
[05:33] <wallyworld> https://codereview.appspot.com/8064043/
[05:34] <jam> wallyworld: yeah, break is fine vs return
[05:34] <wallyworld> cool, landing now
[05:34] <wallyworld> i've pushed my other branch with the error messages, so might even get to land it tonight
[06:41] <wallyworld> jam: i thought about the \n, but the common convention *seems* to be just a long string. i'll tweak it and see how it turns out
[06:41] <jam> wallyworld: I like the amount of context, but it is a little hard to read
[06:41] <jam> our error formatting putting in a bunch of "caused-by" doesn't help either.
[06:41] <jam> wallyworld: as for convention, the convertion isn't to give super-helpful error messages with info for the user, either.
[06:41] <wallyworld> jam: i agree that it's hard to read for sure
[06:42] <jam> so I feel comfortable breaking convention here :)
[06:42] <wallyworld> jam: caused by messages should  all come after a \n imho
[06:43] <jam> wallyworld: yeah, I'd probably agree with that
[06:43] <wallyworld> but i'll just do this one thing perhaps so i can land it before i go
[06:43] <jam> wallyworld: it isn't a blocker, your patch is better in than out, etc.
[06:43] <jam> but if you can tweak it, I think we can make it better.
[06:43] <wallyworld> that's what she said
[06:44] <jam> wallyworld: I'm pretty sure that is "in and out" .
[06:44] <wallyworld> lol
[06:45]  * wallyworld biab
[07:37] <dimitern> morning all
[07:40] <rogpeppe> dimitern: hiya
[07:41] <dimitern> rogpeppe: yo
[07:46] <TheMue> *: morning
[07:55]  * thumper is back
[07:56] <dimitern> can someone send me the HO link?
[08:01] <dimitern> i found it
[09:07] <thumper> fwereade_: I'm looking for something next week...
[09:07] <thumper> fwereade_: how about I take https://bugs.launchpad.net/juju-core/+bug/1131608
[09:07] <_mup_> Bug #1131608: deployed series is arbitrary <juju-core:New for fwereade> < https://launchpad.net/bugs/1131608 >
[09:07] <fwereade_> thumper, hum, ok, let me think a sec -- and I *think* you've actually basically fixed that one
[09:08] <fwereade_> thumper, modulo openstack
[09:08] <thumper> fwereade_: in fact, part 3 of that bug solution is done
[09:08] <thumper> fwereade_: ok, I'll let you think on it
[09:08] <thumper> fwereade_: and pick something on Tuesday next week
[09:08] <fwereade_> thumper, yeah, I did 1 and 2, you did 3
[09:08] <thumper> Monday is a public holiday here too
[09:08] <fwereade_> thumper, I do have an idea
[09:08] <thumper> fwereade_: feel free to email me :)
[09:08]  * thumper is off now
[09:08] <fwereade_> thumper, which is to look into the cards surrounding machine status and reporting back of provisioning errors
[09:08] <thumper> 10 pm before 4 days off \o/
[09:08] <fwereade_> thumper, but I shall mail you
[09:09] <fwereade_> thumper, enjoy your holiday :)
[09:09] <thumper> fwereade_: awesome, ta
[09:10] <dimitern> fwereade_: so now upgrade-charm is done (up to py-juju parity) what next?
[09:10] <dimitern> fwereade_: or can just pick some other card
[09:10] <fwereade_> dimitern, it is probably best to pick some other card
[09:11] <fwereade_> dimitern, trying to swarm is probably a good idea
[09:11] <fwereade_> dimitern, since you have picked up some experience with state
[09:11] <dimitern> ok, any ideas where?
[09:11] <fwereade_> dimitern, I might lean towards the "add machine status field"
[09:11] <fwereade_> dimitern, which is in blue's TODO
[09:11] <dimitern> i'll take a look
[09:13] <rogpeppe> fwereade_: i'd appreciate it if you could take a look at the Relation embedded in Endpoint CL at some point, please. i *think* it worked out pretty well, but YMMV. https://codereview.appspot.com/8055044/
[09:14] <fwereade_> rogpeppe, sweet, I will take a look
[10:24] <rogpeppe> fwereade: ping
[10:24] <rogpeppe> fwereade: "
[10:24] <rogpeppe> Not sure what the distinction is between Limit and Optional -- they're equally
[10:25] <rogpeppe> ignored AFAICT, so we should probably include both or neither.
[10:25] <rogpeppe> "
[10:25] <rogpeppe> fwereade: we do include both AFAICS
[10:27] <fwereade> rogpeppe, ah, sorry, let me reread
[10:28] <fwereade> rogpeppe, yep, I'm an idiot, please disregard
[10:28] <rogpeppe> fwereade: np
[10:28] <dimitern> fwereade: what are the possible machine statuses? are they basically the same as machine agent statuses (as with unit statuses)?
[10:36] <fwereade> dimitern, hum, not necessarily
[10:36] <fwereade> dimitern, I don't think "installed" is meaningful
[10:36] <fwereade> dimitern, all the others might be
[10:40] <dimitern> fwereade: ok, thanks
[10:40] <dimitern> fwereade: so this comment is wrong? // UnitStatus represents the status of the unit agent.
[10:45] <fwereade> dimitern, well -- there is a fuzzy distinction between the "unit" and the "unit agent"
[10:46] <fwereade> dimitern, and there's an additional complication in that agent presence is conflated with unit status (see Unit.Status)
[10:46] <dimitern> fwereade: so it's better to say "UnitStatus represents the status of a unit." and similarly for a machine status
[10:46] <fwereade> dimitern, yeah, sgtm
[10:47] <fwereade> dimitern, btw, since you're hitting that
[10:47] <dimitern> fwereade: and specific status value have a relation to the agent? cf. 	UnitStarted   UnitStatus = "started"   // Agent is running properly
[10:47] <fwereade> dimitern, it would prbably be valuable, as a first step, to move the "down" stuff out of Unit.Status and put it in cmd/juju/status instead
[10:47] <fwereade> dimitern, not really
[10:48] <fwereade> dimitern, "Agent is running properly" applied to every status except pending and down
[10:48] <fwereade> dimitern, and down isn't really a status
[10:48] <dimitern> fwereade: i'm thinking of moving all of unit status in state/status.go
[10:49] <fwereade> dimitern, no firm opinion either way on that, follow your heart :)
[10:49] <dimitern> fwereade: cheers
[10:49] <dimitern> fwereade: and istm i'll just s/agent/unit in those comments
[11:10] <TheMue> lunchtime
[11:18] <jam> niemeyer: blog.labix.org is currently offline
[11:18] <niemeyer> jam: I noticed.. great timing
[11:18] <niemeyer> jam: The whole server is down an inaccessible
[11:18] <jam> that's a hosted site/
[11:19] <jam> ?
[11:19] <niemeyer> jam: Yeah, dreamhost
[11:19] <niemeyer> Already filed a support request
[11:23] <jam> niemeyer: its back up
[11:23] <niemeyer> jam: Phew
[11:24] <niemeyer> jam: I'll wait a bit until making it public, to make sure it won't happen again
[11:32] <jam> wallyworld: are you aronud for standup?
[11:40] <dimitern> mgz: https://codereview.appspot.com/8051043/
[11:41] <mgz> ta
[11:43] <rogpeppe> fwereade: "
[11:43] <rogpeppe> Let's keep that terminology out of charm, I think it's only meaningful at
[11:43] <rogpeppe> runtime.
[11:43] <rogpeppe> "
[11:43] <rogpeppe> fwereade: we already use the term "endpoint" in the RelationScope type doc
[11:43] <rogpeppe> fwereade: should i remove it from there too?
[11:43] <fwereade> rogpeppe, good point -- yes, I think so
[11:44] <fwereade> rogpeppe, does that make it unpleasant?
[11:44] <rogpeppe> fwereade: i'm don't *think* so.
[11:45] <fwereade> rogpeppe, cool, thanks
[11:45] <rogpeppe> fwereade: if we think of a "relation" as a single-ended thing
[11:45] <rogpeppe> fwereade: it's a pity we don't have two terms for "one end of a relation" and "the relation that joins these things together"
[11:46] <fwereade> rogpeppe, agreed, but the terminology seems to be reasonably well bedded in already
[11:46] <fwereade> rogpeppe, without too much confusion, hopefully
[11:47] <rogpeppe> fwereade: yeah
[11:47] <rogpeppe> fwereade: certainly in charm, there's no concept of the double-ended relation anyway
[11:47] <fwereade> rogpeppe, yeah, indeed
=== benji___ is now known as benji
[12:31] <rogpeppe> fwereade: here's some preliminary work to make state/watcher amenable to a faster sync cycle: https://codereview.appspot.com/8078043/
[12:39] <wallyworld> dimitern: jam: https://codereview.appspot.com/8079043
[12:39] <wallyworld> sorry i missed the suggestions the first time
[12:43] <dimitern> wallyworld: no worries, it looks much better now - LGTM
[12:44] <dimitern> thanks
[12:44] <dimitern> fwereade: ping
[12:44] <wallyworld> thanks, i must have been smoking my crack pipe to have missed it, sorry
[12:44] <dimitern> ;)
[12:48] <jam> wallyworld: reviewed
[12:48] <wallyworld> thanks, looking
[12:50] <dimitern> jam: my point with using %s instead of %q was that ["compute, object-store"] looks uglier than [compute, object-store], and esp. [... one of these regions may be suitable instead: "reg1, reg2, reg3"] (ILTM as the region name suggested is "reg1, reg2, reg3"
[12:51] <wallyworld> jam: the wrapping with the error you pasted into the code review - i don't get that in my terminal
[12:51] <wallyworld> i get
[12:51] <wallyworld> error: failed to GET object provider-state from container juju-50548874395ac32d8896f65ab33f3e17
[12:51] <wallyworld> caused by: cannot create service URLs
[12:51] <wallyworld> caused by: the configured region "region-a.geo-1" does not allow access to all required services, namely: compute, object-store
[12:51] <wallyworld> access to these services is missing: compute
[12:51] <wallyworld> one of these regions may be suitable instead: az-1.region-a.geo-1, az-2.region-a.geo-1, az-3.region-a.geo-1
[12:51] <jam> wallyworld: that specific wrapping was probably codereview: http://paste.ubuntu.com/5655074/
[12:51] <jam> right
[12:52] <jam> which is why it is 'uneven'
[12:52] <jam> Not worth worrying about.
[12:52] <wallyworld> yeah. at least each caused by is on a new line etc, so it's not tooooo hard to read
[12:52] <wallyworld> better than a long string
[12:52] <jam> certainly
[12:52] <jam> just not 'ideal'
[12:52] <jam> but good enough for now, I believe
[12:53] <wallyworld> certainly better than what it is replacing
=== teknico is now known as teknico_away
[12:53] <jam> yep
[12:53] <jam> and quite helpful
[12:54] <wallyworld> let's see what users think :-)
[12:56]  * rogpeppe struggles with the crackful WatchUnits tests
[13:06] <jam> dimitern: so, it appears that goamz.S3 requires an aws.Auth, and it *always* signs the request, even when your key is "". And that, in turn, is rejected by Amazon, even though the bucket is public (auth is checked first0.
[13:06] <jam> so I'm thinking just to add a way to supply ec2 creds, even though we "shouldn't" need them.
[13:07] <dimitern> jam: it looks like a bug in goamz
[13:07] <jam> dimitern: well, it is something to fix in the whole stack, which seems out of scope for the benefit.
[13:07] <dimitern> jam: but maybe it's worth a bug report at least to keep it in  mind
[13:07] <jam> as you have to have a way to get creds to accept not having auth entries
[13:19] <jam> rogpeppe: I believe you can land: https://codereview.appspot.com/7925045/
[13:19] <rogpeppe> jam: i believe you might be right
[13:19] <jam> I was going to LGTM, but its already been done
[13:23] <jam> rogpeppe: it seems like you might get the framing wrong if you don't disconnect. Is there an advantage of leaving the connection open? https://codereview.appspot.com/7518052/
[13:23] <jam> it seems like while there might be a small one, it shouldn't be "normal operation" to get UnexpectedEOF
[13:24] <rogpeppe> jam: yeah, i should probably not return nil on UnexpectedEOF actually
[13:24] <rogpeppe> jam: the advantage of leaving the connection open is that the client gets some feedback on what went wrong.
[13:25] <jam> rogpeppe: can we write the response and then disconnect?
[13:25] <rogpeppe> jam: in the actual api, ReadRequestBody does not actually read anything
[13:25] <jam> I do think we want to respond
[13:25] <jam> I agree we want to respond
[13:25] <jam> My concern is just that if we get a partial read, then we might not know where the next message starts
[13:25] <rogpeppe> jam: i'm wondering about allowing the codec to return a special error code that signifies it's not a terminal error
[13:26] <rogpeppe> jam: we shouldn't get an error on a partial read
[13:26] <jam> rogpeppe: isn't that UnexpectedEOF?
[13:26] <rogpeppe> jam: in the UnexpectedEOF case, we do bomb out
[13:27] <rogpeppe> jam: the issue is other requests that may or may not signify a terminal error
[13:28] <rogpeppe> jam: the thing is that the codec *knows* whether it's a recoverable error or not
[13:28] <rogpeppe> jam: and when it is, i'd prefer not to drop the connection
[13:28] <jam> rogpeppe: sure
[13:29] <rogpeppe> jam: (this was a request from the GUI team, BTW)
[13:29] <jam> properly formed messages (even when they are telling you "I can't do that") don't have to close the conn
[13:29] <rogpeppe> jam: the problem is that a message can be syntactically correct (e,g. well-formed JSON) but have some problem with the param types (e.g. null instead of string)
[13:30] <jam> rogpeppe: sure. My concern is specifically about malformed messages, which seemed EOF related.
[13:30] <rogpeppe> jam: the behaviour at EOF and UnexpectedEOF is still pretty much what it was
[13:31] <rogpeppe> jam: i'm more concerned about issues like spurious closing braces
[13:31] <jam> rogpeppe: well, I'd be fine closing a conn if you had a spurious closing brace.
[13:31] <jam> If you're sending stuff that badly formatted, you can connect again.
[13:32] <rogpeppe> jam: yeah, but this change might make it so that doesn't happen
[13:32] <rogpeppe> jam: *if* ReadRequestBody actually read some data
[13:32] <rogpeppe> jam: (which it doesn't, except in the rpc tests)
[13:33] <rogpeppe> jam: my thought is that if such a thing happens, and the connection really is borked, then the next read-header will deal with the issue
[13:34] <jam> rogpeppe: as long as it doesn't become some sort of security hole (user-data getting interpreted as request data, etc) but I think the change is fine.
[13:34] <rogpeppe> jam: i can't currently see any problem with doing that
[13:34] <jam> Just having the conversation about it.
[13:34] <rogpeppe> jam: yeah, it's a good conversation to have
[13:34] <rogpeppe> jam: i'm glad you brought it up, because it's the central possible issue in this change
[13:35] <rogpeppe> jam: if the user data could force a syntax error in the framing data, then i think the client is broken.
[13:37] <jam> rogpeppe: sure, though you can always have a custom client that lets you send broken requests.
[13:37] <jam> HTTP apis aren't sacred :)
[13:37] <jam> anyway,
[13:37] <jam> that is a bit remote
[13:38] <rogpeppe> jam: that's true, but if that's true, then they can make up requests as they need - no need to leverage this method of getting user data to be interpreted as request data
[13:38] <jam> yeah
[13:38] <rogpeppe> s/that's true/they want to/
[13:38] <rogpeppe> second one
[13:38] <dimitern> anybody seen "Modifiers and non-modifiers cannot be mixed" from mgo? tried searching whole or parts of it in all go code, including mgo, but couldn't find anything
[13:39] <rogpeppe> dimitern: sounds like it might be an error from mongod itself
[13:39] <rogpeppe> dimitern: yeah: ./src/mongo/db/ops/update.cpp:40
[13:39] <dimitern> rogpeppe: yeah, seems so. hmm weird..
[13:40] <rogpeppe> omg c++
[13:41] <rogpeppe> dimitern: http://paste.ubuntu.com/5655182/
[13:41] <rogpeppe> dimitern: "e.eoo()" ?!
[13:41] <dimitern> rogpeppe: what's e.eoo() ?
[13:41] <dimitern> :)
[13:42] <rogpeppe> dimitern: my question exactly
[13:42] <rogpeppe> dimitern: EOO == "end of object", i think
[13:43] <rogpeppe> dimitern: ah, you're mixing "$x" with "x"
[13:43] <rogpeppe> dimitern: the crucial bit is: e.fieldName()[ 0 ] != '$'
[13:43] <dimitern> rogpeppe: I can't see how.. I dumped the doc and the key and there's no $ anywhere
[13:44] <rogpeppe> dimitern: have you dumped the entire mongo op?
[13:45] <dimitern> rogpeppe: doing it now
[13:46] <dimitern> >>>> trying to set status of "u#wordpress/0" to &state.unitStatusDoc{Status:"error", StatusInfo:"test-hook failed"}; txn.Op{C:"statuses", Id:"u#wordpress/0", Assert:"d+", Insert:interface {}(nil), Update:(*state.unitStatusDoc)(0xf8401dd720), Remove:false}
[13:46] <dimitern> that's what I have printed just before
[13:49] <dimitern> rogpeppe: can I somehow get a log of what mongo is receiving?
[13:50] <rogpeppe> dimitern: i'd probably put a printf somewhere in mgo
[13:50] <dimitern> rogpeppe: good idea
[13:50] <rogpeppe> dimitern: are you getting this when running a transaction?
[13:50] <dimitern> rogpeppe: yes
[13:50] <dimitern> rogpeppe: insert is fine, update fails
[13:51] <dimitern> rogpeppe: in the same collection
[13:51] <rogpeppe> dimitern: what does the dump of the whole transaction ops look like?
[13:51] <dimitern> rogpeppe: I haven't changed anything else, just added a single op to an exiting transaction
[13:51] <rogpeppe> dimitern: sure, but i'd still like to see it
[13:52] <dimitern> rogpeppe: just a sec
[13:54] <dimitern> rogpeppe: http://paste.ubuntu.com/5655208/
[13:55] <rogpeppe> dimitern: looks fishy to me
[13:55] <rogpeppe> dimitern: what's that Name:"$ne" doing in there?
[13:55] <dimitern> rogpeppe: I think I start to get it - Update: someinterface is not the same as wrapping it up in a D{{...}}
[13:56] <dimitern> rogpeppe: that's notDeadDoc assert
[13:56] <dimitern> rogpeppe: ah!
[13:56] <dimitern> rogpeppe: got it
[13:56] <rogpeppe> dimitern: cool
[13:56] <rogpeppe> dimitern: see, told you it was worth dumping in total :-)
[13:56] <dimitern> rogpeppe: or maybe not..
=== wedgwood_away is now known as wedgwood
[13:58] <dimitern> rogpeppe: so it used to be {C:u.st.units.Name, Id: u.doc.Name, Assert: notDeadDoc, Update: D{{"status", "somestatus"}}} and I removed the update but wanted to keep the assert
[13:58] <rogpeppe> dimitern: there's still an Update in there
[13:58] <rogpeppe> dimitern: which looks possibly bogus
[13:58] <dimitern> rogpeppe: yeah, but the status is moved into a separate collection where's the update now
[13:59] <rogpeppe> dimitern: all the other Update instances have D{{...}} but this just has a unitStatusDoc
[13:59] <dimitern> rogpeppe: well, unitStatusDoc is interface{} to the setStatusOp (opaque)
[14:00] <dimitern> rogpeppe: and I need to insert/update whatever's there
[14:00] <dimitern> rogpeppe: should I wrap it up in a D{{something}}?
[14:00] <rogpeppe> dimitern: what's the type of unitStatusDoc ?
[14:01] <dimitern> rogpeppe: unitStatusDoc is the type - struct {Status, StatusInfo string}
[14:01] <rogpeppe> dimitern: BTW, i recommend dumping with spew.Dump - it formats it nicely and shows all depths
[14:01] <rogpeppe> dimitern: that doesn't look like something you can pass directly to Update
[14:02] <dimitern> rogpeppe: actually no, there are 2 doc types: unitStatusDoc and machineStatusDoc - both have 2 fields (+ the implicit _id) and one has Status UnitStatus, the other Status MachineStatus (both are type *Status string)
[14:02] <rogpeppe> dimitern: i think perhaps you want D{{"$set", unitStatusDoc}}
[14:03] <dimitern> rogpeppe: ah, ok I'll try this 10x
[14:04] <dimitern> rogpeppe: it worked! tyvm
[14:32] <dimitern> fwereade: ping
[14:42] <dimitern> fwereade: I prefer not to change annotations and constraints in this CL to use the globalKeyer, if you don't mind
[14:44] <dimitern> and there it is - https://codereview.appspot.com/8087043/
[15:21] <fwereade> dimitern, heyhey
[15:22] <dimitern> fwereade: hey, it's basically done - not sure about machine status logic; can you take a look please?
[15:22] <dimitern> fwereade: all tests pass, it seems none of the uniter ones depend on unit changes on status
[15:23] <fwereade> dimitern, there are a few things to talk about, not necessarily line-focused; might make sense to do so in a hangout?
[15:23] <dimitern> fwereade: sure, I'll start one
[15:24] <dimitern> fwereade: https://plus.google.com/hangouts/_/555b884bebd0c196b62a3b05ec40cb1ba95a5c19?authuser=0&hl=en
[15:34] <Makyo> Can I request another reviewer for https://codereview.appspot.com/8018043/ ?  It's big, but mostly just a name refactor.
[16:05] <jam> Makyo: is it possible to split it into *just* a name refactor and then another patch for logic changes, that is usually a lot easier to review, though I'll still give this one a look if you want
[16:06] <jam> otherwise each line I have to read carefully to see if there is more than just a mechanical change
[16:07] <jam> though to be fair, everything so far has been only mechanical renames
[16:09] <Makyo> jam, not easily, but yes, it's at least 95% mechanical, with the only logic changes being to get rid of entityNamer/namedEntity interfaces in state tests, as those are now available through the Tagger interface in state/state.go
[16:09] <jam> Makyo: can you point out the specific files, so I can be careful when looking at them
[16:09] <jam> state/state.go and state/?_test.go ?
[16:10] <Makyo> jam, yes, let me see if I can narrow it down further.
[16:10] <jam> Makyo: so one place here: https://codereview.appspot.com/8018043/patch/12001/13026
[16:10] <jam> you rename entityId => tag
[16:10] <jam> where everything else was "entityName"
[16:10] <jam> is Id something different?
[16:10] <Makyo> jam, no, same as entityName
[16:12] <jam> Makyo: so I think you mean here: https://codereview.appspot.com/8018043/patch/12001/13030
[16:13] <jam> where you got rid of the namedEntity interface
[16:13] <jam> for a public state.Tagger interfac
[16:13] <jam> (presumably providing the logically same interface)
[16:13] <Makyo> jam, state/assign_test.go, state/apiserver/api_test.go, state/state_test.go
[16:13] <Makyo> jam, yep.
[16:20] <jam> reviewed
[16:20] <Makyo> jam, Thank you :)
[17:18] <rogpeppe> gosh that was a hard bug to find
[18:11] <rogpeppe> eod. am off now. happy weekends all. see y'all on tuesday.
=== wedgwood is now known as wedgwood_away