| wallyworld | thumper: shiteveld won't accept by latest diff for the rsyslog upgrader. can you take a peek in lp and see if you are happy? i made a small tweak and responded to comments | 00:03 |
|---|---|---|
| thumper | ok, will look | 00:20 |
| thumper | wallyworld: for your Replace file thing, why not use Stat to get the existing premission, and only use 0644 if you can't get it? | 00:25 |
| wallyworld | thumper: cause we want the new perms to overwite any exisitng ones | 00:26 |
| stokachu | hey, i sent an email to the mailing list but just throwing this here as well, anyone know the latest version of the websocket specification juju api server supports? | 00:26 |
| thumper | wallyworld: but you aren't letting the user specify the permissions | 00:26 |
| thumper | you are explicit in 0644 | 00:26 |
| thumper | which may not be right | 00:26 |
| thumper | stokachu: I don't sorry | 00:26 |
| wallyworld | oh shit, i should have that passed in | 00:27 |
| thumper | wallyworld: if we want to overwrite perms, allow the caller to pass them in | 00:27 |
| thumper | :) | 00:27 |
| stokachu | how do i tell which version of the websocket library is used from go.net/websocket? | 00:27 |
| wallyworld | that's what my brain was thinking but my fingers didn;t type that | 00:27 |
| thumper | wallyworld: I get that all the time too | 00:27 |
| wallyworld | stokachu: i'm not sure. rogpeppe is the best person to ask | 00:28 |
| wallyworld | but he's past EOD now | 00:28 |
| stokachu | ok | 00:28 |
| wallyworld | i'll poke him later to make sure he responds | 00:29 |
| stokachu | wallyworld: thanks a bunch | 00:29 |
| wallyworld | np | 00:29 |
| wallyworld | thumper: fixed | 00:50 |
| thumper | kk | 00:50 |
| thumper | axw: no os.Groups though ... | 01:04 |
| thumper | also easier to mock out in the tests with the command :-) | 01:04 |
| axw | thumper: there's Gid? | 01:04 |
| thumper | but thanks for the pointer | 01:04 |
| thumper | not on a os.User | 01:04 |
| thumper | oh | 01:04 |
| axw | yes there is | 01:04 |
| thumper | so there is | 01:04 |
| axw | :) | 01:04 |
| axw | anyway, not a problem, just wanted to make sure you knew | 01:05 |
| thumper | cheers | 01:05 |
| thumper | os.User.Gid is a string and os.Chown needs an int | 01:05 |
| thumper | lovely library coherency there | 01:05 |
| thumper | axw: I'm happy with https://code.launchpad.net/~wallyworld/juju-core/rsyslogconf-upgrader-118/+merge/206090, you happy? | 01:07 |
| wallyworld | yep, for this iteration | 01:08 |
| axw | thumper: yeah I was gonna LGTM | 01:08 |
| thumper | cool | 01:08 |
| axw | FTR, +1 for a sub package for utils | 01:08 |
| wallyworld | axw: i reverted to using TempDir cause then the file perms work put nicer - TempFile creates the file with 600 and you need more code to change the perms | 01:09 |
| wallyworld | i think that's why it was done that way originally | 01:09 |
| === thumper is now known as thumper-afk | ||
| axw | ok no worries | 01:09 |
| === thumper-afk is now known as thumper | ||
| thumper | axw: got a minute? | 01:46 |
| thumper | bugger | 01:46 |
| thumper | completely missed the standup | 01:46 |
| thumper | what a slacker | 01:47 |
| thumper | oops, and sorry | 01:47 |
| wallyworld | f*cking bot | 01:47 |
| * wallyworld stabs it to death | 01:47 | |
| axw | thumper: yes | 01:49 |
| axw | what's up? | 01:49 |
| thumper | axw: was wondering about bootstrapping prcess with maas | 01:50 |
| thumper | and how the sync bootstrap handles the networking bits in it | 01:50 |
| thumper | particulary how we currently restart networking | 01:50 |
| axw | un moment, gotta refresh memory on maas | 01:50 |
| thumper | I'm likely to change the "edit /etc/network/interfaces, restart networking" | 01:51 |
| thumper | to be "ifdown eth0, edit /etc/network/interfaces, ifup br0" | 01:52 |
| thumper | which I have been told will work, but I need to test it | 01:52 |
| thumper | these are all different lines in the cloud init scripts section | 01:52 |
| thumper | wondering how the sync bootstrap handles this... | 01:52 |
| thumper | wondering idly if this relates to : bug 1233751 | 01:53 |
| _mup_ | Bug #1233751: MAAS environment bootstrapped, but not really. <bootstrap> <destroy-environment> <jenv> <maas-provider> <theme-oil> <juju-core:Triaged> <https://launchpad.net/bugs/1233751> | 01:53 |
| axw | thumper: I think cloud-init will run that bit. still trying to get my head around it... | 01:53 |
| axw | if that's the case it should work no problems | 01:54 |
| axw | thumper: yep, maas's StartInstance does that networking bit, so cloud-init will do the networking change | 01:55 |
| axw | so it won't break the ssh connection | 01:55 |
| * axw looks at the bug | 01:55 | |
| thumper | how do we handle the bootstrapping? | 01:55 |
| thumper | axw: doesn't bootstrap call start instance? | 01:56 |
| axw | thumper: I suspect that bug is just cos the bootstrap-state file got left behind on failed bootstrap | 01:56 |
| axw | yes it does, but there's two parts to synchronous bootstrap | 01:56 |
| axw | part 1: run cloud-init with a very basic cloud-config | 01:56 |
| axw | for maas, there's an additional bit of modifying networking | 01:57 |
| axw | part 2: wait for the machine to come up, ssh in and do the rest | 01:57 |
| thumper | ok, cool, so the hack networking is all part 1? | 01:57 |
| axw | and "do the rest" is provider-independent | 01:57 |
| axw | yep | 01:57 |
| thumper | axw: is the bug then fixed? | 01:57 |
| axw | thumper: dunno sorry | 01:57 |
| thumper | kk, so I'll manually test the ifdown/ifup story locally on my precise maching :) | 01:57 |
| thumper | machine | 01:57 |
| axw | doesn't look related to synchronous bootstrap | 01:58 |
| axw | pre-bootstrap setup I think | 01:58 |
| thumper | ok, but didn't you just fix a bug that deleted the .jenv if the bootstrap fails? | 01:58 |
| axw | oh that's a different file | 01:58 |
| axw | I'm talking about how we leave a file in cloud storage | 01:58 |
| thumper | ah... :( | 01:58 |
| axw | thumper: actually, I'm not sure now. you may get the same error for existing .jenv | 02:09 |
| axw | I would need a NUC or something to test on ;) | 02:09 |
| bigjools | curtis has one IIRC | 02:12 |
| thumper | bigjools: can I test something on your maas? | 02:21 |
| bigjools | maaaaaybe | 02:21 |
| bigjools | it means I have to go and turn it on | 02:22 |
| bigjools | thumper: it's on, can you get in? I can't remember if my router config still works for you | 02:31 |
| thumper | um... | 02:31 |
| bigjools | since you're sshing into a network two NATs deep :) | 02:31 |
| thumper | what was the ip address and port again? | 02:31 |
| * bigjools rolls eyes | 02:31 | |
| === lazyPower_ is now known as lazyPower | ||
| thumper | bigjools: I have auth to go buy some NUCs for maas testing locally | 02:43 |
| thumper | I just need to get my a into g and get some | 02:43 |
| bigjools | thumper: expensed? | 02:43 |
| thumper | aye | 02:43 |
| bigjools | suhweet | 02:43 |
| bigjools | how much in NZ? | 02:43 |
| thumper | not sure | 02:44 |
| thumper | have to go price some up | 02:44 |
| === mwhudson is now known as zz_mwhudson | ||
| * axw goes offline for a while to run tests without a network | 04:10 | |
| axw | wallyworld: FYI I have looked at your upgrader CL. It looks good, but I don't think I understand everything well enough to +1 at the moment | 06:10 |
| wallyworld | axw: that's ok, it was meant for william to look at anyway :-) | 06:11 |
| axw | ok, cool | 06:11 |
| wallyworld | thanks for looking though | 06:12 |
| axw | nps | 06:12 |
| wallyworld | it achieves the same result as a previous version, but with a different approach | 06:12 |
| axw | yup, so I see. I understand the idea and see how it works, just not fully aware of how the watcher stuff works for example | 06:15 |
| wallyworld | it's a bit involved | 06:16 |
| wallyworld | fwereade: no urgency, take 2 - https://codereview.appspot.com/63730043/ | 07:25 |
| wallyworld | the main tests i copied across from the other branch and made then pass with the new approach | 07:26 |
| fwereade | wallyworld, nice, thanks | 08:05 |
| fwereade | wallyworld, hey, I was wondering | 08:05 |
| fwereade | wallyworld, using ssl-hostname-verification for metadata-needs-signing seems a little strange to me | 08:05 |
| wallyworld | which bit of code? | 08:07 |
| wallyworld | i'll look | 08:07 |
| fwereade | wallyworld, err let me hunt again, I came upon something that looked like it was doing that while trying to figure out some of the joyent provider tests last night | 08:07 |
| fwereade | wallyworld, if that doesn't ring a bell I probably just misunderstood it | 08:08 |
| wallyworld | i can't recall offhand either | 08:08 |
| wallyworld | gotta go for dinner, back later | 08:09 |
| fwereade | wallyworld, ah it's ok I completely misunderstood it | 08:09 |
| wallyworld | o :-) | 08:10 |
| wallyworld | ok | 08:10 |
| fwereade | wallyworld, it really is about verifying the hostnames | 08:10 |
| fwereade | wallyworld, enjoy your dinner | 08:10 |
| wallyworld | will do | 08:10 |
| fwereade | I've got a trivial intermittent failure if anyone's got a moment: https://codereview.appspot.com/63710044 | 08:41 |
| fwereade | rogpeppe, 479-desired-peer-group appears to be LGTMed from a month ago but still unmerged | 08:54 |
| rogpeppe | fwereade: oh twats | 08:54 |
| rogpeppe | fwereade: it's always that problem when you approve something then switch context back to what you were doing, and forget to check that it's actually merged | 08:55 |
| fwereade | mgz, can we land https://codereview.appspot.com/49050044/ ? | 08:55 |
| fwereade | rogpeppe, yeah, I have a couple of those myself :/ | 08:55 |
| fwereade | TheMue, is 058-debug-log-api abandoned in favour of a different approach? | 08:56 |
| fwereade | rogpeppe, also 487-remove-job-managestate | 08:57 |
| rogpeppe | fwereade: oh, that's worse | 08:58 |
| rogpeppe | fwereade: bugger | 08:58 |
| rogpeppe | fwereade: looks like today is the day for bountiful conflicts | 08:58 |
| fwereade | TheMue, I'm rejecting 058 and WIPing 060 | 08:59 |
| * fwereade wishes rogpeppe luck | 08:59 | |
| TheMue | fwereade: I'm currently testing the 060 live, it is the solution. Only coded testing is missing. But it works for all providers (even local) and with older releases (even here now local). | 09:03 |
| TheMue | fwereade: The 060 contains both, the backend and the cmd changes. | 09:04 |
| fwereade | TheMue, excellent, thanks | 09:05 |
| rogpeppe | wallyworld: this is from a suggestion i made in a review - i didn't want to badger you about it, but i think it's worth doing: https://codereview.appspot.com/63530048 | 09:20 |
| wallyworld | rogpeppe: we prefer interfaces | 09:24 |
| rogpeppe | wallyworld: why? | 09:24 |
| wallyworld | all the usual reasons | 09:24 |
| rogpeppe | wallyworld: when we've just got a bag of data, there is no reason to use an interface | 09:24 |
| rogpeppe | wallyworld: it just obfuscates the code for no reason | 09:25 |
| rogpeppe | fwereade: i'd appreciate your thoughts on this | 09:25 |
| wallyworld | i'm tired tonight so may not be in the best frame of mind for a coherent discussion | 09:25 |
| rogpeppe | wallyworld: i appreciate that interfaces are useful when you might want to mock different behaviour | 09:25 |
| rogpeppe | wallyworld: but in this case there is no behaviour to mock - the data structure *is* the interface | 09:26 |
| wallyworld | now yes | 09:26 |
| wallyworld | bit the design is evolving | 09:26 |
| wallyworld | but | 09:26 |
| wallyworld | the same pattern has been used for agent conf | 09:27 |
| rogpeppe | wallyworld: if we need extra behaviour, that will be easy enough to retro-fit. let's not overengineer in advance - this is not Java | 09:27 |
| wallyworld | huh? | 09:27 |
| rogpeppe | wallyworld: agent conf does actually have some behaviour behind it | 09:27 |
| wallyworld | as will the upgrade stuff most likely | 09:27 |
| rogpeppe | wallyworld: (it encapsulates the way to read different versions of the file) | 09:28 |
| rogpeppe | wallyworld: i don't see that - this is a description of the upgrade steps - that's fundamentally data AFAICS. it's also held inside the binary, not in an external file | 09:28 |
| wallyworld | it is data for sure. but the design is still evolving | 09:29 |
| rogpeppe | wallyworld: sure, so evolve the data | 09:30 |
| rogpeppe | wallyworld: having the extra layer of indirection doesn't help that (actually, it makes it harder) | 09:30 |
| rogpeppe | wallyworld: "KISS" :-) | 09:31 |
| wallyworld | sure. there's arguments either way | 09:31 |
| rogpeppe | fwereade: can we please go the simpler route to start with? | 09:33 |
| rogpeppe | fwereade: if you find that interfaces become useful here, i will eat my humble pie | 09:33 |
| wallyworld | to me interfaces are simpler because they abstract behaviour and mocks can easily be constrcuted for only some of the methods | 09:34 |
| wallyworld | and it's generally just plan good programming practice | 09:34 |
| fwereade | rogpeppe, wallyworld: IMO our reliance on concrete types has fucked the codebase pretty hard | 09:34 |
| wallyworld | our use of structs everywhere in juju core kinda sucks | 09:34 |
| rogpeppe | fwereade: i don't think this case is analagous to that | 09:34 |
| fwereade | rogpeppe, wallyworld: once you turn out to need an interface it's a *massive* hassle to fix the concrete types | 09:35 |
| wallyworld | +1 | 09:35 |
| rogpeppe | fwereade: so do you really think we should never use structs? even when we're just holding bags of data? | 09:36 |
| fwereade | rogpeppe, I want enough seams in the codebase to allow for easy testing of small units, without backing everything with real implementations | 09:36 |
| wallyworld | bags of data now for some things, but the design is evolving | 09:36 |
| wallyworld | +1 | 09:36 |
| rogpeppe | fwereade: there is no "implementation" here - it's just a description of what to do | 09:37 |
| rogpeppe | fwereade: the "implementation" is in the Run functions which are already fully mockable | 09:37 |
| rogpeppe | fwereade: i can't see what use it is to "mock" a function that just returns a description and does nothing more. | 09:37 |
| fwereade | rogpeppe, I'm curious as to what you perceive the benefits of dropping the interface to be | 09:38 |
| rogpeppe | fwereade: it 60 lines less code. i consider that a good reason in itself. | 09:38 |
| rogpeppe | s/it/it's/ | 09:38 |
| wallyworld | rogpeppe: you are the one who likes lots of code :-) | 09:39 |
| fwereade | rogpeppe, and I'm not so sure that we'll never see behaviour associated with things like Targets | 09:39 |
| wallyworld | all those duplicated "contains" functions etc | 09:39 |
| fwereade | rogpeppe, by pickinginterfaces from the get-go we can put extensions to behaviour in the right place when the time comes | 09:39 |
| wallyworld | they gie us more flexibility to evolve a design | 09:40 |
| wallyworld | give | 09:40 |
| rogpeppe | fwereade: it is also much more immediately obvious what the code is doing - if i see a field access, i don't have to look at what some arbitrary code might be doing | 09:40 |
| fwereade | rogpeppe, if we don't, then the "simple" approach becomes to tweak the code that uses those types | 09:40 |
| fwereade | rogpeppe, and behaviour gets smeared out | 09:40 |
| fwereade | rogpeppe, and things just get worse and worse | 09:40 |
| fwereade | rogpeppe, because even if we *know* it's the time to switch to interfaces | 09:41 |
| fwereade | rogpeppe, the hassle and cost of doing so is generally overwhelming | 09:41 |
| fwereade | rogpeppe, you don;t have to anyway | 09:41 |
| rogpeppe | fwereade: i really don't believe that could be the case here - this is limited-scope code | 09:41 |
| fwereade | rogpeppe, the upgrade step knows where it should be applied | 09:41 |
| fwereade | rogpeppe, as a client it's not your responsibility to figure it out, and it's none of your business how it's figured out *anyway* | 09:42 |
| rogpeppe | fwereade: if we want behaviour on Targets, that's trivial to add. | 09:42 |
| fwereade | rogpeppe, and so it begins | 09:42 |
| fwereade | rogpeppe, just a little bit of coupling can't hurt | 09:42 |
| rogpeppe | fwereade: it's not coupling | 09:42 |
| rogpeppe | fwereade: it's evolving the schema | 09:43 |
| fwereade | rogpeppe, and forcing the clients to care about that evolution | 09:43 |
| rogpeppe | fwereade: what clients? | 09:43 |
| rogpeppe | fwereade: this is essentially an internal-use data structure only | 09:43 |
| fwereade | rogpeppe, that doesn't mean there aren't clients | 09:44 |
| fwereade | rogpeppe, the code that uses it, and the code that tests it, are the minimum set of clients for just about any construct | 09:44 |
| rogpeppe | fwereade: so by "client" you mean the package itself? | 09:44 |
| fwereade | rogpeppe, yes | 09:45 |
| fwereade | rogpeppe, programming is *all about* managing interfaces to impose simplicity on complexity | 09:45 |
| fwereade | rogpeppe, package boundaries are way too coarse to be useful | 09:46 |
| rogpeppe | fwereade: sure, but an "interface" can just as well be a concrete data structure there | 09:46 |
| rogpeppe | fwereade: i've switched back and forth between interfaces and concrete types many times. i much prefer starting with the simple, and then graduating to the more elaborate when it proves necessary. | 09:47 |
| fwereade | rogpeppe, I do not agree with that approach | 09:47 |
| fwereade | rogpeppe, I gave it a fair shake | 09:47 |
| rogpeppe | fwereade: if you can't agree that a simple concrete type is appropriate for representing a simple concrete bunch of data, i give up | 09:47 |
| fwereade | rogpeppe, and I'm confident that most of the current complexity of juju is a result of an adherence to flawed notions of simplicity | 09:48 |
| fwereade | rogpeppe, it'd be a different story in python | 09:48 |
| fwereade | rogpeppe, because you can use a descriptor in place of a field when it becomes necessary | 09:48 |
| fwereade | rogpeppe, but with go you *invest* in the struct approach | 09:49 |
| fwereade | rogpeppe, it's an expected value calculation | 09:49 |
| rogpeppe | fwereade: you really think that most of the complexity of juju is from using structs? | 09:49 |
| fwereade | rogpeppe, most of the bits that really hurt and continue to do so, like jujud, are the way they are because we have no seams by which to pick them apart | 09:50 |
| fwereade | rogpeppe, the state/environ horrors are because we have a state struct | 09:50 |
| rogpeppe | fwereade: i think most of the pain there is in the tests, not the code | 09:50 |
| fwereade | rogpeppe, the inability to test state quickly and easily are because of all the structs in mgo which makes it completely unmockable | 09:51 |
| rogpeppe | fwereade: because we had a prior aversion to mocking anything | 09:51 |
| rogpeppe | fwereade: huh? we could mock mgo easily | 09:51 |
| fwereade | rogpeppe, the awful awful uniter stucture is the same story -- there's nothing *that bad* about it in theory, but in practice you can't isolate any of it and have to take the whol lot as an indivisible unit | 09:51 |
| rogpeppe | fwereade: well, except that we'd need to mock mongodb semantics | 09:51 |
| rogpeppe | fwereade: again, i really think that's because of our prior aversion to mocking anything | 09:52 |
| fwereade | rogpeppe, maybe I'm being dense: how would you mock anything without interfaces? | 09:52 |
| rogpeppe | fwereade: there are various places in the code where we *do* manage to mock pieces of the State to good effect | 09:52 |
| rogpeppe | fwereade: you don't need to *export* the type as an interface | 09:53 |
| rogpeppe | fwereade: you just need to define clients in terms of an interface, and then use some trivial bridge code (or nothing) to bridge the gap | 09:53 |
| rogpeppe | fwereade: it's all about ad-hoc interfaces | 09:54 |
| fwereade | rogpeppe, isn't that what wallyworld is doing? it's just that you consider the clients in this case as to be too trivial to bother with | 09:54 |
| * wallyworld apologises - too tired to contribute tonight | 09:54 | |
| fwereade | wallyworld, don't worry, not calling you into the conversation :) | 09:55 |
| rogpeppe | fwereade: if there was any behaviour to mock, i might agree. but there isn't, and i can see no particular prospect of it - it's really a data-driven algorithm | 09:55 |
| rogpeppe | fwereade: for a static table of data, a static table of data seems like the right representation to me | 09:56 |
| fwereade | rogpeppe, I think what he'd doing is relying on instinct for bits-likely-to-change, and inserting seams where they're likely to be useful | 09:56 |
| fwereade | rogpeppe, apart from the Run bits | 09:56 |
| fwereade | rogpeppe, they're types with data *and* behaviour | 09:57 |
| rogpeppe | fwereade: tbh what i think what's happening is just use-interfaces-by-rote | 09:57 |
| rogpeppe | fwereade: the behaviour is independent of the data | 09:57 |
| fwereade | rogpeppe, your refactoring still puts them in the same type; I think that's an indication that it's not *quite* as cut and dried as yu say | 09:58 |
| fwereade | rogpeppe, note: this is not a request for a more extreme refactoring ;p | 09:58 |
| rogpeppe | fwereade: yes, that type composes both the metadata and some behaviour together | 09:59 |
| fwereade | rogpeppe, did I say the thing about expected value? | 10:00 |
| fwereade | rogpeppe, not sure I did | 10:00 |
| rogpeppe | fwereade: not sure you did | 10:00 |
| fwereade | rogpeppe, picking an interface has a small cost now and in future; picking a struct has a lower cost now, and a chance of a much higher cost in future | 10:01 |
| fwereade | rogpeppe, the choice comes down to a judgment call | 10:01 |
| fwereade | rogpeppe, people can legitimately disagree | 10:01 |
| rogpeppe | fwereade: you really think that there's a high cost from moving to use an interface? when there are no external clients? | 10:01 |
| rogpeppe | fwereade: i have not found that | 10:02 |
| rogpeppe | fwereade: i like the code to be as understandable as possible | 10:02 |
| fwereade | rogpeppe, yes, I really do | 10:02 |
| fwereade | rogpeppe, IMO/E interfaces are at least as understandable as structs | 10:03 |
| rogpeppe | fwereade: so you think that changing the code base to use, for example, all the State types as interfaces, would be hard? | 10:03 |
| fwereade | rogpeppe, I'm trying t obe generous in acknowledging that sometimes they're a little cheaper | 10:03 |
| rogpeppe | fwereade: interfaces are less understandable because it's not obvious what's going to happen when they're called | 10:04 |
| fwereade | rogpeppe, yes, it would be a giant hassle and a massive diff | 10:04 |
| fwereade | rogpeppe, because it's *not the client's business* | 10:04 |
| rogpeppe | fwereade: it would be a big diff certainly (*state.State -> state.State) but actually it would be easy | 10:04 |
| fwereade | rogpeppe, and all the other types? | 10:04 |
| rogpeppe | fwereade: in this case we *are* the client! | 10:04 |
| rogpeppe | fwereade: sure, it wouldn't be a problem | 10:05 |
| rogpeppe | fwereade: i could do it this afternoon if we wanted | 10:05 |
| fwereade | rogpeppe, there's no use in having a State interface if Machine returns a *Machine, you have to return Machine as well | 10:05 |
| rogpeppe | fwereade: sure, that's why i said "all the State types" | 10:05 |
| rogpeppe | fwereade: the only hassle would be maintaining the interface types independently from the struct definitions - the interfaces would be enormous. | 10:06 |
| rogpeppe | s/struct definitions/method definitions/ | 10:06 |
| fwereade | rogpeppe, right, so you *can't* usefully use an ad-hoc interface because you have to construct an actual *Machine | 10:06 |
| rogpeppe | fwereade: you can, but it's a bit more work | 10:07 |
| rogpeppe | fwereade: (the address updater uses that technique for testing) | 10:07 |
| fwereade | rogpeppe, it's an *enormous* amount of work to construct one of those that behaves in a specifically tuned way | 10:07 |
| fwereade | rogpeppe, and even *that* is hamstrung because it depends on concrete mgo types | 10:07 |
| fwereade | rogpeppe, it's *really nice* to be able to test state clients against tuned mocks, rather than building up loads of state every time | 10:08 |
| rogpeppe | fwereade: honestly, it's not *that* much hassle - here's the relevant code the instancepoller tests: http://paste.ubuntu.com/6930374/fwereade: actually the best value would come not from mocking * | 10:09 |
| rogpeppe | oops | 10:09 |
| rogpeppe | fwereade: honestly, it's not *that* much hassle - here's the relevant code the instancepoller tests: http://paste.ubuntu.com/6930374 | 10:09 |
| rogpeppe | fwereade: actually the best value would come not from mocking *state.State, but the various other types (*Machine &c) | 10:10 |
| fwereade | rogpeppe, multiply that work by N tests for N state clients and it becomes a bit unwieldy | 10:11 |
| fwereade | rogpeppe, it's the same simple-code-repeated-everywhere problem | 10:11 |
| fwereade | rogpeppe, that I'm not sure yu see as an actual problem | 10:12 |
| rogpeppe | fwereade: so if State was an interface, how would that help matters here? | 10:13 |
| rogpeppe | fwereade: you'd still need to mock out the actual behaviour | 10:13 |
| rogpeppe | fwereade: so you'd probably end up with an enormous "one-size-fits-all" state mocker, which would be its own source of considerable complexity | 10:13 |
| rogpeppe | fwereade: i've thought sometimes that it might be nice just to mock out the mgo interface, and write some in-memory mongodb interpretation code | 10:14 |
| rogpeppe | fwereade: that would mean that you could base all of the State code in memory, so it would be loads faster | 10:15 |
| rogpeppe | fwereade: but... you'd still want to be certain that the mock mgo semantics mirrored the actual mongo semantics | 10:15 |
| fwereade | rogpeppe, sure *that* would be nice, but again I don't see how you could actually do that | 10:15 |
| fwereade | rogpeppe, well, actually, *not* necessarily | 10:15 |
| rogpeppe | fwereade: again, it would be straightforward to do | 10:16 |
| fwereade | rogpeppe, mocking lets you exhaustively test the operation of a client given a range of locally defined behaviours | 10:16 |
| fwereade | rogpeppe, imagining that the only purpose of a mock is to perfectly simulate a real system is to somewhat miss the point | 10:16 |
| fwereade | rogpeppe, what-happens-if-X-bizarre-situation becomes *actually* testable, and cheaply | 10:17 |
| rogpeppe | fwereade: the problem is that the integration tests don't necessarily test all the behaviours that are being tested in the unit tests | 10:17 |
| rogpeppe | fwereade: ideally, you'd be able to test against both the mock (for quick tests) and the real thing (for slower but more complete) tests | 10:18 |
| fwereade | rogpeppe, integration tests as tracer bullets, and exhaustively paranoid unit tests with mocks, are O(N) work; exhaustively paranoid integration tests are O(N^layers-used-directly-or-indirecttly) | 10:19 |
| jamespage | is it really right that 1.16.6 added errgo? | 10:19 |
| fwereade | rogpeppe, oh yes, more direct switch of subject | 10:19 |
| rogpeppe | jamespage: definitely not | 10:19 |
| fwereade | rogpeppe, if we're depending on it can we please put it under github/juju | 10:19 |
| jamespage | rogpeppe, well its in the release tarball... | 10:19 |
| * jamespage sighs | 10:19 | |
| rogpeppe | jamespage: i know why | 10:20 |
| jamespage | I'll ping sinzui later | 10:20 |
| rogpeppe | jamespage: it's because of the way that the tarball is built | 10:20 |
| rogpeppe | jamespage: it's not actually *used* by juju in 1.16.6 | 10:20 |
| rogpeppe | jamespage: but the tarball is build by doing "go get .../juju" then updating to the correct revision, then updating the deps | 10:21 |
| rogpeppe | jamespage: but updating the deps does not remove unused deps | 10:21 |
| jamespage | rogpeppe, oh crap - so it will pull in all trunk deps but not drop ones used in stable | 10:21 |
| jamespage | not used that would be | 10:21 |
| rogpeppe | jamespage: yes | 10:21 |
| jamespage | that's not helpful | 10:22 |
| rogpeppe | jamespage: that's my understanding | 10:22 |
| jamespage | rogpeppe, lemme nag sinzui later | 10:22 |
| jamespage | I was going to upload this for SRU but stuff like this will make the SRU team nervous | 10:22 |
| rogpeppe | jamespage: what's the actual problem with having unused deps? | 10:22 |
| rogpeppe | fwereade: this'll be thumper's errgo, FWIW | 10:23 |
| fwereade | rogpeppe, sure, the request still stands | 10:23 |
| rogpeppe | fwereade: ok | 10:24 |
| rogpeppe | fwereade: you don't think that github.com/errgo/errors is a reasonable path for it? | 10:24 |
| rogpeppe | fwereade: it would be considerably more likely for 3rd party code to use it there, i think, and that would be a Good Thing | 10:24 |
| fwereade | rogpeppe, I'm not sure that's the case -- and I really don't want to further grow our dependencies on external code | 10:27 |
| fwereade | rogpeppe, it'll be a pretty fundamental dependency | 10:28 |
| fwereade | rogpeppe, and if it's good and stable I don;t think being under an organisation named for a (hopefully) successful and popular open-source package will do it any harm | 10:28 |
| rogpeppe | fwereade: perhaps so. | 10:29 |
| rogpeppe | fwereade: i'll probably put it at the above path too, and make sure the two are compatible | 10:30 |
| fwereade | rogpeppe, it remains a distinct package with its own identity but we get to move towards drawing our critical dependencies into a sphere over which we have control | 10:30 |
| fwereade | rogpeppe, that's perfectly fine | 10:30 |
| fwereade | rogpeppe, jamespage: btw, AIUI, we *could* chck out juju, and use deps.tsv to pull in the necessary dependencies on their own, and then not run the risk of pulling in other random unpinned stuff, right? we just don't do that yet | 10:32 |
| rogpeppe | fwereade: the problem with that is my fault | 10:32 |
| fwereade | rogpeppe, jamespage: but I don't see any reason not to? | 10:33 |
| rogpeppe | fwereade: which is that godeps doesn't currently have a way to pull deps - it just updates existing deps | 10:33 |
| fwereade | rogpeppe, yeah, I can see that the pulling-deps bit is a hassle | 10:33 |
| rogpeppe | fwereade: i should just do that - i've just been lazy so far | 10:33 |
| fwereade | rogpeppe, that would be awesome | 10:33 |
| fwereade | rogpeppe, btw can I steal 30s of attention for a review of https://codereview.appspot.com/63710044/ please? | 10:34 |
| rogpeppe | fwereade: sure | 10:34 |
| rogpeppe | fwereade: what's the deal with the changes to UniterAPI.getUnit and getService? | 10:35 |
| fwereade | rogpeppe, oh balls | 10:36 |
| fwereade | rogpeppe, wrong base | 10:36 |
| fwereade | rogpeppe, it's https://codereview.appspot.com/57710043/ | 10:36 |
| fwereade | rogpeppe, the reason is that there's no guarantee that we're actually passing unit or service tags there | 10:37 |
| rogpeppe | fwereade: yeah, i figured that. | 10:38 |
| rogpeppe | fwereade: it could still use FindEntity and just check the type-assertion result, but the ParseTag approach is cheaper and probably better, yeah | 10:38 |
| rogpeppe | fwereade: i don't quite understand how that CL restores the ability of the uniter to read removed units | 10:48 |
| fwereade | rogpeppe, getUnit fails if the unit doesn't exist | 10:48 |
| fwereade | rogpeppe, pulling out the service name still works so long as the service is there | 10:48 |
| fwereade | rogpeppe, and it will be so long as the relation using it is | 10:49 |
| rogpeppe | fwereade: but didn't we just call getUnit to get the RelationUnit ? | 10:49 |
| rogpeppe | fwereade: that's operated on by checkRemoteUnit | 10:49 |
| fwereade | rogpeppe, the RelationUnitis for LocalUnit | 10:50 |
| rogpeppe | fwereade: ah ha! | 10:51 |
| rogpeppe | fwereade: ok, makes sense. | 10:51 |
| rogpeppe | fwereade: LGTM with a suggestion for a comment | 10:53 |
| fwereade | rogpeppe, SGTM, would you LGTM the other one for form's sake too please? | 10:54 |
| rogpeppe | fwereade: am looking currentyl | 10:54 |
| natefinch | morning all. Seems like I missed the standup? | 10:54 |
| fwereade | natefinch, er, I didn't spot the standup reminder :/ | 10:57 |
| fwereade | rogpeppe, natefinch: shall we do it quickly then? | 10:57 |
| rogpeppe | fwereade: i didn't either | 10:58 |
| rogpeppe | fwereade, natefinch: let's | 10:58 |
| rogpeppe | mgz: standup? | 10:59 |
| rogpeppe | mgz: (better late than never...) | 10:59 |
| TheMue | could somebody please give me a little hit at the back of my head! somehow I'm in vi mode and trying to operate my sublime editor with vi commands. *aaaargh* | 11:21 |
| rogpeppe | anyone see launchpad.net down ? | 11:59 |
| natefinch | rogpeppe: fine for me | 12:01 |
| rogpeppe | natefinch: ah, it's started working again | 12:02 |
| natefinch | rogpeppe: you're welcome ;) | 12:02 |
| rogpeppe | natefinch: although it doesn't seem to be registering my branch pushes | 12:02 |
| rogpeppe | i'm trying to approve this: https://code.launchpad.net/~rogpeppe/juju-core/487-remove-job-managestate/+merge/202916 | 12:02 |
| rogpeppe | but it keeps insisting that it's on revno 2250, not the latest revno 2252 | 12:03 |
| rogpeppe | and as expected the bot is saying "There are additional revisions which have not been approved in review" | 12:04 |
| rogpeppe | how frustrating | 12:04 |
| natefinch | rogpeppe: yeah weird | 12:04 |
| rogpeppe | natefinch: if you do, "bzr revno -r lp:~rogpeppe/juju-core/487-remove-job-managestate", it prints 2252, right? | 12:05 |
| natefinch | rogpeppe: it prints ??? as the response, which is less than encouraging | 12:05 |
| rogpeppe | natefinch: ah, that might be just bzr - can you pull the branch? | 12:06 |
| natefinch | rogpeppe: yep, and once that's done it says 2252 | 12:07 |
| rogpeppe | natefinch: right, thanks | 12:08 |
| rogpeppe | natefinch: so *something* is up with lp | 12:08 |
| rogpeppe | mgz: any idea what could cause this? | 12:08 |
| rogpeppe | cd .. | 12:09 |
| rogpeppe | hmm, interesting bzr push error: | 12:12 |
| rogpeppe | bzr: ERROR: Server sent an unexpected error: ('error', 'xmlrpclib.ProtocolError', '<ProtocolError for xmlrpc.lp.internal:8097/codehosting: -1 >') | 12:12 |
| natefinch | rogpeppe: have you tried rebooting? :D | 12:14 |
| mgz | hm, nothing particular comes to mind | 12:14 |
| rogpeppe | natefinch: it doesn't seem like a local problem to me | 12:14 |
| rogpeppe | another error: ConnectionReset reading response for 'BzrDir.open_2.1', retrying | 12:14 |
| rogpeppe | but then the retry succeeded | 12:15 |
| natefinch | rogpeppe: that's the joke. But yes, aggravating. | 12:15 |
| mgz | may be a remote issue | 12:15 |
| rogpeppe | mgz: so no idea of how i might be able to prod lp into recognising that a branch has been updated? | 12:15 |
| rogpeppe | ha, "Sorry, there was a problem connecting to the Launchpad server." | 12:16 |
| rogpeppe | so it's not just one branch | 12:17 |
| rogpeppe | natefinch: what happens if you go to https://code.launchpad.net/~rogpeppe/juju-core/479-desired-peer-group/+merge/201245 now? | 12:17 |
| mgz | rogpeppe: still getting an error now? | 12:19 |
| natefinch | rogpeppe: that's a different MP, is that the right link? | 12:19 |
| rogpeppe | natefinch: yeah, i was getting an error on that one too | 12:20 |
| rogpeppe | mgz: i just succeeded with that one. let me try the first one again | 12:20 |
| rogpeppe | mgz: getting an error with that one still; will keep reloading the page | 12:22 |
| rogpeppe | ah, finally! | 12:23 |
| rogpeppe | approved at the right version | 12:23 |
| natefinch | rogpeppe: what's the proper way to join path strings in a path list? our code does a lot of path+":"+newpath but that'll fail on windows.... but os.PathListSeparator is a rune, not a string, which makes it a pain in the ass | 12:31 |
| rogpeppe | natefinch: i saw a proposal ages ago for a filepath.SplitList, but i think it foundered | 12:33 |
| rogpeppe | natefinch: s/SplitList/JoinList | 12:33 |
| rogpeppe | natefinch: we could always add it to utils | 12:34 |
| natefinch | rogpeppe: ok, yeah, I had hoped I just missed it somewhere | 12:35 |
| rogpeppe | natefinch: func JoinList(paths ...string) {return strings.Join(paths, string(os.PathListSeparator))} | 12:35 |
| rogpeppe | natefinch: actually, given that it's in utils, it should probably be JoinPathList | 12:36 |
| natefinch | rogpeppe: ahh, didn't realize you could cast a rune to a string. | 12:36 |
| rogpeppe | natefinch: you should read the spec again :-) | 12:36 |
| natefinch | yeah, it's been a while. Just haven't dealth with runes much | 12:37 |
| rogpeppe | structural regexp line noise of the moment: ,x/^=.*\n([^=].*\n)+/y/^=.*\n(---.*\n)?(\+\+\+.*\n)?/x/(^[+\-].*\n)+/g/(errors|errgo)\.Cause/= | 12:53 |
| * mgz hits rogpeppe | 12:55 | |
| * rogpeppe protests: "but it's doing useful stuff for me!" | 12:56 | |
| rogpeppe | mgz: perhaps you've got a better idea actually. | 12:58 |
| rogpeppe | mgz: i've got an enormous branch that i want to cherry pick certain bits from automatically | 12:59 |
| mgz | na, not really, rewrites like this are always just finding the right pain balance of automation and manual work | 12:59 |
| rogpeppe | mgz: (or at any rate with a minimum of effort) | 12:59 |
| rogpeppe | mgz: do you know of a tool that lets me just click on the bits i want to choose? | 13:00 |
| mgz | hm, well, with an existing branch, depending on how the bits are layed out, you may find a merge and a proper three-way editor faster | 13:00 |
| natefinch | now you have two problems etc | 13:00 |
| natefinch | yeah, a proper merge tool makes it a lot easier | 13:01 |
| mgz | natefinch: do you have a particular one you tend to use? | 13:02 |
| mgz | I struggle along with vimdiff when I need it generally | 13:02 |
| natefinch | mgz: beyond compare is awesome | 13:03 |
| natefinch | mgz meld is supposed to be pretty awesome but I haven't used it | 13:04 |
| rogpeppe | natefinch: i'm just trying meld, but it's not easy to see how it supports my use case | 13:19 |
| natefinch | rogpeppe: you should be able to diff a clean branch vs. your fixed branch then just do a search for .Cause and copy over the changes that you want to keep | 13:21 |
| rogpeppe | natefinch: thanks, that sounds plausible, i'll try that | 13:22 |
| natefinch | rogpeppe: yeah, since you said there weren't that many spots in which you use cause, it's probably faster and easier than trying to get exactly the right regex to work. | 13:23 |
| rogpeppe | natefinch: yeah, and the regex isn't sufficient anyway, because there are other parts related to Cause changes that don't mention Cause | 13:23 |
| natefinch | rogpeppe: ahh yeah. Sometimes there's no replacement for the human brain | 13:24 |
| natefinch | arg... I kinda wish we hadn't used testing/testbase in utils, because then it means we can't use utils in testing/testbase :/ | 13:57 |
| niemeyer | Hey all | 13:58 |
| niemeyer | fwereade: Is the juju-dist bucket still being used? | 13:58 |
| niemeyer | Hmm, looks like it is | 14:00 |
| TheMue | *: did anybody had a "cannot upload charm: Post https://localhost:8900/charms?series=quantal: local error: record overflow" in client_test.go TestAddLocalCharm()? | 14:08 |
| fwereade | niemeyer, it's still being uploaded to and while it's probably not being used *much* we did come across some guys with a very old environment recently | 14:09 |
| fwereade | niemeyer, where there's one there may be more | 14:09 |
| TheMue | fwereade, rogpeppe: do you know the test failure mentioned above? | 14:09 |
| rogpeppe | TheMue: i haven't seen that | 14:10 |
| TheMue | rogpeppe: thx, strange | 14:10 |
| niemeyer | fwereade: Hmmm | 14:11 |
| niemeyer | fwereade: Do we have any releases in Ubuntu using it? | 14:11 |
| fwereade | TheMue, hmm, haven't seen that | 14:12 |
| fwereade | niemeyer, I don't *think* so -- 1.14 is the latest that depends on it iirc -- but they need it to upgrade to 1.16 | 14:14 |
| niemeyer | fwereade: Ah, still recent then, cool | 14:15 |
| TheMue | rogpeppe, fwereade: running the test solo it passes, running the suite it fails ??? | 14:15 |
| rogpeppe | TheMue: it fails reliably? | 14:15 |
| TheMue | rogpeppe: seems so, had been able to repeat it multiple times | 14:16 |
| TheMue | rogpeppe: the "record overflow" is no text in our code | 14:16 |
| rogpeppe | TheMue: interesting. | 14:17 |
| rogpeppe | TheMue: what does the test output look like? | 14:17 |
| TheMue | rogpeppe: http://paste.ubuntu.com/6931384/ | 14:18 |
| TheMue | rogpeppe: and the piece of code is http://paste.ubuntu.com/6931388/ | 14:21 |
| rogpeppe | TheMue: interesting - i'm not sure that could ever have worked | 14:22 |
| TheMue | rogpeppe: hehe | 14:22 |
| TheMue | rogpeppe: but as I said, running the test function as the only one it works | 14:23 |
| rogpeppe | oh, i see; it sets the server root | 14:23 |
| rogpeppe | TheMue: i suspect you've got two tests colliding | 14:23 |
| rogpeppe | TheMue: that test should definitely not be listening on a fixed port | 14:23 |
| TheMue | rogpeppe: sounds logical | 14:24 |
| rogpeppe | TheMue: if you change that code to listen on a newly chosen port instead, does it still fail? | 14:24 |
| rogpeppe | TheMue: ah, there's another problem with that code too - it may not have actually started listening when you add the local charm | 14:25 |
| TheMue | rogpeppe: changed to 8901, still fails | 14:25 |
| rogpeppe | TheMue: could you change it to listen on a newly chosen port (use net.Listen("tcp", ":0")) | 14:26 |
| rogpeppe | ? | 14:26 |
| TheMue | rogpeppe: have to check how I would have to change the code, but now I've got my standup | 14:28 |
| rogpeppe | TheMue: the same way that all the other test http listeners do it | 14:29 |
| TheMue | rogpeppe: will look there | 14:29 |
| TheMue | rogpeppe: changed it and it runs on the same failure | 15:00 |
| rogpeppe | TheMue: what does the code look like now? | 15:00 |
| TheMue | rogpeppe: paste.ubuntu.com/6931586/ - and it still passes when called alone | 15:02 |
| rogpeppe | TheMue: hmm | 15:03 |
| rogpeppe | TheMue: one possible thing to try: in the juju-core root directory, run "godeps -u dependencies.tsv" | 15:05 |
| TheMue | rogpeppe: could do so, but could you explain the reason for this command before? | 15:07 |
| rogpeppe | TheMue: it updates the package dependencies to be as expected | 15:07 |
| rogpeppe | TheMue: also, what version of Go are you using? | 15:07 |
| TheMue | rogpeppe: 1.1.1 | 15:08 |
| TheMue | rogpeppe: should update ;) | 15:08 |
| rogpeppe | TheMue: yes, you must do that | 15:09 |
| rogpeppe | TheMue: you should be using 1.2 | 15:09 |
| rogpeppe | TheMue: if you're not up to date, you'll probably need to iterate - when godeps complains about a package (because the required revision isn't available), run go get -u <pkgname> | 15:09 |
| TheMue | rogpeppe: ok, will quickly change | 15:09 |
| sinzui | hi rogpeppe , natefinch : Do either of you have a few minutes to review a backport for 1.16 that lets QA run unit-tests? https://codereview.appspot.com/64000043 | 15:09 |
| rogpeppe | sinzui: looking | 15:10 |
| rogpeppe | sinzui: LGTM | 15:11 |
| sinzui | thank you rogpeppe | 15:11 |
| * rogpeppe goes for lunch | 15:16 | |
| * natefinch goes to blow some snow | 15:22 | |
| === natefinch is now known as natefinch-afk | ||
| TheMue | rogpeppe: after update and rebuild of all pkgs still the same | 15:23 |
| rogpeppe | fwereade: https://github.com/juju/errgo | 16:28 |
| rogpeppe | fwereade: (README thanks to github.com/robertkrimen/godocdown ) | 16:28 |
| fwereade | rogpeppe, awesome, tyvm | 16:39 |
| sinzui | any ideas about what I can do to make gobot test my branch again? https://code.launchpad.net/~sinzui/juju-core/run-unit-test-on-trusty/+merge/206503 | 17:54 |
| === JoshStrobl is now known as JoshStrobl-afk | ||
| rogpeppe | sinzui: sometimes it gets stuck | 17:59 |
| rogpeppe | sinzui: it should be working again now | 18:02 |
| sinzui | plars in #juju reports this error on hp-cloud. The config works for 1.16.3, but not 1.17.2 | 18:02 |
| sinzui | ERROR bootstrap failed: cannot start bootstrap instance: index file has no data for cloud {region-a.geo-1 https://region-a.geo-1.identity.hpcloudsvc.com:35357/v2.0/} not found | 18:02 |
| rogpeppe | sinzui: BTW i updated your commit message - it's really nice if the commit message is exactly the same as is in the codereview description, including the codereview link | 18:02 |
| sinzui | I cannot reproduce the issue | 18:02 |
| sinzui | noted rogpeppe | 18:03 |
| rogpeppe | sinzui: i'm away for the weekend now | 18:03 |
| rogpeppe | happy weekends all | 18:03 |
| sinzui | me too | 18:03 |
| === natefinch-afk is now known as natefinch | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!