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