[03:02] aujuju: How can I manage multiple administrators with juju? [07:18] fwereade_: mornin' [07:18] rogpeppe, heyhey [07:18] fwereade_: how's the uniter trauma going? [07:19] rogpeppe, I *think* it's starting to feel sane again :) [07:20] rogpeppe, I'll see what I can pull together this morning :) [07:20] fwereade_: cool. i haven't got what the suggested change is. is it to do with using bzr for the upgrading? [07:21] rogpeppe, that's actually quite a small part of it -- the stuff I've been having difficulty with is the relatively innocuous suggestion that there shouldn't be any distinction between forced and unforced upgrade operations, and uh, something else I forget [07:21] interesting [07:23] fwereade_: BTW i just discovered a piece of cmd/juju testing which is fairly crackful - all the DeploySuite tests are executed 4 times each! [07:23] rogpeppe, golly [07:23] rogpeppe, how did that happen? [07:24] fwereade_: it's 'cos other command test suites are embedding DeploySuite [07:24] fwereade_: not quite sure how that got past my radar [07:24] rogpeppe, gaaaaah [07:24] rogpeppe, well, hey, faster tests :) [07:24] rogpeppe, nice catch [07:24] fwereade_: now i just have to work out how much of DeploySuite can be replaced by JujuConnSuite [07:25] * fwereade_ hopes lots [07:25] fwereade_: lots but... maybe not all; i think perhaps the JUJU_REPOSITORY setting needs to remain [07:25] rogpeppe, still :) [07:25] yeah [08:41] morning [08:44] TheMue, heyhey [08:44] fwereade_: heya [08:54] * niemeyer waves [08:59] niemeyer, heyhey [09:00] niemeyer, nice response :) [09:00] niemeyer, (on the list) [09:02] fwereade_: Morning! [09:02] fwereade_: Cheers.. tricky too [09:07] getting coffee + sunlight, bbs [09:07] Nice plan [09:08] No sun here yet, but I could get some coffee [10:58] niemeyer, Aram: you should get a link to a document via mail. so far e don't use very much watchers in trunk. [10:58] cheers. [10:59] TheMue: Sure, but the question is which watchers are needed/used [10:59] TheMue: You can talk to William to see details of the unit agent, and the Python code base for evaluation [11:00] niemeyer: I tried yesterday an experiment. I saw that you defined a local M type in txn and used that instead of the cluncky bson.M. I tried the same with bson.D in mstate. I did a local type D []bson.DocElem and then s/bson\.D/D/g but it won't work, any idea why. [11:01] niemeyer: yes, i think especially in fwereade_s queue are several usages [11:01] Aram: Yeah, bson.D is handled specially internally [11:01] interesting. [11:01] Aram: I guess we could make it check for the element type instead [11:01] Aram: Will have a look as I'm preparing an update [11:02] (mainly to get a few details of txn polished) [11:18] TheMue, offhand, the watchers I do/will need for the UA are: Service.WatchRelations; Service.WatchCharm; Unit.WatchResolved; Unit.WatchLife; RelationUnit.Watch [11:18] Aram, ^^ [11:18] fwereade_: hehe, thx, just scanning your proposals [11:19] TheMue, Aram: of those, only the last one should be tricky [11:19] fwereade_: thank you kind sir [11:19] yep [11:19] TheMue, Aram: if I find I've forgotten one I'll let you know ;) [11:19] fwereade_: great, thx again [11:20] Aram: I'll add it to the file [11:31] * fwereade_ sighs with relief -- the Uniter has been massaged into a new shape, which passes (near-enough-exactly) the original tests :) [11:32] (excluding actual upgrades, not everything in place there yet, but I feel vindicated all the same :)) [11:32] * niemeyer sighs with fwereade_ [11:32] niemeyer, ok, I has a plan [11:33] niemeyer, I want a damn uniter, so I intend to crudely hack out everything charm-related that I don't need, just to do a single dumb install without sufficiently smart recovery [11:34] niemeyer, happily your suggestions made it a hell of a lot easier to do that, much easier to decouple charmy modes from hooky modes [11:34] fwereade_: +1! [11:35] fwereade_: Glad to hear the refactoring went well [11:35] niemeyer, rough order, then: basic-uniter; charm-smart-upgrades; uniter-upgrade-charm; uniter-with-relations [11:36] niemeyer, in which uuc depends on csu depends on bu; and uwr hopefully just depends on bu [11:36] fwereade_: Found a NeedsUpgradeWatcher in uniter/modes.go, but not yet in trunk. is it in one of your proposals too? [11:36] TheMue, I killed that the other day [11:36] TheMue, replaced by Service.WatchCharm [11:36] fwereade_: ah, fine, will kill it too ;) [11:37] TheMue, cheers :) [11:37] fwereade_: Neat [11:37] niemeyer, yeah, it did -- it took a depressingly long time to find something that didn't acively hurt to look at [11:38] niemeyer, in fact, I'd better try to document the proposed charm.Manager (sorry :( ) API so you can point out all the ways it's crackful before I go too far ;) [11:40] fwereade_: I'd be happy to have a look [11:42] niemeyer, actually, huh, it should obviously be charm.Directory :/ [11:42] fwereade_: I'm not sure.. we have charm.Dir already :) [11:42] niemeyer, hmm :) [11:43] niemeyer, I'll stick with manager for now, then, Directory isn't actually quite right because it needs to keep other stuff in a separate state dir [12:18] niemeyer, http://paste.ubuntu.com/1162551/ [12:18] * fwereade_ tries not to look anxious [12:19] fwereade_: Looks quite nice :-) [12:20] niemeyer, cool :) [12:20] fwereade_: Slightly surprised to see state.Charm there, for some reason [12:20] niemeyer, I *think* it's the right type to use in general; maybe crackfulness will become apparent when you see it used :) [12:21] fwereade_: func (*Manager) ReadStatus() (Status, *state.Charm, error) [12:21] fwereade_: How can the manager know the *state.Charm* in use? [12:21] niemeyer, (man, it did take forever to figure out the right bits -- for a while I had a separate Operation type which really seemed like a good idea) [12:21] fwereade_: I'd expect it to know the URL instead [12:22] niemeyer, it's passed a *state.State on create (whoops, forgot that bit) [12:22] niemeyer, entirely so that it can give back a *state.Charm when asked [12:22] fwereade_: Yeah, I'm arguing that this is a bit awkward [12:22] fwereade_: It should really give back the URL, IMO [12:22] niemeyer, I *think* it works out better like this than messing around creating them in the Uniter [12:23] fwereade_: I think it breaks down a level of encapsulation for little benefit [12:23] fwereade_: state.Charm(url) [12:23] niemeyer, but, well, feedback like this is why I showed you -- thanks :) [12:23] fwereade_: That's all it takes [12:24] fwereade_: Thanks :) [12:25] niemeyer, I was on the fence, and I think it saved a couple of lines of code, but you're right -- cleaner API > cleaner client code [12:29] brb [12:55] lunchtime [12:58] Aram: Curious [12:58] Aram: Turns out I was wrong [12:58] yes [12:59] Aram: bson.D isn't special [12:59] Aram: It's already looking at the element type it seems [12:59] so what's the issue? [12:59] hmm [13:01] Aram: Ah, hmm [13:01] Aram: No, I'm wrong again [13:02] Aram: It's used explicitly through different logic.. [13:02] ok. [13:04] niemeyer: this should work as an Assert in txn, right? [13:04] bson.D{{"$or", []bson.D{ [13:04] bson.D{{"needsupgrade", nil}}, [13:04] bson.D{{"needsupgrade", NeedsUpgrade{Upgrade: true, Force: force}}}, [13:04] }}, [13:04] } [13:05] just an $or query... [13:05] I have a mysterious issue here. [13:05] Aram: You're probably getting caught into the difference of non-existence vs. nil [13:06] Aram: That said, this stuff has changed in state in the past couple of days [13:06] Aram: It's a lot more friendly to mstate now [13:06] Aram: It's probably easier to migrate it [13:06] ok, I'll do that, but I'm still interested in the issue. [13:07] it fails if needsupgrade was already set to that value. [13:07] but why would it, the assertion should hold true in that case, right? [13:08] I have another $or assert like that, also one of the options is nil, and that works fine. [13:15] Aram: Yeah, it's worth understanding why that is happening before moving on [13:15] Aram: Ah [13:15] Aram: []bson.D isn't right [13:16] Aram: Is that even working? [13:16] Aram: I mean., compiling [13:16] sure. [13:16] all my $or queries are []bson.D [13:17] Aram: Hmm, sorry, I'm on crack.. it's right [13:17] Aram: What is the value like in the database?" [13:17] Aram: At the moment the assertion fails [13:17] NeedsUpgrade{Upgrade: true, Force: force} [13:17] exactly what the assertion says [13:18] I add it once in the begining, it matches nil and it puts it in the db, then I try to add it again which should also work [13:18] but I get txn.ErrAborted [13:20] Aram: Ah, hmm [13:20] Aram: I know what's wrong [13:20] Aram: Let me confirm it [13:21] Aram: Yeah [13:21] so what is? [13:21] Aram: txn is using "$or" too [13:21] Aram: I'll add a test and fix it [13:22] Aram: Thanks for persisting on it [13:22] so it's a txn bug? [13:22] heh. [13:22] great. [13:23] Aram: Yeah.. I was (ab)using $or internally to make it cheaper to build the query [13:24] Aram: I have to mix the queries instead [13:25] Aram: So it ended up like {"$or": [{"$or": ...}]} [13:30] will juju-core use mongodb to distribute charm files instead of provider storage? [13:30] no. [13:31] (or at least not yet?) [13:34] Aram: Curious.. i can't reproduce it in a test [13:34] hazmat, Aram: It will, it doesn't yet [13:36] Aram: Ah, got it [13:45] Aram: I can't reproduce it [13:45] Aram: Can you please paste the snippet of code that is failing? [13:46] niemeyer, with the move to that, the only use of provider storage will be the client map to api/db servers? or is the plan to ditch provider object storage entirely? [13:47] hazmat: We may be able to ditch it entirely, but that's a bit off still [13:48] Aram: It looks like the nesting actually works [13:48] niemeyer: I will try to produce a self contained example [13:49] Aram: Can you please just paste the snippet first? [13:49] sure [13:49] niemeyer, cool, thats fine, i'm just talking future speak to an interested provider. [13:49] hazmat: I see, I do hope we're able to handle without the provider storage [13:50] Aram: I've addressed the custom []DocElem, btw [13:50] Aram: Will be out, probably as soon as we nail down why you're having trouble [13:53] niemeyer: http://paste.ubuntu.com/1162690/ [13:53] I've marked the failed assert [13:53] with THIS FAILS [13:56] Aram: Thanks [13:56] Aram: This test passes, btw: http://paste.ubuntu.com/1162673/ [13:57] niemeyer: wrote a self reproducing test [13:57] Aram: Oh, brilliant! [13:57] niemeyer: works with go run [13:57] http://paste.ubuntu.com/1162701/ [13:57] Aram: I'll try moving that onto a test case [13:58] Aram: Uh oh [13:58] Aram: This is bogus [13:58] niemeyer: wait, something is worng [13:58] Aram: Yeah [13:58] Aram: It's this bit: {{"a", nil}, {"a", "a"}} [13:58] Aram: This isn't a list of documents [13:59] Aram: It's a single document with two repeated fields [13:59] Aram: But that's not what you have in the real code [13:59] it isn't? [13:59] sel := bson.D{{"$or", []bson.D{ [13:59] bson.D{{"needsupgrade", nil}}, [13:59] bson.D{{"needsupgrade", nu}}, [13:59] }}, [13:59] } [13:59] that's the real code [14:00] Aram: Right.. that's not what the self-contained example has [14:00] Aram: The example is wrong, the real code is right [14:01] right [14:01] well, let me fix that [14:01] Aram: Btw, the THIS FAILS seems misplaced [14:01] Aram: It's pointing to a get rather than a set [14:02] Aram: Is the the set right above it the proper place? [14:02] niemeyer: yeah, sorry. [14:02] the set above [14:06] niemeyer: fixed my self contained example: http://paste.ubuntu.com/1162716/ [14:06] but I can't reproduce it either [14:06] it works there [14:07] and it's a good test, if I change "a" to b "b" in the assert, it fails the second change, which is correct. [14:07] so nothing wrong on the txn side. [14:10] Aram: and the only way to get ErrAborted is really for the assertion to fail [14:10] Aram: Nothing else returns it.. so it's a bit puzzling [14:10] yeah, what's difference between the self contained example and the real code... the assertion is the same. [14:11] the only difference is that one is a *string and one is a *T [14:11] Aram: Can you please enable mgo.SetDebug(true) right before executing the failing statement, so we can see what's running? [14:11] Aram: Ah, and mgo.SetLogger(c) [14:12] Aram: Hmmmmmm [14:12] Aram: Let me check something [14:13] niemeyer: debug log: http://paste.ubuntu.com/1162725/ [14:13] Aram: I bet it's the reordering of the fields [14:15] Aram: Yeaahhhh [14:16] interesting. [14:16] Aram: http://pastebin.ubuntu.com/1162730/ [14:17] hmm... [14:17] Aram: txn is using a map to inject some data in the inserted document (such as its id, revno, etc) [14:17] Aram: Will have to change it to a bson.D to avoid his [14:17] this [14:18] so it was a real bug, after all [14:22] Aram: Yep, can't reorder fields since people can trust on it [14:31] Aram: Thanks for the care on this, btw [14:34] welcme [15:23] niemeyer, also, 3rd time's the charm... maybe... https://codereview.appspot.com/6482053 [15:23] niemeyer, and on that bombshell, I am *exhausted*, and am going to knock off a little early today -- I'll almost certainly be back on later with my heart in my mouth ;) [15:24] later all [15:24] cheers [15:25] (and if anyone else is kinda lounging around, not sure what to do with themselves, please do take a look...) [15:25] ;p [15:29] fwereade_: Get some good rest! [15:42] * installation (not really resumable, except by luck) [15:42] LOL [16:11] Aram: you got the list of the test comparison, now i'll focus on adding the missing ones [16:13] Lunch [16:15] niemeyer: enjoy [17:37] * niemeyer waves [17:43] i'm off now. struggling with test stuff all day, hopefully make more progress tomorrow. [17:44] see yous tomorrow [17:55] rogpeppe: Have a good time [18:18] fwereade_: I hope the review on the first branch makes sense [19:11] Doc appointment.. back in 30-60mins === fss_ is now known as fss [21:24] fwereade_: Uniter <3 [21:53] niemeyer, w00t! [21:54] fwereade_: I was actually looking at the modes rather than the Uniter itself.. that stuff is superb [21:55] fwereade_: Can almost read it to a child :) [21:55] niemeyer, I was hoping you'd like it :D [21:56] niemeyer, it will get a touch heavier when we add the various other watches, but actually much less so than one might fear [21:56] fwereade_: Of course, but the mechanism itself is straightforward [21:56] fwereade_: Easy to navigate mentally between the states [21:57] niemeyer, yep -- you remember I originally wanted to make them types? I though the setup would be significant enough to hurt, but actually it really isn't [21:58] niemeyer, and yeah, just 2 more modes, upgrading and conflicted [22:00] niemeyer, I have a sketch with a mildly interesting helper type for ModeStarted, but it doesn't become relevant until upgrades *and* relations are in play... and even then it's a touch marginal, will be interesting to see if it's worthwhile when I'm watching lifecycles as well [22:01] fwereade_: Yeah [22:02] niemeyer, good comments on the preceding review, btw, I think I will refrain from addressing them until I'm a bit soberer ;) [22:04] fwereade_: LOL [22:04] fwereade_: Sounds like a plan :) [23:17] davechen1y: Heya [23:18] niemeyer: hey [23:43] davecheney: Regarding the env config [23:43] yes [23:43] davecheney: I think what you did is sane, and I was thinking of something that is not an issue.. [23:43] ok, phew [23:43] davecheney: My main concern is that we shouldn't have in state something that is an invalid environ.Config [23:43] erm [23:44] environs/config.Config [23:44] niemeyer: right, i don't think that is a problem with this proposal [23:44] davecheney: Otherwise the whole thing will break down when we try to make state.EnvironConfig() return a first class type [23:44] davecheney: yeah.. the fields being removed are fine [23:44] davecheney: They're specific to the environ [23:44] yup