[03:49] niemeyer: around? [03:53] I need some help from someone please - I am testing maas deployment and juju deploy seems to work but the provisioning agent blows up with the wrong auth data for maas [03:54] I need to work out what has corrupted the auth data, it seems to have turned a colon-separate string into a list of the substring parts === Leseb_ is now known as Leseb === Leseb_ is now known as Leseb === asavu_ is now known as asavu === TheMue_ is now known as TheMue [12:09] TheMue: a review for you: https://codereview.appspot.com/5834047/ [12:10] TheMue: good morning BTW. lovely and sunny here. [12:10] wrtp: will take a look. is it regarding the zk interface? [12:10] TheMue: it's a preliminary before adjusting state.Initialize [12:10] wrtp: good morning from a sunny norther germany too ;) [12:10] TheMue: (in the the light of recent changes to the zk package, tes) [12:10] s/tes/yes/ [12:13] wrtp: where is the cleanup of the temporary directory? [12:14] TheMue: Server.Destroy removes its directory [12:14] wrtp: does srv.Destroy() handle it? [12:14] wrtp: ah, fine [12:14] TheMue: it always has - it's just that before you couldn't create a server in an existing directory. [12:14] TheMue: so we needed to remove the directory created by TempDir [12:16] wrtp: ic [12:18] TheMue: do you think this is small enough that it should be ok to push without niemeyer's LGTM? [12:18] fwereade__: ^ [12:19] wrtp, I think it duplicates something I just merged [12:20] fwereade__: oh, that's odd. i could've sworn i did a merge from trunk [12:20] wrtp, just merged like 5 mins ago :( [12:21] fwereade__: no email about it. will try merging from trunk again. [12:21] wrtp: currently i won't do so. but we should define a process to discharge niemeyer, a kind of 4-eyes-lgtm [12:21] wrtp, I got the merged mail at 13:08, and your proposal at 13:10 [12:22] fwereade__: hmm, still no mail. [12:23] wrtp, hm, I don't think you reviewed that one, but surely you'd still see *something* [12:23] fwereade__: yeah, i usually do. [12:25] fwereade__: hmm, i see it now, after searching for it. i think i'm probably just blind. [12:26] fwereade__: anyway, that's fine. better actually. i'll just abandon. [12:27] wrtp, sorry collision :) [12:27] fwereade__: that's fine, i should've remembered. [12:27] wrtp, no worries; anyway, lunch :) [12:28] * fwereade__ is down to 1 unmerged python branch \o/ [12:28] fwereade__: enjoy [12:28] fwereade__: yay! [12:36] TheMue: so, as discussed some time ago, i'm changing state.Open so that it waits for the state to be initialized (so i can join some more dots up and make the juju command work for real). it's not a big change, but i'm wondering how this should fit with the tests. i'm thinking of making TestPackage call Initialize. do you think that'll work? [12:39] TheMue: or perhaps every suite should call Initialize in SetupSuite and zkRemoveTree of everything on teardown? [12:39] TheMue: that's probably cleaner, to my mind. [12:41] wrtp: what exactly will Open() wait for? [12:41] TheMue: the /initialized node to exist [12:42] wrtp: and whos task will it be to create this node (and the other ones)? [12:43] TheMue: that's the task of the bootstrap node [12:43] TheMue: it starts zookeeper and does the initialisation. [12:43] wrtp: but the code of it shall be provided by the state package? [12:43] TheMue: yeah. [12:44] TheMue: this is what i've got currently: http://paste.ubuntu.com/884722/ [12:44] wrtp: so in this case it should also provide a kind of Terminate() [12:45] TheMue: i'm thinking that Terminate (or Clean or Uninitialize) could be provided by the testing package. [12:45] TheMue: which would remove everything in the zk tree so we can start with a clean slate each time. [12:46] wrtp: how do we cean-up environments? or doesn't juju provide it? [12:46] TheMue: because i don't think there's ever a time outside of testing when you want to do that. [12:46] TheMue: we call environ.Destroy [12:46] TheMue: but that destroys the whole zk installation. [12:47] TheMue: so it's not necessary to remove the individual nodes. [12:47] wrtp: ok, radical enough ;) [12:47] :-) [12:47] wrtp: yeah, in this case a Cleanup() provided in the testing package should be enough [12:48] TheMue: cool. it can be called in different places in the different suites (e.g. once per test for StateSuite and once per suite for TopologySuite) [12:49] wrtp: hasn't there been any open questions about an extendability by fwereade__ ? [12:49] TheMue: sorry, don't understand the question. [12:49] wrtp: iirc thare have been some questions regarding this topic by fwereade__ [12:50] TheMue, I'm strongly in favour of adding a nuke-zk function to the testing package [12:50] fwereade__: good. [12:50] fwereade__: testing.zkClean() ? [12:51] TheMue, and the only cases I can think of in which we *don't* want to start a test with an initailized state are in tests for Initialize itself and jujud initzk [12:51] wrtp, make it public, but yeah [12:52] fwereade__: ok [12:52] fwereade__: maybe CleanZk, to mirror StartZkServer? [12:52] TheMue, wrtp: that is still 2 cases in different packages in which we want to be able to start from a clean ZK but it may actually be easier to *always* initialize, and just CleanZK() at the start of the tests [12:52] fwereade__: that's what i'm thinking [12:53] fwereade__: except... i'm not sure that testing should be reliant on the state package [12:53] fwereade__: i think i'm happier if StartZkServer just does that. [12:53] wrtp, sounds good to me [12:53] wrtp, anything that needs a state can just set it up [12:54] fwereade__: yeah [12:54] wrtp, and we avoid any need for silly little init-nuke-test dances [12:54] fwereade__: yeah. [12:54] fwereade__, wrtp: sounds fine [12:55] fwereade__: hmm, is there a way of telling which zk nodes are "native" (i.e. created automatically) so i don't try to delete them. or will i just hard code the name (can't remember it now!) [12:56] wrtp: heh, I didn't realise they existed... what would be the negative consequences of a kill 'em all approach? [12:57] fwereade__: i dunno. maybe i'll get a permission denied error though. [12:58] wrtp, the presence tests take the kill-'em-all approach and seem to work ok [12:59] wrtp, that said, I ignore errors [12:59] :-) [12:59] wrtp, but then so do the other state tests [12:59] wrtp, thinking on it, that's a problem [12:59] gotta go for lunch [12:59] wrtp, enjoy :) [12:59] wrtp: enjoy [13:15] TheMue, did you track down the issue with presence nodes? [13:15] TheMue, sorry, I meant to follow up on that this morning... [13:15] fwereade__: part of it, yep. had to do with a different test environment i had to fix [13:16] TheMue, cool, I didn't *think* presence nodes were utterly broken, but one does worry ;p [13:16] fwereade__: now only a non-firing watch is left (after i killed the pinger) [13:17] TheMue, hmm, weird, I thought I'd covered everything in the tests [13:19] fwereade__: order is (a) watch creation w/o pinger (b) start pinger, watch is fine © kill pinger, watch doesn't fire the change [13:19] TheMue, how long do you wait to detect the death? [13:20] TheMue, kill should be picked up almost immediately though [13:20] TheMue, hmm [13:20] TheMue, worst-case stop detection is 4x timeout [13:20] fwereade__: i tried different times from 25 to 100 ms [13:20] TheMue, sorry, 4x period ie 2* timeout [13:21] TheMue, and you have 25ms period? [13:21] fwereade__: no, 50, mom pls, will try with different timings [13:22] TheMue, you can't have my mom, but I'm not here to judge what you do with other peoples' :p [13:22] fwereade__: hmm, period is now 25 and i'm waiting 110 [13:23] TheMue, would you push a branch I can take a look at? [13:23] TheMue, wait, did you rewatch after the change? [13:23] fwereade__: strange how you and niemeyer don't now "mom pls" as abbrev for "one moment please". [13:23] TheMue, it's modelled after ZK watches: one change, and that's it [13:24] TheMue, I use "mo" [13:24] fwereade__: i know it since earliest irc days [13:24] TheMue, less disturbing ambiguity, and over a lifetime you'll save literally dozens of character [13:24] s [13:25] fwereade__: yeah, best it is to use no more abbreviations [13:26] fwereade__: only one change? oh [13:27] TheMue, yeah, sorry -- the idea was to write it as much as possible as though it were a building block as exposed by zk [13:27] fwereade__: wrtp and i changed it that way that a "Watcher" always refreshes the watch and you get an independant channel [13:27] TheMue, that makes perfect sense for a Watcher type [13:28] TheMue, but I'm not even sure there's any justification for an AgentWatcher [13:28] fwereade__: ok, then i know where to change it [13:28] TheMue, what's the use case? [13:30] fwereade__: having a watcher which notifies you if the node, here the agent node, is created or removed. you can query the status as well as wait for a change in any direction. [13:30] TheMue, but what would actually use that? [13:31] TheMue, we can already query status; and I don't think we actually ever watch an agent except when waiting for one to exist [13:31] TheMue, at which point we take some action and we're done [13:32] TheMue, the use cases I'm aware of are essentially Connected() and WaitConnected() [13:33] fwereade__: so the original naming is very misleading [13:33] TheMue, could very well be I'm afraid [13:33] fwereade__: one doesn't watch a node but waits for creation [13:33] TheMue, I *think* so [13:33] TheMue, let me check again [13:34] TheMue, yeah, the only clients are the ssh and debug-hooks commands [13:34] TheMue, each of which just wait for the agent to show up before they continue [13:35] TheMue, also it's an interesting semantic issue [13:35] TheMue, in ZK, "watch" really does mean "watch until there's a single change, and that's all" [13:36] TheMue, doesn't really fit with colloquial usage but in ZK context I think it's the right term to use [13:37] fwereade__: yeah, and so the idea behind the watcher has been to monitor for a change, possibly gather additional informations (if needed in some use cases) and provide it, continuosly [13:37] TheMue, yep: there are plenty of cases where we do want watchers but IMO they're unwanted complexity until they actually are required [13:39] fwereade__: they are so simple, no real complexity [13:39] TheMue, but if they're not needed then they still each represent a little extra cognitive load for no benefit [13:40] fwereade__: one little implementation based on an own type and a behaviour to plug in based on interfaces, it's really dead simple and could be reused everywhere [13:41] fwereade__: standard since more than 20 years [13:41] fwereade__: reusage of code, clean separation of infrastructure and logic, standard concurrent behaior [13:41] TheMue, my point is purely that it's not going to be used even once, so reuse is a moot point [13:42] fwereade__: we'll see [13:42] TheMue, and if it is one day used, which is fine, then we can implement it -- and like you say it'll be nice and simple :) [13:42] fwereade__: already in the state package there are so many watches [13:45] TheMue, and that's OK I think, it's the ZK-level metaphor; in some cases we want more sophisticated behaviour layered on top so we implement a watch*er* that wraps a series of watch*es* [13:48] fwereade__: ok, as long as we only reimplement a non-concurrent software it shall be fine. maybe i'm just too much used to think in larger 24/7 systems. [13:49] TheMue, I'm not sure what you're getting at there, expand please [13:50] TheMue, given the known use cases for agent-watching, what good will the extra layer do us? [13:50] fwereade__: it's no extra layer, it's only a different kind of implementation [13:51] TheMue, you seem to be implying that one-shot watches are somehow not appropriate for concurrent systems, and I don't follow that [13:52] fwereade__: no, i didn't said that [13:52] TheMue, it seems to me that they're perfectly good building blocks, and can be used to do more sophisticated things when a need becomes apparent [13:53] TheMue, all I'm saying is that we don't need any more sophistication on agent watches yet, and that we shouldn't add it until we do [13:53] Good mornings! [13:53] niemeyer, heyhey [13:53] fwereade__: i only said that my thinking about concurrent components comes out of a different area of systems [13:54] fwereade__: +1, whatever that means :-) [13:54] niemeyer: moin (that's a regional greeting here, could be used all day long ;) ) [13:54] TheMue, sorry, I misunderstood, no offence intended :) [13:54] TheMue: Hehe, I like that one [13:54] TheMue, very handy : [13:55] fwereade__: np, still have troubles to express it the right way. but thise evening i'm again at the Oldenburg English Club where we speak english all the time. ;) [13:55] TheMue, cool :) [13:56] fwereade__: but thx for this discussion, they helped a lot to get a better insight of how watches are used in juju [13:57] fwereade__: so my propose for the agent mixin will contain a semantically different naming just to see if it better expresses how it has to be used [13:59] TheMue, cool -- I would personally recommend `Connected() (bool, error)` and `WaitConnected() error` [13:59] TheMue, IMO they match current usage nicely [13:59] fwereade__: yep, exactly [13:59] TheMue, but I'll wait and see :) [13:59] niemeyer: morning :D [13:59] TheMue, cheers [14:00] TheMue: i think i'm with fwereade__ on this. i think we should leave the agent watcher until it's actually used. [14:00] niemeyer: hiya [14:00] niemeyer: the better zk error messages branch is ready for review, BTW: https://codereview.appspot.com/5835045/ [14:01] wrtp: yep [14:01] wrtp: Cheers [14:05] fwereade__: the directory /zookeeper seems to be special. i guess i'll just special-case that. [14:05] wrtp, ah, cool, good to know [14:06] niemeyer: https://codereview.appspot.com/5764043/ [14:10] andrewsmedina: that looks better, but i'm still not sure about that it's worth returning the standard output from commands that don't produce any (what are we going to do with the return value of container.create for example?) [14:11] andrewsmedina: i'd separate stdout from stderr too, so that it's not possible that the output of list will be corrupted by a log message printed to stderr. [14:12] wrtp: I think that we can remove it if we dont will use the output [14:13] andrewsmedina: why would we ever use the output? it doesn't produce any? i'd prefer to return the output (in a more useful form than []byte) if we decide it has something we need. [14:13] s/any?/any!/ [14:14] wrtp: but, I think that the best way is first create the local enviroment and then refactor the lxc lib [14:16] andrewsmedina: AFAICS the lxc lib is an abstraction around the lxc commands. i think the methods in it should reflect the abstraction we want to provide, rather than the commands that are being run underneath. [14:16] andrewsmedina: you're writing functions without knowing how they're going to be used. best to start as simple as possible, i think. [14:17] wrtp: but lxc lib don't is simple for you? [14:18] andrewsmedina: BTW despite the bogus "return first line as error" bit, the code i gave as runLXCCommand in my comment, pretty much reflects what i'd expect to see. [14:18] andrewsmedina: sorry, i don't understand. [14:18] wrtp == rog? [14:18] andrewsmedina: yes [14:19] (sorry, unexpected IRC user names!) [14:20] wrtp: I do not understand what you expect [14:20] wrtp: you expect that returns should be more readable? [14:21] andrewsmedina: i expect that the functions should return useful values. [14:21] andrewsmedina: create will always return an empty byte array AFAIK [14:26] andrewsmedina: something like this for runLXCCommand, BTW: http://paste.ubuntu.com/884842/ [14:26] wrtp: yes, the lxc-create or returns ''foo' already exists' or ''fooa' created' [14:27] andrewsmedina: if that happens, we want to return that as the error. [14:27] wrtp: but when I will define the custom stdout? [14:27] andrewsmedina: because it means the create has failed [14:29] Apparently Rietveld is now eventually consistent as well [14:29] andrewsmedina: like this, for example: http://paste.ubuntu.com/884853/ [14:29] Add comment > Comment disappears > Reload, reload, reload, reload > Nothing > Type comment again > Save > Old comment shows up [14:29] wrtp: in other cases we only need the error output [14:30] andrewsmedina: exactly. [14:30] niemeyer: i get that quite a bit. [14:30] wrtp: now I understand [14:30] niemeyer: i force the comment to appear just by entering a blank comment. [14:30] wrtp: I guess we live in the future, and people decided that things making sense is too boring.. [14:30] andrewsmedina: thanks for bearing with me :-) [14:31] wrtp: Ah, so you mean it consistently shows up if you save another comment on top? [14:31] niemeyer: yes [14:31] wrtp: Good to know, thanks [14:31] wrtp: I will improve the runLXCCommand :D [14:32] wrtp: Hold on.. but today it's _seriously_ broken [14:32] There must be something going on [14:32] wrtp: I've just mailed you a review, and it disappeared! [14:32] and now IT'S THERE! [14:32] niemeyer: weird [14:33] This must be about that "Highly Available Data Store" that they've been introducing in App Engine [14:33] wrtp: Anyway, you just got an eventually consistent review.. [14:33] niemeyer: thanks [14:49] wrtp: It's like a sink.. I can save 10 comments in the same location and all of them disappear immediately.. :) [14:50] niemeyer: i haven't seen that before [14:50] wrtp: It wasn't like that even yesterday [14:50] niemeyer: do they come back eventually? slightly smelly? [14:50] wrtp, niemeyer: it's always *sometimes* been like that for me [14:51] er, if that makes any sense [14:52] fwereade__: Yeah :) [14:53] wrtp: They do come back eventually, all at once [14:53] annoying [14:57] andrewsmedina: Thanks, I'm submitting your branch just now [14:58] wrtp: Disagree regarding separating stdout and err, btw [14:58] andrewsmedina: ^ [14:58] wrtp: If you separate the two streams, the error output from commands is often incomprehensible [14:58] niemeyer: really? isn't that the whole point of the distinction? [14:59] niemeyer: particularly for short-running commands [14:59] wrtp: I haven't talked to the guy that created the distinction, so I can't tell what the whole point is, but I can say that I have practical experience with incomprehensible error messages. [14:59] wrtp: If you take stderr out of context, you don't know what failed anymore. [15:00] launch time [15:00] niemeyer: in this case, i'm more worried about stdout being corrupted by prints to stderr [15:00] andrewsmedina: I'm submitting your branch as-is, thanks.. there are a few other details that we may polish over time, but nothing huge [15:01] niemeyer: if the command prints to stderr and exits without an error, we'll have dud list output for example. [15:01] wrtp: Ok, so let's fix that, but most of these commands don't care about stdout [15:02] niemeyer: that's true. so the stderr won't need any stdout to be understandable, because there isn't any [15:02] Right [15:03] wrtp: Or there is, but it's informational output [15:03] niemeyer: so separating the streams makes sense in this case, i think. [15:03] wrtp: Which we want interleaved with stderr, in case problems happen [15:03] niemeyer: really? i didn't see any. [15:03] niemeyer: but i didn't look too hard, right enough. [15:04] niemeyer: anyway, i'm happy that if stdout is nil, it gets merged with stderr [15:04] niemeyer: but it shouldn't be merged for list. [15:04] wrtp: Agreed [15:04] Separate branch.. we've banged on andrewsmedina enough for this one [15:05] niemeyer: sure. [15:05] andrewsmedina: sorry about all the requested changes. we're still hammering out our conventions and package structuring between ourselves... [15:06] andrewsmedina: thanks for working on this [15:07] mthaddon: Welcome! [15:07] o/ [15:07] mthaddon: So, charm store [15:08] indeed... [15:08] mthaddon: The package you want is golang-weekly [15:08] mthaddon: PPA details in https://wiki.ubuntu.com/Go [15:08] k, thx - and in terms of the bzr branch for the code itself and the build commands you want us to run? [15:08] mthaddon: Build commands: [15:08] go get -u launchpad.net/juju/go/store/charmd [15:08] go get -u launchpad.net/juju/go/store/charmstore [15:09] Erm.. [15:09] Sorry [15:09] go get -u launchpad.net/juju/go/store/charmd [15:09] go get -u launchpad.net/juju/go/store/charmload [15:09] mthaddon: You'll need to export GOPATH to a path where you want the packages and binaries to be put in [15:09] mthaddon: The two commands above will do everything necessary, and will get you two binaries under $GOPATH/bin [15:09] ok, thx - I'll have a play with that and let you know if I have any more questions (I'm sure I will) [15:10] mthaddon: Of course [15:10] mthaddon: Please just ping us if you need anything [15:10] will do [15:13] niemeyer: responded. [15:14] wrtp: "it is necessary, i think, otherwise we'll return when err is nil." [15:14] wrtp: You're right, my bad [15:15] niemeyer: good. my crack habit isn't showing too badly then. [15:17] niemeyer: just to be clear, that version of golang-weekly is needed to both build and run the code, or just to build it? [15:17] wrtp: Good stuff, LGTM [15:17] wrtp, niemeyer: beginnings of hook.Context at https://codereview.appspot.com/5832045 [15:17] mthaddon: Just build it.. take the two binaries home after that ;-) [15:17] niemeyer: cool. i'm happy too. [15:18] niemeyer: so we don't need this package installed on the charmstore server itself? ok, thx [15:18] mthaddon: That's right [15:18] k [15:18] mthaddon: All the nightmares of the security folks regarding static builds are now transformed into dreams for you [15:18] heh [15:29] fwereade__: you've got a review [15:29] wrtp, cheers :) [15:31] There's a bug in lbox submit.. just pushing a new package build now [15:34] wrtp, the constructors are more about documentation and clarity than anything else [15:35] fwereade__: i'm not convinced they help. you can document the fields of the type just as easily. [15:35] wrtp, maybe the term would be "literateness" rather than documentation [15:35] fwereade__: and it's nicer to do make a type with named fields. [15:36] fwereade__: still not sure i buy it. [15:36] wrtp, hmm, also in my mind there's a certain "it's ok to set these fields" implication in a type which exposes them [15:37] fwereade__: isn't it ok to set them? [15:38] fwereade__: I don't have the context, but that doesn't hold in general [15:38] that's true too [15:38] wrtp, well, I can't see any good reason for anyone to mutate them [15:38] fwereade__: If you set a field, there's a goal.. if you're setting fields improperly, bad things will happen [15:38] niemeyer, true [15:39] niemeyer, and so if there's never any good reason to set those fields after creation, why expose them? [15:39] fwereade__: In general, the line I try to draw is whether this is an implementation detail or not, and whether an interface for such type might be useful or not [15:39] niemeyer: +1 [15:40] niemeyer, well, the fact that we happen to store names is an implementation detail really [15:41] fwereade__: As I said, those are general statements.. I don't have the context. Can you point me to the code being debated? [15:41] niemeyer, https://codereview.appspot.com/5832045/ [15:41] fwereade__: Which line/type? [15:41] niemeyer, Context [15:41] niemeyer, https://codereview.appspot.com/5832045/diff/1/hook/context.go#newcode19 [15:43] [15:40] niemeyer, well, the fact that we happen to store names is an implementation detail really [15:43] fwereade__: is it something we'd ever want to change? if not, there's no harm in binding ourselves to that detail IMHO. [15:44] wrtp, I can all too easily imagine a future review saying "why bother storing the names when we could store the objects", for example [15:45] wrtp, and my only argument against it would be that it's an arbitrary rearrangement of code that makes one bit simpler at the cost of making another bit more complex [15:45] wrtp, and I don't seem to have much luck convincing people with that line of argument ;) [15:45] fwereade__: I agree with wrtp on that one.. there are already three different constructors, which do nothing else than assigning to all possible fields of Context in different setups, with no processing. [15:46] fwereade__: And we already have a Members accessor that returns members.. [15:47] fwereade__: In practice, I believe we'll have very few call sites building such a context [15:48] niemeyer, ok then [15:52] wrtp, niemeyer: I'll tack UnitName and RelationName onto ExecInfo as well, ok? [15:52] fwereade__: That said, again this isn't set on stone [15:52] fwereade__: sounds good. [15:52] niemeyer, frankly it may as well be [15:53] fwereade__: Often times I went back and fixed bad assumptions I made in the interface of values, either hiding things that were too exposed, or turning methods that were really data into fields [15:53] fwereade__: It's hard to foresee the perfect way to use a type before actually using it [15:53] fwereade__: So take those suggestions with a grain of salt at all times, please :) [15:53] niemeyer, I don't remotely claim my design is perfect but I'm starting to feel there's no point my thinking about design at all [15:54] that's why it's lovely having a statically typed language - you can make changes in such a way that the compiler will tell you where you need to fix things... [15:54] fwereade__: :( [15:54] niemeyer, ok, I overstated that [15:54] :-( too [15:56] niemeyer, wrtp: it seems like we do spend a lot of time discussing how the code "should" be without really having enough context to make these decisions [15:56] fwereade__: i try to start things off minimal and build up only as necessary. [15:57] fwereade__: which is always the context i'm coming from: "how can we make this smaller, simpler, easier to understand etc?" [15:57] fwereade__: i'm very sorry if my reviews come across as overly critical [15:58] fwereade__: Agreed.. I feel like we went over board on that in the last couple of days too. [15:59] wrtp, don't worry, I'm not feeling hostility in either direction [15:59] fwereade__: yeah, but it sounds as if you're frustrated, which isn't good. [15:59] fwereade__: In a way, I feel like we're building a shared agreement about conventions [15:59] fwereade__: Which is a bit exhausting at times [15:59] * wrtp would agree with that. [16:00] wrtp, that is true, I guess at least I'm talking about it and so I expect we'll fix it ;) [16:00] fwereade__: But I see that as a hump.. once the agreement is clear, it's easier for both parties to dismiss bad claims, or to not even make them in the first place [16:01] niemeyer, yes :) [16:01] niemeyer, wrtp: I think I'm fundamentally feeling railroaded into implementing something in which hook execution is entirely divorced from its context [16:02] niemeyer, wrtp: and that doesn't feel right to me [16:02] fwereade__: i'm thinking that you'll have a slightly higher-level function that will bring the two together nicely. [16:02] fwereade__: I don't think this is the case, to be honest. The way I see it is that we're trying to keep things simpler, while we can. [16:03] wrtp: yeah -- while my feeling was that Exec should be that higher-level function, because it's been cut down to the point where it may as well be inlined into whatever that higher-level function turns out to be [16:04] fwereade__: That may well be true.. I think we have a different feeling about what merging logic means. [16:04] wrtp: er, s/because/but/ (I think) [16:05] niemeyer, could very well be [16:05] fwereade__: Merging is just a statement of "Ok, this is a nicely done little thing that makes sense." [16:05] fwereade__: It's kind of an agreement that "so far, so good" [16:06] fwereade__: The next branch can corrupt that entirely with different needs, though [16:06] fwereade__: Because "so far, so good" by *then*, can mean something else [16:07] fwereade__: perhaps the dilemma goes away a little bit when we start to think about a Context as a potentially independent thing, not *necessarily* inside a hook. then the separation of hook execution from context starts to feel more natural, perhaps. [16:07] niemeyer, yeah, that makes sense, but I'll need to think about it a mo [16:08] fwereade__: We may be able to enlighten you about how much Exec make sense as an independent unit, and the opposite may well happen: a different design that has Exec taking Context may be so much more elegant. [16:08] wrtp, heh, I see the execution as necessarily being inside a Context, but that a Context is a legitimate entity in itself without ever having to execute a hook [16:09] fwereade__: Maybe Exec is even a method on Context? I don't know.. I'm not closed to any of that. [16:10] niemeyer: I setup an lxc container to test the build command, and got https://pastebin.canonical.com/62406/ - is that okay, or a problem in any way? [16:10] niemeyer, I guess the problem is that I have a tendency to pre-design things in my mind, and so the code I deliver ends up somewhat skewed by the plan [16:10] mthaddon: those are just warnings. you can ignore them. [16:10] k, thx [16:10] niemeyer, I think this is exacerbated by having the python code sitting in the background -- ofc I want to do better if I can, but it does shape my thinking [16:10] fwereade__: i think we all do that. [16:11] niemeyer, heh, I had been wondering about Context.Exec, which could indeed be very nice [16:11] fwereade__: that could indeed be nice. [16:12] There we go.. :) [16:12] niemeyer, but the trouble is I don't really feel free to deliver something with Context.Exec, because I can't *necessarily* justify it without an excess of speculative context [16:12] fwereade__: it's trivial to mutate the current Exec into Context.Exec i think [16:12] fwereade__: (when/if it's ready for it) [16:13] niemeyer, wrtp: and Context.Exec would *certainly* deal with the flushing ;) [16:13] :-) [16:13] meeting time [16:13] fwereade__: i think part of my reason for trying to keep things stripped down to the bare minimum is that it makes forward progress more agile. [16:14] fwereade__: That sounds fine, FWIW [16:14] fwereade__: less infrastructure to shift out of the way [16:14] hazmat: I think we're one hour early still.. and I'm about to have lunch! [16:14] niemeyer: i'd be happier to have it now - i've got to leave 30 mins early today. [16:14] niemeyer, time zone changes [16:15] wrtp, it is trivial to mutate: and I think that's the problem, because I've felt that review suggestions have started to tend towards the trivial-to-change-if-it-turns-out-to-be-a-good-idea [16:15] wrtp: Ale is heading to a restaurant right now from her work place.. [16:15] hazmat: No, no time zone changes here :) [16:15] nor here [16:16] niemeyer, :-) i'm fine with +1 hr [16:16] Ok.. biab [16:16]