=== bigjools-afk is now known as bigjools
wrtpfwereade: goat moaning07:37
fwereadewrtp, baaaaa07:37
fwereadeheya TheMue08:28
TheMuefwereade: Today St Peter does his work to not make Roger jealous. It's raining here.08:29
fwereadeTheMue, haha08:29
wrtpTheMue: yo!09:17
wrtpTheMue: it's dry here, at least09:17
wrtpTheMue: but still grey09:17
wrtpand cold09:17
TheMuewrtp: Temperature dropped by about 10° and rain has replaced sun.09:18
fwereadewrtp, good advice last night, I think it's much better: https://codereview.appspot.com/6163044/09:24
wrtpfwereade: cool. yeah, i think that does look cleaner, thanks.09:26
fwereadewrtp, btw, I think it *is* a getCommand not a command -- but do you have a better name in mind?09:58
fwereadewrtp, maybe commands should be commandGetters09:58
* wrtp has another look09:58
fwereadewrtp, alittle voice from my misspent youth is telling me I should use the word "factory", but I'm trying to pretend I can't hear it ;)09:59
wrtpfwereade: perhaps the word "new" should come into it somewhere10:00
fwereadewrtp, not so sure, I feel that "new" binds too weakly10:00
fwereadewrtp, "newCommands" sounds to me like it's doing something diferent10:01
wrtpfwereade: newc ? cmdMaker ? plain "new" ?10:01
wrtpcf ? :-)10:01
fwereadewrtp, "constructors"?10:01
wrtpfwereade: i don't mind the name "commands". it's just "getCommand" that reads oddly to me.10:02
wrtpfwereade: it sounds more like it's getting a command that already exists rather than constructing a new one10:03
fwereadewrtp, a method called "GetCommand" that chooses a "getCommand" helper and returns its result feels pretty natural to me10:03
fwereadewrtp, hmm, yeah10:03
wrtpfwereade: or that it's an actual command that gets something10:03
fwereadewrtp, maybe the method itself should be NewCommand(string name)10:03
wrtpfwereade: +110:04
fwereadewrtp, then "newCommands" and "newCommand" feel good10:04
fwereadewrtp, cool10:04
fwereadethanks :)10:04
wrtpfwereade: tbh in such a local context, a short variable name would read well, i think.10:04
wrtpfwereade: then you have to look elsewhere for context, and you'll see the type of commands etc10:05
fwereadewrtp, "c" it is then :)10:05
wrtpfwereade: or "f"? given it's a function not a command.10:05
fwereadewrtp, +110:05
* wrtp likes "NewCommand"10:06
wrtpfwereade: LGTM10:18
fwereadewrtp, cheers10:18
=== TheMue_ is now known as TheMue
fwereadewrtp, TheMue: should be utterly trivial: https://codereview.appspot.com/618404412:17
=== asavu_ is now known as asavu
fwereade_morning niemeyer15:57
niemeyerfwereade_: Hey man!15:57
niemeyerfwereade_: Good timing, I was "talking to you" :-)15:57
fwereade_niemeyer, dammit, is my astral projection wandering again?15:58
fwereade_niemeyer, what can I do for you?15:58
niemeyerfwereade_: Was just answering https://codereview.appspot.com/6124051/15:58
* fwereade_ reads...15:59
fwereade_niemeyer, fair enough :)15:59
fwereade_niemeyer, although I'm not sure how to communicate exit codes with that model16:00
niemeyerfwereade_: Hmm16:03
niemeyerfwereade_: I almost suggested before that the Main function in cmd should actually return an error instead, precisely for that reason16:04
fwereade_niemeyer, I'd kinda prefer to keep the exit codes meaningful really16:05
niemeyerfwereade_: Maybe that's what we should do.. there's a single situation we return an error different than 1, right?16:05
fwereade_niemeyer, Init errors return 216:06
fwereade_niemeyer, which could really be caused by anything under the general heading of parsing16:06
niemeyerfwereade_: Ok.. anyway.. we can easily get going by merely returning the code from Main16:06
fwereade_niemeyer, ok, sounds good16:07
niemeyerfwereade_: I've used both of these schemes in other applications before16:07
niemeyerfwereade_: Both the early dying and the return err16:07
niemeyerfwereade_: Returning errors generally ended up producing better code16:08
fwereade_niemeyer, except, hmm again: a code of 1 could come either from the "real" command, or from jujuc, and I think we only want to print jujuc help in the latter case16:08
niemeyerfwereade_: Let's please never print help unless the user asks for it16:09
fwereade_niemeyer, in practice I think that means either "never print help" or "complicate things an irritating amount to detect the cases in which the user asked for help and didn't get it from the real command"16:11
niemeyerfwereade_: I don't understand..16:11
niemeyerfwereade_: If the user asked for help and didn't get from the real command, that's a bug in the real command16:12
fwereade_niemeyer, I'm thinking of the case in which we couldn't talk to the agent for whatever reason16:12
niemeyerfwereade_: "error: couldn't talk to the agent"?16:12
fwereade_niemeyer, that was the original intent: that the various things that can go wrong at this end print an appropriate error *and* an explanation of what's going on; otherwise everything is handled through the stuff returned by the agent16:13
niemeyerfwereade_: I understand, but I don't think explaining how things work every single time is necessary, or even nice16:15
niemeyerfwereade_: an error message explaining what went wrong should be good enough16:15
niemeyerfwereade_: ssh => invalid password; vs. ssh => invalid password + ssh is an application that requires a password to be set blah blah blah16:16
fwereade_niemeyer: so I should change this everywhere?16:18
niemeyerfwereade_: Do we have other locations doing the same thing?16:18
niemeyerfwereade_: I recall making the same comment elsewhere before16:18
fwereade_niemeyer, parse errors print help; others don't16:18
niemeyerfwereade_: I'm personally less concerned about those, but I don't mind if you decide to change them to behave that way as wel16:19
fwereade_niemeyer, jujuc seemed to me to be a special case in that an error out of jujuc itself implies extreme brokenness16:19
fwereade_niemeyer, personally I like getting help when I type utter gibberish16:20
fwereade_niemeyer, but that's by the by16:21
niemeyerfwereade_: Sure, if you want to keep showing help on parse errors, I personally don't mind either. That said, I don't *miss* the help, even in those cases16:21
fwereade_niemeyer, assumed context for printing help when jujuc is run in a bad environment is "users should only actually see this when they try to run jujuc on its own"16:21
niemeyerfwereade_: I prefer a one-liner error saying "error: you mistyped --gibberush" than "blah blah blah blah blah blah blah.. and you mistyped --gibberush"16:22
niemeyerfwereade_: "error: jujuc must be called from its symlinks"?16:23
fwereade_niemeyer, hmm, ok, how about printing that iff filepath.Base(os.Args[0]) == "jujuc"16:24
fwereade_niemeyer, and otherwise not offering help (except as supplied by remote commands ofc)16:25
niemeyerfwereade_: Sounds good16:25
fwereade_niemeyer, cool, thanks16:25
niemeyerfwereade_: Thank you, and sorry for not being very clear about those ideas before16:29
wrtpniemeyer: hiya16:36
wrtpniemeyer: any chance of a review of https://codereview.appspot.com/6159044/ ?16:36
niemeyerwrtp: Ah, sorry.. I half-reviewed it already but forgot to finish yesterday16:37
wrtpniemeyer: np. i'm working on another s3 branch which i'd hoped would be independent but turns out it's not.16:37
niemeyerwrtp: LGTM16:38
wrtpniemeyer: cool, thanks.16:38
* TheMue will next propose the relation methods of topology. The code is even more compat than Py. Nice.16:39
niemeyerTheMue: Sweet16:40
TheMueniemeyer: Yes, indeed. But I still would like you to tell me a bit more about the relations concept next week. Inside topology it's simple, but I'm still feeling unsecure regarding the relation manager etc.16:41
niemeyerTheMue: SOunds good16:45
TheMueniemeyer: Thx16:45
jimbakerhazmat, were you able to take a look at lp:~jimbaker/juju/unit-rel-lifecycle-start-invoker ? (https://codereview.appspot.com/6131061/)16:51
hazmatjimbaker, checking it now16:52
jimbakerhazmat, thanks16:52
fwereade_gn all16:52
wrtpfwereade_: see ya tomorrowq16:53
wrtpniemeyer: just discovered a slight oddity in the s3 package16:53
wrtpniemeyer: is it right that the ETag has double-quotes around it?16:53
wrtpniemeyer: the tests check that, but it seems odd16:54
niemeyerwrtp: I don't really have memories about that anymore, but if it's there, it means I've probably seen somewhere at least16:59
wrtpniemeyer: yeah, it seems weird, but i'll add a bit of documentation for others to note.16:59
niemeyerwrtp: Worth testing17:00
wrtpniemeyer: yeah, it bowled out an error in my test server.17:00
TheMueHmm, today my net is pretty unstable.17:05
wrtpniemeyer: https://codereview.appspot.com/618504417:17
wrtpniemeyer: sorry about the gofmt noise, but i think it's easy to see when a file has only changed white space17:18
wrtpright, good moment to stop.17:24
wrtpsee y'all tomorrow17:24
niemeyerwrtp: Cool17:29
niemeyerwrtp: have a good time17:29
hazmatjimbaker, sorry got de-railed for scale test demo of juju on 10k nodes..17:42
jimbakerhazmat, no worries17:54
fwereade_niemeyer, if you have a moment, I'm not sure what the issue is with contexts18:29
niemeyerfwereade_: I'm not sure either, so happy to talk if you're not sleeping yet :)18:30
fwereade_niemeyer, there is server.Context and cmd.Context which are entirely distinct18:30
niemeyerfwereade_: The basic question is: how come we have two contexts, and one of the is completely ignored?18:30
niemeyerfwereade_: Yes, and that's confusing.. why there are two contexts and why is one of them getting into the function without being used?18:31
fwereade_niemeyer, the cmd.Context isn't used if we happen not to use stdout/stderr/working-dir18:31
niemeyerfwereade_: It's quite possible that it's totally fine, but the fact I'm confused is at least an indicator that it'll not be obvious why that exists in a bit18:31
fwereade_niemeyer, I think/hope that the source of the confusion is the fact that we have 2 different typs in different packages whih have the same name and end up being used close to one another18:32
niemeyerfwereade_: It's more than that.. we have two different types, in different types, used for the same thing, and getting to the context in two different ways, with one of them being entirely ignored at times18:33
niemeyers/in different types/in different packages/18:33
fwereade_niemeyer, how are they being used for the same thing?18:33
fwereade_niemeyer, one of them is about th ee18:33
niemeyerfwereade_: Maybe I'm just confused then18:33
* niemeyer looks at the code18:33
fwereade_niemeyer, ...the execution environment, and one of them is about the state we're working against18:33
fwereade_niemeyer, I haven't found it confusing myself *but* that's no doubt because I'm in an extremely privileged position there18:35
fwereade_niemeyer, the name "Context", now I think of it, is not much better than "DataManager"18:36
niemeyerfwereade_: DataManager is as good as Struct :-)18:36
fwereade_niemeyer, I had hoped the cmd.Context/server.Context distinction would be considered appropriately gotastic, but clearly that's not the case ;)18:36
niemeyerfwereade_: and the issue may come out of the naming indeed.. by definition, a thing executing in two contexts at the same time is unexpected18:37
fwereade_niemeyer, part of me wanted to call server.Context something like server.State, but then we hit the same issue with state.State18:37
niemeyerfwereade_: Their name is ok in isolation, but the moment you have two contexts at once, something feels wrong18:38
niemeyerfwereade_: The issue is partly in my head indeed, anyway18:39
niemeyerfwereade_: Don't worry about it.. the issue is less important than I though18:41
niemeyerfwereade_: We can find a better name later if necessary18:41
niemeyerfwereade_: I'll review the code again in light of your explanation, thanks18:41
fwereade_niemeyer, cool, thanks -- I was toying with explicit cmdCtx/serverCtx throughout, but that felt painful too18:41
fwereade_niemeyer, suggestions appreciated :)18:42
niemeyerfwereade_: I don't have any good suggestions either right now.. if I come up with something that you might like I'll post in the review18:42
niemeyerfwereade_: Thanks, and sorry for the noise18:42
fwereade_niemeyer, cheers18:42
fwereade_niemeyer, and no worries :)18:42
niemeyerfwereade_: Ok, got a more concrete suggestion that might take away the minor confusion19:05
niemeyerfwereade_:  Review is up, with an associated LGTM assuming you're happy with it too19:05

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