/srv/irclogs.ubuntu.com/2012/10/05/#juju-dev.txt

davecheneyniemeyer: just a quick double check https://codereview.appspot.com/660105600:00
davecheneyniemeyer: do you want review this goamz patch ? https://code.launchpad.net/~mirtchovski/goamz/ec200:18
niemeyerdavecheney: LGTM on first, thanks00:24
davecheneyniemeyer: ta00:24
niemeyerdavecheney: Do you need something from the second?00:24
davecheneywhat is the code review policy for goamz00:25
davecheneythe key change for that branch is adding DescribeImages00:25
davecheneyas the user (f2f in #go-nuts) has created their own ami's00:26
davecheneywhereas we use the ubuntu cloud image finding service thing00:26
davecheneyniemeyer: does turning on debug in ec2 dump my secret key ?00:27
davecheneyie, is it safe to post this whole debug log ?00:27
niemeyerdavecheney: I don't know..00:28
niemeyerdavecheney: There's a lot pending in goamz.. someone needs to dedicate some time to it00:28
niemeyerdavecheney: We also require people to sign the contribution agreement on external contributions to Canonical code bases00:29
niemeyerdavecheney: Has he signed?00:29
davecheneyniemeyer: i doubt he's signed anything00:29
davecheneyniemeyer: https://bugs.launchpad.net/juju-core/+bug/106194100:29
davecheney^ debug in issue00:30
niemeyerdavecheney: Okay, can you figure why it is broken?00:32
davecheneyi'm sure it's the length of the URL, or some component of that00:33
davecheneybisecting the problem now00:33
niemeyerdavecheney: That debug info doesn't say much.. the meat is usually inside sign.go00:35
niemeyerdavecheney: We had it broken sometime ago due to a change in the way path was handled00:35
niemeyerdavecheney: Ah, wait00:36
niemeyerdavecheney: It could be about the encoding of the + there00:36
davecheneyniemeyer: right o00:37
niemeyerdavecheney: Not how the signature has a %2B00:37
niemeyerdavecheney: Note00:37
davecheneyyup, that will proably screw itup00:37
davecheneydamn spaces00:37
davecheneyniemeyer: there is a certain number of instances where it does work for00:38
davecheneyi'm figureing out that number, it's like 3 or less I think00:38
niemeyerdavecheney: This may match the chances of getting a signature with a given shape00:39
=== meetingology` is now known as meetingology
* niemeyer => bed03:58
niemeyerNight all03:58
rogpeppefwereade_: mornin'06:08
rogpeppedavecheney: hiya06:09
TheMuemorning06:21
davecheneymorning gents06:29
fwereade_rogpeppe, heyhey06:33
fwereade_davecheney, heyhey06:33
fwereade_TheMue, heyhey06:33
TheMuehi all06:33
fwereade_rogpeppe, I've been chatting to niemeyer about jujuc.HookContext06:55
rogpeppefwereade_: ok06:55
fwereade_rogpeppe, we are agreed that it should be an interface, and nothing more specific has been said06:55
rogpeppefwereade_: interesting.06:56
fwereade_rogpeppe, I have had a crazy idea, that the interface we *really* want may actually be a jujuc.Conn, with the same thinking behind it as juju.Conn06:56
rogpeppefwereade_: i think of it as quite concrete, but i'm interested where you end up06:56
rogpeppefwereade_: juju.Conn isn't an interface :-)06:57
rogpeppefwereade_: what different implementations of this interface do you envisage?06:57
fwereade_rogpeppe, the purpose is to separate the server and the commands cleanly from all the crap in RelationContext that is in the wrong place06:59
fwereade_rogpeppe, by making an interface and moving the existing implementation off into uniter we can start to collapse Relationer and RelationContext into something interesting06:59
fwereade_rogpeppe, the fact that it will also be possible to write neater contexts in the jujuc tests is both a bonus and a hassle07:00
rogpeppefwereade_: i'm not sure i see the relationship with juju.Conn. doesn't the context need to know the context within which the commands work (i.e. the current unit, relation, etc) ?07:01
fwereade_rogpeppe, well, yes, but I think we can expose all that clearly and usefully in terms of capabilities rather than things07:02
fwereade_rogpeppe, no need for a *state.Unit in the Conn(?) has an OpenPort method, etc etc07:02
fwereade_s/ in / if /07:03
fwereade_rogpeppe, and then I thought "hmm, juju.Conn presents a convenient high-level interface with state"07:03
rogpeppefwereade_: what about commands that operate relative to the context?07:04
fwereade_rogpeppe, "this situation has at least one interesting parallel"07:04
rogpeppefwereade_: e.g. unit-get, relation-get etc07:04
fwereade_rogpeppe, they basically all do...07:04
rogpeppefwereade_: yeah, so it won't look much like juju.Conn then, i guess07:04
rogpeppefwereade_: but you could certainly provide primitives that look like the ones the commands require07:04
fwereade_rogpeppe, yes, that is the commonality I am thinking of07:05
rogpeppefwereade_: but then, does that really save you much?07:05
rogpeppefwereade_: over just writing the commands, using the stuff inside the context07:05
fwereade_rogpeppe, well, it's basically a straight move of code inside the context07:06
rogpeppefwereade_: i guess i don't really see where the pressure here is. could you explain what the problem is with the current structure?07:07
fwereade_rogpeppe, primarily that relationy responsibilities are unhelpfully smeared across two modules, and I will be able to express things much more clearly07:08
fwereade_rogpeppe, by combining RelationContext and Relationer with some of HookContext07:08
* rogpeppe is thinking07:10
fwereade_rogpeppe, a server module that contains nothing but the server and the commands, and implements them in terms of an interface, will allow me the freedom to butcher the existing types and construct a magnificent assemblage from their parts :)07:10
rogpeppefwereade_: perhaps you could put together a sketch of how you think the types might work?07:13
rogpeppefwereade_: i guess i don't really see how an interface will work. who will implement the interface?07:15
fwereade_rogpeppe, something in uniter whose true nature will only become apparent as i integrate relations07:15
fwereade_rogpeppe, for now, the existing HookContext and RelationContext will move into uniter and be manipulated by Relationer as before07:16
fwereade_rogpeppe, they'll just be tweaked to implement the interface07:16
rogpeppefwereade_: the difficulty i see is that you've got two kinds of context going on, and one is a superset of the other AFAICS07:16
fwereade_rogpeppe, expand please07:16
rogpeppefwereade_: well, aren't the commands that you can execute in a relation hook more-or-less a superset of the commands you can execute in a non-relation hook?07:17
fwereade_rogpeppe, no, they're exactly the same07:17
fwereade_rogpeppe, you just might need to specify things that aren't present in a non-relation context07:17
rogpeppefwereade_: ok, so... i still don't see the interface thing. it's only useful to have an interface if you've got multiple implementations of it, in my view.07:19
rogpeppefwereade_: and i still don't see how that applies here07:19
rogpeppefwereade_: couldn't you just move HookContext into relationer? (with possible renaming/restructuring along the way)07:20
fwereade_rogpeppe, no: import cycles07:21
fwereade_rogpeppe, that *is* the right place for it07:21
rogpeppefwereade_: what's the import cycle there?07:21
fwereade_rogpeppe, the uniter needs to import jujuc to do stuff with the context; jujuc needs to import uniter to know wtf it's been given07:21
fwereade_rogpeppe, it could have its own package07:22
fwereade_rogpeppe, and that package will still exhibit the original problem07:22
rogpeppefwereade_: if the context was in relationer, the uniter wouldn't need to import jujuc, no?07:22
fwereade_rogpeppe, there is no relationer package07:22
fwereade_rogpeppe, and I don;t think there should be07:22
rogpeppefwereade_: i think we're getting closer to the crux of the issue here.07:22
fwereade_rogpeppe, Relationer itself will almost certainly change beyond recognition07:23
fwereade_rogpeppe, it is at heart a part of the uniter, and if it deserves to be a helper type then its shape will be very different07:24
rogpeppefwereade_: so could you put the context in uniter?07:25
rogpeppefwereade_: without having an import cycle?07:25
fwereade_rogpeppe, yes, that is what I'm proposing; that is the situation with the import cycle07:25
fwereade_rogpeppe, an interface STM to be a great way of doing so with minimal disruption07:26
rogpeppefwereade_: why does the uniter need to import jujuc?07:26
fwereade_rogpeppe, so it can run a jujuc.Server07:27
fwereade_rogpeppe, and supply it with contexts07:27
rogpeppefwereade_: ah yes, of course, that seems right07:28
* rogpeppe is still thinking07:29
fwereade_rogpeppe, it is a situation that deserves lots of it, I have been thinking about this for some time :)07:30
fwereade_rogpeppe, hmm, it crosses my mind that OpenPort/ClosePort are somewhat crackful, because they change global state directly07:33
rogpeppefwereade_: rather than wating until the hook has completed successfully?07:34
fwereade_rogpeppe, my fuzzy feeling that the Context interface should not directly expse state at all has just grown firmer07:34
fwereade_rogpeppe, yeah07:34
fwereade_rogpeppe, also, config-get isn't using a frozen config07:34
rogpeppefwereade_: it probably should, i guess07:35
fwereade_rogpeppe, (also we expose stuff like ConfigNodes which have *totally* inappropriate capabilities like Read and Write07:35
fwereade_)07:35
rogpeppefwereade_: here's a perhaps crackful idea: could we obliterate *all* knowledge of units, charms etc from jujuc/server ?07:41
fwereade_rogpeppe, not crackful at all07:41
rogpeppefwereade_: and have it simply provide a command-running and callback service07:41
fwereade_rogpeppe, I've been skating around the issue due to the distracting potential crackfulness07:41
fwereade_rogpeppe, hmm07:41
fwereade_rogpeppe, I'd been thinking more in terms of it not referencing state directly in the interface at all07:42
rogpeppefwereade_: it seems to me that this is where the cyclic stuff is coming from, and that actually the *important* thing that jujuc/server is to run commands and allow commands to call back.07:42
fwereade_rogpeppe, niemeyer and I are in rough agreement that server+commands is a cohesive chunk07:42
rogpeppes:jujuc/server:jujuc/server does:07:42
fwereade_rogpeppe, I think he feels that way more strongly than me though07:43
rogpeppefwereade_: i think i'm more of the opinion that the commands are tied more closely to the uniter than the server mechanism07:43
rogpeppefwereade_: and i *think* that might help things straighten out07:44
rogpeppefwereade_: in fact it looks to me as if even now there's no essential reason for HookContext to live in jujuc/server07:45
fwereade_rogpeppe, I was convinced otherwise :)07:45
rogpeppefwereade_: NewServer doesn't rely on uniter07:46
rogpeppeAFAICS07:46
fwereade_rogpeppe, I have been through this07:46
fwereade_rogpeppe, I think we are in agreement that "pull server out on its own, it'sthe most obviously independent bit" would be a reasonable thing to do07:46
rogpeppefwereade_: yeah07:46
fwereade_rogpeppe, however, I think that all that actually does is push a non-problematic part to one side07:47
rogpeppefwereade_: it seems to me that server+commands is *not* a cohesive unit - it's two things stuck together not-very-strongly-at-all.07:47
fwereade_rogpeppe, well, they are united in their need for a HookContext07:47
rogpeppefwereade_: the server doesn't need a HookContext necessarily.07:48
fwereade_rogpeppe, at least, it needs to "know" about them, even if it doesn't manipulate the type directly07:48
rogpeppefwereade_: what does it "know" about them?07:49
fwereade_rogpeppe, ContextId is one of the Request fields07:49
fwereade_rogpeppe, really07:50
fwereade_rogpeppe, the location of Server is a derail07:50
fwereade_rogpeppe, it does not contribute to solving the problem07:50
rogpeppefwereade_: i'm not so sure07:50
fwereade_rogpeppe, it is trivial to extract it now and it will remain trivial to do so tomorrow07:51
rogpeppefwereade_: the main cyclic problem was between jujuc/server and worker/uniter, no?07:51
fwereade_rogpeppe, the cyclic problem is HookContext, full stop07:51
fwereade_rogpeppe, and that is a symptom of the relation ickiness more than anything else07:52
rogpeppefwereade_: suppose you had a worker/uniter/commands package and defined HookContext in there?07:52
rogpeppefwereade_: might that help?07:53
fwereade_rogpeppe, then I *still* have the precise problem I have been trying to express all morning07:53
rogpeppefwereade_: sorry, please reiterate. it'll penetrate my think skull eventually :-)07:53
fwereade_rogpeppe, Relationer and RelationContext need to be in the same package so I can evolve them towards cohesive sanity without making a mess in other packages07:53
fwereade_rogpeppe, once I have them next to one another I think it will be almost trivial07:54
fwereade_rogpeppe, while they're distant any CL will be laughed out of the room for being monstrous, and quite rightly so :)07:54
rogpeppefwereade_: so... the problem is essentially that the commands need RelationContext and you don't want to put RelationContext in the same package as the commands.07:55
rogpeppefwereade_: (or vice versa)07:55
rogpeppefwereade_: because ISTM that an ultra-simple solution would just be to put the commands in uniter07:56
rogpeppefwereade_: it wouldn't make uniter too huge actually. the two packages combined (including the separable server stuff) amount to only ~2100 lines.08:00
rogpeppefwereade_: and perhaps this is the fundamental underlying semantic issue - the commands *are* tied closely to the uniter.08:01
fwereade_rogpeppe, IMO not so tightly as to justify dumping them all in the same package08:04
fwereade_rogpeppe, being able to express the interface without mentioning state has a benefit as well -- it keeps the jujuc package nice and hygienic, and unable to itself cause the write-on-error bugs we discussed above08:05
rogpeppefwereade_: it seems to me that they're really just functions that are called back into the uniter; they just happen to be expressed as commands.08:05
fwereade_rogpeppe, well, yes; but *everything* under uniter is similarly closely tied to uniter, and I really don't think dumping it all in one package is a good idea08:07
rogpeppefwereade_: hmm, i may be coming around to your point of view :-08:08
rogpeppe)08:08
* fwereade_ feels cheered :)08:08
rogpeppefwereade_: you'll need to be careful how you define the interface though; it might not be easy to avoid cycles or awkward copying type conversions.08:10
fwereade_rogpeppe, I *think* have something moderately sane coming through08:11
rogpeppefwereade_: FWIW i think that worker/uniter/commands is a better name than jujuc/server08:12
rogpeppeor worker/uniter/cmd08:12
fwereade_rogpeppe, the thing is you are right about the callbacky nature of the commands, and I think this will actually make that clearer08:12
fwereade_rogpeppe, as soon as we drop jujuc I will be agitating for worker/uniter/tools08:12
rogpeppefwereade_: tools == commands ?08:12
fwereade_rogpeppe, yeah, specifically borrowing terminology from elsewhere in the project08:13
fwereade_rogpeppe, anyway, while cmd/jujuc exists, worker/uniter/jujuc it will be08:13
rogpeppefwereade_: are you planning to move cmd/jujuc to worker/uniter/jujuc ?08:15
fwereade_rogpeppe, cmd/jujuc is the command, just about all the implementation is now in worker/uniter/jujuc; moved from cmd/jujuc/server08:15
rogpeppefwereade_: hmm. i'm not sure that "jujuc" is such a good name in that context. worker/uniter/server would be better. or worker/uniter/commands (or tools, though i'm not sure about borrowing that terminology)08:17
rogpeppeanyway, that's just a naming issue08:17
fwereade_rogpeppe, for now it is the implememntation side of cmd/jujuc, and I think that remains the best name08:18
fwereade_rogpeppe, when we fold jujuc into jujud that point will become open08:19
rogpeppefwereade_: ok08:20
rogpeppefwereade_: one last murmur: i think you're going to a lot of effort to avoid importing 500 lines of code into the uniter package. a concrete type presented to the commands could potentially be just as clear (as in not making State available) as an interface, i think.08:26
fwereade_rogpeppe, it still feels like necessary scaffolding to allow me to make the change semi-peacefully08:28
rogpeppefwereade_: moving to using an interface feels like it'll be more disruptive, but i'm sure i don't understand the issues well enough.08:30
TheMuefwereade_, rogpeppe: could you please take a short look at https://codereview.appspot.com/6589073/ ? i've got a strange error there when clicking on "View".08:47
fwereade_TheMue, I too see a chunk mismatch08:48
rogpeppeTheMue: i've seen that before. you can look at the raw diff instead: https://codereview.appspot.com/6589073/patch/6001/400308:48
fwereade_TheMue, I don't know how to resolve it I'm afraid08:48
rogpeppeTheMue: i'm not sure what causes it to happen, i'm afraid.08:48
TheMuees, the raw is fine, i've tested.08:48
TheMues/es/yes/08:48
TheMueok, thanks for your effort08:49
rogpeppefwereade_: i consistently get a uniter test failure in trunk: http://paste.ubuntu.com/1261625/10:23
fwereade_rogpeppe, whoa10:23
rogpeppefwereade_: and i think jujuc was failing too, let me find the failure in the copious log output...10:24
rogpeppefwereade_: yeah, uniter/jujuc too: http://paste.ubuntu.com/1261631/10:25
fwereade_rogpeppe, that one's because you haven't updated goyaml I think10:26
rogpeppefwereade_: ah, i didn't realise i needed to10:26
rogpeppefwereade_: yeah, that's the reason for that, thanks10:27
rogpeppefwereade_: that doesn't fix the other problem tho10:28
fwereade_rogpeppe, I'm investigating10:29
rogpeppefwereade_: does it pass for you?10:29
fwereade_rogpeppe, just got to running it, had to clean up a bit of mental state so it makes sense when I look back at it10:30
fwereade_rogpeppe, yeah, works for me, which is odd10:30
fwereade_rogpeppe, would you try logging the actual underlying error in unit.go:578 please?10:31
rogpeppefwereade_: will do10:33
rogpeppefwereade_: you mean uniter.go presumably?10:33
fwereade_rogpeppe, no, state/unit.go, surely?10:34
rogpeppefwereade_: oh, sorry, i wasn't looking at the log...10:34
fwereade_rogpeppe, np -- I think the copious logging is more of a blessing than a curse but I agree it can be a lot10:34
rogpeppefwereade_: it seems wrong that we're ignoring the error there anyway10:35
* fwereade_ agrees10:36
rogpeppefwereade_: [LOG] 10.99256 JUJU set private address error: duplicate key insert for unique index of capped collection10:39
rogpeppefwereade_: ha.10:39
rogpeppefwereade_: maybe i need to update my mgo package10:41
rogpeppefwereade_: that fixed it.10:43
rogpeppefwereade_: phew.10:43
rogpeppefwereade_: if you have a spare moment: https://codereview.appspot.com/662304710:47
TheMuerogpeppe: good hint, just upgraded it too10:47
rogpeppefwereade_, TheMue, davecheney: and another one: https://codereview.appspot.com/6612054/11:05
TheMue*click*11:10
TheMuerogpeppe: In the last one you have two helpers in agent_test.go exported. is there a special reason?11:11
rogpeppeTheMue: i just moved them from another file11:11
rogpeppeTheMue: they're exported to indicate they're intended to be used globally, not that it makes any difference in a test file.11:12
TheMuerogpeppe: yes, i've seen, but they could be private, don't they?11:12
TheMuerogpeppe: ah, ic11:12
rogpeppeTheMue: yes, and so could all our suite types, but they're not. i don't care too much.11:12
TheMuerogpeppe: ;)11:14
TheMuerogpeppe: btw, what's "arble"?11:14
rogpeppeTheMue: a word :-)11:14
rogpeppeTheMue: a nonsense word that was made up by a friend at uni AFAIR.11:14
TheMuerogpeppe: hehe, a foobar alternative11:15
rogpeppeTheMue: yeah11:15
TheMuerogpeppe: you've got a lgtm11:16
rogpeppeTheMue: thanks11:16
TheMuerogpeppe: yw11:16
rogpeppeTheMue: i win? woo! :-)11:16
rogpeppeTheMue: what's my prize?11:17
TheMuerogpeppe: yes, and the prize is to make the alpha bundle11:17
rogpeppeTheMue: booby!11:17
TheMuerogpeppe: hehe11:18
* TheMue fights with headaches today, and that w/o alcohol. maybe that's the reason.11:19
rogpeppeTheMue: i prescribe you a large 16 y.o. Talisker11:20
TheMuerogpeppe: aaaaah, you know my medicine, good11:20
TheMuerogpeppe: just asked my dealer for a 17 y.o. Balvenie DoubleWood, costs about 75 £11:22
TheMuerogpeppe: i hope it's not too expensive here, taxes are lower11:22
rogpeppefairly trivial CL: https://codereview.appspot.com/6622047/12:04
TheMuerogpeppe: is setting the pw to "" really removing it or setting it to ""?12:06
rogpeppeTheMue: for AdminPassword, it's really removing it, as mentioned in the docs12:07
rogpeppelunch12:07
TheMueok thx12:07
rogpeppemramm: i thought the meeting was at 1.3012:15
mrammrogpeppe: it is12:15
mrammI started the hangout too soon12:15
rogpeppemramm: ah, np12:15
davecheneydoes anyone have the hangout link ?12:27
davecheneyfound it12:28
niemeyerdavecheney: yo!12:28
niemeyerHi all12:28
fwereade_niemeyer, heyhey12:29
niemeyerfwereade_: Yo12:30
rogpeppefwereade_: hangout?12:30
davecheneyfwereade_: https://plus.google.com/hangouts/_/ad44942cb79ac76c808c48efaec6b9da87275d6c?authuser=0&hl=en12:30
davecheney!meeting12:35
davecheneydidnt' expect that to work12:36
niemeyer"hello dear, My Name is Dorise i see your email at golang.org i will like us to have a good friendship"12:52
niemeyerWTF?12:52
davecheneyniemeyer: hands off, she's promised to me12:53
davecheneyCONTRIBUTORS for the win12:53
niemeyer:-)12:53
davecheneyno12:53
davecheneyfor the spam12:53
niemeyerdavecheney: Ah, makes sense12:53
TheMue*ROFL*12:53
rogpeppedavecheney: sorry mate, i got there first12:59
davecheneyrogpeppe: D comes before R13:00
davecheneyshe emailed me first13:00
rogpeppe:)13:02
rogpeppeniemeyer: ping13:21
niemeyerrogpeppe: Hi13:21
rogpeppeniemeyer: i'm just wondering about the best place to do the admin secret hash logic13:21
rogpeppeniemeyer: here are a couple of possibilities: http://paste.ubuntu.com/1261889/13:22
niemeyerrogpeppe: Option 2 seems nicer overall13:24
rogpeppeniemeyer: ok, cool. i was trending towards that, but thought there were args both ways.13:25
rogpeppeniemeyer: it's kinda odd perhaps that each environment will need to implement the same hashing logic, but that's perhaps ok.13:25
rogpeppeniemeyer: actually we could implement PasswordHash function in environs13:26
rogpeppeniemeyer: that might work nicely actually; then neither juju nor ec2 need be dependent on the details of the hashing.13:26
niemeyerrogpeppe: That's what I imagined too13:27
rogpeppeniemeyer: cool13:27
rogpeppeniemeyer: will do13:27
niemeyerrogpeppe: Cheers!13:27
rogpeppeniemeyer: i'm piling up a few small CLs BTW if you have a moment some time.13:27
rogpeppeniemeyer: https://codereview.appspot.com/6623047, https://codereview.appspot.com/6612054/, https://codereview.appspot.com/6622047/13:28
niemeyerrogpeppe: I'll do reviews in a moment13:28
rogpeppeniemeyer: np13:28
=== TheMue_ is now known as TheMue
niemeyerfwereade_: Do you want to have that conversation now?13:40
fwereade_niemeyer, http://paste.ubuntu.com/1261919/13:40
fwereade_niemeyer, is a summary of my current thinking which hopefully draws on our shared context13:41
fwereade_niemeyer, can I just go for a ciggie while you read before we get settled in? :)13:41
niemeyerfwereade_: Certainly :)13:41
fwereade_niemeyer, cheers, bbs13:41
niemeyerI'll prepare some chimarrão at the same time13:42
fwereade_niemeyer_, back13:48
niemeyer_fwereade_: I'm here too, let's fire it off13:49
fwereade_niemeyer_, starting a hangout then13:50
niemeyer_fwereade_: Just did it13:50
fwereade_niemeyer_, joining13:50
=== niemeyer_ is now known as niemeyer
niemeyerTheMue: https://codereview.appspot.com/6589073/diff/6001/environs/jujutest/livetests.go14:09
TheMueniemeyer: yes, i already asked roger and william how this is possible. the raw diff is ok.14:10
niemeyerTheMue: Yep.. have you tried to propose it again?14:10
TheMueniemeyer: do you have any idea how to "repair" it?14:10
TheMueniemeyer: not yet, willdo. one moment.14:10
rogpeppeniemeyer: when you've finished with fwereade_, i've encountered another interesting wrinkle which needs a moment's thought.14:11
niemeyerrogpeppe: Just reviewing a branch and will be with you14:11
TheMueniemeyer: strange, now lbox hangs here. *grmblx*14:12
niemeyerTheMue: Hangs? Or is it working?14:12
TheMueniemeyer: oh, had a 50214:12
niemeyerTheMue: That's a Launchpad internal error14:12
TheMueniemeyer: i'm trying it again right now14:13
TheMueniemeyer: ah, it's there now14:14
fwereade_niemeyer, hey, just one thing: naming of the HookContext interface?14:17
fwereade_niemeyer, I would actually be pretty happy with a jujuc.Context interface14:18
rogpeppefwereade_: +114:18
fwereade_rogpeppe, cheers14:18
niemeyerfwereade_: +114:18
fwereade_niemeyer, cheers :)14:18
niemeyerTheMue: Sent a couple of comments14:23
niemeyerTheMue: and LGTM assuming that tests pass on Amazon14:23
niemeyerTheMue: Live tests, that is14:23
TheMueniemeyer: just seen the notification, thx14:23
niemeyerTheMue: I'd like to cover something about the next step14:23
niemeyerTheMue: Do you have a minute?14:23
TheMueniemeyer: yes14:24
niemeyerTheMue: Okay, so14:24
niemeyerTheMue: I believe that one of the reasons why the firewaller works well today is because we have a contract that when a machine comes up, it's coming up with all its ports closed14:24
niemeyerTheMue: In other words, StartInstance always hands off machines with no ports open14:25
niemeyerTheMue: This is of course being broken by the global scheme14:25
niemeyerTheMue: Which is alright on itself, except for one detail14:25
niemeyerTheMue: When a machine dies, what happens with its ports?14:25
niemeyerTheMue: Are you still there?14:27
TheMueniemeyer: Yes, thinking about it.14:27
niemeyerTheMue: The answer is nothing.14:28
niemeyerTheMue: And that's been fine so far14:29
niemeyerTheMue: Because the group is only used by that one machine14:29
TheMueniemeyer: We react in the sense of the lifecycle14:29
niemeyerTheMue: Meaning?14:30
TheMueniemeyer: if i get it right we close the ports of the unit.14:30
TheMueniemeyer: but with a global group that's not good14:31
TheMueniemeyer: it only should be closed if no unit needs that pot anymore14:31
niemeyerTheMue: When do we close the ports of the unit?14:32
TheMueniemeyer: on moment, checking if i have seen it right. thought it is in flushMashine()14:34
niemeyerTheMue: Which machine? It was removed14:34
niemeyerTheMue: We may never have seen it14:35
niemeyerTheMue: I mean, the removal14:35
niemeyerHow can we tell which ports on that global group are improperly opened14:35
TheMueniemeyer: That's what i meant with a lifecycle change. This would be catched. But not a real hard death.14:36
niemeyerTheMue: We cannot trust on luck14:36
TheMueniemeyer:yes, i know. it only has been a view on the status quo14:37
niemeyerTheMue: Anyway, I'll leave you thinking about that problem.. we need a way to close those ports14:37
niemeyerrogpeppe: SOrry, I have a call with flacoste right now.. I'll be with you after that14:37
rogpeppeniemeyer: np14:37
TheMueniemeyer: ok, so i know my next task ;)14:38
fwereade_niemeyer, when you're off your call, https://codereview.appspot.com/6620054 isn't quite what we originally discussed but should still be pretty trivial14:42
niemeyerfwereade_: Sounds good, I'll just be with rogpeppe for a while and will then jump on that15:15
rogpeppeniemeyer: there's a problem with using a salted password15:15
fwereade_niemeyer, cool, thanks15:15
rogpeppeniemeyer: which is that we don't know the salt when we reconnect15:15
niemeyerrogpeppe: Where is the salt put15:15
rogpeppeniemeyer: we need to store the salt in the environment15:16
rogpeppeniemeyer: we can return it in the state.Info15:16
rogpeppeniemeyer: alternatively15:16
rogpeppeniemeyer: we could not use a salted password at all and just say "use a long random password"15:16
niemeyerrogpeppe: Just use a well known salt for now15:17
rogpeppeniemeyer: what's the point?15:17
rogpeppeniemeyer: well, i guess we can add salt later :-)15:17
rogpeppeniemeyer: the changes are fairly small actually15:18
niemeyerrogpeppe: The point is the same of having a salt15:18
niemeyerrogpeppe: You can't attack a juju environment with a pre-made dictionary unless that dictionary was built specifically to attack a juju environment15:18
rogpeppeniemeyer: there's no point in having a constant salt - it's equivalent to having no salt at all15:18
rogpeppeniemeyer: i guess so15:19
niemeyerrogpeppe: No, it's not equivalent15:19
rogpeppeniemeyer: yeah, i see your point15:19
niemeyerrogpeppe: If you use a salt, you force people to start building that dictionary today15:19
rogpeppeniemeyer: ok, constant salt it is15:19
niemeyerrogpeppe: If you use an algorithm that takes 1 second to compute the hash on a modern computer, even a short password will take many years to break15:20
rogpeppeniemeyer: the password hash function i've just used takes about 0.2 seconds per password FWIW.15:20
niemeyerrogpeppe: Increase the count15:20
rogpeppeniemeyer: ok, will do. it'll increase the test time though :-)15:21
niemeyerrogpeppe: Not if you make that a parameter15:21
rogpeppeniemeyer: i'm not sure it should be a parameter. it could be a global variable changed by the test code though.15:21
rogpeppeniemeyer: part of the point of putting it in a function is to hide the parameters, i think.15:22
niemeyerrogpeppe: Or just keep it 0.215:22
rogpeppeniemeyer: i think 0.2 should be fine tbh15:22
niemeyerrogpeppe: I think so as well.. we're overvaluing that problem.. we have a lot to do before that kind of thing becomes a real issue15:23
rogpeppeniemeyer: and we can add a random salt quite easily at a later stage, and people can change their passwords.15:23
rogpeppeniemeyer: yup15:23
niemeyerrogpeppe: All of yours reviewed15:57
rogpeppeniemeyer: brilliant, thanks15:57
niemeyerfwereade_: I'll do yours first thing after lunch15:57
rogpeppeniemeyer: i've just made the change you suggested to https://codereview.appspot.com/6623047/15:57
* niemeyer => food!15:57
niemeyerrogpeppe: Cheers15:57
fwereade_niemeyer, cheers, enjoy :)15:57
* niemeyer is back16:29
rogpeppeniemeyer: a couple of comments on your reviews.16:45
rogpeppeniemeyer: BTW i think it's better to always require that the entity name in the cloudinit's state-info matches the machine's entity name.16:46
niemeyerrogpeppe: Thanks, reviewing fwereade_'s at the moment16:46
rogpeppeniemeyer: np16:46
rogpeppeniemeyer: thanks for the reviews16:46
niemeyerrogpeppe: Doesn't seem reasonable to have a "machine-0" entity name with an admin password16:47
rogpeppeniemeyer: it's the machine password too16:47
niemeyerrogpeppe: As far as the caller is concerned, it's the admin password16:47
rogpeppeniemeyer: ok, sounds reasonable16:47
niemeyerrogpeppe: We just play cheap16:48
rogpeppeniemeyer: ?16:48
niemeyerrogpeppe: Nevermind.. you got it16:48
niemeyerfwereade_: Sent suggestions on the interface.. please let me know how you feel about them17:00
niemeyerrogpeppe: "i don't think the base64 padding will make any difference"17:05
niemeyerrogpeppe: It's just silly to have "==" behind every parssword. Let's avoid that, please.17:05
rogpeppeniemeyer: ok17:06
rogpeppeniemeyer: sorry, i thought it was a security concern17:06
niemeyerrogpeppe: Not a security concern.. just not nice17:07
rogpeppeniemeyer: i'll check the padding17:07
niemeyerrogpeppe: By tweaking the input to a proper size, you can avoid it with no pain or work17:07
rogpeppeniemeyer: i agree17:08
niemeyerrogpeppe: Either way, replied in the CL17:08
rogpeppeniemeyer: thanks17:08
niemeyerrogpeppe: np17:09
fwereade_niemeyer, all look great except maybe Settings17:12
fwereade_niemeyer, I'm afraid fixes/progress will trickle in over the w/e, I think I'm done for the night17:12
rogpeppefwereade_: have a great w/e!17:13
fwereade_rogpeppe, and you :)17:13
niemeyerfwereade_: Indeed, have a great one17:13
niemeyerfwereade_: Settings is the interface of what we'll have as state.Settings17:13
niemeyerfwereade_: No Russian-name programming17:14
niemeyer:)17:14
niemeyerRussian novel programming17:14
niemeyerhttp://www.johndcook.com/blog/2012/09/27/russian-novel-programming/17:14
rogpeppeniemeyer: argh. the EntityName check breaks everything17:19
niemeyerrogpeppe: Because the implementation is not finished, I suppose17:19
rogpeppeniemeyer: because a lot of tests do this, for instance:17:19
rogpeppeinst, err := t.Env.StartInstance(0, InvalidStateInfo, nil)17:19
rogpeppeniemeyer: and now we need to craft a different StateInfo each time.17:20
rogpeppeniemeyer: or perhaps have just two17:20
rogpeppeniemeyer: oh, no17:20
rogpeppeniemeyer: different each time17:20
niemeyerrogpeppe: Why?17:20
rogpeppeniemeyer: because we're starting a different machine each time17:21
rogpeppeniemeyer: so we need a different entity name17:21
rogpeppeniemeyer: that may well be the right thing to do, but it's quite a few changes17:21
niemeyerrogpeppe: I don't think I understand the issue.. grepping for it17:21
rogpeppeniemeyer: yeah sure. it's just more than the trivial change i thought it was going to be.17:21
niemeyerrogpeppe: Why are we giving the instances an invalid state info?17:23
rogpeppeniemeyer: this is in tests17:24
niemeyerrogpeppe: The question too :)17:24
rogpeppeniemeyer: where there's no state to connect to17:24
niemeyerrogpeppe: I'm seeing this in LiveTests17:24
rogpeppeniemeyer: for some tests we only care about the instances being created and destroyed.17:24
rogpeppeniemeyer: we don't care what actually happens on the machine.17:25
niemeyerrogpeppe: Still feels awkward, but okay.. that's not the time to fix it..17:25
rogpeppeniemeyer: and ISTR that those tests were written before we had a dummy state server17:25
niemeyerrogpeppe: Perhaps that's the reasoning17:26
rogpeppeniemeyer: they were some of the earliest juju code actually17:26
niemeyerrogpeppe: Either way, func InvalidStateInfo(machineId int) *StateInfo17:26
rogpeppeniemeyer:17:26
rogpeppe// InvalidStateInfo holds information about no state - it will always give17:26
rogpeppe// an error when connected to. The machine id gives the17:26
rogpeppe// machine id of the machine to be started17:26
rogpeppefunc InvalidStateInfo(machineId int) *state.Info {17:26
rogpeppesnap!17:26
niemeyerrogpeppe: Great minds think alike, as Jamu would say :-)17:27
niemeyerrogpeppe: LGTM on bootstrap-state,  thanks17:27
rogpeppeniemeyer: cool, thanks17:27
niemeyerrogpeppe: LGTM on initial password too, thank you17:39
rogpeppeniemeyer: cool, thanks17:39
niemeyerrogpeppe: it'd be good to run all tests, if you haven't done it.. quite a few touchy changes17:39
rogpeppeniemeyer: problem with all these small branches is keeping track of 'em all...17:39
rogpeppeniemeyer: i'm doing it currently.17:40
niemeyerrogpeppe: Thanks for that17:40
rogpeppeniemeyer: perhaps i should be running them on all branches in parallel17:40
rogpeppeniemeyer: i made the password changes: https://codereview.appspot.com/661504717:48
niemeyerrogpeppe: Looking17:49
niemeyerrogpeppe: Why the big salt?17:49
rogpeppeniemeyer: why not?17:49
niemeyerrogpeppe: Because two bytes should be enough17:49
rogpeppeniemeyer: ok, i'll use a smaller slat17:50
rogpeppesalt17:50
niemeyerrogpeppe: Suggest just a trivial check on the content of the password.. like its length17:51
niemeyerrogpeppe: LGTM otherwise, thanks for the changes17:51
rogpeppeniemeyer: i was right to run the live tests... i had broken something. (i don't *think* in trunk)17:52
rogpeppeniemeyer: i've got to go now, unfortunately.17:55
rogpeppeniemeyer: have a great weekend.17:55
niemeyerrogpeppe: Hmm.. I hope that wasn't a "I've broke trunk and I have to go" :)17:55
niemeyerrogpeppe: have a great weekend :)17:55
rogpeppeniemeyer: i don't think so18:38
rogpeppeniemeyer: the thing that broke was the tests that were added in the most recent (unsubmitted) branch for entity name in cloudinit18:39
=== bcsaller1 is now known as bcsaller
fssniemeyer: ping21:26
niemeyer   fss Yo21:32
fssniemeyer: did you see this cl: https://codereview.appspot.com/6586073/?21:43
niemeyerfss: I hadn't seen it, but this is looking very good in fact21:45
niemeyerfss: Have you signed the contribution agreement yet?21:46
niemeyerfss: Sorry, I don't know if I asked that before21:46
fssniemeyer: I don't think so, where I can check this?21:46
fsscan I*21:46
niemeyerfss: www.canonical.com/contribute21:46
niemeyerfss: It's  straightforward/non-draconian agreement21:47
fss40421:47
fss=/21:47
fssI think I found it21:48
fss /contributors21:48
niemeyerfss: Sorry21:48
fssShoul I put your name in Canonical Project Manager?21:50
niemeyerfss: Yes please21:52
niemeyerfss: I'm providing a review.. pretty simple stuff really. It's mostly ready for integration.21:53
fssdone, I've signed the cla21:53
fssI've already finished the package, also created an iamtest package, I will send it to you in small parts (and adapt what is already done to your feedback)21:54
fssI'm going home now, but I'll be online in 30 minutes21:55
fssbrb21:55
fssniemeyer: I'm back22:50
niemeyerfss: Welcome back.. was just looking at the changes22:51
fssI forgot to add the integration tests suite, I'll send it tomorrow or later today22:51
niemeyerfss: Thanks.. it's fine to do that in a new CL22:52
niemeyerfss: I'm submitting this one22:52
fsscool :-)22:53
niemeyerfss: Hmm.. you'll probably want to name your future branches other than "goamz" :)22:54
fssi was about to ask about that22:54
fsshow should I proceed with lbox after the cl gets merged22:55
niemeyerfss: The name is usually after what's being done22:55
niemeyerfss: add-iam or whatever22:55
fsshmm, so I create a new branch for each cl?22:55
niemeyerfss: The workflow is not enforced.. what I personally do is to grab the master again and pull from trunk22:55
niemeyerfss: I use cobzr22:55
niemeyerfss: That's right22:56
niemeyerfss: This allows you to do things in parallel when you want, and to not mess up changes22:56
niemeyerfss: Your first change is submitted22:56
niemeyerfss: Congratulations! :)22:56
fssniemeyer: \o/22:56
niemeyerfss: Wanna do your first review too?  Something that will look familiar:22:59
niemeyerErm.. if only Launchpad would help22:59
fssniemeyer: thanks for the orientation, I will start sending others methods and types tomorrow22:59
niemeyerfss: https://codereview.appspot.com/661105323:00
fssniemeyer: cool :)23:01
niemeyerfss: If you understand what's going on there, and are happy with it, just reply with LGTM there23:02
fssI was just thinking about the order of Region fields23:03

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!