| rogpeppe | davecheney: mornin' | 06:02 |
|---|---|---|
| davecheney | rogpeppe: hello hello | 06:12 |
| rogpeppe | davecheney: review delivered. | 07:31 |
| davecheney | thank you good sit | 07:31 |
| davecheney | sir | 07:31 |
| davecheney | i'll polish that up now | 07:31 |
| davecheney | re types | 07:32 |
| davecheney | I agree, we should have more of them | 07:32 |
| davecheney | i'm getting tired of typing []*state.Machine | 07:32 |
| rogpeppe | davecheney: with slices, i don't agree actually. | 07:38 |
| rogpeppe | davecheney: i think type MachineList []*state.Machine would be worse. | 07:38 |
| rogpeppe | (and it's the same length) | 07:39 |
| rogpeppe | ish | 07:39 |
| davecheney | Machines | 07:40 |
| davecheney | rogpeppe: do I owe you any reviews ? | 07:51 |
| davecheney | i don't feel that i'm in the best timezone to assist, gustavo has usually got to them before me | 07:52 |
| rogpeppe | davecheney: i'm about to submit https://codereview.appspot.com/6299073/; but you might want to glance over https://codereview.appspot.com/6299082/ | 07:53 |
| rogpeppe | davecheney: yeah, but another pair of eyes is always good | 07:53 |
| davecheney | ok, sam just came home, but i'll check it out after dinner | 07:56 |
| rogpeppe | davecheney: np. it's pretty much code-grind anyway. | 08:00 |
| Aram | morning | 10:25 |
| fwereade | hey Aram | 10:31 |
| TheMue | Aram: moin | 10:37 |
| TheMue | Somehow my 1.0.2 has troubles with gozk and goyaml. | 10:37 |
| Aram | ah, new release | 10:40 |
| Aram | works here | 10:46 |
| TheMue | Aram: Maybe I just have to clean up the 3rd party libs and re-get. | 10:48 |
| TheMue | rogpeppe: ping | 10:48 |
| rogpeppe | TheMue: pong | 10:48 |
| rogpeppe | i haven't tried 1.0.2 if that's what you're gonna ask :-) | 10:48 |
| TheMue | rogpeppe: Ah, is AssignUnit() in unit.go by you? | 10:49 |
| TheMue | rogpeppe: No ;) | 10:49 |
| rogpeppe | TheMue: i've touched it recently... | 10:49 |
| rogpeppe | TheMue: although it was fwereade's originally | 10:49 |
| rogpeppe | i think | 10:49 |
| TheMue | rogpeppe: OK, thx. | 10:49 |
| fwereade | rogpeppe, TheMue: yeah, I think so | 10:49 |
| rogpeppe | TheMue: is there a problem with it? | 10:50 |
| TheMue | fwereade: I just wondered because it's a func taking a State as first arg. | 10:50 |
| TheMue | fwereade: We mostly put this as method to State. | 10:50 |
| fwereade | TheMue, that's at niemeyer's request, he didn't like having it on Unit | 10:50 |
| fwereade | TheMue, I guess it could go on State, that is true | 10:51 |
| TheMue | fwereade: And internally you used u.st, the state of the unit. | 10:51 |
| Aram | TheMue: gozk and goyaml use cgo, maybe it's your gcc at fault, or you lack some dumb lib. | 10:51 |
| fwereade | TheMue, heh that's just me being stupid | 10:51 |
| rogpeppe | +1 on making it a state method | 10:51 |
| fwereade | TheMue, I recommend a little CL fixing it on its own and seeing what niemeyer thinks, I can't see a problem | 10:51 |
| TheMue | rogpeppe: thx | 10:52 |
| TheMue | fwereade: Will you do it or shall I? | 10:52 |
| fwereade | TheMue, would you please? I'm still trying to figure out why I can't talk to launchpad properly... :/ | 10:52 |
| TheMue | fwereade: Check if you're a member of the gophers group. | 10:53 |
| TheMue | fwereade: That has been the problem for me. | 10:53 |
| TheMue | fwereade: Same kind of error yesterday. | 10:53 |
| fwereade | TheMue, looks like that's the trouble | 10:53 |
| fwereade | TheMue, tyvm | 10:54 |
| TheMue | fwereade: np | 10:54 |
| TheMue | Ah, found my missing package/file for ZK. I'm wondering why this happens now. OK, it's the first rebuild after switch to 12.04. | 11:07 |
| rogpeppe | TheMue: i think you should hold off on the tags until we get some feedback from niemeyer. | 11:12 |
| rogpeppe | TheMue: if we agree it's good to change them, i'll do them all in that CL. | 11:12 |
| TheMue | rogpeppe: Great, and it has been a good hint. | 11:12 |
| rogpeppe | TheMue: also, about the error message - i'd like to keep that CL as only about the go vet fixes. error message fixing needs to happen more generally in another place. | 11:13 |
| rogpeppe | s/another place/another CL/ | 11:13 |
| TheMue | rogpeppe: I'm doing the state error messages right now file by file. | 11:13 |
| TheMue | rogpeppe: And in this context I only found your prefix for ec2 | 11:14 |
| fwereade | rogpeppe, just looking at presence_test; you added the "+ a little bit for scheduler glithes" to the longEnough var | 11:23 |
| fwereade | rogpeppe, can you explain what that's about? | 11:24 |
| * rogpeppe looks | 11:26 | |
| rogpeppe | fwereade: things can happen after they are expected to. | 11:27 |
| rogpeppe | fwereade: it's not a real-time language | 11:27 |
| fwereade | rogpeppe, what does go have to do with this at all? | 11:27 |
| rogpeppe | fwereade: so it's best to allow some slack in timings | 11:27 |
| rogpeppe | fwereade: go is firing the watch | 11:28 |
| fwereade | rogpeppe, isn't allowing double the pinger period for detecting timeout good enough already? | 11:29 |
| fwereade | rogpeppe, ah, hmm, I see what you mean in this context | 11:30 |
| * fwereade thinks a bit | 11:30 | |
| rogpeppe | fwereade: no, because as the comments say, the time out might legitimately occur after 99ms | 11:30 |
| rogpeppe | fwereade: it doesn't matter if we add some slack, but it does matter if we get test failures on a heavily loaded system | 11:31 |
| TheMue | fwereade, rogpeppe: Move of AssignUnit() is in as https://codereview.appspot.com/6298081 | 11:34 |
| rogpeppe | TheMue: LGTM | 11:34 |
| TheMue | rogpeppe: That's fast. ;) | 11:34 |
| rogpeppe | TheMue: quickest review ever! | 11:34 |
| TheMue | rogpeppe: Definitely. Cheers! | 11:35 |
| rogpeppe | 29s... | 11:35 |
| Aram | mramm: 15:00 GMT is fine with you? | 11:40 |
| mramm | I believe that is my one not-free hour | 11:42 |
| Aram | 16:00 GMT? | 11:42 |
| mramm | that works | 11:43 |
| Aram | good | 11:43 |
| twobottux | aujuju: Invalid SSH key error in juju when using it with MAAS <http://askubuntu.com/questions/147714/invalid-ssh-key-error-in-juju-when-using-it-with-maas> | 11:46 |
| * TheMue is out of the office this afternoon, will continue work later. | 12:07 | |
| rogpeppe | fwereade: is there a reason why in the python version some parameters are passed as env vars not flags? | 12:22 |
| rogpeppe | (to agents, that is) | 12:22 |
| fwereade | rogpeppe, I don't think so; IIRC niemeyer agreed to drop those | 12:22 |
| rogpeppe | fwereade: cool, thanks | 12:22 |
| fwereade | rogpeppe, (well the reason, I think, was to make python testing more convenient, so it shouldn't aply any more ;)) | 12:23 |
| rogpeppe | fwereade: thanks | 12:24 |
| rogpeppe | fwereade: i'm just looking in juju/machine/unit.py; it seems a bit odd that the provider type is overloaded to be "subordinate" sometimes. i'm thinking that unit deployment really needs only one argument: whether to put the unit agent in a container. | 12:35 |
| rogpeppe | fwereade: does that sound reasonable? | 12:35 |
| rogpeppe | when i say "only one argument" obviously there are other args, but i don't think it needs to know the provider type | 12:36 |
| fwereade | rogpeppe, sorry, went for a ciggie | 12:45 |
| * fwereade reads back | 12:45 | |
| fwereade | rogpeppe, yeah, that makes sense to me... didn't we determine that SubordinateContainerDeployment was actually identical to UnitMachineDeployment anyway? | 12:47 |
| rogpeppe | fwereade: that it is | 12:47 |
| fwereade | rogpeppe, it crosses my mind that, long-term, we need to promote the concept of container somehow | 12:48 |
| fwereade | rogpeppe, as it is we make inferences about containers in a range of places | 12:48 |
| rogpeppe | fwereade: i think that's what i'm doing by making it a bool argument to deploy. | 12:48 |
| rogpeppe | fwereade: but i'm probably missing something important | 12:48 |
| fwereade | rogpeppe, as it is a unit "is" a container, except when some other unit "is" its container, and it confuses me | 12:49 |
| rogpeppe | fwereade: it seems to me that we'd need a bool "containerise" field in the principal unit. but maybe not much more | 12:50 |
| fwereade | rogpeppe, hold on, how does this interact with deploying into local containers? a subordinate needs to be deployed into an existing container... | 12:50 |
| fwereade | rogpeppe, feels like there's something missing | 12:50 |
| fwereade | rogpeppe, but I'm not really up on the context | 12:50 |
| rogpeppe | fwereade: subordinates are deployed by the principal's unit agent | 12:50 |
| fwereade | rogpeppe, ah, cool | 12:50 |
| fwereade | rogpeppe, your perspective seems fine to me then :) | 12:51 |
| rogpeppe | fwereade: i *think* that it'll all just fall out naturally, including the local provider stuff | 12:51 |
| rogpeppe | fwereade: cool, the crack is not with me today then | 12:52 |
| fwereade | rogpeppe, btw, https://codereview.appspot.com/6298082 | 12:53 |
| niemeyer | Guten tag! | 13:37 |
| niemeyer | fwereade: Heya, yeah, sorry for the gophers team issue | 13:38 |
| Aram | moin. | 13:40 |
| niemeyer | Aram: Heya | 13:45 |
| fwereade | niemeyer, no worries :) | 13:58 |
| fwereade | niemeyer, that had me *utterly* foxed for a while though :) | 13:58 |
| niemeyer | fwereade: I figured that yesterday, but I wasn't sure if I should move the branch to another team and forgot to add you to ~gophers, which should be done no matter what | 13:59 |
| rogpeppe | niemeyer, fwereade: here's a sketch for a possible new package (proposed name launchpad.net/juju-core/juju/service) http://paste.ubuntu.com/1040813/ | 14:01 |
| rogpeppe | niemeyer, fwereade: it will encapsulate much of the logic in juju/machine/unit.py | 14:02 |
| fwereade | rogpeppe, in essence, LGTM, but I think we should be careful about Destroy :) | 14:03 |
| rogpeppe | fwereade: careful how? | 14:03 |
| rogpeppe | fwereade: you mean avoid rm -rf / ? | 14:03 |
| fwereade | rogpeppe, IIRC, that's the idea | 14:03 |
| rogpeppe | fwereade: yeah, i'll do my best not to trash your machine or mine :-) | 14:04 |
| fwereade | rogpeppe, nah, it's the same issue as automatically terminating machines | 14:04 |
| fwereade | rogpeppe, we don;t d that because we feel people might want a change toretrieve their data even if they're no longer running the service | 14:04 |
| fwereade | rogpeppe, deleting containers falls under the same category I think | 14:05 |
| fwereade | niemeyer, is theabove broadly accurate? | 14:05 |
| niemeyer | rogpeppe: I'm not sure.. in isolation, that interface doesn't seem very appealing | 14:05 |
| niemeyer | rogpeppe: We have a bunch of things that "represent a deployed service", and we have a very special and well defined meaning for what a "service" is | 14:06 |
| rogpeppe | fwereade: ok. my first thought was a package called "deploy" | 14:07 |
| rogpeppe | oops | 14:07 |
| rogpeppe | niemeyer: ^ | 14:07 |
| niemeyer | rogpeppe: The parameters of New() feel like a bag of unrelated attributes | 14:07 |
| rogpeppe | yeah, "service" is not a great name | 14:07 |
| rogpeppe | niemeyer: another thought was to put those attributes in a struct | 14:07 |
| niemeyer | rogpeppe: I don't know.. it's hard to suggest something in isolation in this case. In my mind what we need is a function that creates the container | 14:08 |
| rogpeppe | niemeyer: what do you think to the basic division of functionality (leaving all setup of directory structure to the unit agent)? | 14:08 |
| niemeyer | rogpeppe: Not an interface, not a struct | 14:08 |
| rogpeppe | niemeyer: ok, so how do we then destroy the container? | 14:09 |
| niemeyer | rogpeppe: Thinking | 14:09 |
| niemeyer | rogpeppe: Ok.. the API is mostly sensible I guess | 14:12 |
| niemeyer | rogpeppe: Maybe the proper name for this is Container | 14:12 |
| niemeyer | rogpeppe: With something along the lines of Create, Start, and Destroy methods | 14:13 |
| rogpeppe | niemeyer: i suppose so. but it's going to be used for starting things that aren't in a container too. at least that was the idea | 14:13 |
| niemeyer | rogpeppe: I'm thinking about Container in a more abstract way.. it's a unit container | 14:13 |
| niemeyer | rogpeppe: (rather than an *LXC* container) | 14:13 |
| rogpeppe | niemeyer: hmm, maybe | 14:14 |
| niemeyer | rogpeppe: That's how we've been referring to it all along, I think | 14:14 |
| niemeyer | rogpeppe: We have relations with scope of "container" for example | 14:14 |
| niemeyer | rogpeppe: Despite the fact they're not deployed via LXC in some cases | 14:14 |
| rogpeppe | niemeyer: i'm slightly uncomfortable because in the default case it doesn't "contain" anything | 14:14 |
| niemeyer | rogpeppe: Uh.. how so? | 14:15 |
| niemeyer | rogpeppe: Maybe I misunderstand what this is about | 14:15 |
| rogpeppe | niemeyer: we're just starting a process and adding an upstart entry for it, if container==false | 14:15 |
| niemeyer | rogpeppe: It contains a unit | 14:16 |
| niemeyer | rogpeppe: That's its reason of existence | 14:16 |
| rogpeppe | niemeyer: starts a unit, really. | 14:17 |
| niemeyer | rogpeppe: Contains, unless I misunderstand what you mean | 14:17 |
| niemeyer | rogpeppe: The Dir there is the root of that container | 14:17 |
| rogpeppe | niemeyer: i suppose it comes down to what we mean by "contain" | 14:17 |
| niemeyer | rogpeppe: Yes, that's what I've been trying to point out :) | 14:17 |
| rogpeppe | niemeyer: we can have several "containers" that all have the same root. | 14:18 |
| rogpeppe | niemeyer: (in the subordinate case) | 14:18 |
| niemeyer | rogpeppe: By definition, all containers will always be in the same root.. we don't have filesystem management today | 14:18 |
| rogpeppe | niemeyer: LXC containers have different roots from one another, no? | 14:18 |
| niemeyer | rogpeppe: Ok, hold on, so I really misunderstand what you mean | 14:19 |
| niemeyer | rogpeppe: Subordinates all live within *one* container | 14:19 |
| rogpeppe | niemeyer: yeah | 14:19 |
| niemeyer | rogpeppe: I don't know what we're discussing then | 14:19 |
| rogpeppe | niemeyer: but the idea behind this API is that a machine agent or a unit agent can start another unit without worrying too much about if it's contained or not. | 14:19 |
| niemeyer | rogpeppe: The machine agent has to create a container (the container I'm talking about) | 14:20 |
| niemeyer | rogpeppe: and start a process within that container | 14:20 |
| niemeyer | rogpeppe: That's a different procedure from what a unit agent must do | 14:20 |
| rogpeppe | niemeyer: yup, and that's what i want this API to do | 14:20 |
| niemeyer | rogpeppe: Okay, that's a Container interface | 14:21 |
| niemeyer | rogpeppe: That's not what a unit agent must do | 14:21 |
| niemeyer | rogpeppe: A unit agent has to simply put a file on the upstart directory, and run "start whatever".. done | 14:21 |
| rogpeppe | niemeyer: however, in the future, the MA must be able to do both things, i think | 14:21 |
| niemeyer | rogpeppe: Why? | 14:21 |
| rogpeppe | niemeyer: because we want it to be able to start a unit both in or out of a container | 14:22 |
| niemeyer | rogpeppe: Nope | 14:22 |
| rogpeppe | niemeyer: so we can avoid LXC overhead if necessary | 14:22 |
| niemeyer | rogpeppe: It's always within the abstract concept of a container that I'm talking about | 14:22 |
| niemeyer | rogpeppe: A unit container may use LXC or not, but it's still a unit container | 14:23 |
| rogpeppe | niemeyer: ah! | 14:23 |
| niemeyer | rogpeppe: We call it "scope: container" even when there's no LXC involved | 14:23 |
| rogpeppe | niemeyer: so perhaps we have a global variable in the container package which is "Current" | 14:24 |
| rogpeppe | niemeyer: as in the container we're currently in. | 14:24 |
| niemeyer | rogpeppe: I don't know.. that's an implementation detail that went over my head at the moment | 14:24 |
| rogpeppe | niemeyer: the difficulty i'm having is that when the MA is deploying a unit without LXC, it's not creating a container. | 14:25 |
| niemeyer | rogpeppe: It is.. it's just a very trivial container | 14:26 |
| niemeyer | rogpeppe: That runs without isolation | 14:27 |
| niemeyer | rogpeppe: Container has Create, Start, Destroy: Create the container itself, Start makes it run now and whenever the machine boots; Destroy gets rid of it all | 14:27 |
| niemeyer | rogpeppe: That's doable both with and without LXC | 14:28 |
| niemeyer | rogpeppe: (and we'll need both) | 14:28 |
| niemeyer | rogpeppe: The other problem you mentioned, starting another unit from the point of view of a unit that is already running, is trivial | 14:30 |
| niemeyer | rogpeppe: It can literally be done in a very short function that dumps an upstart script and starts it | 14:30 |
| niemeyer | rogpeppe: Because we've already agreed that the unit is responsible for itself (downloading charm, etc) | 14:31 |
| rogpeppe | niemeyer: here's what i've got currently: http://paste.ubuntu.com/1040866/ | 14:33 |
| rogpeppe | niemeyer: i'm not sure about the command args to Create though | 14:33 |
| rogpeppe | niemeyer: i'm not sure i want the code outside of container to know about how upstart works. | 14:34 |
| rogpeppe | niemeyer: a few typos rectified: http://paste.ubuntu.com/1040871/ | 14:35 |
| rogpeppe | fwereade: about destroying containers, i'm not sure. the current python code destroys containers when their units disappear from the machine. we'd need a new "terminate-unit" command, i guess. | 14:38 |
| fwereade | rogpeppe, I don't really have a position on this: I'm not really happy with either approach :) | 14:39 |
| rogpeppe | fwereade: if in doubt, follow the existing implementation, right? | 14:39 |
| fwereade | rogpeppe, not sure; depends how soon ubiquitous containerisation arrives really | 14:40 |
| fwereade | rogpeppe, I'm not sure it's even terminate-unit so much as it is terminate-container | 14:40 |
| rogpeppe | fwereade: i don't think the user has any concept of containers, | 14:41 |
| niemeyer | rogpeppe: I was thinking about something along these lines: http://paste.ubuntu.com/1040880/ | 14:41 |
| * rogpeppe thinks. | 14:43 | |
| niemeyer | rogpeppe: Will actually need an extra "Start() error" method | 14:44 |
| niemeyer | Due to LXC.. Create > Deploy > Start | 14:44 |
| rogpeppe | niemeyer: i'm thinking that if we're passing in Unit (i was originally trying to build a low level package without state dependency), we can make the package know whether to deploy as LXC or not, because it can tell from the Unit itself. | 14:45 |
| niemeyer | rogpeppe: No, it can't | 14:45 |
| niemeyer | rogpeppe: This is part of the environment configuration | 14:45 |
| niemeyer | rogpeppe: In fact.. there's no point in having Create and Deploy, I believe.. | 14:46 |
| niemeyer | So it's really just Deploy and Destroy | 14:46 |
| rogpeppe | niemeyer: DeployLXC(*Unit) (Container, error) | 14:47 |
| rogpeppe | niemeyer: or we could just call the package "deploy" | 14:47 |
| niemeyer | rogpeppe: Then you don't have a Container interface to pass around and allow the implementation to vary anymore | 14:47 |
| rogpeppe | niemeyer: deploy.LXC(*Unit) (Container, error) | 14:47 |
| rogpeppe | niemeyer: ? that's what it's returning | 14:48 |
| rogpeppe | niemeyer: what's your "dir" arg? | 14:48 |
| niemeyer | rogpeppe: The directory for the container (!?) | 14:48 |
| rogpeppe | niemeyer: doesn't the LXC subsystem choose that? | 14:49 |
| niemeyer | rogpeppe: I don't know.. | 14:49 |
| rogpeppe | niemeyer: the python code looked like it did | 14:49 |
| rogpeppe | niemeyer: but that may be just the way they've chosen to do it | 14:50 |
| niemeyer | This is the direction I would go: http://paste.ubuntu.com/1040898/ | 14:51 |
| niemeyer | But it sounds like we're simplifying quite a bit, which is great. | 14:52 |
| rogpeppe | niemeyer: what can we usefully do between calling LXC and calling Deploy? | 14:52 |
| niemeyer | So it may turn out to be a single function :) | 14:52 |
| niemeyer | <niemeyer> rogpeppe: Then you don't have a Container interface to pass around and allow the implementation to vary anymore | 14:52 |
| rogpeppe | niemeyer: i don't understand that | 14:52 |
| rogpeppe | niemeyer: you mean for testing? | 14:52 |
| niemeyer | rogpeppe: Ok, don't worry.. it really depends on the implementation. | 14:52 |
| niemeyer | I'm happy to see real stuff happening around that, whatever it is | 14:53 |
| niemeyer | rogpeppe: Just don't name the method as "deploy" please.. you won't want to have a destroy function within a deploy method | 14:54 |
| niemeyer | s/method/package | 14:54 |
| rogpeppe | niemeyer: this is what i was thinking: http://paste.ubuntu.com/1040905/ | 14:54 |
| rogpeppe | oh, ok | 14:54 |
| rogpeppe | niemeyer: this, perhaps: http://paste.ubuntu.com/1040908/ | 14:55 |
| rogpeppe | niemeyer: or even: http://paste.ubuntu.com/1040910/ | 14:56 |
| niemeyer | rogpeppe: How to destroy a container that wasn't created in the current process run? | 14:56 |
| rogpeppe | given that LXC is really an implementation detail | 14:56 |
| rogpeppe | niemeyer: i'm not sure we ever want to do that. | 14:56 |
| niemeyer | rogpeppe: Yes, we do | 14:56 |
| rogpeppe | niemeyer: when? | 14:56 |
| niemeyer | rogpeppe: Sorry, I'm missing what you're missing | 14:57 |
| niemeyer | rogpeppe: Every time?! | 14:57 |
| niemeyer | rogpeppe: Processes are restartable | 14:57 |
| rogpeppe | oh yeah | 14:58 |
| rogpeppe | niemeyer: i'm not sure how to do it though. | 14:58 |
| niemeyer | rogpeppe: The interface I suggested!? :) | 14:58 |
| rogpeppe | niemeyer: i'm wondering how the Simple and LXC functions know what they're attaching to | 15:01 |
| niemeyer | rogpeppe: What does "attach" mean? | 15:01 |
| rogpeppe | niemeyer: if you restart, how do you get a Container that refers to a container created in a previous process run? | 15:02 |
| rogpeppe | niemeyer: i'm thinking that they might take a name as argument | 15:02 |
| niemeyer | rogpeppe: With LXC(...) or Simple(...).. | 15:02 |
| rogpeppe | niemeyer: yes, but how do you tell LXC what container you want it to talk to? | 15:03 |
| niemeyer | rogpeppe: With the arguments to LXC(...) | 15:03 |
| rogpeppe | niemeyer: i.e. what arguments does LXC have | 15:03 |
| niemeyer | rogpeppe: I'd have to implement it to tell you :) | 15:03 |
| niemeyer | rogpeppe: There is existent logic in the Python tree to serve as inspiration | 15:04 |
| rogpeppe | niemeyer: i'm thinking that we'd probably do it by name; so it'll be the name of the unit, most likely, so perhaps my interface would work after all. | 15:04 |
| niemeyer | rogpeppe: I'm sure you can make any interface work | 15:04 |
| rogpeppe | niemeyer: i'm not :-) | 15:05 |
| niemeyer | rogpeppe: It doesn't mean it will look good, necessarily :) | 15:05 |
| rogpeppe | niemeyer: i'm thinking that if the container already exists for a unit, Simple would return it | 15:06 |
| rogpeppe | niemeyer: but yeah, maybe we want to distinguish between create and reattach | 15:07 |
| niemeyer | rogpeppe: deploying an LXC container as a side-effect isn't a great idea | 15:07 |
| rogpeppe | niemeyer: wouldn't LXC() attach to an LXC container, or create it if it didn't exist? | 15:08 |
| rogpeppe | niemeyer: or maybe it doesn't do anything until you call Deploy | 15:08 |
| niemeyer | rogpeppe: We're starting to go in circles.. my suggested Container interface has a Deploy method | 15:08 |
| rogpeppe | niemeyer: i'm wondering what the LXC function actually *does*. | 15:09 |
| niemeyer | rogpeppe: Return an LXC Container implementation | 15:09 |
| rogpeppe | niemeyer: so it's just a factory. i see. | 15:10 |
| rogpeppe | niemeyer: hence the lack of an error return, doh! | 15:10 |
| rogpeppe | niemeyer: sorry, i hadn't appreciated that. | 15:10 |
| niemeyer | np | 15:11 |
| fwereade | TheMue, btw, I like errorContextf :) | 15:12 |
| rogpeppe | niemeyer: https://codereview.appspot.com/6299082/ should be good to go now, i hope. | 15:23 |
| niemeyer | rogpeppe: Done | 15:29 |
| fwereade | TheMue, ping | 15:34 |
| === hazmat is now known as kapilt | ||
| rogpeppe | niemeyer: thanks a lot. | 15:35 |
| rogpeppe | niemeyer: submitted. | 15:35 |
| niemeyer | rogpeppe: Thank you! Really glad with how readable these watchers have become | 15:36 |
| rogpeppe | niemeyer: i still can't help thinking if there's a way to factor out some common pattern from them... but i agree, it's easier to follow than the python logic. | 15:37 |
| niemeyer | rogpeppe: Crosses my mind too, but I also realize that we've already been doing some of that | 15:38 |
| rogpeppe | niemeyer: e.g. stopWatcher | 15:39 |
| niemeyer | Yeah | 15:39 |
| rogpeppe | niemeyer: i don't mind too much. they're similar enough that it's quite easy to look at them side-by-side and see differences. | 15:39 |
| niemeyer | rogpeppe: Yeah, don't see as an immediate issue we need to worry about either | 15:40 |
| niemeyer | rogpeppe: This may even change significantly if Aram's research turns out well | 15:40 |
| niemeyer | Aram: Btw, when do you plan to start pushing some trivial branches? | 15:40 |
| niemeyer | Aram: To get us started on the mstate stuff | 15:41 |
| Aram | niemeyer: today, I started to work on mstate, let me create the launchpad project so I can have a place to push them | 15:41 |
| niemeyer | Aram: I think there are easy first steps to get us going with something concrete and get the momentum going | 15:41 |
| niemeyer | Aram: Woah what? | 15:42 |
| Aram | yes. | 15:42 |
| niemeyer | Aram: The Launchpad project is "juju-core" :-) | 15:42 |
| Aram | ah, so I should push mstate there? | 15:42 |
| niemeyer | Aram: and it should be pushed in very small steps that feel solid, so we can see the experience flowing | 15:42 |
| fwereade | niemeyer, btw, if you have a moment to take a look at https://codereview.appspot.com/6298082/, it should be quick | 15:42 |
| Aram | yes, of course. | 15:42 |
| Aram | so niemeyer, the mstate directory should be in juju-core/juju? I used an out of tree directory for now. | 15:43 |
| fwereade | niemeyer, I'm just updating it now such that it includes the error checks you suggested in the unit-relation-watcher review but that shouldn't affect how good it looks | 15:43 |
| niemeyer | Aram: Yeah, side by side with the real state | 15:44 |
| Aram | niemeyer: ok, understood. | 15:44 |
| Aram | btw niemeyer, we use only a limited subset of the state api: http://paste.ubuntu.com/1040999/ that listing is about 90% accurate. | 15:46 |
| Aram | I generated it to prioritize work. | 15:46 |
| niemeyer | Aram: Yes, because TheMue has been rocking solidly porting the real API | 15:46 |
| niemeyer | Aram: The API is entirely used assuming we finish the port of juju to Go using similar logic to Python | 15:47 |
| niemeyer | Aram: I suggest starting with the easy wins, to get our feet wet | 15:47 |
| rogpeppe | niemeyer, Aram: i wonder whether we should do some watcher stuff early on, because that's where our state stuff differs most from conventional mongo usage | 15:49 |
| niemeyer | fwereade: Reviewed | 15:50 |
| niemeyer | rogpeppe: As I just said, I'd prefer to start with some easy stuff first, to get our feet wet | 15:51 |
| fwereade | niemeyer, good point, thank you | 15:51 |
| niemeyer | rogpeppe: The mstate package doesn't exist, we have zero infrastructure working, we don't know even the basic patterns we want to use | 15:51 |
| niemeyer | rogpeppe: Thinking about watches when we miss all of that will be messy | 15:52 |
| rogpeppe | niemeyer: definitely. just we shouldn't get too far into implementing juju structures and then realise they're incompatible with the way we need to implement watchers | 15:52 |
| niemeyer | rogpeppe: The state package is not too far, in its entirety | 15:52 |
| rogpeppe | niemeyer: but gotta start with something! | 15:52 |
| niemeyer | Let's just get something going, slowly and solidly | 15:53 |
| rogpeppe | niemeyer: BTW sometime it would be great if you could do that goamz resilience stuff, so i can run -amazon tests more reliably. | 15:57 |
| rogpeppe | niemeyer: (another "handshake error"...) | 15:58 |
| niemeyer | rogpeppe: Sounds good | 15:58 |
| niemeyer | First, I'll have lunch! | 15:58 |
| niemeyer | :) | 15:58 |
| niemeyer | See you guys soon | 15:58 |
| rogpeppe | niemeyer: enjoy! | 15:58 |
| TheMue | fwereade: pong | 16:13 |
| fwereade | TheMue, NoRelationError is starting to feel a little bit redundant | 16:13 |
| fwereade | TheMue, I was thinking of replacing it with a bare-faced "relation doesn't exist" | 16:14 |
| fwereade | TheMue, and trusting the clients' errorContextf~s to give necessary context | 16:14 |
| TheMue | fwereade: Sounds reasonable, now with errorContextf. | 16:15 |
| TheMue | fwereade: Btw, it has been niemeyers idea.The trick with using pointers inside is cool. | 16:16 |
| fwereade | TheMue, regardless of initial source, it's cool; thank you for implementing it :) | 16:17 |
| TheMue | fwereade: ;) | 16:18 |
| rogpeppe | niemeyer: i've gotta go now, but this is what i'm looking at currently: http://paste.ubuntu.com/1041100/ | 17:06 |
| niemeyer | rogpeppe: Looks nice! | 17:13 |
| fwereade | niemeyer, btw, I've been meaning to ask | 17:19 |
| fwereade | niemeyer, can you precis your ideas for multi-endpoint relations? | 17:20 |
| fwereade | niemeyer, I can see how they make sense for peer relations | 17:20 |
| fwereade | niemeyer, but beyond that my brain breaks | 17:20 |
| fwereade | niemeyer, (where "multi" means "more than we currently accept for a given relation kind") | 17:21 |
| niemeyer | fwereade: Hmm | 17:22 |
| niemeyer | fwereade: My brain doesn't break.. it simply stops thinking.. :-) | 17:23 |
| niemeyer | fwereade: I'm not foreseeing all kinds of relations we may come up with | 17:23 |
| niemeyer | fwereade: It may well turn out that we need none other than what we have | 17:23 |
| niemeyer | fwereade: The concerns I raised in terms of the architecture are mainly to leave the door open, since we can, rather than preparing for a specific feature that I have in mind | 17:24 |
| fwereade | niemeyer, ok, cool, thanks | 17:25 |
| fwereade | niemeyer, as and when we add them I imagine we'll need to take a fine-tooth comb to the existing relation code anyway, so I probably shouldn't worry *too* much about them now | 17:25 |
| niemeyer | fwereade: Agreed, not too much | 17:26 |
| niemeyer | fwereade: The sensible and simple will likely be a good step no matter what we need in the future | 17:26 |
| fwereade | niemeyer, as always :) | 17:27 |
| fwereade | niemeyer, I've re-proposed https://codereview.appspot.com/6303060/ | 17:27 |
| niemeyer | fwereade: +1 :) | 17:27 |
| niemeyer | fwereade: Thanks a lot | 17:27 |
| fwereade | niemeyer, it's much smaller than it looks really | 17:28 |
| fwereade | niemeyer, and I'm not quite sure ServiceRelation is sane, but fixing that is definitely a job for another CL | 17:28 |
| niemeyer | fwereade: +1 | 17:28 |
| niemeyer | fwereade: Wow, 6 days | 17:29 |
| niemeyer | fwereade: Was it in the review list? | 17:29 |
| fwereade | niemeyer, I took it out | 17:29 |
| niemeyer | fwereade: Aha, phew, ok | 17:29 |
| niemeyer | I'm not crazy then | 17:29 |
| fwereade | niemeyer, that pipeline was going in a bad direction, but I think I've found the right one now | 17:30 |
| niemeyer | fwereade: Superb | 17:30 |
| fwereade | off for now, gn all | 17:33 |
| niemeyer | fwereade: Night man | 17:34 |
| TheMue | fwereade: Bye | 17:40 |
| niemeyer | robbiew: call time? | 18:29 |
| niemeyer | Maybe not.. :) | 18:34 |
| robbiew | niemeyer: yep..just running late | 18:37 |
| robbiew | niemeyer: still around? | 18:39 |
| niemeyer | robbiew: Yo | 18:39 |
| niemeyer | Creepy.. the phone rings with the hangout before the laptop sees it | 18:41 |
| === kapilt is now known as hazmat | ||
| * niemeyer steps out for a nice coffee break | 20:29 | |
| niemeyer | Back later for more reviewing | 20:29 |
| fwereade | rogpeppe, ping | 21:34 |
| fwereade | rogpeppe, if you happen to get this, please let me know what the purpose of the unused state.Unit.isPrincipal field is :) | 21:36 |
| niemeyer | fwereade: Tells if the unit is a principal unit or not? | 22:51 |
| niemeyer | davecheney: Morning | 23:02 |
| davecheney | niemeyer: hello | 23:02 |
| niemeyer | davecheney: Sorry for not having gotten through your branches yet | 23:02 |
| davecheney | niemeyer: that is ok, i know everyone is busy | 23:03 |
| niemeyer | davecheney: I have a meeting right now, but will still try to clean things up a bit tonight | 23:03 |
| davecheney | niemeyer: for f in $(seq 1 5); do go test launchpad.net/juju-core/juju/store ; done | 23:03 |
| davecheney | ^ fails 1 in 5 times for e | 23:03 |
| davecheney | me | 23:03 |
| niemeyer | davecheney: Okay, always the same test I suppose | 23:03 |
| davecheney | [LOG] 35.45727 Socket 0xf841bf5900 to 127.0.0.1:50017: killed again: read tcp 127.0.0.1:50017: use of closed network connection (previously: Closed explicitly) | 23:04 |
| davecheney | OOPS: 25 passed, 1 FAILED | 23:04 |
| davecheney | yup | 23:04 |
| davecheney | actually it's, c.Errorf("counter sum for %#v is %d, want %d", key, sum, expected) | 23:04 |
| davecheney | ... Error: counter sum for []string{"charm-info", "oneiric", "wordpress"} is 0, want 1 | 23:04 |
| niemeyer | davecheney: Okay, that's shaky | 23:06 |
| niemeyer | davecheney: Knowingly shaky, that is | 23:06 |
| davecheney | yeah, i didn't think it was a big deal, i just rerun it | 23:06 |
| niemeyer | davecheney: Should definitely fix it, though | 23:29 |
| niemeyer | davecheney: WIll have a look it | 23:30 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!