/srv/irclogs.ubuntu.com/2012/03/06/#juju-dev.txt

andrewsmedinaanyone can help me with lbox?02:35
hazmatandrewsmedina, what do you need?02:38
andrewsmedinahazmat: I need create a new proposal02:38
hazmatandrewsmedina, lbox propose -cr from within a branch should do it02:38
hazmatandrewsmedina, that will create a proposal (if one doesn't exist) or update an existing one02:38
andrewsmedinahazmat: but "lbox propose -cr" are updated a closed issue =/02:38
hazmatandrewsmedina, i don't parse that02:39
hazmatwhat closed issue?02:39
andrewsmedinahazmat: how lbox knows if a proposal exist02:39
hazmatandrewsmedina, it looks at the branch in lp and look for an attached merge proposal02:39
hazmatwith an expected syntax02:39
hazmatandrewsmedina, if you need to create an entirely new proposal, you should delete the existing merge proposal attached to the branch02:39
hazmatbut you'll lose existing comments02:40
andrewsmedinahazmat: I updated the goetveld to works with the last weekly version02:40
hazmattypically you just propose it again, and lbox will update the existing proposal to let reviewers know theirs something new02:40
hazmatandrewsmedina, yeah.. i don't play the go monkey dance version game02:40
andrewsmedinahazmat but lbox are appending it with another proposal02:41
andrewsmedinahazmat: what?02:41
hazmatandrewsmedina, ;-) just a general comment on 3rd party go libs.. pls ignore02:41
hazmatandrewsmedina, what does lbox appending it to another proposal mean?02:41
hazmatandrewsmedina, what are you trying to do?02:42
andrewsmedinahazmat: https://codereview.appspot.com/5683064/02:42
hazmatandrewsmedina, just run lbox propose -cr in the branch again02:42
andrewsmedinahazmat: a week ago I sent a proposal02:42
hazmatit will append to the existing merge proposal and notify reviewers that there are changes02:43
andrewsmedinahazmat and today and now I sent another proposal02:43
* hazmat nods02:43
hazmatoh.. its already there02:43
hazmat16m ago..02:43
andrewsmedinahazmat: lbox unified both, but are different proposals02:44
hazmatandrewsmedina, lbox uses the branch diff02:44
hazmatif you want two different proposals, you'd have two different branches02:44
hazmatbut in this case, your responding to feedback from the original proposal02:44
hazmatand you've made an update, so that's fine imo being attached to the existing proposal02:45
andrewsmedinahazmat: ty02:45
hazmatnp02:45
andrewsmedinahazmat: You dont sleep?02:51
hazmatandrewsmedina, nah.. i find it distracting ;-)02:57
andrewsmedinahazmat: :)03:01
fsouzais hazmat 24/7?03:42
niemeyerHello everybody14:08
rogpeppeniemeyer: hiya14:09
hazmatniemeyer, g'morning14:11
rogpeppeniemeyer: let me know when you're up for that chat14:13
niemeyerrogpeppe: Sure, let's go14:17
rogpeppeniemeyer: there's something about tomb.Fatal(tomb.Stop) that seems odd to me14:19
rogpeppeniemeyer: it's a common path, and we already have a way of expressing "no error" - just use nil.14:19
rogpeppeniemeyer: so i'm thinking tomb.Stop(nil)14:20
rogpeppeniemeyer: then tomb.Err() would return ErrStillRunning if the tomb hadn't died.14:20
rogpeppeniemeyer: which means the user wouldn't have to mess with particular error values at all in the usual case14:21
niemeyerrogpeppe: I've designed that interface while using it.. the Fatal(nil) is odd, and may easily hide a bug14:21
rogpeppeniemeyer: how's that?14:21
niemeyerthink Fatal(err) when err is nil14:21
rogpeppethat's fine14:21
rogpeppejust like return nl14:22
rogpeppenil14:22
rogpeppeanother thing: i'm not keen on the "Fatal" name. it's not fatal to anything - it just signals the tomb to die in the future. Stop feels better to me.14:23
niemeyerrogpeppe: Sorry, we already went over that one..14:24
niemeyerrogpeppe: It's fatal for the goroutine14:24
rogpeppeniemeyer: only if there's only one goroutine14:24
niemeyerrogpeppe: Besides, the analogy there is sane.. dying, fatal, etc14:24
rogpeppeniemeyer: it's explicitly documented that it may be called multiple times14:24
niemeyerrogpeppe: Yep14:24
rogpeppeniemeyer: the only other instances of Fatal do not return14:24
niemeyerrogpeppe: Yep14:24
rogpeppeniemeyer: but this one does14:25
niemeyerrogpeppe: Yep14:25
niemeyerrogpeppe: The analogy there is strong enough that I'm comfortable with that14:25
niemeyerrogpeppe: tomb, dying, fatal, etc14:25
rogpeppeDie might be better14:25
rogpeppeor even Kill14:25
niemeyerrogpeppe: Yeah, kill might work14:26
niemeyerrogpeppe: Yeah, I guess the ErrStillAlive might be a nice improvement14:28
rogpeppeniemeyer: thanks.14:28
rogpeppeniemeyer: i've got a branch that i could propose14:28
niemeyerrogpeppe: I'm still a bit sad about losing the error check in Fatal/Kill/Whatever, but you're right that this is no different than returning an err by mistake14:28
niemeyerrogpeppe: Sure, please do, thank you14:29
rogpeppeniemeyer: one other thing14:29
niemeyerand I guess I'll have to update the blog post again14:29
rogpeppe(sorry 'bout that)14:29
niemeyernp14:29
rogpeppeniemeyer: i wonder about how much Stopf (or whatever) will be used.14:29
rogpeppeniemeyer: i wonder about just providing Kill14:30
rogpeppesorry, Killf14:30
niemeyerrogpeppe: No, please preserve it..14:30
rogpeppeniemeyer: ok. i was going by the return analogy14:30
niemeyerrogpeppe: This isn't return.. this is Fatal/Fatalf14:30
rogpeppeniemeyer: you can always use fmt.Errorf (there's no need for tomb to depend on fmt)14:30
niemeyerrogpeppe: You can, just like you could always do errors.New(fmt.Sprintf(...)) all the time14:31
niemeyerrogpeppe: That's not what we do, though14:31
rogpeppeniemeyer: i'm thinking that almost all the time you'll be doing Kill(err) anyway14:31
niemeyerrogpeppe: Please..14:31
rogpeppeok14:31
rogpeppeniemeyer: oh, yes one other thing :-)14:32
rogpeppeniemeyer: it would be nice if we could embed tomb.Tomb in the same way we can embed sync.Mutex14:33
rogpeppeniemeyer: i.e. if the zero'd form was good to go14:33
niemeyerrogpeppe: Yeah, it would be nice indeed14:33
rogpeppeniemeyer: it's doable, but it means Dying and Dead have to be functions rather than fields.14:33
niemeyerrogpeppe: Yeah, not an option14:33
rogpeppeniemeyer: no? it works ok like that (and saves space too)14:34
niemeyerrogpeppe: It'll incur in a function call for absolutely no reason every single loop14:34
rogpeppeniemeyer: no it wouldn't.14:34
rogpeppeniemeyer: they'd be inlined14:34
niemeyerrogpeppe: Not really14:35
rogpeppeniemeyer: sure they would. it's a single statement function. it's inlined by default.14:35
niemeyerrogpeppe: If it's a single statement you're not doing anything within the function14:35
rogpeppeniemeyer: ah, good point. yeah, it needs a mutex too.14:36
niemeyerrogpeppe: and make(), and if, and ...14:36
rogpeppeniemeyer: yeah14:36
rogpeppe(i added Tomb.init function when i tried it)14:36
rogpeppei still think it might be worth it though.14:37
niemeyerrogpeppe: Maybe.. New bothers me in that case too14:37
rogpeppeNew?14:38
niemeyerrogpeppe: tomb.New14:38
rogpeppeniemeyer: why? i just deleted that function.14:38
niemeyerrogpeppe: Which means it bothered you as well, right? :-)14:38
rogpeppeniemeyer: ah, i see14:38
rogpeppeniemeyer: i thought there was maybe something more subtle14:39
niemeyerrogpeppe: Hmm.. I guess we can always have a variable when speed actually matters14:39
niemeyerrogpeppe: +1 on trying that14:39
rogpeppeniemeyer: ok, cool.14:39
rogpeppeniemeyer: actually...14:39
rogpeppeoh yeah,14:39
rogpeppeyou mean the client code can have a variable14:40
rogpeppe+114:40
TheMueniemeyer: when i created a watch with ExistsW() on a not yet existing path and the path later will be created i should expect an EVENT_CREATED, shouldn't i?14:41
niemeyerrogpeppe: Thanks for these suggestions... feel like good improvement14:41
rogpeppeniemeyer: thanks for going with me on this. if we're going to be using this a lot, i think it's good that we try to get it right.14:41
rogpeppeniemeyer: cool, as above14:42
niemeyerrogpeppe: Indeed14:42
niemeyerTheMue: Yep, sounds snae14:42
niemeyersane14:42
TheMueniemeyer: hmm, i'm trying here my agent watch test, e'thing fine until the point where i'm waiting for the event. funnily i get none.14:43
TheMueniemeyer: btw, i've got two proposals open. when can i expect your review?14:44
niemeyerTheMue: This week still14:46
niemeyerrogpeppe: When you have a moment, would you mind to have a look at the review that is pending?14:46
niemeyerrogpeppe: I want to close the store work today still14:46
rogpeppeniemeyer: will do14:46
niemeyerrogpeppe: It's a pretty small one14:46
TheMueniemeyer: ok, will continue work on the agent.14:47
niemeyerTheMue: Thanks.. I'll just finish that store work before moving back onto reviews, or we'll never have that14:47
TheMueniemeyer: yep, the store is important14:47
niemeyerrogpeppe: Cheers14:47
rogpeppeTheMue, niemeyer: here's a little working example of how watchers might work: http://paste.ubuntu.com/871544/14:48
rogpeppe(using the changed tomb API, BTW)14:48
niemeyerSorry.. connection dropped14:57
niemeyer<rogpeppe> (using the changed tomb API, BTW)14:57
niemeyer<niemeyer> rogpeppe: FooWatcher.Err should be using tomb.Err14:57
niemeyer<niemeyer> rogpeppe: Looks good, otherwise14:57
rogpeppeniemeyer: what's wrong with using tomb.Wait?15:01
niemeyerrogpeppe: It waits.. :-)15:02
rogpeppehmm, yeah, probably don't want Err to wait, you're right.15:03
rogpeppeniemeyer: the order of the defers in FooWatcher.run is crucial then...15:04
niemeyerrogpeppe: Why?15:06
niemeyerrogpeppe: I think it is reversed indeed, FWIW, but why do you think it is crucial?15:06
rogpeppeniemeyer: otherwise a watcher might see that Change is closed and call err, but before tomb.Done is called, so it'll see the tomb still running.15:08
rogpeppes/call err/call Err/15:08
niemeyerrogpeppe: Good point..15:08
niemeyerrogpeppe: It is good practice to always have defer tomb.Done() as the first thing in the goroutine, since it's supposed to flag "nothing else will happen"15:08
rogpeppeniemeyer: this is perhaps why it might be reasonable to make Err block...15:09
niemeyerrogpeppe: Sorry, I think I lost the reasoning why what we just said above is invalid15:10
rogpeppeniemeyer: i can't see Err being useful to get an error during the watcher run15:10
niemeyerrogpeppe: So remove Err..15:10
niemeyerrogpeppe: Err must not block.15:10
rogpeppeniemeyer: why's that?15:10
niemeyerrogpeppe: Because it's a question about what was the error, rather than a question "please block for as long as necessary"15:11
niemeyerrogpeppe: That's the convention15:11
rogpeppeniemeyer: but perhaps we don't know if there was or wasn't an error yet15:11
niemeyerrogpeppe: And ... ?15:11
rogpeppeniemeyer: using Err before Done has been called is inherently racy, i think15:12
rogpeppeniemeyer: i'm not sure we want to support that15:12
niemeyerrogpeppe: Indeed. Please remove Err.15:12
rogpeppeniemeyer: ok. from tomb as well?15:12
niemeyerrogpeppe: Nope.. tomb is fine as ti is15:12
rogpeppeniemeyer: tomb has the same issue15:13
niemeyerrogpeppe: Nope.. it's a fine question, with a fine error in case it is running. There's no race.15:13
rogpeppeniemeyer: ah, i'd preserved some of the old semantic - if there'd been a non-nil error, it returned it. but i think ErrStillRunning is better.15:14
niemeyerrogpeppe: That's what I understood you were going to propose in the branch15:14
niemeyerrogpeppe: Otherwise Kill(nil) is a no-go15:14
rogpeppeniemeyer: yeah, my thinking was fuzzy15:15
rogpeppeniemeyer: but there's still an issue (maybe a non-issue actually)15:16
rogpeppeniemeyer: that if the Err method is removed from FooWatcher, there's no way for the watcher to fail with an error that isn't discarded.15:16
rogpeppeniemeyer: so maybe just returning ErrStillRunning is fine there too.15:17
niemeyerrogpeppe: Sure, but I suggest waiting until we have the use case15:29
rogpeppeniemeyer: yeah, maybe a simple closed channel is enough. any errors could be logged.15:30
niemeyerrogpeppe: The error comes out of the Stop too15:35
rogpeppeniemeyer: that only works if you Stop it, of course, which you won't if you see EOF on the Change channel.15:36
niemeyerrogpeppe: Why not?15:36
niemeyerrogpeppe: Stop should always be called for all of them15:36
rogpeppeniemeyer: why would you? you know it's already stopped.15:36
rogpeppeniemeyer: ah.15:36
niemeyerrogpeppe: Because the logic is cleaner that way15:36
niemeyerrogpeppe: Which channel was closed? Etc15:36
rogpeppehmm, not sure about that as reasoning. you'll always know which channel is closed, no?15:37
niemeyerrogpeppe: Either way, a bit premature for that discussion.. let's have the branch implementing a watch for review first15:37
rogpeppeok15:37
rogpeppeniemeyer: https://codereview.appspot.com/575505515:51
rogpeppeniemeyer: just on your review now, BTW15:52
niemeyerrogpeppe: Cool, I'm proposing the second one with a trivial runner for the Server just now15:52
niemeyerAfter that, just need to implement /charm support, and I guess it's done15:52
fwereade__niemeyer, sudden thought: JUJU_CLIENT_ID might be better named JUJU_CONTEXT_ID; opinion?15:56
fwereade__niemeyer, given that it's at least in theory the key used to look up a particular context in which to execute hook commands15:56
niemeyerfwereade__: How's it set?16:00
niemeyerfwereade__: Sorry, what's its value16:00
fwereade__niemeyer, heh, well, in python it's always set to "client"16:00
niemeyerfwereade__: I think this is supposed to differentiate which process is running16:01
fwereade__niemeyer, sorry, expand please16:01
niemeyerfwereade__: How to find the relevant jujud process, more specifically16:02
niemeyerfwereade__: Or, I don't see how it is relevant at all..16:02
fwereade__niemeyer, hmm, not sure, JUJU_AGENT_SOCKET should handle that16:02
niemeyerfwereade__: Hmm.. indeed16:03
niemeyerfwereade__: Why is that used at all?  I suggest we don't add that variable until we can figure the answer that tricky question! ;-)16:03
fwereade__niemeyer, I think it's useful so that the hosted commands can be executed directly with the aid of a looked-up hook context that holds relevant data16:04
fwereade__niemeyer, this may be excessive given that ATM there's only one possible active hook context16:05
niemeyerfwereade__: Sorry, I don't get it.. how is that variable useful at all if it's always set to "client"?16:05
fwereade__niemeyer, it's not currently useful, because the python HookExecutor knows that it only ever has one active context16:06
fwereade__niemeyer, but that doesn't IMO actually improve simplicity or sanity16:07
niemeyerrogpeppe: You got a review16:07
fwereade__niemeyer, apart from anything else it means that anyone can execute a hook command at any time a context is active16:07
niemeyerfwereade__: How's that a problem?16:08
fwereade__niemeyer, I dunno, it just seems like a possible way for things to get screwed up16:08
rogpeppeniemeyer: thanks16:08
niemeyerfwereade__: FWIW, I think JUJU_CONTEXT_ID, and using it as you describe is fine.. I'm just suggesting we don't introduce the variable before it is actually being use16:09
niemeyerd16:09
niemeyerfwereade__: IOW, if you16:09
niemeyer're just going to reproduce the Python logic and have client as a constant, drop it16:09
niemeyerfwereade__: If you have plans to improve it and have it as an _actual_ context specifier, than sure, let's see how it loosk16:09
niemeyerlooks16:09
niemeyerI can't type16:09
fwereade__niemeyer, I have every intention of using it as a context specifier ;)16:10
fwereade__niemeyer, cheers16:10
niemeyerLunch time here!16:11
* niemeyer is hungry today..16:11
rogpeppeniemeyer: enjoy. you've got a review too.16:13
jcastrooh hey hazmat16:59
jcastromeetingology: is here16:59
meetingologyjcastro: Error: "is" is not a valid command.16:59
jcastrohazmat: do you know how to use it?16:59
hazmatjcastro, huhu?17:02
hazmatmeetingology help17:02
meetingologyhazmat: (help [<plugin>] [<command>]) -- This command gives a useful description of what <command> does. <plugin> is only necessary if the command is in more than one plugin.17:02
jcastrohazmat: for logging meetings and stuff if you want17:02
hazmatmeetingology help plugins17:02
meetingologyhazmat: Error: There is no command "plugins".17:02
jcastrohttps://wiki.ubuntu.com/meetingology17:03
hazmatjcastro, its been a while since i've used it but sounds good, we do most of our meetings off irc these days17:04
hazmatin g+17:04
jcastroright, I was doing the irc bot stuff and just put him in here just in case.17:04
hazmatjcastro, sounds good17:05
hazmatthanks17:05
jcastroanyway you've got your logging back and stuff17:05
jcastroso you should be all set.17:05
niemeyerrogpeppe: Re-reviewed17:07
niemeyerrogpeppe: Just a few trivials and I'll submit17:07
rogpeppeniemeyer: you caught my global search and replace!17:12
rogpeppeniemeyer: all done.17:17
rogpeppeniemeyer: not so sure about "csapi" as a command name. possible alternatives: i wonder if "charmserve" or "charmsrv" or even "charmd"17:21
rogpeppeoops17:21
rogpeppeniemeyer: not so sure about "csapi" as a command name. possible alternatives: charmserve, charmsrv, charmd, charmstored17:21
niemeyerrogpeppe: cs is from the charmstore17:26
niemeyerrogpeppe: But it's a bit more specific in that this will work with what we call as "cs:" only17:26
rogpeppeniemeyer: i know. but cs can stand for all kinds of things.17:26
niemeyerrogpeppe: Not for us17:26
niemeyerrogpeppe: Note that this is relevant only for IS for now17:27
rogpeppei guess i tend to prefer names that aren't entirely acronyms17:27
niemeyerrogpeppe: charmd sounds good17:27
niemeyerrogpeppe: charmload and charmd.. ok17:27
rogpeppesounds good to me17:27
niemeyerrogpeppe: I'll change that17:28
rogpeppeniemeyer: cool, thanks17:28
rogpeppeniemeyer: other LGTM17:28
rogpeppeotherwise17:28
niemeyerrogpeppe: Haven't pushed the changes yet, but responded to your review on https://codereview.appspot.com/5755047/17:29
niemeyerrogpeppe: Please see if it makes sense to you or if you need anything else17:29
rogpeppeniemeyer: if you're using ServeMux, then you'll want to check the URL anyway. i still think that http://charmserver.com/charm-info/fooey should give a 40417:30
niemeyerrogpeppe: Yes, I agree with that17:35
niemeyerrogpeppe: Will fix it17:35
niemeyerrogpeppe: But will keep the mux, as I don't want to do the basic multiplexing by hand17:35
rogpeppeniemeyer: that's fine.17:35
niemeyerrogpeppe: Really like how the tomb changes turned out, cheers18:30
rogwhat is it with my network currently?18:40
rogi think it might be to do with my network driver rather than the ISP connection.18:41
rog(this morning i was seeing many missed packets and >500ms ping times to my local router, where another adjacent computer was getting consistently getting <5ms18:42
rog)18:42
rogniemeyer: did you manage to sort out your network problems?18:42
rogfwereade_: i think it's not worth proposing more than one branch at once, unless the changes are independent. it's confusing trying to work out if the changes shown in codereview are to do with the proposed branch or not.18:44
rogoff for the night. see yous tomorrow.18:47
niemeyerrog: Nope..18:50
niemeyerrog: I've been using a cable meanwhile18:50
rogniemeyer: i'm moving to cable, as soon as i get the tool that enables me to wire an RJ45 jack, on order now.18:54
niemeyerrog: Are you finding issues too?18:54
niemeyerrog: Or is it just package losing?18:54
niemeyers/package/packet/18:55
=== marrusl_ is now known as marrusl

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