[00:34] <davecheney> niemeyer: new revision of SetConfig incoming
[00:34] <davecheney> you may want to see more caching
[00:41] <davecheney> niemeyer: ready for review, https://codereview.appspot.com/6229046
[01:16] <niemeyer> davecheney: Review on first one delivered
[01:17] <niemeyer> davecheney: "Patch set 8 incorporates https://codereview.appspot.com/6244053."
[01:17] <niemeyer> davecheney: I guess it shouldn't?  Otherwise what's the point of having the other CL?
[01:17] <davecheney> niemeyer: that was a cryptic way of saying i rolled that change up into setconfig
[01:17] <davecheney> but given your comments on the makeBucket branhc
[01:18] <davecheney> that will turn out to be a bigger piece of work
[01:18] <niemeyer> davecheney: The comment remains.. what's the point of having a second CL if it's included in another one
[01:18] <davecheney> i'm finding it hard to manage branches that have multiple parents
[01:19] <davecheney> in the future I won't merge the like patch #8
[01:19] <davecheney> i'll wait til the upstream passes review
[01:19] <niemeyer> davecheney: Ideally things are either independent, or you use -pre-req
[01:19] <niemeyer> davecheney: Yeah, that's a good plan in general
[01:19] <davecheney> so, talking about https://codereview.appspot.com/6244053 for a second
[01:20] <davecheney> you belive this is a bug introduced recently
[01:20] <davecheney> ?
[01:20] <niemeyer> Ok
[01:20] <niemeyer> davecheney: Yeah, definitely.. this logic was worked on by rog last week or so
[01:20] <davecheney> ah, ok
[01:20] <davecheney> i'll review the commit history and try a different approach
[01:21] <davecheney> i'll put SetConfig back into WIP
[01:21] <niemeyer> davecheney: I'm having a look at it just now
[01:21] <davecheney> if you could give it a cursory view that would be good
[01:22] <niemeyer> davecheney: I'm a bit unhappy with the strategy that is spreading through there
[01:22] <niemeyer> return &storage{bucket: e.s3().Bucket(config.bucket)}
[01:22] <niemeyer> davecheney: This is creating a storage+S3+bucket every single time we need to use the bucket
[01:23] <niemeyer> davecheney: Imagine what happens in a trivial loop like this:
[01:23] <niemeyer> for {
[01:23] <niemeyer>     ... whatever ...
[01:23] <davecheney> yup, understood
[01:23] <niemeyer>     environ.PutFile(file)
[01:23] <niemeyer> davecheney: *tons* of garbage
[01:23] <davecheney> I am not overly concerned with performance at this stage
[01:24] <davecheney> as this is talking to a network service
[01:24] <davecheney> but I will add caching
[01:24] <niemeyer> davecheney: For no great reason.. we got into this situation after trying to avoid the caching, which wasn't a big deal at all
[01:24] <davecheney> understood
[01:24] <niemeyer> davecheney: I'm not *concerned* either
[01:25] <niemeyer> davecheney: I just feel bad that we're giving up on both simplicity *and* performance at the same time
[01:25] <davecheney> i'm trying for something that is correct with respect to SetConfig
[01:25] <davecheney> i hope that caching could be added later
[01:25] <davecheney> going back to makeBucket for a second, 'cos that is now a blocker
[01:25] <davecheney> I can't see there the bug was introduced
[01:25] <davecheney> not in the last week at least
[01:26] <niemeyer> davecheney: THe makeBucket issue was created while avoiding the caching as well
[01:26] <niemeyer> davecheney: Well, some of the issues, maybe not makeBucket
[01:26] <niemeyer> davecheney: The race from the last review, for example
[01:28] <davecheney> niemeyer: i'm concerned that making the environs reloadable is becoming a blocker
[01:29] <davecheney> I understand it is important for the final product
[01:29] <davecheney> but would it be possible to propose a provisioning agent that doesn't reload it's configuration after starting
[01:30] <niemeyer> davecheney: No, we have spent a lot of time on this, let's not throw all the work and discussion now that it's pretty much done
[01:30] <niemeyer> davecheney: Hindsight is good, but let's not ignore what was done now that we have it
[01:31] <davecheney> i am concerned that the adding reloadability to environs is a moving target
[01:32] <davecheney> i'm wondering if it would be better to address then after we have a PA that we can use to bring up a trial juju on ec2
[01:32] <niemeyer> davecheney: It wasn't a moving target.. I suggested not changing caching, for example, because it worked, and then you found it a better approach.. that broke things further.. and that had to be fixed.. and ...
[01:32] <davecheney> exactly
[01:32] <davecheney> so lets put all that to once side
[01:32] <davecheney> one side
[01:32] <niemeyer> davecheney: Yeah, let's *fix the problems* and not change further
[01:32] <niemeyer> davecheney: If you're feeling overwhelmed, I can do that myself
[01:33] <davecheney> niemeyer: not overwhelmed, just concerned that all the changes that the PA needs to environs/Environ keeps pushing back the PA itself
[01:33] <niemeyer> davecheney: Yeah, so ignore the madeBucket stuff, if that's what you mean
[01:33] <niemeyer> davecheney: Just drop it, and we'll put on someone else's todo list
[01:34] <niemeyer> davecheney: That sounds fine
[01:34] <davecheney> i'm suggesting a middle ground of delivering a PA that doesnt' try to respond to changes in /environment, so has no requirement for Environ.SetConfig
[01:34] <niemeyer> davecheney: But I do want to see caching added back, and all that extra stuff removed
[01:34] <niemeyer> davecheney: Because that's part of the "never ending roll"
[01:35] <davecheney> niemeyer: yes, i want to get out of this spiral
[01:35] <davecheney> and I can do that if the PA can skip having to reload it's environ once it's up and running
[01:35] <davecheney> then when the rest of the PA is working
[01:35] <niemeyer> davecheney: Again, you spent days on this, and I spend hours reviewing it.. I'm not happy with trashing both your and my time
[01:35] <davecheney> reloading environ configs can be tackled as seperate branch
[01:35] <davecheney> i'm not saying throw it awwy
[01:35] <davecheney> just put it on hold for a while
[01:36] <niemeyer> davecheney: That doesn't work.. if it stays out, it will be broken next week
[01:36] <niemeyer> davecheney: Let's fix it, and move on
[01:36] <davecheney> ok, understood
[01:37] <niemeyer> davecheney: Happy to drop make/madeBucket, though
[01:37] <niemeyer> davecheney: Didn't happen in this branch..
[01:41] <davecheney> ok, so to review, I will continue to work on SetConfig. I will restore all the cached values like ec3Unlocked and friends
[01:41] <davecheney> and makes changes to environ.config atomic with changes to {public,}storage.bucket
[01:42] <davecheney> /s/ec3/ec2/
[01:42] <niemeyer> davecheney: Heh, madeBucket was born broken
[01:42] <davecheney> niemeyer: thought so
[01:42] <niemeyer> davecheney: Sorry about that
[01:43] <niemeyer> davecheney: I suspect the second problem is a NOOP once you deal with the first
[01:43] <niemeyer> davecheney: You got into the second issue because replacing the bucket was done with s3(), which needed a lock
[01:43] <niemeyer> davecheney: Once you have the cached values, it's all done within the lock
[01:43] <davecheney> yup, that'll work
[01:44] <davecheney> seriously, i should stop listening to roger's advice
[01:44] <davecheney> that was his suggestion :)
[01:44] <davecheney> anwywya, i have to go
[01:44] <niemeyer> LOL
[01:44] <niemeyer> Rog have some great advices, though
[01:44] <niemeyer> ''''''''''''''''has
[01:44] <davecheney> i know, it's hard to sift one from the other
[01:44] <davecheney> anyway, i have to go
[01:44] <niemeyer> davecheney: Cheers
[01:45] <davecheney> will submit another round tonihgt
[01:45] <davecheney> thank you for your time
[01:48] <niemeyer> davecheney: My pleasure
[01:50] <andrewsmedina> niemeyer: are you working 24/7 ?
[01:50] <andrewsmedina> :-p
[01:51] <niemeyer> andrewsmedina: Nah.. :-)
[01:52] <andrewsmedina> :D
[01:52] <andrewsmedina> niemeyer: I submitted a new proposal
[01:52] <niemeyer> andrewsmedina: Thanks!
[01:52] <niemeyer> andrewsmedina: This was probably the last one.. with those minors, it should be good to go in
[01:54] <andrewsmedina> niemeyer: thanks :D
[01:54] <andrewsmedina> niemeyer: Rodrigo Senra will be here tomorrow
[02:30] <niemeyer> andrewsmedina: Wow, really?
[02:31] <niemeyer> andrewsmedina: This is awesome.. small world
[02:31] <niemeyer> andrewsmedina: Please deliver him a hug from me :)
[02:32] <andrewsmedina> niemeyer: ok, I will
[02:32] <niemeyer> andrewsmedina: aaaaand your branhc is going in
[02:32] <andrewsmedina> niemeyer: nice!
[02:32] <andrewsmedina> niemeyer: thanks
[02:32] <niemeyer> andrewsmedina: Thank *you* for keeping up with it
[02:33] <andrewsmedina> niemeyer: now I will work on local provider environ interface =D
[02:33] <niemeyer> andrewsmedina: Sweeeeet
[02:34] <niemeyer> andrewsmedina: Small increments is the key
[02:34] <niemeyer> andrewsmedina: Don't feel bad to push half baked stuff
[02:34] <niemeyer> andrewsmedina: As long as it's a correct step forward, it's fine to go in
[02:34] <niemeyer> andrewsmedina: E.g. a type that implements all interface methods returning errors in all of them is a correct step forward
[02:35] <niemeyer> andrewsmedina: the implementation of a single one of these methods is a correct step forward to
[02:35] <niemeyer> too
[02:35] <niemeyer> error: Failed to load data for branch bzr+ssh://bazaar.launchpad.net/~andrewsmedina/juju/go-local-network/: Get https://api.launchpad.net/devel/branches?ws.op=getByUrl&url=bzr%2Bssh%3A%2F%2Fbazaar.launchpad.net%2F~andrewsmedina%2Fjuju%2Fgo-local-network%2F: dial tcp 91.189.89.224:443: connection timed out
[02:35] <niemeyer> error: Failed to load data for bran
[02:35] <niemeyer> Ah, dave isn't here
[02:36] <andrewsmedina> niemeyer: Why this error??
[02:36] <niemeyer> andrewsmedina: NEvermind.. I was going to point to Dave that it wasn't just on him that it was blowing up :)
[02:36] <andrewsmedina> niemeyer: I will sent small proposes
[02:37] <niemeyer> andrewsmedina: Cheers!
[02:44] <niemeyer> andrewsmedina: It's in!
[02:45] <niemeyer> and the queue is zeroed!
[02:45] <niemeyer> Ah, the feeling of mission accomplished.. well, at least for a few hours.
[02:45] <niemeyer> Will take the chance to have a nice sleep
[02:45] <niemeyer> Night all!
[02:45] <andrewsmedina> niemeyer: good night!
[09:05] <davecheney> rogpeppe: whoop whoop!
[09:06] <rogpeppe> davecheney: haroo!
[09:08] <fwereade> davecheney, rogpeppe: heyhey
[09:08] <rogpeppe> fwereade: yo!
[09:08] <fwereade> davecheney, btw, what awful time of day is it for you?
[09:09] <davecheney> fwereade: 7pm
[09:09] <davecheney> rogpeppe: this is what i'm thinking http://paste.ubuntu.com/1006126/
[09:09] <fwereade> davecheney, ok, not so awful then :)
[09:09] <rogpeppe> fwereade: you might want to fix this bug BTW: cmd/jujuc/server/util_test.go:37: not enough arguments in call to st.AddCharm
[09:09] <davecheney> not aweful at all
[09:09] <fwereade> rogpeppe, oof, thanks
[09:10] <davecheney> s/awe/aw/g
[09:10] <rogpeppe> davecheney: that won't work, sadly
[09:10] <davecheney> poop, why not
[09:10] <rogpeppe> davecheney: look at the implementation of deleteAll
[09:10]  * davecheney looks
[09:10] <rogpeppe> davecheney: it wouldn't be good to have the bucket changing underfoot during the execution of that method
[09:11] <rogpeppe> davecheney: hence my "immutable" suggestion.
[09:12] <rogpeppe> davecheney: i thought that's what you meant when you said it was "a bit harder" BTW
[09:12] <davecheney> rogpeppe: good point, but consider why the bucket would be changing
[09:12] <rogpeppe> davecheney: if we change the bucket, does it make sense to have an ongoing delete method suddenly start to remove objects that were listed in one bucket from another bucket?
[09:13]  * davecheney thinks
[09:13] <rogpeppe> davecheney: i think that an "immutable" method gives a nice understandable semantic - if you've started an operation with one set of settings, it will complete with the same.
[09:14] <davecheney> rogpeppe: if only it were that easy, for environ.SetConfig doesn't create a new storage, but alters it's bucket value
[09:15] <davecheney> however, that can be fixed
[09:15] <rogpeppe> davecheney: that's fine, i hadn't intended that it did
[09:15] <fwereade> rogpeppe, trivial: http://paste.ubuntu.com/1006133/
[09:15] <rogpeppe> davecheney: did you look at my immutable method?
[09:15] <davecheney> yup, you look to be taking another reference to the *storage
[09:15] <rogpeppe> fwereade: oh cool, i thought maybe it needed a real sha hash
[09:16] <fwereade> rogpeppe, it may do at some stage
[09:16] <rogpeppe> davecheney: no, i'm copying the contents of the storage.
[09:16] <fwereade> rogpeppe, but we're not at that stage yet :)
[09:16] <rogpeppe> davecheney: well, the contents of the storage struct anyway
[09:16]  * davecheney reads again
[09:17] <davecheney> 7~oooooooooh
[09:17] <davecheney> wow, i never considered that
[09:17] <davecheney> mainly 'cos i've never used a lanugage that could do that
[09:18] <rogpeppe> davecheney: it does mean we've got another allocation each time, but i think that's irrelevant, as mentioned earlier
[09:18] <davecheney> i agree about allocations
[09:18] <davecheney> i don't think allocation counting is important when we're talking to a slow arse ec2 provisioning service
[09:19] <rogpeppe> davecheney: and actually i think probably the compiler can tell that the value never escapes, so it might not even do one.
[09:19] <rogpeppe> davecheney: absolutely
[09:19] <rogpeppe> davecheney: particular when PutReader allocates 1700 times!
[09:19] <rogpeppe> s/ar/arly
[09:21] <davecheney> yeah, but gustavo had a point that I was touching too much code
[09:22] <davecheney> rogpeppe: what if the exported Storage/PublicStorage methods always returned an immutable *storage ?
[09:22] <davecheney> then DeleteAll would just work
[09:24] <fwereade> rogpeppe, btw, can I just submit that directly please?
[09:24] <davecheney> hmm, that might need a little more finesse, deleteAll isn't part of the public Storage API
[09:24] <rogpeppe> fwereade:  i think you should make a CL, i'll LGTM it and you submit it immediately. that way the commit log looks consistent.
[09:25] <fwereade> rogpeppe, SGTM
[09:25] <rogpeppe> davecheney: i don't think that would work
[09:25] <davecheney> rogpeppe: would you say that deleteAll is the exception that breaks the rule
[09:25] <rogpeppe> davecheney: because there's nothing stopping someone holding on to a Storage object, but i guess we want it to change when the settings change.
[09:25] <rogpeppe> davecheney: ... or maybe not. i wonder.
[09:26] <davecheney> rogpeppe: there are arguments for an against
[09:26] <fwereade> rogpeppe, https://codereview.appspot.com/6243054
[09:27] <rogpeppe> fwereade: you've checked that all tests pass?
[09:27] <fwereade> rogpeppe, heh, I'll do a full run through
[09:27] <rogpeppe> fwereade: i think it's worth it :-)
[09:27] <davecheney> rogpeppe: which do you think gustavo had in mind ?
[09:27] <fwereade> rogpeppe, quite so :)
[09:28] <rogpeppe> davecheney: i dunno. as you say, arguments for and against.
[09:28] <davecheney> if we call Environ.SetConfig, should that apply to any Storage that was requested, or should the Storage you previously asked for remain valid ?
[09:28] <davecheney> given that the reason for calling SetConfig is most likely to update some credentials
[09:28] <rogpeppe> davecheney: exactly. that's the question
[09:28] <davecheney> it's likely that your previous Storage would be inoperable
[09:29] <fwereade> rogpeppe, huh, strange things are going on elsewhere; I'll poke around and see if I can figure out what's up
[09:29] <davecheney> actually, SetConfig can _only_ change credentials, or the bucket
[09:29] <davecheney> as those are the only configuration values consumed
[09:31] <rogpeppe> davecheney: if we're changing credentials, to avoid screwing things up, we'll have to change the credentials *before* deprecating the old credentials
[09:31] <davecheney> rogpeppe: how about this, SetConfig never changes the value of storage.bucket, that is immutable once created
[09:32] <rogpeppe> davecheney: that won't work if we want a Storage to work long term
[09:32] <davecheney> it simply replaces (and safely publishes) new storage and publicStorage references
[09:32] <davecheney> right, so then the opposite is the better chioce
[09:32] <davecheney> the mutex guards storage.bucket, and changes to that are visible immediately
[09:37] <rogpeppe> davecheney: i think the crux is deleteAll - do we treat that as an "external" method, so the bucket can change underfoot as it's progressing?
[09:38] <rogpeppe> davecheney: what i'm concerned about is someone changing not just the security credentials (but leaving them pointing to the same original AWS account) but the name of the bucket too.
[09:38] <rogpeppe> s/(but /c/(/
[09:39] <davecheney> so, what if I implement 'immutable' inside deleteAll, as it is the special case
[09:39] <davecheney> or at least use it in just the deleteAll case
[09:39] <rogpeppe> davecheney: tbh i think we might be better for all concerned if we make storage not change with SetConfig
[09:40] <davecheney> rogpeppe: i would be more than happy with that
[09:40] <rogpeppe> davecheney: and we can document that SetConfig works only on the Environ and not on storage objects returned from it
[09:40] <davecheney> what is your feeling about gustavo's view on this ?
[09:40] <rogpeppe> davecheney: then anything that calls Storage gets a nice coherent view
[09:40] <rogpeppe> davecheney: i *think* he'd be ok with that.
[09:40] <davecheney> i've seen you raise the point a few times
[09:41] <rogpeppe> davecheney: i have mixed feelings - there are trade-offs all over
[09:41] <TheMue> morning
[09:41] <rogpeppe> davecheney: but my current feeling is that this is the sweet spot
[09:41] <davecheney> hey
[09:41] <rogpeppe> TheMue: hiya!
[09:41] <davecheney> rogpeppe: i'm happy to float that
[09:41] <davecheney> rogpeppe: in your extimation, who are these long term holders of Storage references ?
[09:42] <rogpeppe> davecheney: i don't think there are any
[09:42] <rogpeppe> davecheney: that's the reason for my current thoughts
[09:45] <davecheney> rogpeppe: then why suggest that storages' not change with SetCOnfig ?
[09:46] <rogpeppe> davecheney: because even though there are no long term holders of Storage references, there will be *concurrent* holders of Storage references
[09:46] <rogpeppe> davecheney: so we can provide them with a coherent view
[09:46] <rogpeppe> davecheney: by letting each call to Storage return a new *storage instance.
[09:47] <davecheney> rogpeppe: yup, i'd support that
[09:47] <rogpeppe> davecheney: in fact it's easier than that
[09:47] <rogpeppe> davecheney: we just alloc a new storage on each SetConfig
[09:48] <rogpeppe> davecheney: and return that from Storage
[09:48] <rogpeppe> davecheney: easy peasy
[09:48] <rogpeppe> davecheney: (changing the type of the storage field in environ to be a pointer, of course)
[09:49] <rogpeppe> davecheney: then the current storage implementation stays identical to current.
[09:49] <davecheney> let rogpeppe it's slightly less easy, 'cos we have to publish that reference properly
[09:49] <rogpeppe> doh, s/identical to current/the same/
[09:49] <rogpeppe> davecheney: that reference is inside the environ instance, no?
[09:49] <davecheney> i'd vote for constructing a new storage for each call to environ.Storage() using the current values of config()
[09:50] <rogpeppe> davecheney: i don't think there's a need to publish it
[09:50] <rogpeppe> davecheney: we will get the new instance when we call Storage again
[09:50] <davecheney> publish across goroutines
[09:50] <rogpeppe> davecheney: why do we need to do that?
[09:50] <rogpeppe> davecheney: we've said that there are no long-term users of Storage instances, right?
[09:51] <rogpeppe> davecheney: which means that when the current storage operations are complete, the goroutines will call Storage again and get the new version.
[09:51] <rogpeppe> davecheney: there, published.
[09:51] <davecheney> if goroutine X calls SetCOnfig, we need a mutex to ensure the value of environ.storage is properly published
[09:51] <rogpeppe> davecheney: yeah, obviously we still need the mutex on the environ
[09:52] <rogpeppe> davecheney:  but we make sure that any Storage returned from Environ is immutable.
[09:58] <davecheney> rogpeppe: yup, i can do that
[09:59] <rogpeppe> davecheney: it's important, then, that environ doesn't call deleteAll directly on the storage instance inside the environ. it should call Storage() first, and then cast .(*storage).
[10:02] <davecheney> i'll add a comment block if appropraite
[10:23] <davecheney> rogpeppe: this it looking like a much smaller diff
[10:23] <davecheney> storage.go is almost untouched
[10:23] <rogpeppe> davecheney: that's what i hoped
[10:24] <davecheney> # launchpad.net/juju/go/environs/ec2_test
[10:24] <davecheney> ./live_test.go:281: undefined: "launchpad.net/juju/go/environs/ec2".DeleteStorageContent
[10:24] <davecheney> o_O
[10:24] <davecheney> this is against trunk
[10:24] <davecheney> let me check I haven't delted that by accident
[10:25] <rogpeppe> davecheney: trunk works for me
[10:25] <davecheney> ignore that error, ny fault
[10:25] <davecheney> my fault
[10:25] <TheMue> *rofl*
[10:42] <fwereade> rogpeppe, ok, I just plain cannot figure out what is up with the store tests... I think I've nuked everything and started perfectly clean, and I still get 1 pass, 2 fixture panic, 25 missed
[10:42] <fwereade> rogpeppe, have you seen this before?
[10:42] <rogpeppe> fwereade: yeah, i see it every so often. i'll try again and see if i can reproduce it
[10:48] <davecheney> rogpeppe: https://codereview.appspot.com/6256050, tell me what you think
[10:49] <davecheney> specifically around immutable storages
[10:54] <rogpeppe> davecheney: LGTM
[10:54] <rogpeppe> davecheney: and i think this shows the goodness of the approach actually
[10:54] <davecheney> i'm relieved that storage.go is virtually unaffected
[10:54] <rogpeppe> davecheney: it means that we can still continue to use environs.EmptyStorage when there's no public storage
[10:54] <rogpeppe> davecheney: otherwise that wouldn't work.
[10:55] <davecheney> coolcoolcool
[10:55] <davecheney> so, from the POV of someone who wrote Storage, are you happy with the tradeoffs there ?
[10:55] <davecheney> ie, Storage() gives you an immutable reference
[10:55] <rogpeppe> davecheney: yeah, i think this works nicely
[10:56] <rogpeppe> davecheney: it means that someone external can write the equivalent of deleteAll without worrying about things changing underfoot
[10:56] <davecheney> rogpeppe: the only time I had to step outside the API was in instance.Destroy, but it was only two lines
[10:56]  * rogpeppe didn't notice that. one mo, while i look.
[10:57] <rogpeppe> davecheney: oh, you mean e.Storage().(*storage) ?
[10:57] <davecheney> yup
[10:57] <davecheney> e.Storage() gives you your own reference, the cast it too (*storage)
[10:58] <rogpeppe> davecheney: yeah, i think that's inevitable, unless we provide a Destroy method on environs.Storage
[10:59] <davecheney> this is just so you go through the mutex to get the properly published reference to environ.storageUnlocked
[10:59] <rogpeppe> davecheney: because the bucket's existence isn't something dealt with in the storage API
[10:59] <rogpeppe> davecheney: exactly; i think i mentioned this case above.
[10:59] <rogpeppe> [11:00:00] <rogpeppe> davecheney: it's important, then, that environ doesn't call deleteAll directly on the storage instance inside the environ. it should call Storage() first, and then cast .(*storage).
[10:59] <davecheney> jynx
[11:00] <davecheney> i'll fix the typos
[11:02] <davecheney> whoa, how did that change to storage slip into NewConfig
[11:12] <davecheney> right - time for a break, and probably a scotch
[12:30] <fwereade> rogpeppe, after much messing around it appears that the only thing that makes any difference is adding a ludicrously long sleep after starting the server in MgoSuite
[12:30] <fwereade> rogpeppe, by "ludicrously", I mean that sometimes 10s isn't long enough but it hasn't yet failed with 20s
[12:30] <rogpeppe> fwereade: hmm.
[12:31] <rogpeppe> fwereade: how is it failing again?
[12:31] <fwereade> rogpeppe, failing to dial
[12:31] <hazmat> fwereade, the server hasn't started?
[12:31] <fwereade> rogpeppe, no reachable servers
[12:31] <fwereade> hazmat, yeah
[12:31] <rogpeppe> fwereade: maybe the dial should retry?
[12:32] <fwereade> rogpeppe, reading the docs it appears it should anyway
[12:32] <fwereade> rogpeppe, I'll try DialWithTimeout again (10s is usual I think; I tried 20s, but I should probably try 30s to match the always-succeed-with-sleep rate)
[12:33] <rogpeppe> fwereade: sounds like a plan
[12:33] <rogpeppe> fwereade: what is it with these servers that forever to come up?
[12:34] <fwereade> rogpeppe, I have no idea, I really wasn't expecting that
[12:34] <fwereade> rogpeppe, I've spent all morning poking around at entirely unrelated things :/
[12:34] <rogpeppe> fwereade: i know the feeling!
[12:35] <rogpeppe> fwereade: hmm, having fixed the bug in madeBucket, one of my old tests fails. it *should* fail given the new semantics, but i'm not sure now if they're right.
[12:36] <fwereade> rogpeppe, heh :(
[12:36] <rogpeppe> fwereade: the test scenario is we have two Environs, both pointing to the same remote environ, e and e2; we do this: e.Bootstrap(); e2.Destroy(); e.Bootstrap()
[12:37] <rogpeppe> fwereade: do we care about that working or not?
[12:37] <fwereade> rogpeppe, tbh that feels somewhat edgey to me
[12:37] <rogpeppe> fwereade: me too. when are we ever gonna call Bootstrap twice on the same Environ, right?
[12:37] <rogpeppe> Aram: yo!
[12:38] <Aram> hi.
[12:38] <fwereade> rogpeppe, indeed; if reported the response should probably be "don't do that please"
[12:38] <rogpeppe> fwereade: yeah. i think i'll change the test.
[12:40] <rogpeppe> fwereade: and perhaps the docs: "When Destroy has been called, all Environs referring to the same remote environment may become invalid", or something.
[12:41] <fwereade> rogpeppe, yeah, sounds pretty clear
[12:43] <rogpeppe> fwereade: you gonna submit that AddCharm fix?
[12:45] <fwereade> rogpeppe, yeah, I got distracted with trying to make all tests pass :/
[12:45] <fwereade> rogpeppe, I'll submit that one separately right now
[12:45] <rogpeppe> fwereade: there are still more that fail too. i get sporadic StateSuite.TestUnitWatchPorts failures.
[12:45] <fwereade> rogpeppe, I saw that one once, indeed
[12:46] <rogpeppe> fwereade: i think TheMue has it in hand
[12:46] <fwereade> rogpeppe, cool
[12:47] <TheMue> rogpeppe, fwereade: Not yet really, only the idea of a race condition.
[12:50] <TheMue> rogpeppe: We talked about inserting a little sleep. But it could be a similar flaky behavior in production environment. So I've got to analyze your laste change deeper and how to avoid this behavior directly in the content watcher.
[12:50] <rogpeppe> TheMue: no need to insert a sleep.
[12:50] <rogpeppe> TheMue: i can see the problem, and i know the fix.
[12:51] <TheMue> rogpeppe: Oh, so feel free. I've got two branches in queue before.
[12:51] <TheMue> rogpeppe: What is your idea now?
[12:51] <rogpeppe> TheMue: i think! i haven't tested it yet, but i've just written the code (one line)
[12:51] <TheMue> rogpeppe: Small and beautiful ;)
[12:52] <rogpeppe> TheMue: insert a null op as the first test in unitWatchPortsTests
[12:52] <rogpeppe> TheMue: expecting an empty ports array
[12:52] <TheMue> rogpeppe: It's not only the port test.
[12:52] <TheMue> rogpeppe: There are more failing, all based on the content watcher.
[12:52] <rogpeppe> TheMue: the same would apply to any similar test
[12:53] <TheMue> rogpeppe: But so you fix the test, but not the nehavior.
[12:53] <rogpeppe> TheMue: the tests need to take account of the fact that the watcher *always* sends an initial value on the channel
[12:53] <rogpeppe> TheMue: i think that's reasonable behaviour
[12:53] <rogpeppe> TheMue: if we don't think that, then we can fix the behaviour
[12:54] <TheMue> rogpeppe: The idea of always sending is ok. But the fact, that we sometimes get errors and sometimes not.
[12:54] <rogpeppe> TheMue: but that behaviour was deliberately made that way; i don't mind if we change it to be different.
[12:54] <rogpeppe> TheMue: that's a bug in the tests.
[12:54] <rogpeppe> TheMue: they're creating the watcher and setting the content immediately after. there's a race there.
[12:55] <TheMue> rogpeppe: And what do you do when this happens in production?
[12:55] <rogpeppe> TheMue: there's no race unless you're trying to predict what the events are coming on to the channel, as the tests are.
[12:56] <rogpeppe> TheMue: each of those events is a valid representation of the contents at some point in the past.
[12:56] <rogpeppe> TheMue: and that's all we want from the contents watcher
[12:56] <TheMue> rogpeppe: I'll take a look at your change.
[12:57] <rogpeppe> TheMue: i'll push it in a mo
[12:59] <TheMue> fwereade: I've got to change the endpoint related methods to not separate between provider/requirer and peer. Instead I shall change it to …(endpoints …RelationEndpoint). OK, less functions, but more complex code imho.
[13:00] <fwereade> TheMue, yeah, I think that's a shame, IMO it makes it much harder to see what's going on
[13:00] <fwereade> TheMue, but hey ho
[13:00] <fwereade> TheMue, sorry for the noise, I guess :(
[13:01] <TheMue> fwereade: np ;)
[13:08] <rogpeppe> TheMue: there's an issue with ResolvedWatcher
[13:09] <rogpeppe> TheMue: when the content is empty, parseResolvedMode fails.
[13:09] <TheMue> rogpeppe: Will have to look at the change history. IMHO I've tested it.
[13:10] <rogpeppe> TheMue: yes, but before we weren't seeing that initial blank value. i guess it's being created blank, but perhaps it should be created with a valid resolved content.
[13:11] <rogpeppe> TheMue: ah, no! i guess that it just didn't exist before.
[13:11] <TheMue> rogpeppe: Yep.
[13:11] <rogpeppe> TheMue: so the first value on the content-changed channel is "no node existing"
[13:12] <rogpeppe> TheMue: what should the value of "resolved" be then?
[13:14] <rogpeppe> TheMue: maybe it should just be ResolvedNone if there's no node.
[13:15] <TheMue> rogpeppe: Sorry, I'm in a totally different context currently. IMHO the resolve change is relative new even for the Python version. Maybe fwereade can help.
[13:17] <TheMue> rogpeppe: I have to take some to to see the change history here too and look why it now fails.
[13:17] <TheMue> some time
[13:18] <rogpeppe> TheMue: i think it's quite straightforward - if you do WatchResolved on a unit without a resolved node, what should you read from the channel?
[13:19] <rogpeppe> TheMue: ah! the answer is there in Unit.Resolved
[13:19] <rogpeppe> TheMue: ResolvedNone is correct.
[13:20] <TheMue> rogpeppe: Didn't the original implementation did its first shot when the node is created, not before?
[13:21] <rogpeppe> TheMue: yes, but in keeping with the current thinking on watchers, we want to send the current state as the first event on the channel.
[13:22] <rogpeppe> TheMue: which in this case is ResolvedNone.
[13:23] <TheMue> rogpeppe: Yes, that's ok. But I'm a bit astonished that the change is in the code based but dependencies havn't been changed so far.
[13:23] <TheMue> code base
[13:23] <rogpeppe> TheMue: which dependencies?
[13:24] <TheMue> rogpeppe: You now see several follow-up, like in the tests.
[13:24] <rogpeppe> TheMue: sorry, i don't understand
[13:25] <TheMue> rogpeppe: The checkin of the changed watcher has been too early. The same branch IMHO has to change the tests or the problem with the parseResolvedMode.
[13:25] <rogpeppe> TheMue: i haven't checked in anything
[13:26] <rogpeppe> TheMue: i'm fixing those problems currently.
[13:26] <rogpeppe> TheMue: and the fact that ChildWatcher doesn't always send a first event.
[13:27] <TheMue> rogpeppe: The behavior of the content watcher changed. That's what I'm talking about.
[13:28] <rogpeppe> TheMue: ah yes. that's true. the problem was that the tests usually passed!
[13:28] <rogpeppe> TheMue: so i'm fixing that now.
[13:28] <TheMue> rogpeppe: Yip, here we have to find a proper way, like a continous integration server running all tests after each submit.
[13:29] <rogpeppe> TheMue: i think it's fine. as niemeyer said recently, we just need to make sure we run the tests ourselves before submitting.
[13:30] <rogpeppe> TheMue: ok, i have all tests passing reliably (i think!) now.
[13:30] <TheMue> rogpeppe: Do you run all tests? Off each package?
[13:30] <TheMue> rogpeppe: Great, cheers.
[13:31] <rogpeppe> TheMue: just the state package currently.
[13:31] <TheMue> rogpeppe: The cheers is for the fixing. ;)
[13:31] <TheMue> rogpeppe: I would like some kind of running all (!) tests at least once per day.
[13:31] <rogpeppe> TheMue: i'm gonna fix the watcher tests too.
[13:32] <rogpeppe> TheMue: we all run the tests at least once a day, i hope!
[13:32] <TheMue> rogpeppe: Do you have a script for it or do you go into each package manually?
[13:33] <rogpeppe> TheMue: go test launchpad.net/juju/go/...
[13:33] <rogpeppe> TheMue: or just go test ./...
[13:33] <rogpeppe> from the root of the go hierarchy
[13:33] <TheMue> rogpeppe: Oh, the elipses works? I didn't know, great, that helps a lot.
[13:33] <rogpeppe> the go tool is lovely like that
[13:33] <TheMue> rogpeppe: Wow, thx for that hint.
[13:34] <rogpeppe> TheMue: the ellipses are a wildcard - you can do go test .../state
[13:34] <Aram> rogpeppe: do you use bzr manually or do you use niemeyer's wrapper?
[13:34] <rogpeppe> or go install .../test/.../foo
[13:34] <rogpeppe> Aram: i use niemeyer's cobzr wrapper
[13:34] <Aram> ok, I'll look into it.
[13:34] <rogpeppe> Aram: it makes the branches work well with $GOPATH
[13:34] <rogpeppe> Aram: otherwise it's a right pain
[13:44] <rogpeppe> TheMue: https://codereview.appspot.com/6245053
[13:44] <TheMue> *click*
[13:46] <rogpeppe> TheMue, fwereade: i'm going to be away for an hour or two soon.
[13:46] <TheMue> rogpeppe: OK
[13:46] <fwereade> rogpeppe, np, I'm finishing in a mo too
[13:47] <fwereade> rogpeppe, might manage to push a couple of CLs before I do
[13:47] <rogpeppe> fwereade: cool. will have a look later.
[13:47] <TheMue> Btw, I've got a public holiday on Monday. So don't wonder if you don't reach me.
[13:48] <rogpeppe> TheMue: ah, ok. sometime we're going to have to try to do a meeting!
[13:48] <rogpeppe> TheMue: you know Aram is starting on monday, right?
[13:48] <TheMue> rogpeppe: Yep, good news. ;)
[13:49] <rogpeppe> TheMue: indeed. maybe we should have a meeting on tuesday?
[13:49] <TheMue> rogpeppe: Would be fine to me.
[13:49] <rogpeppe> fwereade: ?
[13:50] <rogpeppe> fwereade: i.e. would you be up for a meeting on tues?
[13:50] <fwereade> rogpeppe, tuesday would be fine
[13:50] <rogpeppe> fwereade: probs around 8pm oz time
[13:50] <fwereade> rogpeppe, is there only one time zone there?
[13:50] <rogpeppe> fwereade: which i think was what we agreed before
[13:50] <rogpeppe> fwereade: no idea. i mean sydney time, of course.
[13:52] <fwereade> rogpeppe, ok, so my midday I guess; that sounds great
[13:52] <rogpeppe> fwereade: not so good for niemeyer i think
[13:52] <TheMue> rogpeppe: Yep, midday is ok, lunch is later. :)
[13:53] <rogpeppe> fwereade: maybe he should call the time.
[13:53] <TheMue> rogpeppe: It's 7:00 then for him.
[13:53] <rogpeppe> TheMue: exactly.
[13:53] <rogpeppe> TheMue: maybe 9pm and 8am might be better
[13:53] <fwereade> rogpeppe, TheMue: I'm easy, but as you say niemeyer should probably call it
[13:54] <TheMue> rogpeppe: Isn't he in UK next week?
[13:54] <rogpeppe> TheMue: that's true
[13:54] <Aram> what timezone is dave cheney in?
[13:55] <rogpeppe> Aram: sydney, australia
[13:55] <Aram> ah.
[13:58] <TheMue> rogpeppe: LGTM
[13:58] <rogpeppe> TheMue: thanks
[13:59] <rogpeppe> niemeyer: yo!
[13:59] <Aram> hi ni
[13:59] <Aram> niemeyer:
[13:59] <niemeyer> Hello!
[13:59] <rogpeppe> niemeyer: you've arrived just as i'm leaving for a couple of hours
[13:59] <rogpeppe> niemeyer: got a couple more CLs for you to look at though. just bug fixes.
[14:00] <niemeyer> rogpeppe: Super
[14:00] <niemeyer> rogpeppe: Thanks
[14:02] <TheMue> niemeyer: Moin
[14:03] <niemeyer> Heya
[14:04] <rogpeppe> see y'all in a bit
[14:46] <TheMue> niemeyer: Just for info, the earring is back. ;)
[14:46] <niemeyer> TheMue: Hah, awesome :)
[14:47] <TheMue> niemeyer: One of the side jobs: plumber.
[14:47] <niemeyer> TheMue: Hehe, I know how it feels :)
[15:01] <Aram> meh, vmware website is getting worse every day.
[15:02] <TheMue> niemeyer: Could you please take a look at topology.py line 567 ff?
[15:03] <niemeyer> TheMue: Local or branch?
[15:03] <TheMue> niemeyer: Current trunk (python).
[15:03] <niemeyer> TheMue: Ah, .py
[15:04] <TheMue> niemeyer: There the interface of the actual relation in the loop is tested only against the interface of endpoint 0.
[15:04] <niemeyer> TheMue: There
[15:04] <niemeyer> TheMue: Ok
[15:04] <niemeyer> TheMue: ?
[15:05] <TheMue> niemeyer: When doing a test now with two endpoints with different interfaces, but the rest is ok, it seems to return a key instead of an error.
[15:05] <TheMue> niemeyer: Is that ok?
[15:05] <niemeyer> TheMue: How could this possibly happen?
[15:06] <niemeyer> TheMue: That would mean there's a relation with two different interfaces on it?
[15:06] <TheMue> niemeyer: No, adding is tested ok. But the user of the function may test two invalid endpoints (I've done so in my unit test, extra).
[15:07] <TheMue> niemeyer: IMHO a test that all passed endpoints share the same interface could be added.
[15:08] <niemeyer> TheMue: Yeah, that sounds fine
[15:09] <TheMue> niemeyer: OK, will add it, the rest of the new function RelationKey() has been changed according to your review. That's the last point.
[15:09] <niemeyer> TheMue: Super, thanks!
[15:10] <TheMue> niemeyer: For the NoRelationError string I've tried %+v, which is really good due to the field names, but long, and %v, which only shows the values of the endpoints. What do you prefer, short or long?
[15:11] <niemeyer> TheMue: Neither.. that's an error message.. should be oriented towards the user, not with types and debugging information
[15:12] <niemeyer> TheMue: An endpoint is uniquely identified by service name + relation name
[15:12] <TheMue> niemeyer: Hehe, that's why I wrote a RelationEndpoint.String() where you told me that it's not needed, instead %v.
[15:13] <niemeyer> TheMue: There can't be two different relations between two services with the same (service name, relation name) => (service name, relation name) characteristics
[15:13] <niemeyer> TheMue: Yeah, because you were *already* printing debugging information there
[15:13] <niemeyer> TheMue: In a way that is not understandable to a user nor to a developer
[15:13] <TheMue> niemeyer: OK, what about a method Id() string for RelationEndpoint returning exactly this with a colon?
[15:13] <niemeyer> TheMue: That's what I was against
[15:14] <TheMue> niemeyer: And I would use that id in the error generation.
[15:14] <niemeyer> TheMue: foo:bar:baz:baco:bar is not readable
[15:14] <TheMue> error message
[15:15] <niemeyer> TheMue: Yeah, sounds fine in principle
[15:15] <niemeyer> TheMue: Not sure I have the whole picture right now, but we can try it
[15:16] <niemeyer> I'll head to lunch and be back in a bit
[15:16] <TheMue> niemeyer: The message would be like ("… between %q and %q", ep1.Id(), ep2.Id()) , where %q later is "wordpress:blog" etc.
[16:03] <niemeyer> TheMue: I suggest using String()
[16:03] <niemeyer> TheMue: So you can say between %s and %s
[16:03] <niemeyer> , ep1, ep2,
[16:08] <rogpeppe> niemeyer, TheMue, fwereade: yo!
[16:10] <niemeyer> rogpeppe: Welcome back!
[16:10]  * niemeyer sips a delicious post-lunch coffee
[16:11] <rogpeppe> niemeyer: looks out over the sunny Tyne river
[16:11]  * rogpeppe looks out over the sunny Tyne river :)
[16:14] <rogpeppe> i just took a little time to see a final year recital from a friend who's a folk degree students here. t'was amazing. now using the free wi-fi at the Sage, Gateshead.
[16:14] <niemeyer> rogpeppe: Where's that?
[16:15] <rogpeppe> niemeyer: http://en.wikipedia.org/wiki/The_Sage_Gateshead
[16:16] <rogpeppe> niemeyer: am currently sitting just inside the glass just above the smaller of the two boats you can see in that picture. and the sky is just as blue.
[16:16] <rogpeppe> niemeyer: it's a fantastic building
[16:25] <rogpeppe> niemeyer: have you got any suggestions for the best way to do the putFakeTools thing?
[16:25] <niemeyer> rogpeppe: Woah
[16:25] <niemeyer> rogpeppe: This is amazing
[16:25] <rogpeppe> niemeyer: what is?
[16:26] <niemeyer> rogpeppe: That building
[16:26] <rogpeppe> niemeyer: it certainly is
[16:26] <rogpeppe> niemeyer: although some have compared it to a sea slug :-)
[16:28] <niemeyer> rogpeppe: It reminds me of the rubber that protects a car's wheel transmission
[16:28] <niemeyer> rogpeppe: Has the same shape
[16:28] <rogpeppe> :-)
[16:28] <niemeyer> rogpeppe: But I digress wildly :)
[16:28] <rogpeppe> niemeyer: i'm sitting just near to where the woman on the left is in this picture: http://en.wikipedia.org/wiki/File:Newcastle_the_sage_innen.jpg
[16:29] <niemeyer> rogpeppe: Regarding the putFakeTools, if you want to keep the tools fake, sounds fine, but I still see that as a lot of logic for what is basically "List + Get"
[16:29] <rogpeppe> niemeyer: and that arc you can see outside above her head is also an amazing piece of engineering, the millenium bridge
[16:29] <rogpeppe> niemeyer: well, yes.
[16:29] <rogpeppe> niemeyer: i'm mildly inclined to export ToolsPath from environs
[16:29] <niemeyer> rogpeppe: Sounds sensible too
[16:30] <rogpeppe> niemeyer: which would simplify the logic a lot, but slightly clutter the public API for the sake of testing.
[16:30] <rogpeppe> niemeyer: which i don't like doing.
[16:30] <niemeyer> rogpeppe: I'd prefer that we had at least a couple of variables on it, to make it explicit what it is depending upon
[16:31] <rogpeppe> niemeyer: ? variables on what?
[16:31] <niemeyer> rogpeppe: environs.ToolsPath(version, arch, "12.04")
[16:32] <rogpeppe> niemeyer: definitely. i think that function already might exist.
[16:32] <rogpeppe> niemeyer: just not exported
[16:32] <rogpeppe> 12.04 or precise? :-)
[16:33] <rogpeppe> niemeyer: BTW, i caught this piece of your conversation last night with davecheney: http://paste.ubuntu.com/1006651/
[16:33] <rogpeppe> niemeyer: i beg to disagree :)
[16:33] <niemeyer> rogpeppe: Yeah, good question, I kind of put the literal to talk about it :)
[16:33] <rogpeppe> niemeyer: do you have any idea how many allocations PutFile does?
[16:34] <niemeyer> rogpeppe: I don't, and I see no reason to increase it whatever that number looks like
[16:34] <rogpeppe> niemeyer: 1780
[16:34] <rogpeppe> more than
[16:34] <niemeyer> rogpeppe: Nice, let's make it larger then! Not.
[16:34] <rogpeppe> niemeyer: 3 allocations is lost in the noise
[16:34] <rogpeppe> niemeyer: (literally - it's within the standard deviation)
[16:35] <niemeyer> rogpeppe: Yeah, sounds great. Let's do caching please, it's simpler.
[16:35] <rogpeppe> niemeyer: the code is more complex actually. i don't mind too much, but i just had to take a little issue with the reasoning.
[16:36] <rogpeppe> niemeyer: anyway, "precise" vs "12.04"
[16:36] <niemeyer> rogpeppe: No, it's not, and we don't have to discuss this because we have evidence.. two different bugs were already introduced on the way of removing caching..
[16:37] <rogpeppe> niemeyer: i like the former because it's obviously an ubuntu release name. i like the latter because it's more amenable to program manipulation.
[16:38] <niemeyer> rogpeppe: Well, let's have "precise" then.. it's already around the code base, and we can actually manipulate it too now that I think of it.
[16:38] <niemeyer> rogpeppe: Still curious about what happens after 'z', but I'm sure *some* form of the logic will remain
[16:38] <rogpeppe> niemeyer: yeah, i'm slightly concerned about that.
[16:39] <rogpeppe> (if the code's simpler, i'm all for it in general - it was just the "it allocates more therefore let's do it differently" reasoning i wasn't keen on.
[16:39] <rogpeppe> )
[16:41] <niemeyer> rogpeppe: If you read properly, you'll see in the context that this is just yet another argument among the others
[16:41] <niemeyer> rogpeppe: and that argument is *also* true.
[16:47] <niemeyer> fwereade: ping
[16:53] <rogpeppe> niemeyer: fair enough
[16:55] <niemeyer> fwereade, rogpeppe, TheMue, Aram: Folks, just as a reminder, next week I'll be in London
[16:55] <rogpeppe> niemeyer: do you think you'll be able to make a meeting on Tues?
[16:56] <niemeyer> I'm not sure about how the flow of the sprint will look like
[16:56] <niemeyer> But I'll try to keep on top of reviews and communication as usual
[16:56] <TheMue> niemeyer: *envy*
[16:56] <Aram> btw niemeyer, do you think I'll be able to access the wiki by monday?
[16:56] <Aram> I don't have permission right now :).
[16:56] <niemeyer> Aram: I'm hoping so
[16:56] <TheMue> niemeyer: I only have been there on the airport so far. Would like to see more.
[16:56] <niemeyer> Aram: There's a "nice" list of tasks for you to go through after starting
[16:57] <niemeyer> TheMue: I won't be able to see much more than the airport, and the office, I'm afraid :)
[16:57] <niemeyer> TheMue: Well, the office is still in a fantastic spot, though..
[16:57] <niemeyer> That's changing soon unfortunately
[16:57] <niemeyer> Aram: That said, hmm
[16:57] <niemeyer> Aram: DOn't be super enthusiastic about it.. As far as juju goes, it's pretty much all public
[16:58] <TheMue> niemeyer: The new one seems to be nice too. Only not such a good outlook.
[16:58] <niemeyer> rogpeppe: Most probably, re. the meeting
[16:58] <rogpeppe> niemeyer: i'm thinking that it would be nice if we could attempt at least one face-to-face with Aram next week.
[16:58] <niemeyer> rogpeppe: Wasn't it Monday, btw?
[16:58] <niemeyer> rogpeppe: Yeah, that sounds like a great idea actually
[16:59] <rogpeppe> niemeyer: yeah, but TheMue has a public holiday on mon
[16:59] <rogpeppe> niemeyer: how about early one morning?
[16:59] <niemeyer> rogpeppe: Ah, sounds good then
[16:59] <niemeyer> rogpeppe, Aram: What about Friday?
[16:59] <rogpeppe> niemeyer: then it'll be ok for davecheney
[16:59] <Aram> niemeyer: next friday? as in one feek from now?
[16:59] <Aram> sure.
[17:00] <Aram> anytime.
[17:00] <niemeyer> Aram: Yeah, in London
[17:00] <niemeyer> :)
[17:00] <niemeyer> rogpeppe: Sounds good.. good chance given we're all closer in terms of timing
[17:00] <rogpeppe> niemeyer: BTW by "face to face" i meant G+...
[17:01] <niemeyer> rogpeppe: ROTFL
[17:01] <niemeyer> rogpeppe: That's what face-to-face has turned into? :-)
[17:01] <niemeyer> rogpeppe: I actually mean *real* face-to-face.. :)
[17:01] <rogpeppe> niemeyer: more so than IRC :-)
[17:01] <niemeyer> Aram: Can you get to London on Friday?
[17:01] <rogpeppe> niemeyer: actually next fri would work for me
[17:01] <niemeyer> Aram: How troublesome would it be for you?
[17:02] <rogpeppe> niemeyer: seems like a long way to come for a short time though.
[17:03] <niemeyer> rogpeppe: It pays off..
[17:03] <niemeyer> rogpeppe: The next several weeks will be a lot nicer for Aram after having met people personally
[17:03] <Aram> niemeyer: I guess I could come, have to check up flights and whatnot, never bought tickets from here.
[17:05] <rogpeppe> niemeyer: FWIW i can get to london in about 3h15m
[17:05] <rogpeppe> niemeyer: yeah, i agree with that
[17:05] <rogpeppe> niemeyer: when do you fly back?
[17:05] <niemeyer> rogpeppe: Friday too
[17:06] <niemeyer> rogpeppe: EOD
[17:06] <niemeyer> Maybe Thursday would be better
[17:06] <niemeyer> Aram: Which city are you in again?
[17:07] <Aram> vienna
[17:08] <niemeyer> Aram: I'll send a probe to the agency that books flights for us
[17:08] <niemeyer> Aram: Will CC you
[17:08] <Aram> please, thanks
[17:09] <rogpeppe> looks like flights from vienna to heathrow are ok
[17:09] <rogpeppe> niemeyer, Aram: http://www.skyscanner.net/flights/vie/lond/120530/120602/airfares-from-vienna-to-london-in-may-2012-and-june-2012.html
[17:10]  * Aram has to leave for a few hours, sorry.
[17:10] <Aram> I'll be back.
[17:11] <rogpeppe> Aram: see you monday!
[17:11] <niemeyer> Mail sent
[17:12] <niemeyer> rogpeppe: Yeah, not to bad
[17:12] <niemeyer> too
[17:12] <niemeyer> Aram: Cheers
[17:13] <rogpeppe> niemeyer: i've just proposed a new version of https://codereview.appspot.com/6245048 with simplified putFakeTools
[17:15] <niemeyer> rogpeppe: Looking
[17:20] <rogpeppe> niemeyer: i've got to head off now. will poke my head in later.
[17:20] <rogpeppe> niemeyer, TheMue, Aram, fwereade: have a great w/e
[17:20] <TheMue> rogpeppe: Thx, you too.
[17:21] <niemeyer> rogpeppe: Review delivered
[17:22] <niemeyer> rogpeppe: Thanks for the quick turn around
[17:28] <TheMue> niemeyer: Btw, changed Id() to String() of the RelationEndpoint too.
[17:49] <niemeyer> TheMue: Sweet, is it ready for another round?
[17:49] <TheMue> niemeyer: Yep, it's in the box. ;)
[17:49] <niemeyer> TheMue: Checking
[17:54] <niemeyer> TheMue: Diff looks bogus
[17:54] <niemeyer> TheMue: https://codereview.appspot.com/6198055/
[17:55] <TheMue> niemeyer: Ooops, but I set the prerequisite.
[17:55] <niemeyer> TheMue: Yeah, this is a bug in lbox that I still haven't fixed
[17:55] <TheMue> niemeyer: Hmm, but that is now in the trunk. May that be the reason?
[17:56] <niemeyer> TheMue: The problem is that the pre-req has been merged
[17:56] <niemeyer> TheMue: But lbox is still comparing against the pre-req
[17:56] <niemeyer> TheMue: I'm sorry about that
[17:56] <niemeyer> TheMue: To fix it, merge trunk on the pre-req, and push it
[17:56] <niemeyer> TheMue: (with bzr push only)
[17:56] <niemeyer> TheMue: Then, re-propose the follow up
[17:56] <niemeyer> TheMue: This will get the diff clean again
[17:59] <niemeyer> Ouch, forgot to buy the new battery
[18:01] <TheMue> niemeyer: OOK, wait a moment, will try.
[18:02] <TheMue> niemeyer: Ah, great, thx, looks cleaner now.
[18:12] <niemeyer> TheMue: Wow, yeah :)
[18:25] <niemeyer> TheMue: Review delivered
[18:25] <niemeyer> TheMue: The logic there is not quite right
[18:27] <TheMue> niemeyer: In which way?
[18:27] <niemeyer> TheMue: Please check the review and let me know if you need further details
[18:28] <TheMue> niemeyer: Done, that's why I ask. ;)
[18:28] <niemeyer> TheMue: Ok.. so you don't know that a *relation name* and a *service name* are not the same thing?
[18:30] <TheMue> niemeyer: I took line 571 in topology.py as example
[18:31] <TheMue> niemeyer: There is service["name"] != endpoint.relation_name
[18:32] <niemeyer> TheMue: The code is misleading indeed, but it's important to keep an eye on what you are actually doing
[18:32] <niemeyer> TheMue: What is inside that "service" dictionary?
[18:32] <niemeyer> TheMue: Have a look at assign_service_to_relation
[18:35] <TheMue> niemeyer: Ah, the relation name of the service.
[18:37] <niemeyer> TheMue: Do you have a good mental model of what a relation name is?
[18:38] <niemeyer> TheMue: That's not a trick question.. if you don't, let's talk about it
[18:39] <TheMue> niemeyer: It's a part of the relation identifier, but I find the name "name" mislieading.
[18:39] <niemeyer> TheMue: Ok.. so let's pick a metadata.yaml to look at
[18:39]  * niemeyer looks
[18:40] <TheMue> niemeyer: One moment, have to help my wife.
[18:40] <niemeyer> TheMue: https://juju.ubuntu.com/docs/charm.html#the-metadata-file
[18:40] <niemeyer> TheMue: At the end of the doc there are some samples
[18:41] <niemeyer> TheMue: The relation name is that "db" under provides or requires
[18:41] <niemeyer> TheMue: Now, imagine that I have an application called mywiki that takes a couple of mongodbs
[18:42] <niemeyer> TheMue: One of them is for caching, and the other contains the actual app data
[18:42] <niemeyer> TheMue: Both of these relations will have the "mongodb" interface, for example
[18:42] <niemeyer> TheMue: But we need to disambiguate them so we can refer to them
[18:42] <niemeyer> TheMue: So we might call one of them "data" and the other "cache"
[18:42] <niemeyer> TheMue: Both with interface "mongodb"
[18:43] <niemeyer> TheMue: So when I'm adding a relation, I could say something like
[18:43] <niemeyer> TheMue: juju add-relation mywiki:data themongoservice
[18:43] <niemeyer> For example..
[18:43] <niemeyer> TheMue: TO connect that specific relation
[18:43] <niemeyer> TheMue: So it is indeed a "relation name"
[18:43] <niemeyer> TheMue: Makes sense?
[18:49] <TheMue> niemeyer: Ah, ok, thank you, makes it more clear.
[18:49] <niemeyer> TheMue: np
[18:50] <TheMue> niemeyer: OK, so I'll change it (with a glas of whine beside me)
[18:50] <TheMue> niemeyer: For the interface error I used a plain error to return informations about the interface name.
[18:50] <niemeyer> TheMue: Will keep that in mind when reviewing.. LOL
[18:51] <TheMue> niemeyer: But indeed, I can change it to a NoRelationError too.
[18:51] <TheMue> niemeyer: Hehe, only one wine (or two or three *hicks*)
[18:53] <niemeyer> ;-)
[19:26] <TheMue> niemeyer: One question, coult you please take a look at https://codereview.appspot.com/6200044/diff/3001/state/topology.go.
[19:27] <TheMue> niemeyer: This has been my first approach before UDS. In line 50 and 55 you see that I used a type to contain the name information.
[19:28] <niemeyer> TheMue: Checking
[19:28] <niemeyer> TheMue: Ok?
[19:28] <TheMue> niemeyer: After the discussion with William I switched to a map of role to service key. Our idea has been that the name relates to the service name and that the key is better here. A wrong assumption, as you now showed to me.
[19:29] <TheMue> niemeyer: So now I would like to reintroduce this type, but containing the serviceKey (like today the sting in the map) and the relationName. What do you think?
[19:29] <niemeyer> TheMue: Sounds reasonable
[19:30] <TheMue> niemeyer: Fine, thanks.
[19:30] <niemeyer> TheMue: Thank you!
[19:36] <niemeyer> go-environs-setconfig is ready to go.. OMG.. so happy
[20:22] <andrewsmedina> niemeyer: nice!
[20:42] <niemeyer> Woohoo.. clean queue again
[20:42] <niemeyer> I'm heading out for some exercising
[20:57] <TheMue> niemeyer: I have to disappoint you. Just reproposed the latests branch. ;)