=== jamespage is now known as 13WAATFJC === meetingology` is now known as meetingology [06:01] wallyworld_: would you rather do G+? [07:45] mornin' all === tasdomas` is now known as tasdomas === mthaddon` is now known as mthaddon [07:50] morning [07:51] dimitern: hiya [08:59] dimitern: /wave, I'm just grabbing a coffee, and then I'll be back for mumble [08:59] jam: i'll be there shortly [09:05] dimitern, ping [09:05] fwereade__: heyhey [09:05] dimitern, I don't recall: did we write code for joining new peer relations on Service.SetCharm? [09:05] fwereade__: not yet, this is upcoming [09:06] dimitern, ah -- am I right in thinking we *did* write it but it's been pushed back behind the command? or did we just talk about it? [09:08] fwereade__: that's right - it's behind the cmd [09:08] dimitern, cool, thanks, just trying to construct a model of reality ;p [09:11] fwereade__: np :) [09:12] jam, dimitern, rogpeppe1: so, I have a testing conundrum wrt the order in which the unit agent joins relations; I'm going to ramble about it for a bit in the hope that one of you will tell me what I should obviously do [09:13] fwereade__: i'm listening [09:13] fwereade__: you should obviously drink some coffee and go for a walk :) [09:13] 1) the unit agent should join relations in a particular order: peers first, then all others, and within those categories in creation order (ie id order) [09:13] jam, I've had a weekend to think about it, hasn't really helped ;p [09:13] I will read traceback, currently OTP [09:14] jam, cheers [09:15] 2) black-box testing of relation-join order is very tricky in the first place -- I can come up with tests that sometimes demonstrate a problem, maybe [09:15] "ordering" definitely seems like a whitebox test [09:16] 3) it's not currently possible to even *create* a peer relation for a service that has a higher id than any of the non-peer relations, because of missing functionality in Service.SetCharm [09:16] 4) but I could probably do that with a test internal to state [09:17] 5) how reasonable would it be to just define `type ByPriority []*Relation` in state -- which I think *can* be tested internally by doing inappropriate things to mongodb [09:17] (that's probably fine) [09:18] fwereade__: sorry, what exactly are you trying to test here? i seem to have missed that crucial detail. [09:19] 1) the unit agent should join relations in a particular order: peers first, then all others, and within those categories in creation order (ie id order) [09:19] fwereade__: ah, you're testing the unit agent [09:20] rogpeppe1, yeah [09:20] fwereade__: is it possible/valid to remove(destroy) peer relations? [09:20] fwereade__: what would ByPriority do? [09:21] 6) and then, in the uniter, try to come up with an internal test that demonstrates that join order is consistent with a ByPriority sort even if I can't show that's what we're using [09:21] dimitern, not really, only as part of service destruction [09:21] rogpeppe1, sort as described in (1) [09:21] 7) that's it [09:22] fwereade__: is the reason you can't test priority externally that the id isn't visible outside state? [09:22] rogpeppe1, no -- id is easy [09:22] rogpeppe1, but there's no clear way to get the order of relations joined on another goroutine [09:23] rogpeppe1, watchers coalesce events, remember ;p [09:23] fwereade__: please remind me why the order that the unit agent joins relations is important [09:24] rogpeppe1, a DB in a peer relation that handles replication, say, should not start talking to a client until it knows that peers exist [09:25] rogpeppe1, that said, *that* precise scenario may be impossible to tell anyway [09:25] fwereade__: ok, that's a reason why peers need to be joined before others. what about the secondary ordering? [09:26] rogpeppe1, I think it's just so that charms get a picture of the world that's as consistent as possible [09:27] rogpeppe1, and if you're defining a sort order you may as well do it completely rather than give up half way through ;p [09:27] rogpeppe1, I should probably sync up with hazmat, actually, the more I think the less sure I am that this will help at all [09:28] rogpeppe1, join order doesn't actually impact hook order anyway, even in python, afaict [09:29] rogpeppe1, and there's no point a hook checking whether a peer relation exists -- it must always do so implicitly [09:29] rogpeppe1, so, hmm [09:30] rogpeppe, yeah, screw this, I'll talk to hazmat later and find something else to do for now [09:31] oops, sorry, my computer just spontaneously rebooted [09:31] fwereade__: last thing i saw was [09:31] [Monday 25 March 2013] [09:29:04] rogpeppe1, and there's no point a hook checking whether a peer relation exists -- it must always do so implicitly [09:31] rogpeppe, I said "so, hmm", and thought until I said "screw this" [09:32] sorry for the noise gents [09:32] fwereade__: oh yes, i saw that. [09:32] fwereade__: np. [09:32] fwereade__: you might be interested to see what i did on saturday: https://codereview.appspot.com/8010043/ [09:33] davecheney, can I WIP 107b please? trying to get an accurate +activereviews [09:34] fwereade__: it generalises the transaction-space explorer i did before, and applies it to State. so far i only test a single operation though. [09:34] * fwereade__ looks [09:40] fwereade__: sure === negronjl` is now known as negronjl === racedo` is now known as racedo [09:49] rogpeppe, that is very neat, but I'm not 100% sure it's a clean fit for the problem -- in my mind, it would be nicer to just allow tests to hook into a point where they can make arbitrary state changes just before the txn is run [09:49] rogpeppe, that seems more conducive to deterministic testing [09:51] fwereade__: i think both things may have a place [09:51] fwereade__: the advantage of my scheme is that it allows black-box testing [09:52] fwereade__: and also potentially the writing of very compact tests that test existing operations against one another. [09:54] fwereade__: actually, thinking about it, my scheme does provide the capability to allow make arbitrary state changes just before a txn is run too [09:54] rogpeppe, I am probably not seeing the consequences clearly [09:54] rogpeppe, but I was imagining something like http://paste.ubuntu.com/5645845/ [09:55] fwereade__: i'm not sure that's sufficient - some operations execute several transactions in their normal course of action [09:55] fwereade__: so you'd want to be able to hook before or after any of those, i think [09:55] rogpeppe, what operations are these, that are not broken? ;p [09:56] fwereade__: if circumstances change underfoot [09:57] fwereade__: then an operation might legitimately execute several transactions [09:57] rogpeppe, oh, you mean when they abort due to state change rather than invalidity? [09:57] fwereade__: yup [09:57] rogpeppe, I don't see the problem there tbh [09:58] fwereade__: so, how do you test that the second transaction does the right thing even when the assumptions of *that* one are broken? [09:59] rogpeppe, close over a counter, if you can come up with a situation that really requires it [09:59] fwereade__: and i think you want to be able to test what happens if the state is mutated both before and after a transaction is run (it's not clear when SetTxnCallback would be called) [09:59] rogpeppe, I'm not following -- why afterwards too? [10:00] rogpeppe, that thing I posted was a complete test case that needs minimal machinery in state AFAICT [10:00] fwereade__: because if you get ErrAborted, you then inspect the state, which might have mutated in an unhelpful way since the transaction, and we'd like to be able to test that logic [10:01] rogpeppe, is that not just testing mgo/txn, rather than testing our own transactions? [10:01] fwereade__: i don't think so [10:02] fwereade__: for instance, consider Service.Destroy [10:02] rogpeppe, go on [10:02] fwereade__: if it gets ErrAborted, it does a Refresh, then proceeds to act on the basis of that state. [10:03] fwereade__: how do you test that logic? [10:03] rogpeppe, yes... [10:03] fwereade__: i suppose you can assume that it's always a straightforward loop and the logic at each cycle is the same as the previous [10:04] rogpeppe, well, you know what state change you made in the callback... that should either be sufficient to kill completely or to allow a second attempt to go through, regardless of the implementation [10:04] rogpeppe, so you test that the expected thing happens, and you're done [10:04] rogpeppe, right..? [10:06] fwereade__: i'm not quite following you there [10:07] rogpeppe, would you expand on why you need to assume it's a straightforward loop? [10:08] rogpeppe, I expect that in most interesting cases the construction of the new txn will be totally different from the old one [10:08] fwereade__: ok. first a little bit of terminology that i've been using when thinking about this: [10:09] fwereade__: an operation cycles between two states: "look" and "mutate". in look state, an operation queries the database and decides what op to perform; in mutate state, an operation runs a transaction. [10:10] fwereade__: i think that any operation can be considered to start in look and then alternate between look and mutate. [10:10] fwereade__: until it terminates [10:10] rogpeppe, ok... [10:12] rogpeppe, well, some start in a pre-looked state, but I expect we can handwave past that [10:12] fwereade__: so, in general, we want to test that the correct thing happens regardless of the interleaving of look and mutate states of concurrently running operations. [10:13] fwereade__: yeah, that's awkward, but we can get around it [10:13] rogpeppe, well, what I actually want to do is to test what happens to single operations when the state changes in prescribed ways between look and mutate [10:14] fwereade__: well yes - that "change in a prescribed way" is just simulating some other operation happening there, right? [10:14] rogpeppe, it's actually running the operation, in my model [10:15] fwereade__: ok, sure - so you're entirely encapulating all the look/mutate transitions of that other operation inside the SetTxnCallback hook [10:15] rogpeppe, yeah, if you like [10:16] rogpeppe, from my perspective the details of that other txn are a red herring, I think [10:16] fwereade__: look at it this way: could you use SetTxnCallback to actually produce an ErrExcessiveContention error? [10:16] rogpeppe, ofc [10:18] rogpeppe, in service.Destroy, for example, add/remove the single unit of the service every time you go through [10:19] fwereade__: yeah, i suppose so, by running a different operation at each stage, yeah [10:19] rogpeppe, maybe I'm not following your approach right, though [10:19] rogpeppe, how would you do it? [10:21] fwereade__: in a similar way actually, but rather than have a closure, we'd run an operation that did for i := 0; i < 5; i++ {add unit; remove unit} concurrently with the Destroy op [10:23] fwereade__: in general, we can get arbitrary interleaving of look/mutate states between concurrent operations, and i'm interested in verifying that behaviour is correct in all those cases. [10:23] rogpeppe, I'm not seeing how you'd synchronize them to ensure proper coverage [10:24] fwereade__: well with txntest there are two possibilities. [10:24] fwereade__: if the state space is small enough, you can let it do it itself, and it'll run through all possible interleavings [10:25] fwereade__: otherwise you can specify a set of interleavings to test [10:25] mgz: morning [10:26] fwereade__: say we've got two concurrent operations, a set of interleavings looks something like: 0.1.0t.1t.0*.1* [10:27] fwereade__: where the number signifies the operation in question (one of two, in this case); a bare number indicates a look state; a "t" suffix indicates a transaction (mutate), and a '*' suffix indicates that the op terminates at that point. [10:28] fwereade__: so the above trace says something like: both ops look at the state, and execute a transaction based on that state; they each look at the resulting state and stop. [10:29] rogpeppe, that STM like you're mixing up the tests for two operations, which is just about ok for 2 but becomes more tricky when there are 6, and that that case it would be clearer to test 0 in the context of 1, 2, 3, 4, and 5 separately [10:29] rogpeppe, especially if 4, 5, and 6 don't interact with one another [10:30] jam: hey [10:30] fwereade__: i think that we are mixing up the tests for two operations because actually we want to know what happens when those two operations execute concurrently. [10:30] mgz, just quickly, did my suggestion on your review look like crack? [10:31] fwereade__: given that we know that those operations operate on the same part of the state. [10:31] fwereade__: nope, but I do wonder if that's preferable to just having a base struct that contains 'config map[string]interface{}' and methods for operation on [10:31] I still have no feel for what idioms make sense [10:32] mgz: I do think having a separate TestConfig attribute, and then that gets copied to something when it is actually used [10:32] overriding the Config member vs copying from another member [10:32] mgz, my experience has been that the prefer-composition-over-inheritance-turned-up-to-11 approach actually leads to surprisingly neat code, even though my OO instincts led me to reach for inheritance-alike approaches by default [10:32] (eg) [10:33] I shall give it a hack. [10:33] mgz, there's another approach that says, hey, what I suggested is doing almost exactly that anyway in effect [10:33] fwereade__: your SetTxnCallback idea is definitely progress (and might well be all we need for the time being), but it still doesn't give me confidence that there's not some odd interaction between different concurrent operations that might not be predictable. [10:34] mgz, let me know how it goes, if it starts getting up your nose then your current approach is still progress [10:35] rogpeppe, it does mean we need to write the individual tests [10:36] fwereade__: sure; that's true in both cases i think [10:37] rogpeppe, I'm woried about how that'd look with the general approach though [10:37] rogpeppe, here's a pile of transactions, here's a pile of possible acceptable errors for each transaction, GO [10:38] fwereade__: it would probably be more like: here's two concurrent operations; verify that this condition holds after all these listed interleavings between the two. [10:40] rogpeppe, I worry that the condition will just be "either 0 won or 1 did" [10:40] rogpeppe, and that getting better than that, in the sense of detecting unknown weirdness, will be rather tricky [10:41] rogpeppe, some of them should of course be "both succeeded" [10:41] rogpeppe, anyway, thank you, that was an enlightening conversation [10:41] fwereade__: in the general (automatically explored) case, that's true, because that actually is the expected fuzzy result. but when specifying a specific set of interleavings, you can specify them such that you know the expected result and verify accordingly. [10:42] rogpeppe, yeah, that does make sense [10:43] rogpeppe, seems to me to make it harder to unit test individual operations clearly [10:44] fwereade__: possibly so. i'm not sure yet. [10:44] fwereade__: i've found it an interesting experiment, even if we don't use it. [10:45] rogpeppe, definitely -- and I'll try to give it another look later, because I definitely didn't follow all the details on the first run through :) [10:46] fwereade__: when i get a spare moment, i'll run up a test for Service.Destroy and see how it looks [10:46] fwereade__: 'cos there's quite a lot of untested logic there. [10:49] rogpeppe, the errRefresh action is untested, but I'm not sure what else you're referring to [10:50] fwereade__: errRefresh and ErrAborted [10:51] oh dear, my canonistack instance has just become very unhappy... [10:51] rogpeppe, but ErrAborted is tested really quite a lot [10:51] rogpeppe: fwereade__: please take a look at this error (config-get while deploying juju-gui): http://pastebin.ubuntu.com/5645957/ It seems something is going wrong while calling state/service.go:*Service.Config() [10:52] fwereade__: ah, because we don't query mongo before making the initial transaction? [10:52] rogpeppe, yeah [10:52] rogpeppe, well, we do a bit, with the relations [10:53] fwereade__: yeah [10:53] fwereade__: i just saw that [10:53] Mar 25 10:49:54 go kernel: [1433100.892933] EXT4-fs error (device vdb): ext4_journal_start_sb:370: Detected aborted journal [10:53] Mar 25 10:49:54 go kernel: [1433100.894501] EXT4-fs (vdb): Remounting filesystem read-only [10:53] frankban, looking [10:54] fwereade__: anyway, Service.Destroy does seem to be one of the more complex transactions around [10:54] fwereade__: hence, i thought, a good example for trying to test thoroughly. [10:54] rogpeppe, Destroy txns are in general total bitches [10:55] frankban, what happens if you do a `juju get juju-gui` from the CLI? [10:56] frankban, hey, did you bootstrap with --upload-tools? [10:56] fwereade__: yes [10:56] frankban, bah, ok, I keep doing that and being confused by biarre errors ;p [10:57] fwereade__: and deployed using "juju deploy -e go --repository=/home/frankban/devel/juju/gostore local:precise/juju-gui" [10:57] frankban, would you paste me the full unit log please? [10:58] fwereade__: sure [10:59] frankban, a thought: did you also rebuild the CLI tools rcently? [11:01] fwereade__: http://pastebin.ubuntu.com/5645995/ [11:01] fwereade__: how to do that? I usually just run "go install ./..." in launchpad.net/juju-core [11:02] frankban, yeah, that's what I do, usually followed by a paranoid ls -al $(which juju) [11:03] fwereade__: retrying [11:04] fwereade__: I usually go in $GOPATH/bin and use ./juju from there [11:06] frankban, I've got $GOPATH/bin on my path but I never quite trust that I haven't been screwing around with something and forgotten about it ;p [11:07] frankban, (my thinking here, fwiw, is that the keys for service settings documents *have* changed recently, but I'm pretty sure they're tested in some detail, so I'm hoping it's just a mismatch on your end) [11:08] * rogpeppe still thinks we should be changing our major version number every time we make a backwardly incompatible change. these kinds of issue are a pain to diagnose. [11:12] fwereade__: I am retrying after re-compiling trunk [11:13] frankban, cheers [11:21] bbiab === andrewdeane is now known as devhell === devhell is now known as awd [11:31] * TheMue dances around as he found why those too tests fail after the latest change [11:32] mgz: standup? [11:35] fwereade__: are you good for lunch aroung 13:30 (after standup)? [11:44] lunchtime [11:48] fwereade__, rogpeppe: same error. either using the local charm or cs:precise/juju-gui [11:49] frankban: could you paste a script which replicates the problem from scratch? [11:52] rogpeppe: it's just "juju bootstrap --upload-tools" and then "juju deploy cs:precise/juju-gui". it fails in the install hook. [11:52] frankban: ok, i'll try it and see if i can reproduce the problem [11:52] rogpeppe: thanks [12:00] frankban, sorry, everything went weird for a bit -- are you still seeing problems? [12:01] fwereade: yes, rogpeppe is trying to reproduce, basically I see the same install hook failure running "juju bootstrap --upload-tools" and then "juju deploy cs:precise/juju-gui". === gary_poster|away is now known as gary_poster [12:24] * dimitern lunch [12:28] frankban, fwereade: ok, i've reproduced the problem [12:29] rogpeppe: thanks, :-/ [12:30] fwereade: here's the whole error log that i see: http://paste.ubuntu.com/5646211/ [12:30] fwereade: sorry, uniter log [12:34] rogpeppe: that seems a charm error, unrelated to the problem I am encountering, and probably not depending on juju-core. It seems that config-get worked for you. [12:34] frankban: yes, i was just coming to that conclusion [12:36] rogpeppe: so, it must be something in my configuration, maybe it's worth retrying after deleting stuff in $GOPATH/bin and /pkg? [12:36] hmm, it would be nice if juju status could print the agent-version urls [12:36] rogpeppe, yeah, it would [12:36] frankban, rogpeppe: that's how it looks to me as well [12:36] frankban: are you sure you're executing the juju from $GOPATH/bin ? [12:37] * fwereade has to go to lunch, late already, back later at dimiter's [12:37] fwereade: enjoy [12:38] rogpeppe: yes I am, I am explicitly calling $GOPATH/bin/juju. [12:38] rogpeppe: in juju status I saw agent-version: 1.9.12 on machine 1 and agent-version: 1.9.11 on machine2 [12:38] frankban: and you've done go install launchpad.net/cmd/juju, presumably? [12:39] frankban: ah! [12:39] frankban: did you do a destroy-environment before bootstrapping? [12:39] rogpeppe: yes [12:40] frankban: what does juju --version print? [12:40] $ ./juju version [12:40] 1.9.12-quantal-amd64 [12:40] rogpeppe: ^^^ [12:41] frankban: what happens if you try: juju upgrade-juju --upload-tools ? [12:42] frankban: it's really weird that you're seeing 1.9.11 on the second machine [12:42] rogpeppe: yes [12:42] frankban: hold on, do you mean machine 0 and machine 1 ? [12:43] rogpeppe: do I need a bootstrapped juju before executing upgrade-juju? [12:43] frankban: yeah [12:43] frankban: but you've got that, no? [12:43] rogpeppe: yes, I need to bootstrap again, I destroyed the environment [12:44] frankban: ah, in that case, i recommend blowing away $GOPATH/bin and $GOPATH/pkg and doing go install launchpad.net/juju-core/... [12:44] frankban: before trying again [12:44] rogpeppe: ok === teknico_ is now known as teknico [12:57] rogpeppe: same error :-/, and same agent version mismatch: http://pastebin.ubuntu.com/5646274/ [12:58] rogpeppe: FWIW, I am using "default-series: precise" in environments.yaml [13:08] frankban: what result does juju status print if you use the juju command from this branch: lp:~rogpeppe/juju-core/256-more-status ? [13:11] rogpeppe: http://pastebin.ubuntu.com/5646309/ [13:12] frankban: ah! i see the issue [13:12] rogpeppe: cool! [13:13] frankban: i'm not sure of the best way to fix it though :-) [13:13] frankban: note that the bootstrap node is running quantal, and the other node is running precise [13:14] rogpeppe: yes, that's maybe because I am bootstrapping the env using a juju compiled in my quantal machine [13:15] Hello rockstars [13:16] frankban: ah! this is because your charm is precise not quantal [13:16] frankban: so it's doing exactly what we asked it to do [13:16] niemeyer: hiya [13:16] rogpeppe: indeed, I need the charm to be deployed on precise [13:16] rogpeppe: Yo [13:16] frankban: so you're asking the impossible here [13:17] frankban: because you're building the tools on quantal and expecting them to be used by a precise charm [13:18] frankban: if we had changed the major version number as we should when we changed the db schema, you should have got a better error (no tools found) [13:18] rogpeppe: aha! so the machine in precise will always get the latest released version of the agent for that platform? [13:19] frankban: the machine in precise will grab what tools it can find that match its series. [13:19] frankban: this is arguably wrong. the latest thinking, i think, is that if there's anything at all in the private bucket, it should ignore anything else. [13:20] frankban: anyway, your best best for fixing the problem is to create a quantal version of the gui charm [13:21] rogpeppe: ok, got it. so, I am deploying from trunk because I need "default-series" to be respected, but this way I am preventing the tools to be shared, because my machine is quantal, right?] [13:21] frankban: yeah [13:21] frankban: so if you deployed --upload-tools from precise, it would work (as it does for me) [13:29] * rogpeppe goes for lunch [13:42] back [13:52] rogpeppe: so, I should be able to develop a precise charm bootstrapping from trunk (quantal) once a newer precise juju (containing latest settings changes) will be added to the cache, right? [13:56] frankban: yeah [13:57] rogpeppe: cool, any plan on when this will happen already? [13:57] frankban: next release date [13:57] frankban: i'm not quite sure when that will be [13:57] frankban: i think it's once every two weeks [13:58] rogpeppe: great, thanks for your help === wedgwoodz is now known as wedgwood_away [14:38] rogpeppe, ping [14:38] fwereade: pong [14:38] rogpeppe, Conn.PutCharm just adds 1 to revision -- was this thought about carefully or just cloned from python? [14:39] rogpeppe, (when bumping revisions, that is) [14:39] rogpeppe, (if you recall) [14:39] fwereade: it was reasonably carefully thought about (i started with one approach, then gustavo suggested the current one) [14:41] fwereade: erm, actually i might be thinking of bump-version [14:41] rogpeppe, that's what I want you to be thinking about :) [14:41] rogpeppe, I'm trying to figure out the right approach when upgrading [14:41] fwereade: i mean upgrade-juju --bump-version [14:41] rogpeppe, ah, ok [14:41] rogpeppe, I guess it's essentially the same question: are we deliberately handwaving multiple users with their own local repos? [14:41] fwereade: what other behaviour might you consider? [14:42] fwereade: i think so. [14:42] fwereade: the whole issue of local charms is wrong anyway [14:42] fwereade: because one user's local charm might be nothing like another's [14:42] rogpeppe, one greater than max rev of matching charms in state and local repo [14:43] * fwereade tries to figure out what's the worst thing that can happen if we just use PutCharm from upgrade-charm [14:43] ...or if we actually can use it at all [14:44] fwereade: i think we *should* use PutCharm from upgrade-charm [14:44] rogpeppe, ok, but we need to get the charm twice then [14:44] fwereade: and fix it if it's got the wrong behaviour [14:44] rogpeppe, I fear it's more a rewrite than a fix [14:45] fwereade: once when we deploy and once when we upgrade? [14:45] rogpeppe, no, twice on upgrade [14:45] fwereade: ah, how so? [14:45] rogpeppe, we can't know what bumpRevision should be before we have a charm [14:45] fwereade: bumpRevision only works with local charms, no? [14:46] rogpeppe, charm dirs, specifically, local charms don't have to be dirs [14:46] fwereade: bumpRevision doesn't work unless it's a dir [14:46] rogpeppe, and we don;t know charm kind until we've got it [14:46] fwereade: and i think that's probably ok [14:46] rogpeppe, yes [14:47] rogpeppe, so we can't know what's safe to pass for bumpRevision unless we get the charm and find out the type [14:47] fwereade: this is client-side, right? [14:47] rogpeppe, yeah [14:48] fwereade: so what's wrong with just calling PutCharm with bumpRevision if specified, and just failing if it's not a dir? [14:48] rogpeppe, bumpRevision, if sane, is automatic when upgrading [14:48] rogpeppe, the question is determining sanity [14:48] fwereade: ah, i see [14:49] fwereade: hmm, i'm not sure that's right [14:49] fwereade: that means that if i upgrade-charm a bundle, it will silently do a different thing than if i upgrade-charm a dir [14:50] rogpeppe, yes -- when working with dirs, we do different things [14:50] fwereade: i think i'd hope that upgrade-charm would do exactly the same thing regardless of the source of the charm. [14:50] fwereade: currently we only do different things if you explicitly *ask* to do different things, no? [14:51] rogpeppe, define "currrently" [14:51] ;) [14:51] fwereade: in the current trunk [14:51] * fwereade shrugs [14:51] fwereade: of juju-core [14:51] it's not implemented ;) [14:52] fwereade: well, we have deploy --bump-revision [14:52] fwereade: which i'd expect to be the same as upgrade --bump-revision [14:52] sorry, upgrade-charm [14:53] fwereade: i think making it explicit forces the user to think about potential multi-user issues [14:54] rogpeppe, it was a heavily requested feature [14:55] fwereade: bump-revision? or the automatic nature of it? [14:55] rogpeppe, automatic bump [14:56] fwereade: i'd be happier if upgrade-charm didn't change the local charm at all, but uploaded it at one revision higher than the charm in the state [14:56] fwereade: the side-effect on disk seems unnecessary [14:57] rogpeppe, not unreasonable [14:57] fwereade: and in that case you can do it with any charm at all [14:57] rogpeppe, hmmmm, not sure that makes sense to me [14:58] fwereade: yeah, you probably want it to happen only with local charms [14:58] fwereade: where there's no automatically updated revision when it changes [14:58] rogpeppe, yeah [14:59] rogpeppe, trying to figure out if I can forget about the charm in state and just do one higher than the local one though [14:59] fwereade: if you're not changing it on disk, that won't work [15:00] fwereade: BTW is there some way of finding out the highest revision number for a given charm in the state? [15:00] rogpeppe, I didn't specifically agree to not-changing-on-disk, but I'm still considering it [15:00] rogpeppe, nope [15:01] fwereade: i think it might be best to go for the simplest possible approach here [15:01] fwereade: for the time being [15:01] rogpeppe, I think, though, that if we say local repos are only supported in single-user mode, we can get away with just bumping locally [15:01] fwereade: yeah [15:01] rogpeppe, ok, thanks, we'll see where we can get to with that [15:01] fwereade: and don't worry about getting the charm twiece [15:02] fwereade: (you could have PutCharm return a particular error if it can't bump the version) [15:02] rogpeppe, as long as I check repo type I'll only do that when it's local [15:02] rogpeppe, ah, hmm, yeah [15:02] rogpeppe, cheers [15:02] fwereade: np [15:11] fwereade: does this code look right for adding then removing a relation? http://paste.ubuntu.com/5646600/ [16:03] anyone know a reason I might be getting connection timed out to when trying to run juju status? [16:11] mgz: ping [16:11] rogpeppe: hey [16:12] mgz: i was just looking at the test images stuff in environs/ec2 [16:12] mgz: what's supposed to be the difference between TestImagesContent and imagesData ? [16:13] mgz: because as it is, anyone that runs the tests on i386 will get failing tests [16:13] mgz: because TestImagesContent has no i386 entries [16:13] rogpeppe: right. [16:13] do you think the juju test suite passes on i386? [16:14] mgz: i know it doesn't [16:14] mgz: for this one reason (at least, i guess) [16:14] mgz: i don't see any other reason why it shouldn't [16:14] so, this is an intermediate state. there's no reason to have a static imagesData at all. [16:14] mgz: it's just for tests, right? [16:15] the tests that need it aren't testing the mapping of cloud thingy data to image ids [16:15] so, they should just be given something that will work [16:15] mgz: yeah, the problem is that various pieces of logic use version.Current [16:15] mgz: that is being fixed, but for the time being, i guess i'll add an i386 line to TestImagesContent [16:16] I envisioned the next step as just generating the right string from the same values as the lookup will use [16:16] mgz: i think just fixing version.Current for the tests should do the job [16:17] mgz: which is a little bit less whiteboxy [16:18] mgz: it took me ages to work out why the lines in imagesData weren't being used :-) [16:45] rogpeppe: care for a review? :) https://codereview.appspot.com/7927043/ [16:45] dimitern: will do [16:45] rogpeppe: cheers [17:06] fwereade: ping [17:07] TheMue, pong [17:07] fwereade: just too a look into deploy.go. could it be that deploy --config is already implemented? [17:08] * fwereade looks [17:09] fwereade: it reads the YAML and passes it to conn.DeployService() [17:10] TheMue, hmm, seems so [17:10] TheMue, that's good news :) [17:10] fwereade: seldom been so fast. *lol* [17:11] TheMue, nice :) [17:11] TheMue, just reviewed the $HOME stuff [17:11] TheMue, please let me know if there's any uncertainty [17:12] fwereade: yep, will read it now. did you find any Init() outside of main()? sh**, should be all removed. *sigh* [17:12] TheMue, only a couple [17:13] fwereade: ok, will clean it. thx for the review [17:14] dimitern: am looking at your review. while i'm at it, if you want a simple one: https://codereview.appspot.com/8016043 [17:14] TheMue, as a suggestion, the two nastiest places are repo.go and cloudinit_test.go [17:14] rogpeppe: sure [17:14] fwereade: this branch should fix trunk tests under i386 [17:14] https://codereview.appspot.com/8016043 [17:15] TheMue, single small CLs to unmess those particular areas would probably make your life easier [17:15] * fwereade looks [17:16] fwereade: yes, it started small, but then the effect growed [17:16] TheMue, I know how it happens :) [17:18]