=== fuzzy is now known as Ponyo === kadams54 is now known as kadams54-away [03:40] wallyworld_: does health/qos cover surfacing status messages from charms? e.g. "waiting for storage X"? [03:41] axw: yep [03:41] cool [03:41] there will be a juju-status hook tool === kadams54 is now known as kadams54-away [04:31] wallyworld_: thanks for merging that fix [04:31] np, thanks for fixing [04:31] turned out to be a simpl eone === wwitzel31 is now known as wwitzel3 [07:34] fwereade: around? === ashipika1 is now known as ashipika [08:08] fwereade: ping === liam_ is now known as Guest79610 [08:39] wallyworld_, oops, sorry [08:39] wallyworld_, how's it going? [08:40] fwereade: hi, am late for dinner, got a few things to discuss, can i ping ypu later? [08:40] wallyworld_, ofc [08:40] np, gotta run [08:40] ttyl [08:40] wallyworld_, oh crap I forgot we had one scheduled this morning [08:41] wallyworld_, went out for coffee with cath [08:43] morning folks === liam_ is now known as Guest73903 [10:01] TheMue, standup? === AndChat|9081 is now known as bogdanteleaga [10:42] rogpeppe1, do you know: what's the current state of the art on making real charm directories to test with? [10:43] rogpeppe1, offhand I can think of a disturbingly large number of places with roughly connected functionality [10:43] fwereade: i was chatting about this with menno yesterday [10:43] fwereade: we had a couple of thoughts [10:44] fwereade: 1) having lots of things depending on the one single testing charm repo is Bad [10:44] rogpeppe1, +1 [10:44] fwereade: 2) quite a few places would probably be better off just making in-memory charms rather than relying on a charm repo on disk [10:45] rogpeppe1, agreed up to a point, but I'm currently thinking about needing actual charms on disk -- those cases do exist [10:46] fwereade: i think that a reasonable solution to that is to make it easy to create an in-memory charm that's also straightforward to expand to disk if needed [10:46] fwereade: the most straightforward thing is just to provide a straightforward way to make a *charm.CharmArchive [10:47] too many "straightforwards" :) [10:47] rogpeppe1, (oh yeah, I was meaning to ask, why the stuttering?) [10:47] rogpeppe1, (vs *charm.Archive?) [10:48] rogpeppe1, (not that that's anything but a derail) [10:48] fwereade: because its symmetric with charm.BundleArchive [10:48] fwereade: we could probably have gone with charm.Archive and charm.BundleArchive actually [10:48] fwereade: but the shed has already been painted [10:48] rogpeppe1, yeah, I assumed there was a reason, just wanted to know what it was [10:49] rogpeppe1: I don't think you were talking to me about this... davecheney maybe? [10:50] rogpeppe1, fwiw, reading back, I'm not so much thinking of a charm archive on disk, I'm thinking of custom-built charms for particular test cases [10:50] rogpeppe1, and I feel like I've seen/written related code several times over in the past [10:51] mjs0: oops, it was bodie, sorry [10:51] rogpeppe1, and weighing up the cost/benefit of writing yet more vs consolidating it a bit and what I can actually afford to spend to get those benefits [10:51] rogpeppe1: no worries [10:51] :) [10:51] fwereade: i'm also thinking of that. but you said that we needed actual charms on disk [10:52] rogpeppe1, well, I need stuff on the filesystem to run the tests against, that doesn't automatically imply a repo [10:52] rogpeppe1, and a *general* in-memory charm representation makes me a bit nervous [10:52] rogpeppe1, will get very big/complex to handle all the cases wecurrently worry about [10:52] fwereade: we already have one [10:53] fwereade: it's called CharmArchive :) [10:53] rogpeppe1, heh, indeed [10:53] rogpeppe1, but charm archive creation is not exactly optimised for testing usage [10:53] fwereade: definitely not [10:54] fwereade: which is why i was thinking of a testing helper function to do that [10:54] fwereade: it wouldn't need to touch disk [10:54] rogpeppe1, yeah, and that's what I worry will get super-complex, and will take me a couple of days to survey the field well enough to even get a handle on the true size of the task [10:55] fwereade: well, if you don't need any actual working hook contents, it would be very easy [10:55] rogpeppe1, is there much reason to create an archive when it won't need to ever hit the disk? [10:55] fwereade: so we can just use *charm.CharmArchive... [10:55] rogpeppe1, yeah, I'm all involved with the uniter again, so I care more about what lands on disk than anything else [10:56] rogpeppe1, but an archive is frequently a step too far [10:56] rogpeppe1, all I need is the dir [10:56] fwereade: an archive is a good first step - if you've got an archive, you can trivially call ExpandTo [10:57] fwereade: but there are many cases (most cases) where we don't need or want anything on disk [10:57] rogpeppe1, granted [10:57] fwereade: and creating a bundle of files and directories is almost certainly going to be more expensive than creating a zip archive (which doesn't even need to do compression if we don't want) [10:58] rogpeppe1, the way I see it there are 3 cases [10:58] fwereade: re: my first point, i'm considering deleting the testing.Charms variable [10:58] rogpeppe1, one, we just need to implement the interface, definitely no need to put it on disk [10:58] fwereade: and replacing it with: [10:58] // NewRepo returns a new testing charm repository [10:58] // rooted at the given path, relative to the package directory of [10:58] // the calling package. [10:58] func NewRepo(path, defaultSeries string) *Repo { [10:59] rogpeppe1, stopping talking, listening to you instead [10:59] fwereade: the interface is not enough [10:59] fwereade: because it doesn't cover anything other than metadata and config (and actions) [10:59] rogpeppe1, it's enough for state tests, surely? [10:59] rogpeppe1, and metrics [10:59] fwereade: indeed [10:59] fwereade: i'm... not sure [11:00] rogpeppe1, but those are all we need in certain situations, aren't they? [11:00] fwereade: probably [11:00] rogpeppe1, I accept it's useless for most of what you need to do [11:00] rogpeppe1, where you care about content [11:00] fwereade: it depends whether it's worth having two parallel mechanisms, one without files and one with [11:01] rogpeppe1, but have no reason to risk the involvement of spinning rust in your tests [11:01] fwereade: i agree [11:01] fwereade: but there's no reason why creating an archive needs to involve spinning rust [11:02] rogpeppe1, agreed [11:02] rogpeppe1, but I don't see *that* many more cases of ArchiveTo(f) than ArchiveTo(buf) [11:02] rogpeppe1, although [11:02] * rogpeppe1 goes to look at the state tests === rogpeppe1 is now known as rogpeppe [11:03] rogpeppe1, yes, it's only Dirs that have that [11:03] bah [11:03] ignore me [11:03] fwereade: yeah, look for CharmDir [11:05] fwereade: free? [11:05] wallyworld_, sure [11:05] rogpeppe, thanks, will think more [11:06] rogpeppe, and probably talk to bodie when he comes on [11:06] wallyworld_, joining our 1:1 [11:11] fwereade: the hamster died :-) [11:21] wallyworld_, I can hear yu, guess you can't me?> [11:21] no:-( let me check my end [11:22] wallyworld_, ha, tab actually crashed [11:27] wallyworld_, ffs think I'm gone again, brb [11:27] fwereade: cooome baaaack, i didn't mean it [11:27] * fwereade waits for sound to start... [11:28] * fwereade sighs, grumbles [11:29] * wallyworld_ taps fingers waiting for hamster to start running === liam_ is now known as Guest35660 [11:58] dimitern: ping [11:58] voidspace, pong [11:59] dimitern: this is how goamz configures the response for the test server for calls to AssignPrivateIPAddresses [11:59] dimitern: http://pastebin.ubuntu.com/8817898/ [11:59] dimitern: is it ok to do the same or do we have a different abstraction? [11:59] dimitern: it configures the xml response directly [12:00] voidspace, looking [12:00] voidspace, that's it yes - I suppose it was made like this to allow greater flexibility while testing [12:00] dimitern: we have some tests like this in cmd/juju/publish_test.go [12:00] and a couple more [12:01] but not many that directly configure responses [12:01] dimitern: I'll do this :-) [12:01] dimitern: if someone objects they can suggest a better way in review [12:01] voidspace, the publish tests are not ec2-specific [12:02] dimitern: I'm fine with it, it's a bit "low level" [12:02] ah, interesting [12:02] voidspace, it's another testing server afaics [12:02] what is gitjujutesting.Server then [12:02] checking [12:02] voidspace, well, hide it in a helper :) [12:02] well, indeed [12:03] voidspace, I think it's a simple http server [12:03] just reusing the same api [12:03] dimitern: it is [12:03] dimitern: NewHTTPServer [12:03] dimitern: that's probably what is under the hood for the goamz test server [12:04] voidspace, possibly quite similar, but the goamz one has a few ec2-specific helpers I think [12:04] right [12:05] dimitern: anyway, I'll procede in this direction... [12:05] ah dammit [12:05] I need to configure a call to "Instances" first to return the network interface [12:06] we need the network interface id before we can ask for the ip address [12:06] never mind, it's only more xml [12:08] maybe I can add a network interface to the instance via goamz instead [12:15] voidspace, yes, you can [12:15] dimitern: it's a call to CreateNetworkInterface then AttachNetworkInterface and I need an ec2 instance [12:15] dimitern: probably still less opaque than the xml [12:15] voidspace, in fact I did modify the goamz test server to create a network interface when you run an instance [12:15] dimitern: but it didn't get merged yet? [12:16] dimitern: that would be helpful as it matches the default behaviour [12:17] voidspace, check out TestRunInstancesVPCCreatesDefaultNICWithoutSubnetIdOrNICs [12:18] dimitern: thanks [12:19] voidspace, look also at ec2test.Server.addDefaultNIC and how it's used [12:20] dimitern: private method though [12:21] and needs a subnet [12:21] so I'll still have to do it somewhat manually [12:21] but these examples show how [12:21] voidspace, I didn't mean to call it directly, it'll get called if you specify a SubnetID in RunInstanceParams [12:22] ah right [12:22] maybe we can just add that to our testcase setup [12:22] voidspace, it's meant to simulate what the real ec2 api does [12:22] as the other tests shouldn't care about a subnet [12:24] dimitern: cool, thanks for your help [12:27] voidspace, np, it's a bit messy to test, but ping me if you run into something else === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 [12:56] trivial update anyone - this fixes juju-dev so that the tests can work under the upcoming Go 1.4: https://github.com/juju/juju/pull/1034 [12:57] dimitern, voidspace: ^ [12:58] dimitern: so the trouble with having goamz create the network interface is that I need an EC2 instance, so I need the auth credentials [12:58] rogpeppe: that revision of testing has already been reviewed? [12:58] voidspace: yes [12:59] voidspace: (otherwise it wouldn't have been landed) [12:59] voidspace, I'm not sure I follow.. [12:59] rogpeppe, looking [12:59] voidspace: currently if you use jc.DeepEquals under Go1.4, it will panic [13:00] dimitern: to call RunInstance or anyother ec2 api directly from goamz (which I'll need to do to setup the network interface) [13:00] dimitern: I need an ec2.EC2 instance [13:00] dimitern: and creating one of those requires auth credentials [13:00] rogpeppe: LGTM [13:00] rogpeppe: :-) [13:00] voidspace: ta [13:00] voidspace: where are you doing this from? [13:01] rogpeppe, it would've been nicer to add a comment to the change in juju/testing :) [13:01] rogpeppe: provider/ec2/local_test.go [13:01] voidspace: you're calling ec2 apis from the tests? [13:01] rogpeppe: a new method finds the network interface from the ec2 instance and then uses the id of that to provide a new static ip address [13:01] rogpeppe: into the test server... [13:02] voidspace, just a sec [13:02] voidspace: the tests have auth creds [13:02] rogpeppe: I need to configure the response to Instances to have a network interface [13:02] rogpeppe: where? [13:04] voidspace: see the EnvironEC2 function [13:05] dimitern: i'm not sure what you mean. i did add a relevant comment in github.com/juju/testing/checkers [13:05] rogpeppe: ah, cool [13:05] rogpeppe: he meant as a comment on the PR linking to the change [13:06] rogpeppe, no I meant to add a comment on the #1034 PR with a link to the other PR in juju/testing, which introduced the change [13:06] voidspace, sorry, so you can do that, but you need to modify how runInstances is called in provider/ec2/ [13:07] dimitern: ah, ok [13:07] dimitern: will do [13:07] dimitern: so modify runInstances to provide the subnet id? [13:07] dimitern: so that the goamz test server will do the right thing? [13:07] voidspace, if you call it and set SubnetID in ec2.RunInstances struct, it will use the VPC-aware version and create a NIC (both the real ec2 and the test server) [13:07] voidspace, exactly :) [13:07] dimitern: i've added a link [13:08] dimitern: LGTY now? [13:08] dimitern: that sounds dangerous.... [13:08] voidspace, but, please be aware the subnets are linked to AZs and this has to be done carefully, so not to break axw's AZ distribution code [13:08] dimitern: exactly... [13:09] dimitern: so then I have to test that change - which is a deeper rat-hole to get into [13:09] rogpeppe, LGTM, thanks! [13:09] dimitern: maybe I'll try the XML in a helper first... [13:10] dimitern: or I can move the test to be in ec2_test.go (white box) and have access to Environ.EnvironEC2 function (exported in export_test) [13:10] voidspace, maybe you can propose a pre-req that just tests setting an AZ (or picking one automatically with the distribution code) also sets SubnetID in RunInstances? [13:11] voidspace, that works for me, but please add a comment [13:12] dimitern: moving out of localServerSuite loses me a lot of setup though, so that's not ideal either :-) [13:12] the black box test is a bare Suite [13:12] voidspace, hmm.. that's a bit unfortunate [13:13] dimitern: messing with the way we create ec2 instances just for testability sounds more dangerous than I'd like to venture into [13:13] dimitern: so I'll check the XML route - it's easy enough to try it [13:15] voidspace, how about using ec2.InstanceEC2(inst *instance.Instance) to get access to the underlying instance struct? [13:17] dimitern: that might work [13:17] voidspace, or t.srv.ec2srv.Instance(strId) - there seem to be a few useful examples in provider/ec2/local_test.go [13:17] yeah [13:18] dimitern: if adding a network interface to that struct works then that could be it [13:18] it has the NetworkInterfaces slot [13:18] thanks [13:19] lunch first [13:20] voidspace, that'll also work, but it's not the same path we'd take eventually (i.e. we'll rely on the automatic NIC creation); but I'm fine with it as a workaround for testing (+please add a comment if you do this) [13:21] dimitern: ok === kadams54 is now known as kadams54-away [13:22] dimitern: if we go the route of setting SubnetID in RunInstances I'd rather pair on it [13:22] dimitern: I'd be very concerned about the intricacies of that [13:22] voidspace, sure, it's perhaps best [13:23] dimitern: and as we don't *need* it yet, it seems wiser to avoid it for now [13:23] dimitern: I'll add the comment that creating a default network interface for testing is a temporary measure [13:24] voidspace, +1 === liam_ is now known as Guest73986 === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: None [14:59] natefinch, jam, mgz: Do you have a few minutes to review https://github.com/juju/juju/pull/1035 [15:00] sinzui: lgtm [15:01] we also want to create the branch now? [15:01] mgz ...already done and... [15:03] mgz, https://github.com/juju/juju/pull/1036 [15:04] natefinch: omw, having plugin issues [15:05] sinzui: also lgtm. trying to remember if there's anything else that needs to be done for new branches... I think the landing bot just works? [15:05] mgz, abentley is adding the new branch to the ci-director cron job now [15:06] mgz, we will see if the lander sees my $$merge$$ in a few minutes [15:11] sinzui, mgz: done. [15:12] bugger. I think I need to prepare a vivid source package too [15:21] * sinzui polls mail to see vivid accepted [15:31] davechen1y: ty for the reviews [15:32] mgz, can you send an email to the juju-devel list explaining master is 1.22-alpha1 and we have a new stable branch 1.21 at 1.21-beta1...merges still in progress of course [15:33] sinzui: sure thing [15:33] what are we expecting people to want to land on 1.21? [15:33] I guess I can just link the launchpad milestone pages? [15:35] he there mgz [15:35] hey alexis [15:35] I see you but you are silent :) [15:35] argh, ahngouts again [15:35] yes they can be fickle things [15:36] sec, going chrome [15:50] fwereade: ping [15:50] ericsnow, pong [15:51] fwereade: do you have any reservations with making --download the default bahavior for "juju backups create"? [15:52] ericsnow, I'm heartily +1 [15:52] fwereade: cool [15:53] ericsnow, if the backup never makes it off the system being backed up, it ain't much of a backup [15:53] fwereade: :) [15:53] fwereade: thank you for the review; i'll be revising after i land some PRs [15:53] katco, cool [15:53] ericsnow, fwereade: now the problem is, what do you call the flag to not download? [15:54] natefinch: --no-download [15:54] natefinch, ericsnow: --eatmydata? ;p [15:54] haha [15:55] :) [15:56] can you print out a big warning when someone uses whatever nodownload flag, that says something like "Your backup will be stored on the server. This is NOT a long term solution and the backup should be downloaded as soon as possible." [15:58] do we have any util library to perform slice-item deletions? [15:59] natefinch: yep [16:00] katco: meh... a := append(a[:i],a[i+1:]...) [16:00] er = not := [16:00] natefinch: boundary edge cases =/ [16:01] what if i is 0, or len(a)-1 [16:01] clutters the clarity of the code =| [16:23] natefinch: bah, so i always forget this. after you reduce down all the code branches, you only really need 1 bounds check. so not so cluttered :) [16:24] :) [16:28] going to pick up my Lily from preschool and then off to vote. [16:29] voted this morning! [16:29] happy election day :) [16:30] katco: cool. Happy election day :) === natefinch is now known as natefinch-vote [16:31] niemeyer: ping [16:31] voidspace: Hey [16:32] niemeyer: hey, hi [16:32] niemeyer: I have code calling EC2.AssignPrivateIPAddresses [16:32] niemeyer: I'd like to be able to configure the response of the test server [16:33] niemeyer: I'd like it to fail with InvalidParameterValue and PrivateIPAddressLimitExceeded [16:34] niemeyer: as far as I can tell I can't hack the test server to return specific errors for calls, without the test server itself being coded to produce those responses [16:34] niemeyer: is that correct? [16:34] niemeyer: (goamz just in case that wasn't completely clear) [16:35] voidspace: I'm not sure.. it's been a while since I looked at that code, but if there's no public API for doing that, it should be correct [16:35] ok [16:36] niemeyer: and to the best of your knowledge, if the test server implements an api (for example there's specific code for AssignPrivateIPAddresses) [16:36] niemeyer: the test server *should* modify the instances it holds to reflect the actions [16:37] I haven't dug into that code yet, I'm about to - but on my first cut I wasn't seeing the IP address added to the instance after a call to AssignPrivateIPAddresses [16:37] voidspace: Yes, it should be realistic [16:37] cool, so it should be added and it's probably my fault that I can't see it yet [16:37] voidspace: Erring on the side of the problematic behavior, when there's choice [16:37] cool [16:37] niemeyer: right [16:37] voidspace: E.g. if a response may come without an IP address on the first try, it should force the requester to retry [16:38] ah right, interesting [16:38] in our case we're providing the IP address to assign [16:38] we're managing them ourselves [16:38] as the actual ec2 api doesn't tell you which one it assigns if it does succeed - so you have to work it out [16:39] and that's inherently racy if multiple addresses might be being assigned - so instead we'll manage them ourselves [16:49] voidspace: Right, so the test server should mimic that problematic behavior [16:49] voidspace: Forcing implementations to do the right thing [16:53] fwereade: https://github.com/juju/charm/pull/73 is a start on one of the points i mentioned earlier [16:54] bodie_: you might want to take a look too [16:57] rogpeppe, will do. was just discussing some charm testing needs with fwereade a little while ago [16:58] bodie_: i have a plan to add some more stuff above this, to address custom charms that don't want to come from disk === kadams54 is now known as kadams54-away === kadams54-away is now known as kadams54 [18:24] g'night all === natefinch-vote is now known as natefinch [18:26] natefinch, ping? [18:26] mattyw: yo [18:27] natefinch, I picked your name at random - congratulations... Who's a good person to talk to about the manual provider? [18:27] I have some knowledge of it [18:41] folks, I'm calling it a day, see you all tomorrow [18:55] natefinch: how's it looking for those 2 reviews? [18:55] ericsnow: I'm looking at the other one now, fyi [18:55] wwitzel3: thanks! [18:56] ericsnow: sorry, busy day today with voting and stuff. I'm working on it, honest :) [18:56] natefinch: :) [18:57] natefinch: FYI, I also have a patch up now for download-by-default [19:22] is it just me, or is it impossible to actually tell what fixes addressed what comments in a review? [19:32] natefinch: not just you [19:32] wwitzel3, hey, re http://reviews.vapour.ws/r/318/ [19:33] wwitzel3, I'm suggesting that juju-run should accept the same syntax for relations as do the various hook tools [19:34] ericsnow: can you help me understand the reviews I'm re-reviewing? If you can show me where to look, I can better understand what I'm looking at. [19:34] wwitzel3, it is interesting to note that we would accept `-r 21`, or `-r db:21` or, in fact, `-r lolwhatever:21` and all would be accepted and handled as "21" [19:34] natefinch: sure [19:35] fwereade: right, but I but isn't that translastion going to be handled by "juju run" [19:35] ericsnow: like, basically, what comments were left that you addressed that need to be approved? [19:35] ericsnow: I assume the people who reviewed before did a good job, I just want to approve whatever hasn't been reviewed [19:36] natefinch: I'm not exactly sure; they were pretty thorough [19:36] wwitzel3, well, as a charm author I'm likely to want to invoke juju-run in whatever way I'm used to invoking relation-get [19:36] wwitzel3, ie with "-r db:21" [19:36] natefinch: I addressed everything that came up [19:36] wwitzel3, and not to support that syntax feels like we're missing a trick [19:37] fwereade: ok, I see what you mean now by "ignorebit:", I was really confused before. [19:38] fwereade: you mean .. -r ignoredbit:21 [19:38] fwereade: yeah, we can just reuse the newRelationIdValue as you suggested [19:38] wwitzel3, cool [19:38] need to take my dog to the vet (UTI), bbiab [19:38] ericsnow: if you feel you addressed all the problems that were mentioned, and that there's unlikely to be anything outstanding, I can rubber stamp it for you. [19:39] natefinch: fine with me :) [19:39] fwereade: also when you say, make jujuc accept --relation that's different from cmd/juju right? [19:40] fwereade: you mean uniter/jujuc right? [19:40] natefinch: thanks [19:41] wwitzel3, yeah, exactly, I'm suggesting that if we're having --relation, or --relation-id in juju-run, it would equally be nice to accept the same spelling in the various uniter/jujuc commands [19:41] fwereade: .. where would they be needed in jujuc? [19:42] holy nested if clauses, batman! [19:44] wwitzel3, relation-list, relation-get, relation-set [19:44] wwitzel3, just search the tests for "-r" but I think that should be all [19:45] fwereade: wait, that doesn't make sense to me .. why would you pass in a relation to relation-list? [19:46] wwitzel3, relation-list is "tell me the units in some relation" [19:46] wwitzel3, relation-ids is "tell me what relations use this endpoint" [19:46] wwitzel3, which indeed returns relation ids ratherthan accepting them [19:49] fwereade: wait, so you are just saying make it accept -r / --relation (mapped to the same input) [19:49] wwitzel3, yeah, exactly [19:49] fwereade: no that it should accept another relation .. oh ok [19:49] fwereade: phew, I thought you'd gone mad, turns out it was just me ;) [19:49] wwitzel3, just make juju-run and juju consistent wrt how we specify relations [19:49] wwitzel3, jolly good ;p [19:50] s/ juju / jujuc / [19:50] fwereade: yep, that all makes sense, will get that taken care of now, thanks [19:50] wwitzel3, cool, thanks [19:54] morning all [20:03] morning waigani [20:03] alexisb: morning :) === mjs0 is now known as menn0 [20:05] davechen1y, mwhudson, waigani: morning all [20:06] Anyone need a review? It's sorta hard to understand what reviews still need attention [20:09] natefinch: I normally start with anything that doesn't have a Ship it or issues against it (i.e. nothing in the 3rd column of the dashboard) [20:15] natefinch: did you take a look at http://reviews.vapour.ws/r/268/? === kadams54 is now known as kadams54-away [20:22] ericsnow: no, I can look at it though. There were just already a lot of comments, which makes it really hard to review [20:23] natefinch: yeah, sorry, feel free to pass--I'll hit up menn0 and axw :) [20:36] natefinch: http://reviews.vapour.ws/r/331/ if you have a moment [20:39] davechen1y: looking [20:50] davechen1y: gah, the problem with multiple repos... the changes here reflect changes in some other repo I can't see in the review. I presume the changes in SpecializeCharmRepo are the fix for the data race you're talking about? [20:55] menn0: as a background task, if you could look at bug 1389389 that would be awesome; it raises it's head semi regularly [20:55] Bug #1389389: TestUpgradeStepsFailure intermittent test failure [20:56] natefinch: while i'm whining about tests - any progress on fixing the ^@#!@$!@$ replicaset ones? [20:56] wallyworld_: will do [20:56] ty [21:05] wallyworld_: no, sorry. I haven't spent time on it recently. I'll try to find time to do so, I know it's long overdue. [21:06] natefinch: np, thank you. we still get many landing failures, and replicaset tests are one of the main causes. there are a few others as well, i'm on the case for those also :-) [21:14] katco: did you get the feedback and/or +1 you needed to unblock? [21:14] wallyworld_: i did... reviewing feedback now [21:14] great [21:14] wallyworld_: thank you :) [21:15] katco: np, i talked with wlliam last night, he said to shout at him if you didn't get the info, so i can now put away my megaphone [21:16] wallyworld_: haha [21:17] wallyworld_: good news is that there are no major disagreements, so i can start implementing while we iterate to an agreeable state. [21:18] whoohoo [21:18] the approach to do the remote api first was well received [21:18] cool, good idea [21:19] it will be the oprah of leadership services. "you get leadership! and you get leadership!" [21:20] that's one of my favorite memes lately. Just so useful in so many situations. You get ebola, and YOU get ebola! etc :) [21:20] haha [21:21] https://www.youtube.com/watch?v=xAhuSDRIDHE [21:21] (NB: you won't really get ebola) [22:12] perrito666: FWIW, I recognize how complex the restore process is :) === kadams54-away is now known as kadams54 === kadams54 is now known as kadams54-away [22:23] what is dbFixer? [22:23] perrito666: just what I called a hypothetical type that hides away the state-dependent stuff going on there [22:23] * perrito666 forbids himself to code because its late in the night here and he knows bugs are going to appear [22:23] :) [22:24] ericsnow: you have a subscription to the abstraction of the month magazine :p [22:24] dont take this as an offense but sometimes you remind me of the zope code base :) [22:24] perrito666: I'm a contributing writer ;) [22:24] perrito666: those were the good old days... [22:26] * perrito666 tries to make some sense of reiewboard separations [22:26] * mwhudson finally reappears [22:27] ericsnow: so, I have an indecent proposition for you [22:28] perrito666: I'm listening [22:29] mm, nevermind, I was going to propose something but then I decided I am not sure I want to code that :p [22:29] * perrito666 rented a room for OpenStackSummit that has the particularity to have several powerplugs... none of them at powercord distance of the desk on the room [22:30] in the up side, the room was cheap [22:30] ericsnow: so, I believe I can get rid of client call for prepare [22:30] perrito666: cool [22:31] I am pretty sure that Finish is there to stay, since it has to be called with a different client connection [22:31] perrito666: yuck [22:31] ericsnow: well it is bound to be [22:31] ericsnow: I am actually destroying the api server and the state server [22:31] you would expect that connection to fall [22:32] I agree its less than ideal [22:32] but it is also a unique case [22:32] perrito666: Can't client.Restore() create a new client and call Finish on it? [22:32] perrito666: considering it's a unique case [22:33] ericsnow: mm, well client restore finishes with rpc.ErrShutDown [22:33] let me re-read this to make sure I can do it [22:33] k [22:34] mmm, yes, it can, but we would have the methods in the facade [22:34] perrito666: having them in the facade I think is unavoidable but okay [22:35] well if unavoidable, we better consider ok [22:35] or we will end up with an ulcera [22:35] :p [22:35] perrito666: I meant "but not the end of the world" :) [22:40] I am too old for Airbnb rent a room [22:41] they actually require for me to be silent after a certain hour.. in a room with parquet floor and in a house with a toilet that has a grinder (something I never saw before) [22:42] and I am doing my best to avoid typing like a mad man as I usually do because I know I produce a noise similar to a typewriter :p [22:44] ericsnow: btw, so far you are one of the only reviewers that actually uses the multiline selection feature, this makes my life way easier when reading comments [22:45] perrito666: I think people are catching on little by little [22:45] yeah, but every now and then I get acomment on a line that is if err != nil and I already changed that line [22:45] so get it requires some devination === kadams54 is now known as kadams54-away [22:54] ericsnow: question === kadams54-away is now known as kadams54 [22:54] perrito666: yeah === kadams54 is now known as kadams54-away [22:55] backups.NewClient [22:55] how fake can httpApiCloser be? [22:56] * perrito666 is blatantly breaking his no code promise === kadams54-away is now known as kadams54 [22:57] perrito666: in the API client tests we only care that the low-level methods were called properly [22:59] ok, anyway reading at it, it seems to me that Ill need a restore root [22:59] perrito666: so for unit-testing the API client we want to keep that low-level implementation as simple as possible [22:59] perrito666: ah, yep [23:00] ericsnow: word of advice, even if they are not public, when you create an interface (which I actually believe are more cumbersome than useful when not public, at that volume of code) throw a couple of lines of comments [23:01] mostly for the purpose of the extending members [23:02] perrito666: k [23:04] perrito666: I wrote up a basic archive workspace type that should help clean up some of the state/backups.Backups.Restore code (https://github.com/ericsnowcurrently/juju/commit/50623a104438c7c61d6921b9db4873ae5c619522) [23:05] perrito666: OpenFile() could be used to open the agent conf from the archive [23:08] ericsnow: it can actually but for now I care only to have this in a solid enough state to be merged, the rest is just an academic exercise, which I will most likely dedicate to once merged [23:08] nice method btw [23:08] and I agree, that could go in utils/tar [23:08] perrito666: k [23:08] perrito666: yeah, it was just a quick stab at it [23:09] sounds like a useful feature for utils/tar [23:33] ericsnow: question [23:34] perrito666: yeah [23:34] cmd/juju/backups/backups.go:86 why? [23:35] not returning a concrete type is making very difficult to actually use this outside [23:37] perrito666: you mean newAPIClient? what makes it hard to use outside? outside where? [23:38] ericsnow: well I wanted to pass NewAPIClient as a sort of closure but the fact that it returns an APIClient kind of defeats the purpose since I was declaring the function as returning a Client [23:38] I am trying to avoid circular imports here [23:38] still trying to figure out how to actually make client reconnect [23:38] perrito666: what circular imports? [23:41] between cmdbackups and api backups restore, looking for a better solution worry not [23:53] * wallyworld_ types MAAS to make jool's irc client go "bing" [23:53] wallyworld_: fuck off [23:53] lol [23:53] maas [23:53] maas [23:53] bigjools: seen this one? bug 1387262 [23:53] Bug #1387262: bootstrap fails with failure to execute queries before end of atomic block [23:54] make me a copffee and I'll look [23:54] wallyworld_: sorry, just wanted to confirm you said maas? [23:54] ok, you come over here and i will [23:54] lol lolo lol [23:54] might come over this arvo then [23:54] or does it have to be upper-case like this: MAAS? [23:54] wallyworld_: why are we supposed to say maas? [23:55] /o\ [23:55] perrito666: cause bigjools has an irc alert that goes "ping!" whenerver we say MAAS [23:55] perrito666: i think we're supposed to say maas because wallyworld_ is interested in maas or something [23:55] * wallyworld_ can't stop laughing [23:55] either way, maas is a fun word, so (shrug) [23:55] wallyworld_: really just saying maas? [23:55] wallyworld is secretly wanting to work on maas [23:55] yep, just MAAS [23:56] bigjools is a maas -ster of irc [23:56] a maas-ter something or other, for sure [23:57] wallyworld_ is a maasturbator [23:57] what a pleasure test deployments to amazon while on a country with decent connections [23:57] perrito666: where are you? [23:57] wallyworld_: paris [23:57] lucky you! [23:57] my uploads fly [23:58] wish i were there [23:58] wallyworld_: I am here for a juju on windows sprint, think again [23:58] so do i [23:58] perrito666: ah, well, there's good and bad :-) [23:58] lol [23:58] heh yeah [23:59] the thing is, this old lady that rents me a room has a faster internet that my office :p [23:59] or at least is better positioned in terms of topology