=== andrewsmedina_ is now known as andrewsmedina [08:11] fwereade: mornin' [08:11] rog, heyhey [08:11] rog, about the pipeline requests, feel free to ignore anything with a prereq [08:11] fwereade: ok. [08:12] rog, or I guess I could try to remember to always -prep -req [08:12] fwereade: yeah, that's what i try to do [08:12] fwereade: (but you have to remember to manually move the branch state out of "work in progress" when you finally do propose) [08:13] rog, fwereade: morning [08:13] rog, oh! really? "-unprep" is not implicit? [08:13] heya TheMue [08:13] fwereade: no. it's a bug, and i've reported it... [08:13] TheMue: hiya [08:14] rog, ok, cool, I'll try to remember :) [08:15] fwereade: i've been thinking about your command context stuff [08:15] fwereade: i'm not sure about it [08:15] rog, hm, ok, go on [08:16] fwereade: the motivation is so that the hook commands can run the commands inside the agent, right? [08:16] rog, maybe, sort of [08:16] rog, let's say yes for now [08:16] fwereade: if it's not, then my concern is orthogonal [08:16] fwereade: anyway [08:17] fwereade: i think we should provide a "hook" API so that Go programs can use relation-get etc without invoking the command line commands [08:17] fwereade: then jujuc can layer onto that [08:18] fwereade: 5 second sketch: http://paste.ubuntu.com/872668/ [08:19] rog, I think I need a bit more context for why you think that's necessary or desirable [08:19] rog, IMO invoking a command-line command is not a big deal [08:19] fwereade: it would be useful (IMO) to be able to write hooks in Go [08:20] rog, it would be cool but it's pretty niche really [08:20] fwereade: i think it makes sense from a modularity perspective, just like the juju command [08:20] fwereade: (it's certainly niche now, but not necessarily in the future) [08:20] sorry, just like the juju package [08:21] we can think of the Hook type as the primary API. the command line is just an interface to it. [08:21] rog, what will be the increased costs of implementing it at the time it turns out to be needed? [08:22] fwereade: what are the increased costs of implementing it now? [08:22] fwereade: i think it makes things simpler if anything [08:22] rog, I haven't implemented a single hook command yet [08:23] rog, we know they need to be invoked via the command line [08:24] rog, we have this idea that it might be nice to write hooks in Go [08:25] rog, but os/exec doesn't feel like such a lame horse that we should devote dev effort to privileging go hooks over other languages by giving them their own API [08:25] fwereade: it would enable other languages to write their own API if they want. [08:25] fwereade: Go is privileged here because it's the implementation language. [08:27] rog, IMO the *lack* of privilege is an important feature [08:27] fwereade: if we provide exactly the same capabilities, there's no real privilege, just convenience (and type safety) [08:28] fwereade: IMO it's a better separation of concerns [08:28] fwereade: and you don't need the command context :-) [08:28] rog, but it's additional convenience for a language that is unlikely to be a normal user's first choice for hook implementation, and doing so carries a subtle implication that you "should" write hooks in go [08:29] fwereade: i really don't think so. it's just that we can easily provide a nice API and layer onto that with the command line stuff [08:29] rog, the context is a replacement for far too many layers of plumbing in python [08:30] fwereade: i think it's only needed because the innards of the code is printing to stdout and stderr. [08:30] fwereade: if there's an API, there's no need for that. [08:30] rog, it's not necessarily even the innards [08:31] rog, really it shouldn't/won't be the innards [08:33] rog, ok, I think there's something I'm missing: the intent of the Context is entirely to simplify what strikes me as an unnecessarily convoluted architecture in python [08:34] rog, if "native" hook command execution were a priority, we would have exposed it in python [08:34] fwereade: i'm not sure we need it though, if we're not doing remote execution of arbitrary commands. [08:35] fwereade: it's not a priority, but i think it would work well as a way of structuring the system. [08:36] sending command line commands across the pipe seems like it's making things more abstract than they need to be [08:37] rog, can we take a step back because I'm not sure of the extent of your proposal [08:37] fwereade: ok. [08:38] rog, are you suggesting that we should basically have an RPC server implementing N methods, and N executables each calling one of those methods? [08:39] not quite [08:39] g'morning [08:39] fwereade: i'm proposing that we have an RPC server implementing N methods, yes, but that the jujuc command calls one of those methods depending on the command line args [08:40] hazmat, heyhey [08:40] hazmat: yo! [08:40] greetings [08:40] hazmat: i guess it's not morning for you... [08:40] fwereade, did you have a a chance to look at the review on service constraints? [08:40] rog, the birds are chirping ;-) [08:40] hazmat, yeah, I rote you an essay [08:41] hazmat: tell 'em to shut up and go back to sleep :-) [08:41] hazmat, it may be filled with crack but I'd be interested to hear your views [08:41] fwereade, sounds good.. waiting for the intertubes to catch up.. [08:42] fwereade: the point is that all the awkward command-line stuff is inside jujuc, and the unit agent only deals with nice fluffy typed abstractions. [08:42] rog, IMO that is itself a significant increase in complexity -- N request types, N response types, N methods -- just to avoid one layer of abstraction that doesn't affect any other code [08:44] rog, what makes you think the unit agent won't have nice fluffy typed abstractions to work with? [08:44] rog, the command line needs to be dealt with *somewhere* [08:44] fwereade: sure. i'd just prefer to deal with it as close to the metal as possible. [08:46] fwereade, ah.. re (4) it was about the storage (dict) but about the choice of names (layer_data) which implied it was a layer in some lookup that's not germane to the entity, to the given entity that data are its serializable constraints. [08:46] hazmat, sorry, having trouble parsing that [08:46] er.. it wasn't about the storage, just the naming [08:47] hazmat, ha, ok; sorry I completely missed that [08:48] fwereade, no worries, thanks for keeping up with it [08:49] rog, ok, but a cmd.Context is all-the-complexity-we-need, right here, right now, and done; and it lets us add accessible commands to a hook context in one place only (the hook context itself) [08:49] rog, your proposal implies we need to change jujuc, and the server, every time we implement a new hook command [08:50] fwereade: one other consideration is that doing it this way constrains our future implementation options. all hook commands must run as RPCs to the unit agent, which isn't necessarily always going to be the case. [08:51] fwereade: yeah, that's true. jujuc would need to know what commands it's implementing [08:51] rog, if we do end up with hook commands that don't depend on their execution context, we can just expose those as standalone commands [08:52] rog, jujuc is solving the very specific problem of hook commands that need to know an unhealthy amount of stuff controlled by the unit agent [08:52] fwereade, if a service is deployed with no constraints, the service state will end up storing a copy of the environment constraints as the actualized constraints for the service? [08:53] hazmat, hmm, tbh I forget exactly when things are collapsed [08:53] fwereade: agreed [08:54] hazmat, but I'm pretty sure my intent was to defer collapse until we have an actual unit that needs them [08:54] fwereade: but that stuff might vary according to the command. [08:56] rog, indeed it could; but I don;t see where you're going with this [08:57] fwereade: i just have a strong feeling that a hook API would be a Good Thing. perhaps i'm on crack. [08:57] apart from anything, it would provide a nice documentation of the hook capabilities. [08:58] rog, I'm not saying it would automatically be a *bad* thing, but I think that it's (1) premature and (2) perfectly easy to add at a later stage assuming I implement the commands themselves with some sanity [09:01] fwereade: FWIW i think the code would be almost as simple with an API. there's no need to define N request types and N response types - existing types will do the job. [09:01] rog, that is to say that the true meat of the code will still be "call some hook context method" [09:02] rog, the Commands we expose will be responsible purely for parg parsing, and transforming the result for stdout [09:02] fwereade: yeah. the question is: does that code happen inside jujuc or inside the unit agent [09:02] ? [09:04] the difference is: (commandline -> RPC -> demux -> call) vs (commandline -> demux -> RPC -> call) [09:05] i'm contending that the latter is nicer, and gives us a Go hook API for free. [09:06] rog, I'm contending that the former is, because it allows us to keep all the knowledge about what commands we want to expose in one place [09:06] rog, and that a go hook API will still be cheap if and when we need it [09:08] fwereade: we've still got to make the symlinks from jujuc to the various commands, so that information's not *quite* in one place :-) [09:08] rog, but that's utterly trivial [09:08] rog, and we can do that for each execution context [09:09] rog, it's a simple transformation of a list of Commands; the very same list of Commands that we use in the server [09:09] hmm. would we prefer "command not found" or "command not appropriate for hook context"? [09:10] rog, IMO command not found, if for no other reason that it implies less wanton shitting in my command space [09:10] rog, as a user, I don;t really appreciate having relation-get on my path ;) [09:11] rog, alternate conception: relation-get is not a right, it's a privilege ;) [09:12] fwereade: the question is not whether you've got relation-get in your path, but whether relation-get is in the path for hook executions where it's not allowed [09:13] rog, it feels to me like fundamentally the same question; what's the point of exposing commands whose only purpose is to error out? [09:13] fwereade: i *think* i prefer it to be in the path - that way there's the possibility of getting it to log a debug message saying "inappropriate hook call" when a charm gets it wrong [09:13] fwereade: as above - because the error can be more informative that way. [09:14] fwereade: also we wouldn't have to muck around having a directory with a different set of symlinks for each possible context. [09:14] rog, "command not found" seems pretty informative to me [09:15] rog, intent is that the symlinks dir lives only as long as the hook does [09:15] fwereade: so we create the symlinks dir on every hook execution? [09:15] rog, the dir is trivial to create -- list of commands [09:15] fwereade: seems a bit unnecessary, but i suppose all this is very slow anyway [09:17] rog, it seems fine to me to spend a couple of ms per hook execution creating a directory that gives us exactly the interface exposed by the hook context -- again letting us keep all the interface info on the hook context itself, and growing seamlessly as we implement the commands one by one [09:19] fwereade: i think that updating jujuc at the same time as the juju agent would be just as simple as updating just the agent. [09:21] rog, isn't "if you change X, you have to change Y" rather the hallmark of inappropriate factoring? [09:22] fwereade: you'd have to change two files anyway. whether those two files end up in two separate commands seems fairly by the by. [09:22] rog, ok, we have to implement a Command in some way regardless, assume that's a sunk cost [09:23] agreed [09:23] rog, in my case, once we have a command, it's a matter of adding a line reading `&FooCommand{Context: ctx},` to the appropriate contexts [09:25] fwereade: and, presumable, the actual call that FooCommand makes to get or set the information? [09:25] presumably [09:26] rog, yes, FooCommand will indeed be talking to ctx [09:26] rog, I think that's a sunk cost as well...? [09:27] rog, the ctx needs to know what to do in either case [09:28] fwereade: the question is simply whether the FooCommand does a local or a remote call, i think. [09:29] fwereade: and, yes, if it's a remote call, there would be one additional layer of indirection, to hide the remote method invocation. [09:29] rog, quite so; and it's an indirection layer that we need to keep updating as the exposed commands change [09:30] fwereade: true. but i see that as a good thing. the API would mirror the capabilities of the hook. [09:31] rog, as it is the exposed environment already does that [09:31] fwereade: BTW, just out of interest, does this API reasonably represent the capabilities of the current hook execution environment? http://paste.ubuntu.com/872742/ [09:31] rog, and it does it dynamically rather than forcing us to manually mirror the same information in multiple places [09:32] fwereade: the exposed environment gives the names of the commands, but not the interface to those commands [09:32] rog, there's juju-log as well and possibly one or two others [09:34] rog, 8 in total [09:36] rog: relation-get, relation-set, relation-list, juju-log, config-get, unit-get, open-port, close-port [09:37] fwereade: like this, then: http://paste.ubuntu.com/872750/ [09:37] rog, yes, we'll end up with hook contexts implementing something very much like that regardless [09:37] rog, we can expose them in another way if it ever turns out to be necessary [09:38] rog, and in the meantime we don't need a layer of redundant code for dealing with remote calls with all the different types and results [09:42] rog, eg, what happens when we need to add a flag to something? we can: fix the Command and the HookContext method; or, fix the Command, fix the HookContext method, fix the RPC server and fix the RPC client [09:43] rog, not forgetting the Request type [09:43] fwereade: the Request type? [09:44] rog, assuming we use net/rpc, that is [09:44] fwereade: i was assuming that. but what do you mean by the request type? [09:45] rog, call(req, &resp) [09:45] fwereade: ah, yeah if you add options, you might need a new request type. [09:45] rog, if we add another flag that affects what we want to send... we need to send it [09:46] fwereade: i agree it's a little more work. but i still think it would be worth it. [09:47] rog, tbh I'm still not seeing the benefits [09:47] rog, apart from this speculative hook API that would still be easy to implement if we ever needed it [09:50] rog, (and I have my doubts about the hook API's utility *anyway* -- I really think that any hint of a "preferred" implementation for hooks is a pretty fundamentally bad thing) [09:50] rog, and I think that a golang hook API *would* be seen that way [09:51] fwereade: isn't there a python API to the hook context? [09:51] fwereade: (layered onto the command line) [09:52] rog, well, technically yes; but I don't think we've ever suggested that anyone use it [09:52] fwereade: i thought some people write hooks in python [09:53] rog, maybe they do, but I don;t think they have any business whatsoever importing code from juju itself and interacting directly with the unit agent [09:53] i wasn't suggesting that they did [09:53] rog, if they were they're be Doing It Wrong IMO [09:53] rog, ok... I missed your point then [09:54] rog, people can write hooks in python or go right now [09:54] fwereade: if i was writing a hook in python, i'd probably write an API so i could do relation_get() rather than calling exec and getting its piped output. [09:55] (obviously relation_get would do exec under the covers) [09:55] rog, ok, and that'd be a good thing for charm-tools or whatever [09:56] rog, but to me the exec-under-the-covers is the important thing [09:57] fwereade: so, i see the Go hook API in the same way as that, except because we're implementing it in Go, it can be a little more efficient, and synced more directly with the core [09:57] rog, because it means you're *actually* using the same interface everyone else is, as you should be [09:57] fwereade: the inefficiency is important? [09:57] rog, why would it be a bad thing to use the python API but not the go one? [09:58] fwereade: it wouldn't. just a little slower. [09:59] rog, hell yes it would be a bad thing to use the python API [09:59] rog, eithet your charm doesn't work in the go implementation, or we're stuck reimplementing the backend of the python api in go [10:00] fwereade: that's what we're doing... relation-get will be implemented in Go [10:01] rog, we have a perfectly good way of calling hook commands [10:01] rog, what benefit do we get from introducing another? [10:01] rog, especially when it makes development more inconvenient [10:02] rog, and the only people who'll feel the benefits will be those writing charms in go [10:02] that is indeed true [10:03] fwereade: i had this idea that all the user-scriptable functionality in juju would be nicely exposed in a Go package (the juju package, in fact) [10:03] maybe it's a silly idea [10:03] rog, I think the distinction is between user-scriptable and charmer-scriptable [10:04] i see charmers as users [10:04] (just a different category from someone that just types "juju deploy") [10:04] rog, fair enough, but we're still talking about two classes of users with very distinct needs [10:05] fwereade: they both benefit from a nice API, AFAICS [10:08] fwereade: if we *do* provide a Go hook API, the code in it would look remarkably similar to the code we'd write if we did a hook API to start with... except more inefficient. perhaps that's where my reluctance comes from. maybe it's the usual premature optimisation "but... i can see how it can be more efficient!" [10:08] rog, well, to be generally useful to either class we need an API that can be consumed from more languages than just go, surely? [10:09] fwereade: if we used json as the rpc transport, that would be quite straightforward actually. [10:11] rog, and that's all well and good, but it's still an additional api, and I don't think we can currently justify taking on the cost and hassle of maintaining two when we have a perfectly serviceable one already [10:11] rog, "exec" is kinda lowest-common-denominator but that's the point [10:12] * rog 's premature-optimisation head is screaming "but we can do better!" [10:12] but i'm ok to go your route [10:13] rog, by introducing another we're implicitly saying "you shouldn't be writing bash scripts, it's better to do it in go" ;) [10:13] rog, I hope I've at least half-convinced you rather than just talked you to death ;p [10:15] fwereade: i still have a strong intuition that keeping the command-line parsing in jujuc is somehow more "right". but i'm working hard to try to suppress it. [10:17] rog, my intuition that it's "wrong" largely comes from the feeling that the python code takes on a disturbing amount of complexity (apparently) to accommodate that very feeling [10:18] fwereade: which complexity are you talking about there, BTW? [10:19] rog, juju/hooks/protocol.py (and all the associated stuff) [10:19] @defer.inlineCallbacks [10:19] def open_port(self, client_id, port, proto): [10:19] """Open `port` for `proto` for this unit identified by `client_id`.""" [10:19] yield self.callRemote( [10:19] OpenPortCommand, client_id=client_id, port=port, proto=proto) [10:19] defer.returnValue(None) [10:19] etc etc [10:23] i don't mind that file too much. it's fairly naive (in a good way), and it documents the capabilities quite nicely. the Go source would be quite a bit shorter too, i think. [10:23] fwereade: i do see where you're coming from now though. i'm suggesting exactly what's in the current python implementation! [10:24] rog, yeah :) [10:25] fwereade: so, for the record, i still think that's the right approach. but i appreciate your arguments in favour of a "command line across the pipe" too. [10:25] rog, and it's not just that file -- if's relation-set which calls relation_set which constructs a RelationSetCli which calls the client which calls the server whcih calls the context [10:26] the "client" being the code in protocol.py, yes? [10:26] rog, the client/server both being in protocol.py, yes [10:27] rog, at some point the ravioli code got to me and I rejected the whole aproach ;) [10:27] rog, (lots of tiny little things with hardly any meat in them) [10:29] fwereade: yeah, i appreciate that feeling very well [10:30] fwereade: so in go, it would be command RelationSetCommand which invokes Hook.SetRelation which does rpc.Call("Hook.SetRelation") which invokes unitAgent.SetRelation. [10:32] the ravioli would be in Hook.SetRelation which wouldn't do much more than marshal its arguments and make the call. [10:33] rog, all the marshalling is busywork that forces us to touch at least extra two places in the code any time we make a change [10:34] fwereade: yes, i agree. but it's only three lines per call. and we've only got 8 calls, and no prospect of significant numbers of incomers. [10:35] rog, but it's still taking on repetitive extra code in exchange for speculated future benefits [10:36] fwereade: i think that having the API laid out in one place (the Hook type) is a benefit in itself. [10:37] rog, won't the HookContext interface have all that, without the marshalling busywork? [10:37] rog, can't we just add marshalling that talks directly to that when we turn out to need it? [10:37] rog, which I again consider to be a somewhat remote prospect [10:38] rog, IMO even if you want to write hooks in go you can damn well wrap os/exec like everybody else does :p [10:40] rog, well, I guess it'll be Context, RelationContext, DepartedContext interfaces, but details details [10:41] fwereade: hmm, not quite sure how that would work [10:41] rog, hm, no need for a separate Departed *interface*, just another implementation of it [10:41] rog, sorry brb [10:44] rog, b [10:44] fwereade: just thinking about how the unit agent's hook execution would work [10:45] rog, take a look at juju.state.hook [10:45] fwereade: from a Go perspective, that is. [10:46] rog, understood, but I think there'd be some similarities [10:46] fwereade: here's a sketch: http://paste.ubuntu.com/872817/ [10:47] so startHookServer interprets the command line and invokes the appropriate HookContext method (which might return an error if a function was called in an inappropriate context) [10:47] (if the wrong symlink was made, for example) [10:47] rog, I think I want to step further away from that idea actually [10:47] so, yeah, the hook context capabilities would be nicely exposed there [10:48] fwereade: which idea? [10:48] rog, hook contexts shouldn't be implementing the *commands* themselves; they should just be exposing capabilities made use of by the commands [10:49] rog, eg in python, context.get_value(unit_name, key) [10:49] fwereade: so you'd just pass a bitmask of available capabilities? [10:49] rog, no, I'd define an interface of available capabilities [10:50] fwereade: isn't that what the HookContext is? [10:50] rog, two in particular; one which knows about relations and one which doesn't [10:51] so you'd do a dynamic type check to see if the given interface is implemented? [10:51] rog, no...? [10:51] rog, take juju-log [10:53] rog, the juju-log Command would just have a `ctx PlainContext`; where the relation-set one would have a `ctx RelationContext` [10:53] rog, the departed/non-departed implementations of RelationContext would act diferently but there's no reason for the commands to see a difference [10:54] rog, where the RelationContext interface includes the PlainContext one [10:54] fwereade: but where does the ctx argument to juju-log come from? [10:54] fwereade: the rpc server can't know in advance that the juju-log command is going to be run [10:54] rog, JUJU_CONTEXT_ID [10:55] huh? [10:55] that's not my question. [10:55] i'm talking about within the unit agent [10:56] fwereade: referring back to my code, how would you write RunHook? [10:57] rog, pretty much like this: https://codereview.appspot.com/5753057/ [10:58] rog, so the ultra-minimal Context interface there is all that's necessary to exec a hook in a context [10:59] rog, you'd need actual context struct types to implement more fleshed-out interfaces to be useful to their commands [11:00] [10:37] rog, won't the HookContext interface have all that, without the marshalling busywork? [11:00] it seems not [11:00] rog, yep, that was horseshit [11:00] sorry :) [11:00] rog, on that point I fall back to the "we shouldn't be implementing an alternative interface until we know we need it" argument [11:02] rog, that said there's nothing stopping us from implementing those methods on HookContext [11:03] rog, I'm just not sure it'd be very neat in practice [11:06] fwereade: i think it would be quite neat actually. i think the command-line stuff is clunky, and it's nice to isolate it. [11:06] fwereade: here's what i'd do: http://paste.ubuntu.com/872832/ [11:07] then each context just passes a value that exactly implements its needs, and all the command line stuff is confined to startHookServer [11:09] rog, tbh it doesn't seem strange to me that an execution context should provide Commands [11:09] it's also potentially easier to test, because you can test the context independently of the command parsing [11:10] rog, that's already perfectly possible isn't it? [11:12] fwereade: to my mind the callbacks within the hook execution are fundamentally operations, not commands. [11:12] fwereade: they happen to implemented with commands (currently) but conceptually i think an interface containing the operations to be made available makes better sense to me [11:13] rog, in which case IMO they should still be `func RelationSet(ctx RelationContext, blah blah)` [11:13] fwereade: why not an interface containing the set of possible operations, as in my example? [11:14] rog, because it includes impossible operations [11:14] fwereade: that's fine. impossible operations return an error. [11:14] rog, and because DepartedContext and RelationContext will be disgustingly similar ;) [11:15] fwereade: really? what's the difference between them? [11:15] rog, this may come down to the "should we expose meaningless commands" question, and I still think "no" [11:15] fwereade: i think that operationally there's no difference [11:16] rog, RelationContext and DepartedContext make different information available [11:17] fwereade: we can always use embedding to remove duplication. [11:17] fwereade: different information available through which operation, BTW? [11:18] rog, departed relations only make the local unit's settings accessible [11:18] rog, and should IMO not even expose relation-set etc [11:18] fwereade: i don't have a problem if relation-set runs but returns an error. i don't see that as an issue. [11:19] rog, it's not exactly an advantage, though [11:20] fwereade: plus i think there *is* an advantage to avoiding the need to create and populate a directory of symlinks on every hook execution [11:21] fwereade: it means there's no need to write that code... [11:21] rog, that 30 lines? [11:22] yeah! 30 lines gone, hurray! [11:22] and one less moving part, also good. [11:22] rog, you did just dismiss 24 lines of marshalling, plus all the types and so forth, as insignificant :p [11:23] rog, and those aren't exactly non-moving parts either [11:23] fwereade: nah, worth it for the benefits i saw... [11:23] fwereade: but i don't see any particular benefit to creating the command set dynamically. [11:24] rog, one moving part in one place vs 8+ distributed here and there? [11:25] rog, the benefit is that it allows us to do the rest of our work without worrying about the plumbing [11:25] fwereade: ? [11:26] fwereade: i'm not sure what you mean by "worrying about the plumbing" [11:27] BTW of course i think that some moving parts are worth it... just not this one :-) [11:27] rog, I mean that the symlinks need to come from somewhere (or even the individual executables); we can do it manually for each command -- adding types, changing the server, changing jujuc, worrying about how we expose them at deploy time [11:28] rog, or we can have the execution context know how to create itself [11:28] rog, by just implementing another Command [11:28] rog, for, y'know, command-line parsing [11:29] rog, which can then interact with the actual context however appears to make sense (I think this in particular is orthogonal) [11:29] i'd thought that the symlinks would just be part of the package. the hook execution code would just add the directory to $PATH and that's it [11:29] the apt-get package, that is [11:29] rog, that's still extra packaging work [11:30] fwereade: can't the symlinks simply be in the VCS? [11:30] fwereade: i.e. we create them once and then commit them [11:31] (i admit i know nothing at all about the workings of apt-get) [11:31] rog, that's *still* an extra step along with the stuff I was just complaining about [11:32] fwereade: a step that one person has to make once, with no code involved. i don't see that as an issue. [11:32] someone has to install the jujuc command too... [11:33] rog, once per command, along with all the other once-per-command steps... [11:34] rog, and still the only benefit to all the extra work is that it *might* become easier to implement a speculative feature at some point in the future... [11:34] fwereade: no, not at all [11:34] fwereade: i've moved on from there [11:34] fwereade: i've accepted that the RPC interface is command-line-centric [11:34] rog, ok, sorry -- we're purely talking complexity distribution now then? [11:35] rog, consider the argument for making agents implement Command [11:35] fwereade: i'm arguing for the use of a Go interface to provide the callback interface rather than a set of Commands [11:36] rog, and I'm arguing that we need the Commands for parsing command lines, because we're exposing them to the execution context as command-line tools [11:36] fwereade: i.e. hide the fact that the hook callbacks are implemented with commands, because it's cleaner to do so [11:37] rog, and that whether or not Context has that interface, or a different one, is not material [11:37] rog, the Commands themselves can talk back to the context over whatever interface makes sense [11:38] rog, it may be yours, it may look more like the python ones [11:39] fwereade: i think the Commands can be hidden inside Exec. [11:39] rog, then what does all the command-specific command-line parsing? [11:39] fwereade: i think things would be nice if the agent code itself never had to know about Commands [11:39] rog, you're saying *Exec* should now know about all the different possible commands and how they should be handled? [11:40] fwereade: that's inside startHookServer (or whatever you might call it) which maps from the command line stuff to calls on the hook context. [11:40] fwereade: yeah, i'm saying that, yes. [11:41] fwereade: it would know about the HookContext interface (which encapsulates all possible hook operations) [11:42] rog, the knowledge of the commands is entirely within the context itself; I don't see why the agent should care [11:44] rog, seriously: we have a hook execution context that needs to provide env vars and executables; what is the possible benefit of pretending, at this level, that the executables are not really executables? [11:46] rog, the actual implementation of RelationSetCommand.Run will either be `c.ctx.RelationSet(c.foo, c.bar)` or `RelationSet(c.ctx, c.foo, c,bar)` [11:46] rog, plus some transformation of the result for stdout, where necessary [11:48] the env vars are for the hook context, not necessarily for the callback operations. [11:48] rog, ie it's doing exactly what's needed at this level and not forcing anything on how we implement the operations themselves [11:48] rog, yes, exactly so [11:49] rog, so are the executables for the hook context [11:49] rog, and they're responsible for parsing command lines, running the operations (which can be independent) and returning the results [11:49] i see that your hook package is very general. i think i'm just thinking that something with a little less abstraction would be easier to understand. [11:50] so a Context would be an interface containing the actual operations, rather than a set of arbitrary commands. [11:51] and the environment would be an argument to Exec rather than something returned from the context. [11:52] rog, a given Context will expose [1] .Commands() and .Env() for use by Exec and [2] RelationSet, or GetValue, or whatever, for use by the Commands themselves [11:52] [2] [11:53] rog, passing the env to exec doesn't alter the fact that it still comes from the context in the first place... [11:53] rog, I'm less and less sure that RelationSet should be on the hook context [11:54] rog, I think the capabilities exposed by the python hook contexts are at the right level [11:54] rog, a RelationSet operation that takes a RelationContext and uses its SetValue method seems pretty reasonable to me [11:55] rog, when seen in the context of the total number of ways we need to interact with a context anyway [11:55] rog, ie what's implemented in juju.state.hooks [11:57] fwereade: i see a context containing all the operations as a nice reflection of the way that the charm author will see things [11:57] fwereade: i don't see a need to have one type for each operation [11:58] rog, in practice a charm author will see the operations-that-don't-always-return-errors as the real interface she has available [11:58] rog, RelationSet isn't a type, it's a function [11:58] fwereade: sure. that's what they get in practice [11:59] rog, ...so if you're arguing from conceptual integrity, exposing only the operations an author cares about seems sensible to me [12:00] fwereade: Go works better when there's a known set of operations, even if some return errors. [12:01] fwereade: i like the idea of mapping the set of command line callback operations onto a single go interface that encapsulates all those calls. [12:03] rog, what was the last thing you saw me say? [12:03] rog, that I saw from you was: [12:03] fwereade: sure. that's what they get in practice [12:04] fwereade_: in practice, a charm won't tell the difference between a command that returns an error and a command that doesn't exist [12:04] fwereade_: hence my comment [12:05] fwereade_: even though we can actually implement all the calls all the time [12:06] fwereade_: BTW does the unit agent write to disk for any other reason? [12:06] rog, it does [12:06] ah, the socket [12:07] rog, also stuff like remembering state in case of unexpected process death etc [12:07] fwereade_: the zk reconnection id? [12:07] rog, nah -- what hooks have been queued and not executed, what relations we're currently part of, etc [12:08] interesting [12:08] rog, stuff like that is not appropriate for ZK -- nothing else needs to know about it [12:10] rog, basically just local state that needs to be persisted for one reason or another [12:10] rog, also stuff like upgrading the charm itself :p [12:10] fwereade_: is there an operation log or something? (i'm wondering how it deals with atomicity of writes) [12:11] rog, I'm not sure exactly what context we're talking about [12:12] fwereade_: so, if you've just queued a hook, you need to write to disk that you've queued it, right, in case you die there and then? [12:12] rog, that's the general idea yes [12:13] fwereade_: so i was wondering how it managed that data such that it doesn't grow without bound but it's ok whenever the process dies unexpectedly. [12:14] rog, whenever we've executed something we rewrite the queue without the executed thing [12:14] fwereade_: so if it dies half way through rewriting the queue? [12:14] rog, mv is atomic, right? [12:15] fwereade_: ah, so we create a new file each time we queue an operation. makes sense. [12:15] rog, if it dies after executing but before rewriting, no big deal, we execute that hook again [12:15] rog, the danger is executing a hook 0 times; sometimes executing one twice is not a major issue [12:16] rog, anyway, I suspect the does-the-agent-write-to-disk question fed back into the original discussion in some way [12:17] fwereade_: yeah, but only if it didn't write to disk :-) [12:17] rog, haha :) [12:28] fwereade_: in the end perhaps it comes down to this: i really like the look of this type (http://paste.ubuntu.com/872911/) and i like the directness of passing that as an argument rather than passing a context that can return a list of Commands that when Run can excecute the relevant operations. [12:33] fwereade_: but i understand your approach, i think, and it will work fine too [12:34] fwereade_: thanks for going round the garden path with me :-) [12:34] rog, a pleasure :) [12:43] HOLY SH... [12:44] i'm trying to find out why my watcher isn't firing. and what did i missed? starting the goroutine which listens to the watch. [12:44] so stupid, aaargh [12:45] TheMue: i did exactly that when writing the example code! [12:46] TheMue: (did you see it, BTW?) [12:48] rog: opened several pastes regarding this topic. which one you're referencing? [12:49] HA! great, now it fires. you only have to do it right. *smile* [12:49] TheMue: this one: http://paste.ubuntu.com/871544/ [12:50] rog: oh, no, will study it. tom tom.Tomb is a funny declaration. ;) [12:51] tomb tomb Tomb TOMB! [12:51] rog: and i have to update my Tomb, i have still Fata instead of Kill [12:52] Fatal [12:52] TheMue: there was one thing wrong in that example: FooWatcher.Err should return w.tomb.Err not w.tomb.Wait [12:53] rog: yep, sounds reasonable [12:54] other than that, i think it represents pretty well the structure we were talking about. [12:55] TheMue: oh yes, that example won't work without my tiny local/testzk package [12:56] TheMue: ... the implementation of which is here: http://paste.ubuntu.com/872945/ === Leseb_ is now known as Leseb [12:57] rog: mine is now working and needs only some last adjustments and more tests [12:57] TheMue: cool [12:57] TheMue: i wonder how similar it is... [12:59] rog: so far very similar. only that my watcher is only observing the creation and deletion of a node [13:00] rog: and i retrieve the agent key out of the path [13:00] TheMue: yeah. i chose the children example because it can be used to demonstrate delta events, but it could've been anything [13:01] rog: it's a good example, more complex than mine [13:02] TheMue: i hope you find it useful :-) [13:05] rog: for sure, gives more security === marrusl_ is now known as marrusl [14:19] Greeetings [14:23] greetings [14:23] niemeyer, its alive! [14:23] * hazmat points at the soon to be deployed store [14:23] hazmat: Almost there! :-) [14:24] fwereade_, rog: https://codereview.appspot.com/5758061 [14:24] Should be a trivial one given previous reviews [14:27] Hmm.. how do specify cross-PPA dependencies again.. [14:34] niemeyer: review delivered [14:34] rog: Cheers! [14:37] niemeyer: I sent two reviews, one for initial lxc port to Go [14:37] niemeyer: and another to goetveld [14:38] andrewsmedina: Thanks a lot.. I'll finish sorting out the store details and will get back on track in review land [14:38] niemeyer: ty [14:52] hmm, I guess I should eat something, bbiab [14:52] niemeyer: what can cause zk to close a connection after a Delete() of a node? [14:53] niemeyer: i create my agent node => watch fires correctly; now i delete it => next event is "zk connection closed" [15:02] TheMue: a connection closed event is unrelated to deletions [15:02] TheMue: You just happen to be seeing that event because that's when you asked about it, but that's not what the event is being caused by [15:04] Has it ever happen to anyone that some kind of white rectangle shows at the top of the screen, on the left-hand side? [15:05] It's a bug in some app and I can't spot which one it is because xwininfo shows the underlying window rather than the float [15:06] niemeyer: i know that it isn't related and hadn't it ever before. so i'm wondering this time. [15:06] TheMue: It's something else.. the delete is a red-herring [15:08] niemeyer: i know - but still have no clue [15:08] niemeyer: will try something to gather more infos [15:36] "That's what Russ said, but besides the point above, this is definitely not ok to ignore. There's both a read and a write side." [15:36] niemeyer: not quite sure what you mean by that [15:36] niemeyer: this is just the write side, yes? [15:36] rog: Copy reads and writes [15:37] niemeyer: ah, good point. that's fine then. [15:37] niemeyer: (i was thinking about the http client reading) [15:40] rog: Cool [15:40] * rog wishes it was possible to easily distinguish read and write errors when returned from io.Copy [15:45] "not entirely convinced, but fair enough." [15:45] rog: People log 200 results from web sites on a regular basis [15:46] rog: Logging errors sounds like an extremely natural thing to me, FWIW [15:46] niemeyer: ok. i guess i was just extrapolating from russ's remark. [15:46] rog: Russ pointed out that there's no reason to *return* an error [15:46] niemeyer: it sounds like they're not really errors, because they'll happen whenever the client is aborted [15:46] rog: And I agree with him on that [15:47] Knowing about errors in an app we care about is a different story, though [15:47] niemeyer: huh? it's ok to ignore write errors but only sometimes? [15:47] rog: Not sure how you got to that conclusion [15:48] [15:46] rog: Russ pointed out that there's no reason to *return* an error [15:48] that's ignoring the error AFAICS [15:48] rog: Are we ignoring the error in that handler, even though we're not returning it!? [15:48] niemeyer: yes [15:49] rog: No, we're not ignoring it.. we're logging the error, and we're reporting to the client that errors happened [15:49] sorry [15:49] yes, but if the write error happens in Flush or WriteHeader then we have to ignore it [15:49] because it's not returned for us to log [15:50] rog: Ok, nevermind.. are you happy with the current branch content or do you want something else there? [15:50] obviously the handler can't return the error - it has no return value [15:51] niemeyer: LGTM. you might want to consider StripPrefix to remove the /charm repetition and the need for a panic check, as i just mentioned in my reply. [15:51] niemeyer: but i don't mind either way. [15:51] rog: I still want a panic if it doesn't match.. [15:51] niemeyer: you won't see /charm in the URL if you use StripPrefix [15:51] niemeyer: so there's nothing to check. [15:52] niemeyer: (and nothing to strip off either) [15:52] rog: Exactly.. I want to check if the prefix being stripped is wrong, and panic so that when someone changes /charm to whatever else and forget to update that code, it blows up rather than failing silently [15:53] there's only one occurrence of /charm needed, AFAICS [15:53] so there'd be nothing to update [15:53] rog: Which prefix is being stripped? [15:54] niemeyer: the /charm prefix. [15:54] niemeyer: i'd write a tiny helper function that added the mux handler and put a strip prefix handler on it [15:55] rog: Sounds good.. I'll move on with what's there though, as it seems to be safe and work well as far as I can see. [15:55] niemeyer: yeah, that's fine. when you have more handlers you might want to move in that direction though. [15:56] niemeyer: if there are ever more handlers :-) [15:56] There will be.. by then I worry about it [16:00] Lunch time.. biab to sort out deployment details.. [16:01] robbiew: i:m ready when are [16:01] TheMue: ack...need 5min [16:02] robbiew: ok [16:03] rog: followed your concept of using new watches in each loop run. now it works fine. [16:06] TheMue: 5 more minutes...waiting on an update to complete :/ [16:07] robbiew: :D [16:13] TheMue: https://talkgadget.google.com/hangouts/extras/canonical.com/the-hulk-hideout [16:23] TheMue: not sure what you mean by that, but i'm glad you've got it working... [16:30] rog: in my first approach i tried to use only one watch. now i'm creaing a new one like you inside the loop each time. you do it with a GetW, i with an existsW [16:30] TheMue: ah - you hadn't appreciated that a watch fires only once... [16:31] TheMue: i was confused by that initially too [16:34] rog:exactly [16:38] jimbaker, could you have a final look over the maas provider branch, i think its good to go, i'm planning on landing it today, but wanted to make sure you had a chance to look over it first. [16:43] back again [16:48] hazmat, sounds good. i looked at last night and i think it's fine, but i'll do one more pass [16:49] hazmat, also i have a new version of the spec on the relation support changes ready for review; it's about half the size of the original [16:54] jimbaker, awesome [16:56] * niemeyer waves [17:00] bcsaller, any updates on subordinates, i saw the subordinate-relation-types is in for review, i'm curious what's next/left, is it just the status support branch? [17:01] hazmat: no no, working on the changes to unit lifecycle now [17:01] actually triggering the deploy and all that [17:01] hazmat: its starting to come together I think [17:01] bcsaller, ah.. right jim's deploy refactor wired into the unit rel watcher [17:01] bcsaller, cool [17:02] hazmat: yeah in unit/lifecycle, I had to merge it into this branch by hand as it wasn't on trunk yet [17:03] bcsaller, luckily its your review day, so you can help move it along ;-) [17:03] hazmat: looks likes its already approved, but point taken [17:03] yeah.. i'm going to be spending most of the day merging extant approved branches [17:05] i've been refactoring the scheduler to have some better behavior (at least once wrt to events, instead of at most once) [17:07] niemeyer, i believe we had some discussions on the last meeting wrt to this. I believe the conclusion was on failure for hook execution on an event we should keep the event around, and if resolved --retry rexecute the hook, or if not pop it off the queue. [17:07] s/on/in [17:08] hazmat: Yeah, unless there's some bad idea, doing the same we do for other cases sounds sane at least [17:08] niemeyer, indeed it does [17:09] bcsaller, hazmat, sounds good about getting refactor-machine-agent into trunk [17:09] hazmat: s/bad/better/, sorry [17:09] hazmat: But I guess you go tit [17:09] lol [17:09] :-) [17:09] must be a golang thing ;-) [17:10] hazmat: If it was go tit(), it might be.. but that's just a very funny typo.. :D [17:10] there's also a minor branch depending on that, robust-test-removed-service-unit [17:13] jimbaker, cool, it looks like it still needs a review though. [17:13] hazmat, btw, when do you go out to pycon? i arrive pretty late tomorrow night [17:13] hazmat, re refactor-machine-agent, yes, just comments so far [17:13] jimbaker, i arrive to sfo tomorrow at 6pm, i should be at the hotel by 8pm [17:14] hazmat, i should be at the hotel by 11p, so i guess i'll see you fri [17:14] unless i happen to catch you in the bar ;) [17:14] jimbaker, sounds plausible ;-) [18:33] sigh.. i'm starting to think it might just be simpler to rewrite the scheduler [18:34] if starting to feel like there's too many edge cases with the current data structures [18:34] if/its [18:40] * rog is off for the evening. until tomorrow. [18:40] although even with a simplified list we have versions and memberships to track, but i hope we can fold those into the list [18:40] * hazmat lunches [18:52] niemeyer, SpamapS so agent-state for machines sound good, for consistency that also means turning the unit's 'state' key to an agent-state key [18:53] hazmat: +1 [18:53] hazmat: Even though that's a little bit more subtle [18:53] hazmat: Since the "unit" is actually represented by the agent itself [18:53] niemeyer, yeah. its quite a bit richer wrt to semantics [18:54] hazmat: While the "machine" has an independent life [18:54] hazmat: I'm happy with the change, though..won't hurt to be clear [18:55] niemeyer, i'm happy either way re unit 'state' to 'agent-state'.. i can banish the hobgoblin of consistency.. even though there both agents, as you say the state reported there is the unit's not really the agents. [18:56] hazmat: What are the values we have for the unit's state? [18:56] hazmat: not-started and .>? [18:56] started? anything else? [18:57] hazmat: Also, I wonder about what happens if the agent dies.. is that state updated? :-) [18:57] State is hard.. [18:57] niemeyer, started, start_error, install_error, stop_error, stopped [18:58] hazmat: What about for the machine agent? [18:58] niemeyer, just running or pending [18:58] there is no state machine for the machine agent [18:58] hazmat: Hmm.. that's instance-state.. I mean the current "state" that we're talking about chnaging [19:17] The go tool+conventions and launchpad's daily builds were made for one another [19:18] hazmat: Just check that out: https://code.launchpad.net/~niemeyer/+recipe/charmstore [19:18] hazmat: You've hacked around with PPAs before, right? [19:18] niemeyer, very nice [19:19] hazmat: The whole build is "go install launchpad.net/go/store" [19:21] niemeyer, so re the unit's state.... the machine agent state is really just running, pending, or down. the machine instance state has pending or running. [19:22] hazmat: I got not-started, so this must be at least not reflecting the actual names [19:22] down is basically the same as pending, ie the agent isn't connected, we just infer that it must have been running if it has an assigned unit with a state. [19:22] niemeyer, yeah.. pending == 'not-started' [19:22] hazmat: Ok [19:23] actually that's not true [19:23] hazmat: Does running => down happen automatically based on ephemerals? [19:23] not-started means no machine id.. so provisioning agent hasn't acted, pending means its got an id, but no address [19:23] niemeyer, yes [19:24] hazmat: Ok, that's nice.. what about the unit's state.. does it shift automatically to down based on ephemerals as well? [19:24] hmm.. interesting.. we actually stick 'pending' into the instance-id field of the machine instead of the state.. hrm. [19:25] niemeyer, it does [19:25] for status display, a unit is 'down' if its agent is not connected [19:25] and the unit workflow state is shown if its connected [19:26] hazmat: The pending in the instance-id sounds reasonable I guess? [19:26] hazmat: Hmm.. or maybe not.. [19:27] hazmat: We're supposed to have the id from the moment we request.. so I'm not sure about why we'd show pending [19:27] hazmat: Or do you mean instance-state rather than instance-id? [19:28] niemeyer, we might not have processed the zk state into a provider request and instance-id [19:28] ie. the provisioning agent hasn't caught up yet [19:28] hazmat: Ah, bingo [19:30] make: dh_clean: Command not found [19:30] WTF? [19:30] hazmat: So, yeah.. I'm happy with changing both to agent-state if you'd like that [19:31] hazmat: Btw, is it start_error or start-error that we show? [19:32] niemeyer, underscore [19:35] niemeyer, i'm starting to feel like it should remain 'state' for unit.. because its not really the agent's state, its the charm and unit's state that's being represented.. the unit agent by itself is either running or its not, if its running we report the unit state, if its not we report the unit is down. [19:36] hazmat: bummer re. the underscore :( [19:37] niemeyer, we could change it to display.replace("_", "-") [19:37] hazmat: It's actually the agent.. the software in the unit might be running even if the agent is down [19:37] hazmat: For consistency, I'd find that quite adequate.. I think we've managed to stick to dashes in pretty much every other option [19:38] right.. but its not really the agent's state.. its the unit aka charm instance's state. the underling service is of course independent [19:38] hazmat: It's the agent really.. start-error means the *agent* is in a start error state [19:38] hazmat: Because the hook failed, true.. but it's the agent that is locked in that state [19:39] okay.. its good to have the consistency of key and to distinguish service vs agent. [19:39] agent-state for both [19:39] niemeyer, sorry that got long winded. [19:40] * hazmat looks for some caffeine [19:43] hazmat: No worries, thanks for explaining.. I had vague memories about it [19:55] Found the dh_clean error.. for some reason it was setup to upload to the wrong ppa [19:56] Which didn't have the proper dep [19:59] Awwwwww [19:59] 3h estimated build time for the charmstore [20:00] Suddenly the Go build time was severely downgraded by Launchpad :-( [20:16] Folks, I'll have a shorter day today as we have a family gathering tonight.. [20:16] See you all tomorrow [23:11] wtf tests seem to have stalled out a few weeks ago