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

fwereadeniemeyer, that one should be a trivial, it's the State.Relation(...Endpoint) -> State.EndpointsRelation(...Endpoint) + state.Relation(int)00:00
fwereadeniemeyer, that IIRC we agreed on a few days ago00:00
niemeyerfwereade: +100:00
mrammniemeyer: cool00:03
davecheneyniemeyer: % juju bootstrap --upload-tools00:05
davecheneyerror: cannot start bootstrap instance: cannot make user data: invalid machine configuration: initial password is empty00:05
davecheneysubmitting branch now00:05
davecheneywell, for review00:05
fwereadeniemeyer, how pleasing, a palindromic CL id: https://codereview.appspot.com/660106600:06
niemeyerfwereade: Wow, beautiful :)00:07
niemeyerdavecheney: Hmm, ok.. curious about the cause00:07
davecheneyniemeyer: looking for it now00:08
niemeyerfwereade: Looking already00:08
davecheneyat least we know where the problem is now00:08
fwereadeniemeyer, great -- and https://codereview.appspot.com/6650043 shouldn't be too controversial either00:08
niemeyerfwereade: nicely straightforward00:08
niemeyerfwereade: Cool, looking00:09
davecheneyniemeyer:         password := e.Config().AdminSecret()00:09
davecheney        if password != "" {00:09
davecheney                password = trivial.PasswordHash(password)00:09
davecheney        }00:09
davecheneyec2/bootstrap00:09
niemeyerdavecheney: Hmm.. that looks okay..?00:10
davecheneyAdminSecret() must be returning ""00:10
* fwereade calmly resists the temptation to submit tonight, and really really goes off to bed, he can look forward to it in the morning :)00:10
niemeyerfwereade: Hmm.. one last question? :)00:12
* fwereade hasn't really gone00:12
* fwereade is sadly predictable :)00:12
niemeyerfwereade: ROTFL00:12
davecheneyniemeyer: ahh, found it00:12
davecheneyi was missing an admin-secret key for one of my environments00:13
davecheneybut the schema didn't complain00:13
niemeyerfwereade: Why is it not sending the plain slice it gets?00:13
niemeyerfwereade: It seems to be sending all relations it ever received at all times00:14
fwereadeniemeyer, it clears them out when it sends00:14
fwereadeniemeyer, f.relations = nil00:14
fwereadeniemeyer, then it collects a set of ids it's seen until someone receives again00:14
niemeyerfwereade: Ahh, I see00:15
fwereadeniemeyer, I didn;t want to just collect by appending because dupes would be icky00:15
niemeyerfwereade: I suggest keeping the form as a list of ids still00:15
niemeyerfwereade: But that's minor.. it looks pleasantly straightforward too00:16
davecheneyniemeyer: from ec2/config.go00:16
davecheney                "admin-secret":   schema.String(), // Unused. Here just for compatibility.00:16
niemeyerfwereade: Have a good sleep!00:16
davecheney^ clearly, this isn't true00:16
niemeyerfwereade: and thanks00:16
fwereadeniemeyer, all I think I'll ever want to do is iterate over it00:16
fwereadeniemeyer, and it feels a bit icky to (potentially) merge into a list without dupes every change00:16
niemeyerdavecheney: Indeed, but that shouldn't, I think, be affecting anything00:16
fwereadeniemeyer, what's the advantage of a list?00:17
davecheneyniemeyer:                 "admin-secret":  schema.Omit,00:17
niemeyerfwereade: I'm willing to bet this is orders of magnitude faster and consumes less space in the real cases00:17
davecheneymeans AccessSecret always retuns a value00:17
davecheneyand that value is ""00:17
fwereadeniemeyer, I guess this is indeed not really one of the N=100000 cases00:18
niemeyerfwereade: not only that, but real cases will generally see no appends at all, and allocating and handling a sequence of bytes is pretty friendly to the hardware00:18
fwereadeniemeyer, all true -- thanks :)00:19
niemeyerfwereade: np, and have a good night! :-)00:19
fwereadeniemeyer, consider it done... but tomorrow ;)00:19
fwereadeniemeyer, davecheney, gn00:19
davecheneyfwereade: gn00:19
niemeyerdavecheney: Isn't that the "default" value?00:19
davecheneyyup, found it in a few places00:19
davecheneywhich means a valid config can have a "" for the inital password00:20
niemeyerdavecheney: It can, but that's not related to this logic00:21
niemeyerdavecheney: and I thought you had said you didn't have an empty admin-secret?00:21
davecheneyniemeyer: that was my fault00:21
davecheneyi didn't read my environments.yaml closely enough00:22
davecheneythat key was missing from some envs00:22
niemeyerdavecheney: Ah, okay, so without the admin secret indeed it will fail I suppose00:22
davecheneyjust confirming now00:22
davecheneyok, yes, with an admin-secret it is working00:22
niemeyerdavecheney: We should prevent that, but ec2 is not the right place.. that should happen in environs/config/config.go00:22
niemeyerdavecheney: Woohay! That's great news.00:22
davecheneyniemeyer: already fixing00:22
davecheneyniemeyer: and i've added double underpants in cloudinit00:23
davecheneyniemeyer: before I get too far into this00:39
davecheneyis this statement correct -> admin-secret is now a required configuration item ?00:39
niemeyerdavecheney: I think so00:52
niemeyerdavecheney: Assuming it all works well00:52
davecheneyniemeyer: ok, i'll propose this and we can discuss it with rog tonight00:52
davecheneywell, once I get all the tests passing00:53
rogpeppedavecheney: morning!06:32
TheMuemorning06:34
davecheneymorning06:35
TheMuedavecheney: not yet time for dinner? ;)06:44
davecheneyTheMue: soon06:44
davecheneystill fighting with tests06:45
davecheneyhttps://codereview.appspot.com/665904806:45
TheMue_hmmmp, network06:46
=== TheMue_ is now known as TheMue
davecheneyrogpeppe: i've hit a roadblock with admin-secret06:50
davecheneyniemeyer is pretty sure it is now a required configuartion item06:50
rogpeppedavecheney: oh dear. tell me about it06:51
rogpeppedavecheney: fuck, yes it is06:51
rogpeppedavecheney: alternatively i *could* change things so it would work without it06:51
davecheneyrogpeppe: https://bugs.launchpad.net/juju-core/+bug/106575006:51
davecheney^ long version06:51
rogpeppedavecheney: is there some problem you're having even when you *do* specify admin-secret?06:52
davecheneynope, it works then06:52
davecheneybut without something liek  https://codereview.appspot.com/6659048 (unfinished)06:52
rogpeppedavecheney: cool, i'll just make it mandatory and all problems will go away :-)06:52
davecheneyyou can happily deploy a non working config06:52
davecheneyrogpeppe: yes, but if you make it manditory, then things like BootstrapConfig stop working06:53
rogpeppedavecheney: yeah, it was my intention to make it mandatory - it slide off my todo list, i'm sorry.06:53
rogpeppedavecheney: why's that?06:53
rogpeppes/slide/slid06:53
davecheney        // We never want to push admin-secret to the cloud.06:53
davecheney        delete(m, "admin-secret")06:53
davecheney        m["agent-version"] = tools.Number.String()06:53
davecheney        return config.New(m)06:53
davecheneyif m is lacking "admin-secret"06:53
davecheneyconfig.New() will fail06:53
rogpeppehmm06:54
rogpeppeso it's mandatory at one level, but not at another06:54
davecheneyunpossible06:54
rogpeppei'd forgotten that wrinkle06:54
rogpeppei think probably juju.NewConn (?) should fail if there's no admin secret,06:55
rogpeppedavecheney: but config should continue to work without it06:56
davecheneywell, my first effort was https://codereview.appspot.com/6659048/patch/2001/300306:56
davecheneybut that just spiraled out of control trying to make all the tests pass06:56
rogpeppedavecheney: yeah, that won't work06:56
rogpeppedavecheney: you've gotta stop the problem at source06:56
rogpeppedavecheney: i'm happy to fix this problem if you haven't done so already. it's all my fault.06:58
davecheneyrogpeppe: have a crack, i'm done for hte day06:59
davecheneythe diff is just getting longer and longer06:59
rogpeppedavecheney: ok, thanks for taking a look06:59
rogpeppedavecheney: can you push your latest attempt?06:59
davecheneyrogpeppe: i just did07:00
rogpeppedavecheney: cool, so https://codereview.appspot.com/6659048 is it?07:00
davecheneyy07:01
fwereade__rogpeppe, TheMue: I imagine, but would like to confirm, that none of you are implementing service or relation lifecycle stuff?07:32
rogpeppefwereade__: correct in my case at least07:33
rogpeppefwereade__: morning, BTW!07:33
fwereade__rogpeppe, cheers07:33
fwereade__rogpeppe, heyhey :)07:33
TheMuefwereade__: yes, i'm doing firewaller stuff right now07:33
fwereade__TheMue, thanks07:33
TheMuefwereade__, rogpeppe: morning07:33
fwereade__TheMue, (morning :))07:33
* fwereade__ has just spotted where he broke the uniter08:33
fwereade__rogpeppe, am I to understand that we can't deploy things atm for authy reasons?08:45
rogpeppefwereade__: no.08:45
rogpeppefwereade__: but you do need to add an admin-secret to your environ config08:45
fwereade__rogpeppe, ah, ok, I've always had one of them lying around anyway ;p08:45
rogpeppefwereade__: i'm just trying to work out where to put the checks to ensure that it's set08:46
rogpeppefwereade__: config.Config is unfortunately *not* the right place08:46
* fwereade__ has always maintained that an input config and an internal config are not the same thing, and that the hassle of pretending they are is not wrth it08:47
* rogpeppe tends to agree, but we have what we have now.08:47
* fwereade__ wishes rogpeppe good luck08:49
rogpeppefwereade__: part of the problem is that it's quite hard to enable passwords for local tests... but maybe it's ok now since my "restart mongo on unauthorised-access-failure" hack08:51
rogpeppeTheMue: hiya08:52
TheMuerogpeppe: ;)08:52
Arammoin.09:37
fwereade__Aram, heyhey09:49
fwereade__TheMue, I'm a bit confused, would you confirm for me what security groups I should be seeing with a bootstrap + deploy mongodb?09:50
TheMuefwereade__: today one named "juju-<groupname>" and one per machine named "juju-<groupname>-<machineid>".10:04
fwereade__TheMue, ok, cool -- but I am confused, because none of them seem to have any rules; and yet I can connect to state just fine: is it immediately apparent that I am being stupid?10:05
TheMuefwereade__: the juju-group afaik has all ports open between each other and 22 to the world. the other group should have those ports worldwide open which are needed.10:07
fwereade_rogpeppe, can I get a trivial LGTM on https://codereview.appspot.com/665605111:29
rogpeppefwereade_: looking11:30
TheMueyummy, had a good lunch, kraut11:30
rogpeppefwereade_: not *that* trivial :-)11:31
fwereade_rogpeppe, the code change really really is -- and, bother, I thought the test changes would be clear11:31
rogpeppefwereade_: the code change really is (although i'm not sure i see the implications - why didn't it crash before?); the test changes need a little thought though.11:32
fwereade_rogpeppe, it did, I think11:32
fwereade_rogpeppe, I believe this was what was hitting davecheny 2 nights ago11:32
rogpeppefwereade_: ah. have you got a test that failed before and now passes?11:32
fwereade_rogpeppe, yes; the expanded config-changed test11:33
fwereade_rogpeppe, in it, config-changed now records the output of config-get, and we verify that we see what we should11:33
rogpeppefwereade_: by "verify lack of hooks" do you mean that you're verifying lack of hook *executions* ?11:36
fwereade_rogpeppe, ha, yes11:36
rogpeppefwereade_: ok, that makes more sense11:36
rogpeppefwereade_: i'm still really enjoying your little test DSL BTW11:37
fwereade_rogpeppe, yeah, it makes me happy too :)11:37
rogpeppefwereade_: who needs LISP, eh? :-)11:37
fwereade_rogpeppe, haha :)11:37
rogpeppefwereade_: what does node.Update do?11:41
rogpeppefwereade_: scratch that11:41
rogpeppefwereade_: i just read the code, doh!11:41
fwereade_rogpeppe, ha, yeah, I find myself doing that :)11:42
rogpeppefwereade_: i mean i actually read the code for changeConfig.step properly and noticed the little "s" arg to Update...11:42
fwereade_rogpeppe, ha, yeah11:42
rogpeppefwereade_: so config-get never worked at all before?11:43
rogpeppefwereade_: anyway, LGTM11:43
rogpeppefwereade_: i'll leave it up to you to judge whether it's trivial enough to submit now11:44
fwereade_rogpeppe, it did until I did a half-baked (agreed) post-LGTM change the other day11:44
rogpeppefwereade_: ahhh11:45
fwereade_rogpeppe, since it fixes my own monstrous howler, I feel it is very important that it be sumbmitted asap :)11:45
fwereade_rogpeppe, and I'm preety sure the test changes are all positive11:45
rogpeppefwereade_: go for it11:45
fwereade_rogpeppe, cheers11:45
rogpeppefwereade_: a similarly almost-trivial CL: https://codereview.appspot.com/660106711:46
fwereade_rogpeppe, LGTM -- need to be off for lunch now though11:48
rogpeppelunch too12:10
AramI need some information about the firewaller and machiner.13:50
AramI need to understand better what to replace change.Remove with.13:51
AramDying, Dead, or (Dead OR Removed).13:51
Aramrogpeppe: fwereade_: TheMue ^ ^13:51
rogpeppeAram: for which watcher?13:52
Aramrogpeppe: Machine(Principal)?UnitsWatcher.13:52
AramI used Dead OR Remove, and tests pass.13:53
Arambut I am not sure if that's right or not.13:53
Aramactually I used Dead OR Remove OR Unassigned13:53
TheMueAram: i would prefer dead to keep it consistent with the lifecycle13:53
fwereade_Aram, AIUI that is what we need there, yes13:53
fwereade_Aram, but I had a vague feeling that we wanted to implement all the life-group watchers as sending lists of ids?13:54
rogpeppefwereade_: Dead OR Remove OR Unassigned seems about right13:54
Aramfwereade_: that's what I did, I reimplemented the units watcher to send list of ids.13:54
fwereade_rogpeppe, agree, I think, haven't 100% thought through unassigned13:54
fwereade_Aram, ah, excellent -- I thought the idea was that we should just be sending every life change, rather than filtering those at the watcher level?13:55
Aramfwereade_: I'm not sure I follow. you want the watcher to return map[Life][]string instead of []string?13:56
fwereade_Aram, not at all13:56
fwereade_Aram, above, I got the impression that you were not sending changes from alive to dying; was that wrong?13:57
Aramthis is the behavior now:13:58
Aram/ MachinePrincipalUnitsWatcher notifies about assignments and lifecycle13:58
Aram/ changes for all principal units assigned to a machine.13:58
Aram/13:58
Aram/ The first event emitted contains the unit names of all principal13:58
Aram/ units currently assigned to the machine, irrespective of their life13:58
Aram/ state. From then on, a new event is emitted whenever a unit is assigned13:58
Aram/ to or unassigned from the machine, or the lifecycle of a unit that is13:58
Aram/ currently assigned to the machine changes.13:58
Aram/13:58
Aram/ After a unit is found to be Dead, no further event will include it.13:58
Aram 13:58
Aramso the watcher fires both when life is changed, and when is a change in assignment, reglardless of lifecycle.13:58
rogpeppefwereade_: i'm having second thoughts about that earlier CL. here's an alternative: https://codereview.appspot.com/6601068/13:58
rogpeppefwereade_: i'm seriously considering adding a NoBootstrap field to JujuConnSuite, but fear it might be condemned as crackful.13:59
fwereade_rogpeppe, not especially keen myself, I think I liked the earlier one more14:01
* Aram loved $CDPATH14:01
Arams14:01
rogpeppefwereade_: the problem is i need to do exactly the same thing in CmdSuite14:01
rogpeppefwereade_: and it seems a bit silly to duplicate *almost* all of JujuConnSuite's functionality in two places, when we actually want something only slightly less than it does14:02
fwereade_rogpeppe, hmm, good point -- I think I'd prefer a NoBootstrap field to what you have there14:02
fwereade_rogpeppe, but I can't predict niemeyer's response14:02
rogpeppefwereade_: ok, that's useful thanks14:02
rogpeppefwereade_: maybe i should just bite the bullet and do the duplication14:03
fwereade_rogpeppe, I suspect that will give yu the smoothest path through review14:03
rogpeppefwereade_: done: https://codereview.appspot.com/666004814:42
Aramholy cow, bazaar really doesn't have the option of reverting a commit.14:55
Aramyou need to do some merge magic14:55
rogpeppeAram: bzr uncommit ?14:57
Aramrogpeppe: that only removes the last commits, and it really removes them, I don't want that, I want to undo an arbitrary change bu applying the reverse diff and commit the new undo operation.14:58
rogpeppeAram: yeah, you're out of luck there.14:58
rogpeppeAram: i've used patch for that before14:58
Aramyou can do a merge . -r -X..-X+115:00
rogpeppeAram: i didn't know merge accepted revno ranges15:01
Aramrogpeppe: yeah, it worked just fine15:06
Aramrogpeppe: fwereade_: TheMue: PTAL at the firewaller and machiner changes: https://codereview.appspot.com/662506915:45
Aramfirewaller and machiner tests pass with those changes and the new watcher.15:46
TheMueAram: in firewaller, when are units added after watcher changes?15:52
TheMueAram: ah, found, is ok15:53
TheMueAram: missed the line15:53
AramAmazon doesn't like the diacritical marks in my name.16:08
Aramwtf.16:08
AramI can't set a name if it's not ASCII16:08
Aram"We could not verify the address you have provided. If you are sure this is a correct address, please try again."16:12
Aramhow is amazon veryfying my address and why?16:12
Aramfuck you.16:12
Aramalready spent to much time on this.16:12
Aramthis should not have taken more than 30 seconds.16:12
rogpeppefwereade_, Aram: trivial (code movement only): https://codereview.appspot.com/6651062/16:19
rogpeppeAram: i'm sorry i'm not sure i'll have time to go through the watcher changes today. i'll try though.16:19
Aramrogpeppe: don't worry.16:24
* Aram is done for today.16:24
Aramcheers everyone, have a great weekend.16:24
rogpeppefwereade_: can i get a quick LGTM on the above branch please?16:30
rogpeppeniemeyer: ^16:31
rogpeppehttps://codereview.appspot.com/6651062/16:31
niemeyerrogpeppe: On holiday, and on a call16:31
rogpeppeniemeyer: np at all16:31
rogpeppeniemeyer: hope you're having a good day!16:31
niemeyerrogpeppe: Hello, btw :)16:31
rogpeppeniemeyer: likewise16:31
rogpeppeTheMue: ping16:32
TheMuerogpeppe: pong16:37
rogpeppeTheMue: any chance of a quick LGTM on the above CL?16:37
rogpeppeTheMue: code movement only, no code changes16:37
TheMuerogpeppe: already looking16:37
TheMuerogpeppe: you got it ;)16:38
rogpeppeTheMue: ta!16:38
rogpeppeTheMue, fwereade_: here's another one if you fancy having a look: https://codereview.appspot.com/6653050/16:43
TheMuerogpeppe: thumbs up16:48
rogpeppeTheMue: thanks16:48
TheMueso, i'm off. have a nice weekend17:00
rogpeppeme too, have a great weekend everyone!17:22
niemeyermramm: I have a good chuckle when I'm in a phone meeting with Mark S. and then there's that message saying "The LEADER has left the conference!" and all goes silent at the end..17:24
niemeyerrogpeppe: Have a good one17:24
* niemeyer steps out for more family activities..17:25
niemeyerHave a good weekend all17:26
mrammniemeyer: have a great weekend17:27
mrammniemeyer: I never saw it that way, but it is great.17:27
SpamapSwould be a great prank to have IS change that message to 'Elvis has left the building'17:59

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