| === andrewsmedina_ is now known as andrewsmedina | ||
| niemeyer | fwereade_: Yeah, sorry, I wasn't actually suggesting that | 01:17 |
|---|---|---|
| niemeyer | fwereade_: Just to put it in its own line, out of the block | 01:17 |
| fwereade_ | niemeyer, cool, that was what I took it to mean, but best to check ;) | 01:18 |
| niemeyer | fwereade_: Thanks for checking out :) | 01:21 |
| bigjools | niemeyer or fwereade_, what starts up zookeeper? | 01:47 |
| bigjools | I am still not sure | 01:48 |
| niemeyer | bigjools: I believe it starts itself.. cloud-init installs it | 01:48 |
| niemeyer | bigjools: I might be wrong, though.. the cloud-init setup will tell for sure | 01:48 |
| bigjools | thanks, I was afraid of that | 01:48 |
| bigjools | it's not installed on my testing odev thing :( | 01:48 |
| bigjools | niemeyer: is it triggered via the kickstart file used in the cobbler's juju profile? | 01:52 |
| niemeyer | bigjools: It's triggered via cloud-init | 01:53 |
| bigjools | niemeyer: there's cloud-init stuff at the bottom of the kickstart | 01:53 |
| bigjools | and there's a different profile for non-juju nodes | 01:54 |
| bigjools | just trying to piece this together, it's hard | 01:54 |
| niemeyer | bigjools: Yeah, I can imagine, sorry about that | 02:04 |
| niemeyer | bigjools: The whole cobbler thing, though, was intended to simply run cloud-init, in a way attempting to make it a bit saner | 02:05 |
| bigjools | niemeyer: do you know if it relies on preseed data from juju to install it? | 02:05 |
| niemeyer | bigjools: So most of the magic there is intended to get closer to what EC2 would do | 02:05 |
| niemeyer | bigjools: It relies on the cloud-init user-data which we indeed deliver via kickstart | 02:06 |
| bigjools | ok | 02:06 |
| bigjools | niemeyer: does it do that when bootstrapping? | 02:14 |
| niemeyer | bigjools: Does it do what, more precisely? | 02:15 |
| bigjools | niemeyer: supply the zoopkeeper info to cloud-init for bootstrap | 02:15 |
| bigjools | or is it done later? | 02:15 |
| niemeyer | bigjools: All of the process is a side-effect of "juju bootstrap" | 02:15 |
| bigjools | ok thanks | 02:15 |
| niemeyer | bigjools: np | 02:37 |
| bigjools | niemeyer: ok I think I have it now - smoser changed cloud-init to pull all this from the metadata service instead of passing loads of stuff in the preseed. | 02:38 |
| bigjools | thanks again | 02:38 |
| niemeyer | bigjools: RIght, so that we can stay closer to EC2 and remain working as the logic advances | 02:39 |
| === flaviamissi_ is now known as flaviamissi | ||
| === asavu_ is now known as asavu | ||
| TheMue | morning | 12:43 |
| wrtp | TheMue: hiya | 13:01 |
| niemeyer | Good morning jujuers! | 13:42 |
| wrtp | niemeyer: hiya | 13:43 |
| niemeyer | wrtp: Ho | 13:43 |
| wrtp | niemeyer: i updated lbox, but it didn't seem to make a difference to my checksum error. | 13:43 |
| fwereade_ | heya niemeyer | 13:43 |
| niemeyer | wrtp: Hmm :( | 13:46 |
| niemeyer | fwereade_: Heya! | 13:46 |
| niemeyer | wrtp: What's the output again? | 13:46 |
| wrtp | hmm, it's just gone and worked! | 13:47 |
| niemeyer | wrtp: Hah.. love when that happens :) | 13:47 |
| wrtp | niemeyer: this was the output earlier this morning: error: Failed to send patch set to codereview: can't upload base of environs/ec2/internal_test.go: ERROR: Checksum mismatch. | 13:48 |
| fwereade_ | niemeyer, heh, if anything that sort of thing utterly creeps me out ;) | 13:48 |
| wrtp | niemeyer: oh | 13:48 |
| wrtp | niemeyer: i thought it had worked, but it's still borked | 13:49 |
| wrtp | https://codereview.appspot.com/5796078/diff/1/environs/ec2/ec2.go | 13:49 |
| niemeyer | wrtp: I suggest deleting the CL, removing the reference to it from the description in the merge proposal, and running lbox again | 13:50 |
| wrtp | niemeyer: i did that last night, but i'll try again | 13:50 |
| niemeyer | wrtp: The new lbox might help | 13:50 |
| wrtp | niemeyer: i think it's possible it has something to do with a contents conflict in the history | 13:50 |
| wrtp | niemeyer: will try again | 13:50 |
| wrtp | niemeyer: same error | 13:52 |
| wrtp | niemeyer: error: Failed to send patch set to codereview: can't upload base of environs/ec2/internal_test.go: ERROR: Checksum mismatch. | 13:52 |
| niemeyer | wrtp: What's the new CL? | 13:53 |
| wrtp | niemeyer: it didn't tell me. but... i'll have a look in codereview | 13:53 |
| niemeyer | wrtp: Can you please paste the error then? | 13:53 |
| wrtp | niemeyer: http://codereview.appspot.com/5798076/ | 13:54 |
| wrtp | niemeyer: error: Failed to send patch set to codereview: can't upload base of environs/ec2/internal_test.go: ERROR: Checksum mismatch. | 13:54 |
| niemeyer | wrtp: This CL was not created by the new lbox | 13:55 |
| wrtp | niemeyer: the branch is at lp:~rogpeppe/juju/go-zk-connect if you want to clone it and try for yourself | 13:55 |
| niemeyer | wrtp: Can you please update and try again? | 13:56 |
| wrtp | niemeyer: i did apt-get update, and apt-get install says i've got the newest version | 13:56 |
| wrtp | niemeyer: ok | 13:56 |
| niemeyer | wrtp: Well, your new version is not the new version that fwereade_ installed last night somehow | 13:56 |
| wrtp | niemeyer: running apt-get update again | 13:56 |
| wrtp | (what *is* it doing when it says "Waiting for headers"?) | 13:57 |
| wrtp | still updating... | 13:59 |
| wrtp | niemeyer: it says lbox is already the newest version | 13:59 |
| niemeyer | wrtp: What's the version you've got? | 14:00 |
| wrtp | niemeyer: how do i tell that? | 14:00 |
| niemeyer | wrtp: dpkg -l lbox | 14:00 |
| wrtp | niemeyer: 1.0-42.58.37.1 | 14:00 |
| niemeyer | 37.1 or 37.10? | 14:01 |
| wrtp | 37.1 | 14:01 |
| wrtp | niemeyer: i could try go getting it. | 14:02 |
| niemeyer | wrtp: What does "which lpad" tell you? | 14:02 |
| niemeyer | Erm | 14:02 |
| niemeyer | wrtp: What does "which lbox" tell you? | 14:02 |
| wrtp | niemeyer: bingo! | 14:02 |
| niemeyer | wrtp: Are you using a locally installed one? | 14:02 |
| wrtp | niemeyer: i'd done go get lbox before i guesws | 14:02 |
| niemeyer | wrtp: Aha, cool | 14:03 |
| niemeyer | wrtp: Not sure if this will really resolved the issue, though.. but let's see | 14:04 |
| wrtp | niemeyer: sorry, same error | 14:06 |
| niemeyer | wrtp: Can you please paste the command output again? | 14:06 |
| wrtp | niemeyer: do you want the verbose version? | 14:07 |
| niemeyer | wrtp: Yeah, that'd be helpful | 14:07 |
| niemeyer | wrtp: Please let me know of the final CL as well | 14:07 |
| wrtp | niemeyer: here's the CL it created BTW | 14:07 |
| wrtp | http://codereview.appspot.com/5795079/ | 14:07 |
| wrtp | although i'm just about to delete that again, so hold on | 14:07 |
| wrtp | niemeyer: http://paste.ubuntu.com/881866/ | 14:09 |
| wrtp | niemeyer: and the CL is this: http://codereview.appspot.com/5797083/ | 14:10 |
| niemeyer | wrtp: Thanks | 14:11 |
| niemeyer | wrtp: When you mentioned there was a conflict in the history, what was that about? | 14:11 |
| wrtp | niemeyer: the file i referred to has been deleted and recreated in the branch history | 14:12 |
| wrtp | niemeyer: the branch i'm trying to push is not a direct descendant of the branches i've previously pushed, although it's been merged with them. | 14:12 |
| niemeyer | wrtp: Ouch | 14:13 |
| niemeyer | wrtp: Did the file exist at tip? | 14:14 |
| niemeyer | s/Did/does/ | 14:14 |
| wrtp | niemeyer: yes, i think so | 14:14 |
| niemeyer | wrtp: So why was it removed and added? | 14:14 |
| wrtp | niemeyer: at some points in the history, there was no need for any internal tests, hence internal_test.go was removed. | 14:15 |
| niemeyer | wrtp: I see | 14:15 |
| niemeyer | wrtp: Hmm | 14:15 |
| wrtp | niemeyer: if i do a bzr diff, it should internal_test.go being removed and then created again with exactly the same content. | 14:17 |
| wrtp | s/should/shows/ | 14:17 |
| niemeyer | wrtp: Yeah, that means the file was removed and re-added.. it's unfortunate that it loses the whole history when doing that | 14:18 |
| wrtp | niemeyer: yeah, it is | 14:19 |
| wrtp | niemeyer: and history is the reason i'm wanting to submit the branch as is - i have a sentimental attachment to some of the historical pieces... | 14:20 |
| niemeyer | I'm now just wondering if Rietveld itself is breaking down when it notices that the file has the same checksum before and after | 14:20 |
| niemeyer | Ah, maybe not.. | 14:21 |
| niemeyer | wrtp: The file was removed and added in the same commit | 14:28 |
| wrtp | niemeyer: how is that possible? | 14:28 |
| wrtp | niemeyer: i guess i must have done bzr rm and then changed my mind | 14:29 |
| niemeyer | wrtp: I don't know, but that's what history shows | 14:29 |
| niemeyer | removed: | 14:29 |
| niemeyer | environs/ec2/internal_test.go internal_test.go-20120215173103-mc3cbfukg0redhy0-1 | 14:29 |
| niemeyer | added: | 14:29 |
| niemeyer | environs/ec2/internal_test.go internal_test.go-20120222161404-ukvwz2ypn0g6plcc-1 | 14:29 |
| wrtp | niemeyer: interesting question: what's the best way to undo a bzr rm ? | 14:30 |
| niemeyer | wrtp: That's exactly what I'm looking for right now | 14:30 |
| wrtp | niemeyer: most edits between commits are ephemeral... | 14:30 |
| niemeyer | wrtp: This seems to work: % bzr revert -r 101 ./internal_test.go | 14:31 |
| niemeyer | +N environs/ec2/internal_test.go | 14:31 |
| niemeyer | wrtp: Please try to do this: | 14:32 |
| wrtp | niemeyer: i'm trying now | 14:32 |
| niemeyer | wrtp: cp internal_test.go{,.tmp} | 14:32 |
| niemeyer | wrtp: bzr revert -r 101 ./internal_test.go | 14:33 |
| niemeyer | wrtp: mv -f internal_test.go{.tmp,} | 14:33 |
| niemeyer | wrtp: bzr commit | 14:33 |
| niemeyer | Not sure if it'll work, actually | 14:33 |
| niemeyer | Nope | 14:34 |
| wrtp | no indeed | 14:37 |
| wrtp | lunch | 14:43 |
| niemeyer | wrtp: I believe I fixed it | 14:48 |
| niemeyer | wrtp: When you're back, please try to "bzr pull lp:~niemeyer/juju/go-zk-connect-fixup" | 14:49 |
| niemeyer | wrtp: Then, lbox propose | 14:49 |
| niemeyer | wrtp: If that works, I'll explain how I got it fixed, and how to avoid the problem in the future | 14:50 |
| wrtp | niemeyer: bzr: ERROR: These branches have diverged. Use the missing command to see how. | 15:02 |
| wrtp | oops | 15:02 |
| niemeyer | wrtp: I guess you have committed something else in your local branch? | 15:02 |
| wrtp | wrong directory! | 15:02 |
| wrtp | erm | 15:03 |
| wrtp | niemeyer: ah yes, i did a commit to try and solve the problem earlier | 15:04 |
| wrtp | (as suggested) | 15:04 |
| wrtp | i'll uncommit it | 15:04 |
| fwereade_ | gents, I'm really tired; I'll be around again at some stage later today but right now I need a walk and a lie down | 15:05 |
| fwereade_ | laters | 15:05 |
| wrtp | fwereade_: ok. enjoy the walk... | 15:05 |
| wrtp | niemeyer: ok, that worked, thanks! | 15:06 |
| wrtp | niemeyer: so... how did you fix it? | 15:07 |
| niemeyer | fwereade_: Have some good rest man | 15:07 |
| niemeyer | wrtp: In general, reviving a file is trivial: revert -r N file_name and bang.. we have the file back | 15:08 |
| niemeyer | wrtp: The problem in this case, is that the file has been revived with a different id | 15:08 |
| niemeyer | s/revived/added/ | 15:08 |
| niemeyer | wrtp: So when we attempt to "bring it back", bzr actually picks the latest id used for that name | 15:08 |
| niemeyer | wrtp: The trick for getting the real original file back was to create another branch that still had the old file | 15:09 |
| niemeyer | wrtp: Then, remove the file locally, and add it back again with the new content, but using bzr add --file-ids-from $OLD_BRANCH | 15:09 |
| niemeyer | wrtp: This tricked bzr into looking at the original revision | 15:09 |
| wrtp | niemeyer: ah. i had no idea about --file-ids-from | 15:10 |
| wrtp | interesting | 15:10 |
| wrtp | niemeyer: BTW the CL now seems to have a file called "[revision details]" | 15:12 |
| niemeyer | wrtp: Yeah, that was intentional | 15:12 |
| niemeyer | wrtp: this used to be at the head of every diff, but unfortunately this makes Rietveld get lost in terms of diffing the diffs | 15:13 |
| niemeyer | wrtp: Now we'll be able to see which files actually changed between revision, as usual | 15:13 |
| wrtp | niemeyer: i'm not sure i understand - what exactly is that file showing? | 15:15 |
| niemeyer | wrtp: Have you clicked on it? | 15:15 |
| wrtp | niemeyer: yeah. i get two revision ids | 15:15 |
| niemeyer | wrtp: That's what this file is showing :-) | 15:16 |
| wrtp | niemeyer: what do the revision ids refer to? i'd've expected that one would refer to me, given i've created the latest revision in that branch. | 15:16 |
| niemeyer | wrtp: Sorry, I'm not sure I understand your question, since the revision ids are labeled | 15:17 |
| niemeyer | wrtp: They show the old revision id and the new revision id.. | 15:17 |
| wrtp | niemeyer: ah! of course. you created the latest revision! | 15:17 |
| niemeyer | wrtp: :-) | 15:17 |
| niemeyer | wrtp: This information enable any patch set to be recovered exactly as proposed | 15:18 |
| niemeyer | enables | 15:18 |
| wrtp | niemeyer: hmm: | 15:19 |
| wrtp | % bzr diff --old themue@gmail.com-20120312190709-6cki3f36c8clo63t | 15:19 |
| wrtp | % bzr diff --old gustavo@niemeyer.net-20120313144703-jz03znqyo2qt56qu | 15:19 |
| wrtp | % | 15:19 |
| wrtp | or maybe i can't use a revision id as a --old target | 15:19 |
| niemeyer | --old=ARG Branch/tree to compare from. | 15:20 |
| niemeyer | wrtp: you want -r | 15:20 |
| wrtp | i've not used revision ids in bzr before. | 15:20 |
| wrtp | i wonder why it didn't give an error | 15:20 |
| niemeyer | wrtp: Yeah, curious indeed | 15:21 |
| wrtp | niemeyer: ah, this worked: bzr qdiff -r revid:themue@gmail.com-20120312190709-6cki3f36c8clo63t | 15:22 |
| wrtp | i was missing the revid: prefix | 15:22 |
| wrtp | and the -r of course | 15:22 |
| niemeyer | wrtp: bzr diff -r themue@gmail.com-20120312190709-6cki3f36c8clo63t | 15:23 |
| niemeyer | wrtp: This works for me | 15:23 |
| wrtp | niemeyer: so it does. | 15:23 |
| wrtp | (for me too | 15:23 |
| wrtp | ) | 15:23 |
| wrtp | niemeyer: thanks, that's useful. | 15:24 |
| niemeyer | wrtp: np | 15:24 |
| niemeyer | wrtp: I'll see if I take that lbox hackaton and implement the pre-req stuff later | 15:24 |
| niemeyer | after some further reviews | 15:24 |
| * wrtp is hoping to get this branch reviewed today :-) | 15:25 | |
| wrtp | niemeyer: here's the cherry on the cake: https://codereview.appspot.com/5754103 | 15:31 |
| wrtp | it feels very good to have that finally proposed! | 15:34 |
| niemeyer | wrtp: I've delivered a LGTM review | 15:41 |
| wrtp | niemeyer: yay! | 15:41 |
| niemeyer | wrtp: but I'm a bit unsure about what's going on there.. do we really have only a couple of changes to tests in that branch? | 15:42 |
| niemeyer | wrtp: Doesn't seem to match the description | 15:42 |
| wrtp | niemeyer: all it does is connect to the zookeeper that's already started as a result of the userdata changes in the previous branch. | 15:42 |
| niemeyer | wrtp: The description seems bogus then.. you're changing tests only | 15:43 |
| niemeyer | wrtp: It should say something like "Fix lack of tests!" :-) | 15:43 |
| TheMue | niemeyer: next step for the continued machine branch, will now use williams 'presence' for my 'agent' draft. | 15:43 |
| wrtp | hmm. you're right! | 15:43 |
| TheMue | niemeyer: oh, forgot the link, https://codereview.appspot.com/5690051/ | 15:44 |
| niemeyer | wrtp: That TestBootstrap should be reverted too | 15:44 |
| wrtp | niemeyer: TestBootstrap can't work against the ec2test server | 15:44 |
| wrtp | hmm | 15:44 |
| * wrtp goes to look | 15:45 | |
| wrtp | niemeyer: yeah, maybe we should have TestBootstrap and TestBootstrapAndOpen | 15:45 |
| wrtp | niemeyer: and avoid running the latter against ec2test | 15:45 |
| niemeyer | wrtp: It certainly feels pretty bad that we won't be testing all of that logic locally anymore | 15:46 |
| wrtp | niemeyer: that will make the live tests take a lot longer though | 15:46 |
| niemeyer | wrtp: Isn't that the whole purpose of having two different set of tests, one that opens and runs all tests, and one that does not | 15:47 |
| niemeyer | wrtp: I feel like we're getting lost in that mess of suites | 15:47 |
| wrtp | niemeyer: well, the point of the Live tests is that they're designed to be run against amazon. and as an added bonus we can run most of them locally as well. | 15:48 |
| niemeyer | wrtp: That's not what I'm talking about.. we have 4 suites, right? | 15:48 |
| niemeyer | wrtp: Why? | 15:48 |
| wrtp | two cross-provider suites; two ec2-specific suites. | 15:49 |
| wrtp | niemeyer: each for live vs local | 15:49 |
| wrtp | niemeyer: that Bootstrap logic is also tested inside the non-live test suite | 15:51 |
| wrtp | niemeyer: running the live suite locally is just a nice thing to do when we can. but in this case we can't. | 15:51 |
| niemeyer | wrtp: Running the live suite locally is the base strategy we've been using for testing juju's ec2 provider | 15:53 |
| niemeyer | wrtp: If we drop those tests, logic is effectively going untested | 15:53 |
| wrtp | niemeyer: yeah, we do rely on some of those tests, but we don't really want to duplicate the code. | 15:55 |
| wrtp | niemeyer: maybe it would be better the other way around - if LiveTests selectively imported from LocalTests. | 15:56 |
| niemeyer | wrtp: I'm starting to dislike that organization of tests altogether | 15:56 |
| niemeyer | wrtp: We're getting lost very easily | 15:57 |
| wrtp | niemeyer: yeah, it's not ideal. but i think the ideal of having some provider-independent tests is a good one. | 15:57 |
| niemeyer | wrtp: Yeah, maybe we should try to have the zk running in the local tests as well.. | 16:06 |
| niemeyer | wrtp: But we'll face a road block very soon | 16:06 |
| niemeyer | wrtp: So I'm not really sure it's worth it | 16:06 |
| wrtp | niemeyer: yeah, i dunno. | 16:07 |
| niemeyer | wrtp: I think LiveTests should bootstrap just once | 16:08 |
| niemeyer | wrtp: Unless we're testing something that knowingly destroys the environment | 16:08 |
| niemeyer | wrtp: We have to avoid that fear that introducing new tests will slow things down | 16:09 |
| niemeyer | wrtp: Otherwise we'll stop writing tests.. | 16:09 |
| niemeyer | wrtp: So the bootstrap itself would happen at SetUpSuite | 16:09 |
| wrtp | niemeyer: what about the tests that don't require a bootstrapped environment? | 16:09 |
| wrtp | niemeyer: maybe it should happen on demand, but once only. | 16:10 |
| niemeyer | wrtp: Right, that sound snice | 16:11 |
| wrtp | niemeyer: it's a pity in a way that gocheck runs tests in alphabetical order. it would be nice to arrange things so that the weaker tests run first. | 16:12 |
| niemeyer | wrtp: In fact, we already do that.. | 16:12 |
| niemeyer | wrtp: That was the whole idea behind t.Open | 16:12 |
| niemeyer | Somehow we lost it | 16:13 |
| wrtp | niemeyer: Open is quite different from Bootstrap though | 16:13 |
| niemeyer | wrtp: Maybe.. | 16:13 |
| wrtp | niemeyer: but i agree, that kind of thing should work | 16:13 |
| niemeyer | wrtp: I'll have to step out for lunch as Ale is waiting.. let's continue that conversation in 1h | 16:13 |
| wrtp | niemeyer: sounds good | 16:14 |
| robbiew | niemeyer: fyi...movement on the charm store RT...they are requesting a call with you | 16:52 |
| TheMue | fwereade_: arg, thx, when creating a function it indeed should be used. will fix it. | 17:07 |
| fwereade_ | TheMue, cheers | 17:07 |
| niemeyer | robbiew: Aweosme! | 17:27 |
| niemeyer | wrtp: So, tests.. | 17:30 |
| wrtp | niemeyer: yeah | 17:30 |
| wrtp | niemeyer: i've just added a bootstrap flag | 17:30 |
| niemeyer | wrtp: flag? | 17:31 |
| wrtp | niemeyer: so that live tests can call Bootstrap and it'll only bootstrap if another test hasn't already bootstrapped | 17:31 |
| wrtp | niemeyer: a state variable rather than a flag, really | 17:31 |
| niemeyer | wrtp: This is a test-specific detail.. | 17:32 |
| niemeyer | wrtp: It's a problem for the suite to handle | 17:32 |
| wrtp | niemeyer: the variable is inside the suite | 17:32 |
| niemeyer | wrtp: So by "state" you mean "suite"? | 17:32 |
| wrtp | niemeyer: yeah. i meant state in the generic sense of a variable that records the state of something (in this case whether we've bootstrapped already) | 17:33 |
| niemeyer | wrtp: Ah, ok.. we have too many states :_) | 17:33 |
| wrtp | :-) | 17:33 |
| niemeyer | wrtp: I think the method in the suite should be something like BootstrapOnce, to differentiate from the Bootstrap method we already have in the Tests suite | 17:34 |
| wrtp | niemeyer: i'm also adding a flag inside jujutest.LiveTests which indicates whether it's possible to connect to the juju state. | 17:34 |
| niemeyer | wrtp: Ok | 17:34 |
| wrtp | BootstrapOnce sounds good. (although it might do it again if you deliberately call Destroy) | 17:35 |
| wrtp | niemeyer: BTW i'm interested to hear ideas as to how to better structure the test suite. | 17:36 |
| niemeyer | wrtp: Yeah, not sure yet.. the complexity is just a bit overwhelming at the moment | 17:37 |
| niemeyer | wrtp: 2 base suites, 4 suites in ec2, multiple scenarios, etc etc | 17:37 |
| wrtp | niemeyer: yeah. perhaps the multiple scenario stuff should go. | 17:37 |
| wrtp | niemeyer: (although it did catch some useful errors earlier on) | 17:38 |
| wrtp | niemeyer: the other stuff seems difficult to avoid though. | 17:39 |
| niemeyer | wrtp: I think we can just pick the harsher scenario and go with it | 17:40 |
| wrtp | niemeyer: not always easy to know which is "harsher". different scenarios can expose different bugs. | 17:41 |
| wrtp | niemeyer: but i think i'll just go with "extra-instances" | 17:42 |
| niemeyer | wrtp: I find it easy to see which one is likely to yield bugs | 17:43 |
| wrtp | niemeyer: i don't think we care too much if instances come up in initial running state. | 17:43 |
| niemeyer | wrtp: Pre-existing instances is trickier to handle than an empty environment | 17:43 |
| wrtp | niemeyer: yeah, that one's an easy call. | 17:43 |
| niemeyer | wrtp: Handling an instance in an intermediate state before running also is the less trivial path than getting instances running upfront | 17:44 |
| wrtp | niemeyer: another one though, that we might want to do in the future, is eventual consistency. but perhaps we just always test for maximum eventual consistency delay. | 17:44 |
| niemeyer | wrtp: Yeah.. I think we should have a reasonable delay that is likely to yield bugs but does not cause the suite to be extremely boring | 17:44 |
| niemeyer | wrtp: Then, we can have something like Go's -cpu setting for tests | 17:45 |
| niemeyer | wrtp: So that we can tweak that value for specific runs | 17:45 |
| wrtp | niemeyer: seems plausible | 17:45 |
| wrtp | niemeyer: (it'd be nice if gocheck could run tests concurrently BTW) | 17:46 |
| niemeyer | wrtp: Yeah, it will eventually | 17:46 |
| niemeyer | rebooting.. apparently Unity is now fixed and won't pop up the HUD all the time.. Woohay. | 17:51 |
| niemeyer | Heh.. the upgrade *removed* unity.. | 18:03 |
| niemeyer | But the HUD issue is gone indeed | 18:03 |
| wrtp | niemeyer: this is why we need better error messages from state: | 18:03 |
| wrtp | /home/rog/src/go/src/launchpad.net/juju/go/environs/jujutest/livetests.go:72: | 18:03 |
| wrtp | c.Assert(err, IsNil) | 18:03 |
| wrtp | ... value syscall.Errno = 0x2 ("no such file or directory") | 18:03 |
| wrtp | niemeyer: i've gotta go | 18:03 |
| wrtp | niemeyer: PTAL; i haven't had a double check of it, but it might be ok :-) | 18:04 |
| wrtp | https://codereview.appspot.com/5754103 | 18:04 |
| niemeyer | wrtp: This is why we need better *error messages* | 18:04 |
| niemeyer | wrtp: Let's not take the leap from there onto saying we *have* to implement a given approach | 18:04 |
| niemeyer | wrtp: We already discussed improving error messages, and haven't done it yet | 18:04 |
| wrtp | niemeyer: sure. i'm not saying that my suggested approach is the only one that'll work. | 18:05 |
| wrtp | anyway, i'm being shouted for! | 18:05 |
| niemeyer | wrtp: Have fun :) | 18:05 |
| wrtp | niemeyer: see you tomorrow | 18:05 |
| niemeyer | wrtp: These error messages look like an issue in your local setup, btw.. there are no elements of "launchpad.net/goamz/aws" in /home/rog/src/go/src | 18:07 |
| niemeyer | wrtp: Sorry, that was about a pretty old paste you made | 18:08 |
| niemeyer | wrtp: ECONTEXT | 18:08 |
| niemeyer | hazmat: ping | 18:30 |
| hazmat | niemeyer, pong | 18:30 |
| niemeyer | hazmat: Yo | 18:30 |
| niemeyer | hazmat: I'm reviewing the Go branch that implements RemoveMachine in state | 18:30 |
| * hazmat nods | 18:31 | |
| niemeyer | hazmat: I recall we had some good exchanges around the idea that machines shouldn't simply disapear | 18:31 |
| niemeyer | disappear | 18:31 |
| niemeyer | hazmat: Have you ever touched that area in the Python incarnation? | 18:31 |
| hazmat | niemeyer, that api should still be present for the final removal, but the machine agent shouldn't be killed by just removing its state and killing the machine, instead it should have a state communication/coordination around its termination to shutdown graceful before being executed permantently | 18:32 |
| hazmat | ie. we don't kill identity of an actor before it has a chance to shutdown gracefully | 18:32 |
| hazmat | but after it does or a suitable timeout, we do go ahead and remove the state | 18:33 |
| hazmat | so the state api for removal should still be present | 18:33 |
| hazmat | but the api act of stopping a machine uses a state protocol before the provisioning agent uses removemachine | 18:34 |
| niemeyer | hazmat: Yeah, I recall that.. but have you ever implemented anything? | 18:34 |
| hazmat | niemeyer, no.. its the last bug marked critical for 12.04, i'm going to a spec to the list this after i return from pycon sprints tomorrow | 18:35 |
| hazmat | ^get | 18:35 |
| niemeyer | hazmat: Ah, cool | 18:35 |
| hazmat | niemeyer, i've got draft in lp:~hazmat/juju/unit-stop | 18:35 |
| niemeyer | hazmat: RemoveMachine will certainly change.. it has to be split in two | 18:35 |
| hazmat | niemeyer, i figure just a different api ... like stopmachine.. that will in turn remove machine when its done | 18:36 |
| niemeyer | hazmat: "stop machine" implies something else.. "stop" has the "start" counterpart | 18:36 |
| hazmat | true | 18:36 |
| niemeyer | hazmat: Well, I guess we could model it that way even.. | 18:37 |
| niemeyer | hazmat: stop => remove.. or stop => start.. | 18:37 |
| niemeyer | hazmat: How's subordinates going, btw? | 18:42 |
| niemeyer | TheMue: You've got another review | 19:23 |
| TheMue | niemeyer: thx, done a first scan. your comment critics is ok. i'm just used to take them sometimes for optical block building. but i can live w/o them here. | 19:28 |
| niemeyer | TheMue: Thanks! I feel a bit on the other side.. if there's a comment, I'll always read it while skimming through the code.. the fact the comment says nothing is distracting | 19:29 |
| TheMue | niemeyer: i'll remove them and be more keen on better helping comments if i feel they are needed | 19:30 |
| niemeyer | TheMue: Thanks a lot | 19:41 |
| niemeyer | TheMue: On the bright side, there are only trivial details really.. the branch is very nice | 19:41 |
| TheMue | niemeyer: thx, williams argument regarding RemoveMachine() has been good | 19:43 |
| niemeyer | TheMue: Please don't forget to lbox propose again (just got the review DOnes) | 20:01 |
| TheMue | niemeyer: done the propose | 20:02 |
| niemeyer | TheMue: LGTM! | 20:07 |
| niemeyer | WOohay, one more branch.. | 20:07 |
| TheMue | niemeyer: great, thx, will submit it | 20:09 |
| niemeyer | TheMue: Have you done any changes in https://codereview.appspot.com/5782053/ that are ready for review? | 20:17 |
| niemeyer | TheMue: I'm asking only because you mentioned something earlier on | 20:17 |
| niemeyer | TheMue: I know it's late for you, so just wondering if you have something that is already ready | 20:18 |
| TheMue | niemeyer: the proposal works, but currently w/o the presence solution of william. i'll integrate it tomorrow. | 20:20 |
| niemeyer | TheMue: Ok, super | 20:20 |
| TheMue | niemeyer: but for today i say good night ;) | 20:21 |
| niemeyer | TheMue: have a good night! | 20:22 |
| TheMue | niemeyer: thx, cu tomorrow | 20:23 |
| andrewsmedina | niemeyer: hi | 21:15 |
| niemeyer | andrewsmedina: Hey | 21:32 |
| andrewsmedina | niemeyer: tonight I will send another code for review related with lxc | 21:33 |
| andrewsmedina | niemeyer: and I have a doubt | 21:33 |
| niemeyer | andrewsmedina: Sure, what's up? | 21:34 |
| andrewsmedina | niemeyer: I need create another branch for work with the local environment or I can use the same branch that I'm using for lxc? | 21:35 |
| niemeyer | andrewsmedina: One branch, one proposal, one submit | 21:35 |
| andrewsmedina | niemeyer: Will be a proposal for lxc and another for local environment? | 21:36 |
| niemeyer | andrewsmedina: The smaller and more self contained a proposal/branch is, the best for us to review, and the best for you to fix it for inclusion | 21:37 |
| niemeyer | andrewsmedina: Your initial lxc/local branch is great | 21:37 |
| niemeyer | andrewsmedina: But it's pending some fixes | 21:37 |
| andrewsmedina | niemeyer: I know | 21:37 |
| niemeyer | andrewsmedina: It has to be fixed accordingly, and updated ("lbox propose" again) | 21:38 |
| andrewsmedina | niemeyer: II already have fixed that issues, and I add another methods to lxc container type | 21:38 |
| niemeyer | andrewsmedina: Other methods should be in a different proposal | 21:40 |
| niemeyer | andrewsmedina: Once a proposal is made, the issues pointed out should be fixed, so that we can include it | 21:40 |
| andrewsmedina | niemeyer: hm.. | 21:40 |
| niemeyer | andrewsmedina: Otherwise we get into a never-ending cycle | 21:40 |
| andrewsmedina | niemeyer: you're right | 21:41 |
| andrewsmedina | niemeyer: thanks | 21:42 |
| niemeyer | andrewsmedina: Thank you | 21:42 |
| niemeyer | Heading out for dinner with some friends.. back later | 22:19 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!