| === weblife is now known as web-brandon | ||
| axw | thumper: what's your stance on things like ctx/txn? | 00:43 |
|---|---|---|
| thumper | axw: I strongly prefer what uncle Bob calls "Unse Pronounceable Names" | 01:44 |
| thumper | personally I use context | 01:45 |
| thumper | and prefer transaction | 01:45 |
| axw | thumper: ok. my reasoning is that, for things like ctx, the translation to context is automatic (for me) | 01:45 |
| axw | so I pronounce it "context" | 01:46 |
| thumper | but the point is, you need to translate | 01:46 |
| thumper | there is a cognitive process there | 01:46 |
| thumper | that takes ctx, and makes context | 01:46 |
| axw | not consciously | 01:46 |
| thumper | but it is there | 01:46 |
| thumper | when I see it, I still see "see tee ex" | 01:46 |
| davecheney | sure, so is translating source code into your native language | 01:46 |
| davecheney | i think it's a very minimal cost | 01:46 |
| davecheney | and we're trained to do it very effectively | 01:46 |
| thumper | davecheney: but writing code is all about people reading it | 01:46 |
| axw | if it's not the same for everyone that's fair enough | 01:46 |
| thumper | axw: I agree that some don't have that | 01:47 |
| thumper | but do you have a problem reading "context" as a variable? | 01:47 |
| thumper | davecheney: computers are easier to train | 01:47 |
| axw | thumper: only when it means the source line > 80 chars, or becomes a series of ugly lines | 01:47 |
| * davecheney is sad that we can't apply common sense here | 01:48 | |
| thumper | axw: I agree, in which case the logic should be simplified | 01:48 |
| thumper | computers are good at optimising | 01:48 |
| thumper | davecheney: common sense isn't as common as we'd hope | 01:48 |
| thumper | axw: I remember writing some code one, it was c++ | 01:49 |
| thumper | where I managed to use a single for_each function | 01:49 |
| axw | thumper: btw I'm not advocating ctx necessarily, just playing devil's advocate | 01:49 |
| thumper | in the end though, I rewrote it with a normal for loop that took two or three more lines | 01:49 |
| thumper | because it was more readable | 01:49 |
| thumper | one of my favourite programming mantras is: | 01:49 |
| thumper | just because you can, doesn't mean you should | 01:50 |
| thumper | however, smart C++ programmers would have been able to see what my extremely clever function was doing | 01:50 |
| thumper | but it wasn't obvious | 01:50 |
| thumper | I'd go for obvious over clever every time | 01:50 |
| thumper | however, I feel that there are some on our team that prefer clever | 01:51 |
| axw | thumper: agreed. I used to fall into the clever code trap, but try hard to steer away from it now. It doesn't scale well in teams, and I tend to forget what my code is meant to do | 01:51 |
| thumper | agreed | 01:51 |
| * axw remembers in horror at his former colleagues' naming conventions | 01:52 | |
| axw | or lack thereof | 01:53 |
| axw | basically all the variables would be the first letter of the variable's compound word type | 01:53 |
| axw | var atp AggregateTableProvider | 01:53 |
| axw | fun times | 01:53 |
| thumper | haha | 01:58 |
| axw | bbl, lunch | 01:59 |
| === jtv2 is now known as jtv | ||
| bigjools | thumper: txn reads perfectly well! :) | 02:24 |
| bigjools | thumper: however when I see ctx my mind pronounces it as "cuntox" | 02:25 |
| thumper | heh | 02:25 |
| bigjools | blame big kev ... | 02:25 |
| thumper | well, everyone is different | 02:25 |
| bigjools | yeah | 02:25 |
| bigjools | which is precisely why you are right | 02:25 |
| thumper | the idea is to code for the majority | 02:25 |
| kvt | poking through the state pkg, and noticing some odd items a couple of the docs have counts, like serviceDoc has relation count, relations have unitcount.. is that just an optimization vs querying those? | 03:03 |
| thumper | kvt: probably | 03:49 |
| davecheney | bigjools: ping | 04:26 |
| bigjools | davecheney: hail | 04:27 |
| * thumper read that as 'fail' | 04:28 | |
| thumper | perhaps I have too much on my mind | 04:28 |
| bigjools | understandable | 04:28 |
| thumper | jam: I've added Dubai to my clock city list so I can know what time it is there. | 04:55 |
| jam | thumper: :) | 04:56 |
| thumper | jam: so you are 8 hours out from me | 04:56 |
| thumper | jam: got time for a quick hangout? | 04:56 |
| jam | thumper: right. I'm right at the end of your day, I believe. though it depends whether I wake up and poke around before officially "starting". | 04:56 |
| jam | sure | 04:56 |
| * thumper starts one | 04:56 | |
| thumper | https://plus.google.com/hangouts/_/f16d2fd28ebc5923a5ceb1a2e1ffc57f5b152404?hl=en | 04:57 |
| thumper | jam: ^^ | 04:57 |
| jam | thumper: trying to connect, it rings on my phone where I don't want to connect | 04:58 |
| jam | and isn't loading on the desktop where I do | 04:58 |
| thumper | haha :( | 04:58 |
| thumper | hmm.. | 04:58 |
| thumper | jam: did you want to set one up | 04:58 |
| thumper | ? | 04:58 |
| jam | let me make sure I'm signed into all of my accounts quickly | 04:59 |
| jam | thumper: I'll call you back, it is just getting stuck at "joining video call". | 05:00 |
| thumper | kk | 05:01 |
| jam | calling you | 05:02 |
| jam | https://plus.google.com/hangouts/_/f16d2fd28ebc5923a5ceb1a2e1ffc57f5b152404 | 05:02 |
| jam | thumper: ^^ | 05:02 |
| jam | I really don't prefer the "calling" style of hangout, vs just creating one and having people join it. | 05:02 |
| thumper | night people | 05:41 |
| === tasdomas_afk is now known as tasdomas | ||
| davecheney | bigjools: 16:19 < kurt_> davecheney: do you know this error? error: cannot create bootstrap state file: gomaasapi: got error back from server: 400 BAD REQUEST | 06:21 |
| davecheney | any ideas ? | 06:21 |
| bigjools | davecheney: yes, maas was not accepting empty files. There's an SRU about to go out for it | 06:22 |
| bigjools | if desperate, use the daily PPA | 06:22 |
| davecheney | ok, will tell 'em | 06:24 |
| axw | bigjools: did you see my comment on the gwacl/vhd bug? | 06:37 |
| bigjools | axw: from this morning? | 06:37 |
| bigjools | if so yes, and I replied | 06:38 |
| axw | oh | 06:38 |
| axw | (yes) | 06:38 |
| axw | bigjools: thanks. didn't get an update - I thought it would auto-subscribe me | 06:40 |
| davecheney | axw: hang on, you are upset because LP _didn't_ spam you ? | 06:42 |
| davecheney | you'll quickly change your tune | 06:43 |
| bigjools | haha | 06:48 |
| bigjools | most of the spam I get is because stupid people stupidly do stupid team nesting in teams that have stupid subscriptions | 06:49 |
| bigjools | this is why I left ~uju as soon as I could | 06:49 |
| bigjools | ~juju even | 06:49 |
| davecheney | yes, that guy in china who subscribes me to bugs with the intel chipset | 06:49 |
| davecheney | WTF | 06:49 |
| jam | allenap: poke about https://code.launchpad.net/~allenap/juju-core/makefile-stuff/+merge/181113 I commented on it, but haven't heard a response from you yet. | 06:53 |
| jam | axw: is there a reason https://code.launchpad.net/~axwalk/juju-core/testing-mgo-nounixsocket/+merge/181218 isn't landed? | 06:54 |
| axw | jam: nope, I just forgot to. thanks - I'll do it now | 06:55 |
| jam | axw: I was guessing so. I'm just going through activereviews and cleaning it out. | 06:56 |
| axw | jam: looks like the bot's /tmp is full of gocheck dirs | 07:13 |
| axw | are you able to clean that? | 07:13 |
| jam | axw: if you're able to see it, you're able to clean it, but I'll do it | 07:13 |
| axw | I can see it because my MP failed | 07:13 |
| axw | tests failed, because /tmp/gocheck-blah exists | 07:13 |
| axw | jam: gocheck doesn't set a seed for rand when it generates the temp dirs | 07:14 |
| rogpeppe | mornin' all | 07:39 |
| jam | morning rogpeppe | 07:54 |
| rogpeppe | jam: hiya | 07:54 |
| jam | rogpeppe: I have a couple more CLI API patches up that would appreciate a quick review. | 08:01 |
| rogpeppe | jam: ok, will have a look | 08:01 |
| jam | https://codereview.appspot.com/12744052/ | 08:01 |
| jam | and | 08:01 |
| jam | https://codereview.appspot.com/12744053/ | 08:02 |
| rogpeppe | jam: BTW we can work around the int64 vs float64 thing if we want to, but given that the API is accessed from javascript, I'm not entirely sure we should. | 08:04 |
| jam | rogpeppe: right. My concern is if someone out there is getting their settings as YAML and then parsing it into a struct that requires an int. | 08:05 |
| jam | We do have a "type" object in that struct, though. | 08:05 |
| jam | So the other option is that GetConfig/SetConfig start understanding the actual content of those messages | 08:05 |
| jam | but I'd like to just punt on all that :) | 08:06 |
| jam | rogpeppe: gustavo mentioned we can change the decode function to "UseNumber" but I'm not sure what that actually changes? If it doesn't have a decimal you get an int64? | 08:07 |
| jam | It uses arbitrary precision integers? | 08:07 |
| rogpeppe | jam: you can change it to get any type you want - one possibility is just to use a string. | 08:09 |
| jam | rogpeppe: I tracked it down, it is "json.Number" which is actually a string, and then you "Number.Float64()" | 08:10 |
| jam | it still wouldn't let us use DeepEquals easily | 08:10 |
| jam | but it would let us get what we want later on. | 08:10 |
| jam | But if we have to do that step | 08:10 |
| jam | we can int64(floatValue) | 08:10 |
| TheMue | jam: thx for review. setting an option to nil means unsetting it, so that the default is valid. and setting an option to "" will not be translated to nil anymore (as before). so for string options "" is a valid value while it's an error for other ones (like in pyjuju). | 08:10 |
| jam | and it just matters when you have > 40bits of precision in your int74 | 08:10 |
| jam | int | 08:10 |
| jam | TheMue: right that is where I thought we were going: Allow "" to be a valid setting, and use nil to indicate you want a default value. So your patch fits that just fine. | 08:11 |
| rogpeppe | jam: exactly | 08:11 |
| rogpeppe | jam: and that might actually be a problem | 08:11 |
| TheMue | jam: yep | 08:12 |
| jam | rogpeppe: you mean losing precision on ints? | 08:12 |
| rogpeppe | jam: yes | 08:12 |
| rogpeppe | jam: we might be causing subtle breakage in existing charm usage | 08:12 |
| jam | rogpeppe: float64 has exact precision up to ~52 bits so it only matters if someone enters 2**61+1 that they care about that +1 | 08:13 |
| rogpeppe | jam: yes | 08:13 |
| rogpeppe | jam: but that still might be important | 08:13 |
| rogpeppe | jam: for instance someone might be storing bytes in there or something | 08:13 |
| jam | rogpeppe: don't store bytes in a number type | 08:14 |
| jam | I think that is a fine statement | 08:14 |
| jam | we have strings | 08:14 |
| rogpeppe | jam: indeed | 08:14 |
| jam | and base64 | 08:14 |
| rogpeppe | jam: but it's still a possible issue | 08:14 |
| jam | rogpeppe: I would be willing to take a patch that changed "juju set" to notice that you might lose precision and warn you | 08:14 |
| jam | rogpeppe: I'm happy to deal with it when someone shows it is an actual problem. | 08:14 |
| rogpeppe | jam: tbh i think it's ok to just say we only cope with 52 bits of precision | 08:15 |
| jam | rogpeppe: I feel the same way. | 08:15 |
| rogpeppe | jam: in fact, we *could* deprecate the "int" type entirely | 08:15 |
| jam | which is why I JFDI with float() | 08:15 |
| rogpeppe | jam: +1 | 08:15 |
| rogpeppe | jam: those two CLs reference the same MP | 08:36 |
| rogpeppe | jam: i think i just reviewed the one without the prereq | 08:36 |
| rogpeppe | jam: so it proabably counts as a review of both :-) | 08:37 |
| jam | rogpeppe: yeah, I meant https://codereview.appspot.com/12943045/ | 08:50 |
| jam | rogpeppe: so for Client object. What fwereade_ really wanted is to avoid exposing "Environ" to the CLI code as much as we can (because we shouldn't need stuff like provider creds for almost everything we do). | 08:51 |
| rogpeppe | jam: yeah, that seems reasonable | 08:51 |
| jam | rogpeppe: beyond that, there isn't much that api.State provides CLI above "api.State.Client()". | 08:51 |
| rogpeppe | jam: agreed | 08:52 |
| jam | so I'd like to start with restricting it to just Client, and see if we can make it with that | 08:52 |
| rogpeppe | jam: i think that sounds fine too | 08:52 |
| jam | but I'm happy to fix the comment | 08:52 |
| jam | rogpeppe: ah, back to you. IT turns out that multiple Close of api.State actually isn't ok | 08:52 |
| jam | underneath api.State.Close() just calls rpc.Conn.Close() | 08:53 |
| jam | which returns: errors.New("already closing") | 08:53 |
| jam | rogpeppe: should multiple Close calls be an error? | 08:53 |
| rogpeppe | jam: it's not clear | 08:53 |
| jam | rogpeppe: we had a test that state.State.Close() can be called multiple times (though like I copied, it didn't actually check the err return :) | 08:54 |
| rogpeppe | jam: on network connections, i think the second close will return an error | 08:54 |
| jam | you can add that, and it passes | 08:54 |
| jam | it fails for api.State.Close() | 08:54 |
| rogpeppe | jam: we could make rpc.Conn.Close return nil if it's already closing | 08:56 |
| jam | rogpeppe: that would be my preference, but you wrote it and I wanted to check what your reasoning was | 08:56 |
| rogpeppe | jam: i think i was principally following the original logic of net/rpc | 08:57 |
| rogpeppe | jam: which returns an error in that case | 08:57 |
| rogpeppe | jam: (if in doubt i followed net/rpc's conventions) | 08:58 |
| jam | rogpeppe: right: http://golang.org/src/pkg/net/rpc/client.go?s=7213:7248#L262 | 08:58 |
| jam | though it returns a typed error | 08:58 |
| jam | which we could suppress at a higher level if we wanted. | 08:58 |
| rogpeppe | jam: that's true. but it's a typed error that actually means two possible things | 08:58 |
| rogpeppe | jam: 1) you just closed it; 2) the server shut down on you | 08:59 |
| jam | rogpeppe: sure, though if I'm calling Close() and that happened to be shutdown by the remote side, that doesn't sound like an error | 08:59 |
| jam | so I don't know if you need to distinguish between the two cases in this path | 09:00 |
| jam | in *my* opinion, Close() is one of those idempotent functions that you can call multiple times to make sure resources were cleaned up | 09:00 |
| jam | rogpeppe: if you agree, I'm happy to change rpc.Conn and assert it higher in the stack. | 09:00 |
| rogpeppe | jam: I think that sounds reasonable (presumably you mean "lower in the stack"?) | 09:02 |
| jam | rogpeppe: well I mean assert it across the stack | 09:02 |
| jam | so we assert api.Client.Close and APIConn.Close() and api.State.Close and rpc.Conn.Close() | 09:02 |
| jam | They happen to be implemented in terms of eachother, but it is nice to have the statement "Close() is idempotent at the various levels" | 09:03 |
| rogpeppe | jam: what's APIConn ? | 09:03 |
| jam | rogpeppe: juju.APIConn | 09:04 |
| rogpeppe | jam: oh doh! | 09:04 |
| jam | Contains an api.State and api.Environ | 09:04 |
| jam | rogpeppe: mirroring juju.Conn | 09:04 |
| rogpeppe | jam: yes, that all seems reasonable | 09:04 |
| jam | but *probably* going away | 09:04 |
| jam | I didn't want to be ripping it out at this point, but we have api.Client() where functionality that needs a place like juju.Conn can go, there doesn't seem much point in having both, and we *don't* want most CLI things to need an object that has Environ on it. | 09:04 |
| rogpeppe | jam: yes, i think it will probably go away, though there are one or two lingering bits | 09:05 |
| jam | rogpeppe: hopefully most of it involves moving things into the API :) | 09:05 |
| rogpeppe | jam: yeah, i'm very much +1 on putting everything behind api.Client | 09:05 |
| rogpeppe | jam: it wasn't possible before, because we couldn't mix logic that involved both Environ and State in a State method. | 09:06 |
| jam | rogpeppe: out of curiousity, why is rpc.Conn defined in "server.go" rather than "client.go" ? | 09:08 |
| rogpeppe | jam: it could be either | 09:08 |
| rogpeppe | jam: it's both the server and the client side | 09:08 |
| rogpeppe | jam: it could have been in a separate file, i guess, but i don't think it's a big issue | 09:09 |
| rogpeppe | mgz: morning! | 09:10 |
| mgz | qmorning! | 09:10 |
| jam | rogpeppe: no, it just wasn't where I was looking for it when dealing with connection on the client side. And the word "Conn" exists all over the place in our source code. :) | 09:11 |
| jam | mgz: morning | 09:11 |
| rogpeppe | jam: you should really use better tooling :-) | 09:11 |
| jam | rogpeppe: we should use better names | 09:11 |
| rogpeppe | jam: rpc.Conn seems just fine to me | 09:11 |
| rogpeppe | jam: it's unambiguous | 09:12 |
| jam | rogpeppe: except for net/rpc/Conn | 09:12 |
| rogpeppe | jam: they're both connections | 09:12 |
| jam | rogpeppe: but they both use "rpc.Conn" and are entirely different code. | 09:12 |
| jam | even if they are "similar" | 09:12 |
| rogpeppe | jam: ah, sorry, thought you were referring to net.Conn vs rpc.Conn | 09:13 |
| jam | rogpeppe: there may not actually be a net/rpc/Conn, but punning 'rpc' with a stdlib package is also not great naming. | 09:14 |
| rogpeppe | jam: in a large s/w environment, there are always going to be duplicate names. the package name spacing means that it's possible to reliably and quickly find the actually definition for any given name | 09:14 |
| rogpeppe | jam: and there are tools that integrate with your editor that makes that easy to do | 09:14 |
| rogpeppe | jam: (assuming you use emacs or vim :-]) | 09:15 |
| rogpeppe | s/actually/actual/ | 09:15 |
| rogpeppe | jam: do you think that all our names should be globally unique? | 09:17 |
| jam | rogpeppe: I'm not sure about globally unique, but avoiding repeated 10 times would be nice | 09:18 |
| jam | eg State | 09:18 |
| jam | Conn | 09:18 |
| jam | and Client | 09:18 |
| rogpeppe | jam: i think that that's actually a benefit - those names are well known concepts, and they're unambiguous when used in context (rpc.Conn vs net.Conn; uniter.State vs machiner.State, etc) | 09:20 |
| rogpeppe | jam: i guess it comes down to the "no needless repetition" thing | 09:20 |
| rogpeppe | jam: which is a conventional approach in Go | 09:20 |
| rogpeppe | jam: but does have some down sides, as you point out | 09:20 |
| jam | rogpeppe: I *do* like not having var foobar foo.FooBar = new foo.FooBar(fooing the bars) | 09:21 |
| rogpeppe | jam: but you prefer foo.FooBar to foo.Bar ? | 09:21 |
| jam | rogpeppe: I like it being really easy to translate "foo := SomethingReturningAFoo()" into what Foo that is returning. | 09:22 |
| jam | and using "tools that are already installed on my system" rather than having to dig up "nostandard but useful tools" from 3rd parties. | 09:23 |
| rogpeppe | jam: presumably if it's foo.NewBar(), that's fairly obvious | 09:23 |
| jam | rogpeppe: except it is "rpc.Conn" and that is actually net/rpc/Conn or foo/bar/baz/rpc/Conn | 09:25 |
| jam | rogpeppe: so I have to look up what "foo" is actually being imported here. | 09:25 |
| rogpeppe | jam: it's true that package identifiers aren't unique (we use "testing" many times, for example) | 09:26 |
| jam | rogpeppe: and it is a source of confusion occasionaly. | 09:26 |
| rogpeppe | jam: yeah, it can be. but hopefully in situations where ambiguity might be a problem, we rename the identifiers appropriately. | 09:27 |
| === tasdomas is now known as tasdomas_afk | ||
| rogpeppe | jam: we only use net/rpc in one place in our tree, for example | 09:27 |
| === tasdomas_afk is now known as tasdomas | ||
| jam | rogpeppe: so there is "it is unambiguous when you've paged in enough context" and there is "it is unambiguous when you read 3 lines of code" | 09:27 |
| rogpeppe | jam: personally, i think it's reasonable to assume some contextual knowledge. but that might be just me. | 09:29 |
| rogpeppe | jam: and i have to say that when browsing code i'm not familiar with, i use godef a lot | 09:29 |
| jam | rogpeppe: in a team of 10+ having people look at code that they aren't as familiar with it is *useful* albeit not *required* to minimize the need for contextual knowledge. | 09:29 |
| jam | rogpeppe: it is a goal (generally) to code is such a way that it is understandable to people who aren't as intelligent as myself (especially since *I* will not remember everything in 1 month either :) | 09:31 |
| rogpeppe | jam: i totally agree with that | 09:31 |
| rogpeppe | jam: but i don't think that the redundant naming helps greatly there | 09:31 |
| rogpeppe | jam: in general, i find external Go code (using similar naming conventions to our current ones) is quite easy to browse and understand | 09:32 |
| rogpeppe | jam: but again, i do use godef (and godoc.org) a lot | 09:33 |
| jam | rogpeppe: one thing I would *really* like for godoc, is if you could click the Bar in "func Something() Bar)" and have it take you to the definition. | 09:33 |
| rogpeppe | jam: i think it does that, doesn't it? | 09:34 |
| rogpeppe | jam: yeah, it does | 09:34 |
| rogpeppe | jam: it linkifies all type names | 09:35 |
| jam | rogpeppe: not in the one-line overview, and the "func Something" doesn't give you the arguments or the return type, but if you keep drilling down until you get to the grey-box with more stuff that does eventually get there. | 09:35 |
| rogpeppe | jam: i might be looking at a different godoc.org to you | 09:36 |
| rogpeppe | jam: in the one i'm looking at, it's two clicks | 09:36 |
| rogpeppe | jam: (one to click on the one line overview, which takes you to the actual description, then another to click on the type name) | 09:38 |
| jam | rogpeppe: and a bit of visual searching to find the right thing, but with practice not too bad. I'm not sure if it used to do that, or if the specific layout of the page made it hard for *me* to see it. | 09:38 |
| rogpeppe | jam: it's quite possible. and the site does change - it might not have been so good in the past. | 09:39 |
| jam | I think I would also like to see the overview of functions for a Struct when you go to the structs definition (like there is in the top level index). | 09:39 |
| jam | AFAIK there isn't a way to jump to the place in the index | 09:40 |
| jam | and the index mixes | 09:40 |
| jam | things that return Foo | 09:40 |
| jam | with methods on Foo itself | 09:40 |
| jam | eg, http://godoc.org/net/rpc has Client | 09:40 |
| jam | which has functions returning a Client | 09:40 |
| jam | and functions on Client | 09:40 |
| jam | but no concise "these are the methods on Client" | 09:40 |
| rogpeppe | jam: yeah, it tries to classify factory funcs | 09:40 |
| jam | I don't mind them being nearby | 09:40 |
| jam | but it would be useful to have a simple "here are the methods" section | 09:40 |
| rogpeppe | jam: that's the overview, i guess | 09:41 |
| jam | in my head it would exist here: http://godoc.org/net/rpc#Client | 09:41 |
| jam | rogpeppe: they aren't visually distinct from the "here are factory functions" | 09:41 |
| jam | they are slightly different | 09:41 |
| rogpeppe | jam: yeah, you have to look for the (client *Client) | 09:41 |
| rogpeppe | jam: if you can think of a nice way of visually distinguishing them, i'm sure they'd be open to suggestions | 09:42 |
| rogpeppe | jam: it's not something that had occurred to me | 09:42 |
| jam | rogpeppe: Are all of the tests for "state/api/*" code over in state/apiserver? | 09:43 |
| jam | I don't see any _test.go files in state/api | 09:43 |
| jam | which is.. a bit surprising | 09:43 |
| jam | (I realize you sort of need apiserver in order to test api code, but I would expect to find api tests in the api package) | 09:44 |
| jam | rogpeppe: which might be why I put the APIClient.Close test in juju/* | 09:44 |
| jam | (I'm only *just now* remembering that I saw some api.Client tests in apiserver a few days ago) | 09:44 |
| jam | rogpeppe: as in, I don't have an immediately obvious place to put new tests that api.State.Close() and api.Client.Close() are idempotent. | 09:45 |
| jam | rogpeppe: so how do you find where there would be tests for api.State? | 09:45 |
| jam | rogpeppe: having a unique name instead of State | 09:45 |
| jam | would make that pretty obvious (i would think) | 09:46 |
| jam | so references of X not definitions of X | 09:46 |
| jam | and some of those references might be in the package itself | 09:46 |
| rogpeppe | jam: my original scheme for testing API functionality was to put all tests in the client. | 09:47 |
| jam | rogpeppe: there are no tests that I see in state/api/*, and there is no state/api/client/ directory | 09:48 |
| jam | rogpeppe: is it just that they were moved elsewhere? | 09:48 |
| jam | rogpeppe: also, no tests for state/api.State object that I can immediately find | 09:48 |
| * rogpeppe goes to look | 09:49 | |
| rogpeppe | jam: the tests that are now in state/apiserver/client did originally live in state/api, i believe | 09:50 |
| rogpeppe | jam: yeah, that seems to be the case. they should really be in state/api/client, i think | 09:52 |
| jam | rogpeppe: I'll file a techdebt bug, but not fix it right now. | 09:52 |
| rogpeppe | jam: sgtm | 09:53 |
| rogpeppe | jam: FWIW, i still think that testing all API stuff through the client makes sense, as there's so little actual logic in the RPC layer. | 09:54 |
| rogpeppe | jam: and the way things are, we've got vast swathes of almost-but-not-quite identical tests at each layer | 09:54 |
| rogpeppe | jam: i guess the main reason we can't do that is that we decided to expose stuff in the API server that we don't make available in the client.; | 09:55 |
| jam | rogpeppe: which I've already run into, though it was in the "and this isn't tested" category AFAICT | 09:56 |
| jam | AddUnit supported ToMachineSpec but that wasn't exposed in api.Client | 09:56 |
| rogpeppe | jam: ha, yes | 09:57 |
| rogpeppe | jam: i have difficulty keeping track of what our test coverage is in this area | 09:57 |
| rogpeppe | jam: because the tests are so overlapping | 09:57 |
| jam | rogpeppe: so where do you think tests about api.State are today? (where should I add a test that calling api.State.Close() multiple times doesn't give an error) | 09:58 |
| rogpeppe | jam: i think that tests on api.State should live with the other tests on api.State. | 09:58 |
| rogpeppe | jam: it seems that that's currently (but wrongly) in state/apiserver/client | 09:58 |
| rogpeppe | jam: so i'd add it to those, to be moved over at some later date | 09:59 |
| fwereade_ | rogpeppe, I was taking a casual look at Prepare... there's a 3.5 second sleep in jujud/machine.go | 10:00 |
| rogpeppe | fwereade_: yes, i've removed that since. | 10:00 |
| rogpeppe | fwereade_: but not re-proposed the branch | 10:00 |
| rogpeppe | fwereade_: (that was me trying to reproduce a saw-it-once-only test failure) | 10:00 |
| fwereade_ | rogpeppe, :) | 10:01 |
| fwereade_ | rogpeppe, seems fine to me, go ahead and land it, if anything turns out to be a problem for the followup work we'll find out soon enough | 10:05 |
| rogpeppe | fwereade_: i'm pretty sure it was nothing to do with my changes | 10:05 |
| jam | fwereade_: did you have anything to add to the float64() vs int64() discussion about the ServiceGet() api? | 10:09 |
| fwereade_ | jam, I'm fine accepting the json precision restrictions (well, I don't like it, but I don't think it's worth the effort of fixing) | 10:11 |
| jam | fwereade_: well *JSON* supports arbitrary precision integers AFAICT | 10:12 |
| jam | golang json.Unmarshal defaults to float64 though you can tweak it to return "type Number string" | 10:12 |
| jam | but that would make testing it harder rather than easier AFAICT | 10:12 |
| fwereade_ | jam, shall we just call it a bug and move on? | 10:13 |
| fwereade_ | jam, unless you judge it simple enough to make it Right rightnow | 10:14 |
| jam | fwereade_: my personal feeling is that 'juju get' will return similar-enough types that it probably isn't a problem, and I'm willing to deal with it when charms themselves really do need it | 10:15 |
| fwereade_ | jam, seconded | 10:15 |
| jam | fwereade_: and I would say the test should be in state/api, except there are 0 tests in there :) | 10:15 |
| jam | which is what I was just chatting about with roger | 10:15 |
| fwereade_ | jam, but let's document it as a bug regardless so it's easier to hunt it down and figure it out when it *does* happen | 10:15 |
| rogpeppe | jam: presumably we have a potential issue even if we do use int64 because python would (presumably) use arbitrary-precision ints when needed | 10:16 |
| jam | fwereade_: bug #1217282 | 10:16 |
| _mup_ | Bug #1217282: api.Client tests should be in state/api not state/apiserver/client/ <tech-debt> <juju-core:Triaged by jameinel> <https://launchpad.net/bugs/1217282> | 10:16 |
| fwereade_ | jam, oh bollocks, they're all in apiserver | 10:16 |
| jam | fwereade_: as yet I haven't actually found direct tests of api.State either. | 10:16 |
| jam | rog mentions they might be in state/apiserver/client/* but I haven't actually found a suite that is focused on the api.State object. | 10:17 |
| fwereade_ | jam, oh yay | 10:17 |
| natefinch | jam: I sent an email that got blocked by the message size limit on juju-dev.... the autoreply I got said it was awaiting moderator approval... are there actual moderators that might approve it, or should I just recreate it? | 10:18 |
| jam | natefinch: I am not one of the moderators. Gustavo might be, but it is probably easier to just resubmit. | 10:18 |
| jam | I believe you can reject the existing message yourself. | 10:18 |
| natefinch | jam: yeah.... bah. 40k limit? What is this, 1998? :) | 10:19 |
| jam | morning evilnickveitch | 10:23 |
| jam | I don't remember you having the "evil" goatee, though. | 10:23 |
| evilnickveitch | jam, hey! My facial hair is in a constant state of flux to confuse my enemies | 10:23 |
| TheMue | fwereade_: in https://codereview.appspot.com/12752044/diff/8001/cmd/juju/get_test.go you made a comment regarding "default" in "juju get" | 10:25 |
| rogpeppe | jam: i'd like to see tests in state/api focused on the functionality that's in that package. you're right, tests for api.State itself seem to have vanished. i'm pretty sure i wrote some originally. | 10:26 |
| TheMue | fwereade_: could you explain your thoughts a bit more? | 10:26 |
| TheMue | fwereade_: i haven't touched the logic of get here, only moved. but i could handle it in a third CL. | 10:27 |
| rogpeppe | has anyone seen this when testing? | 10:27 |
| rogpeppe | PANIC: machine_test.go:58: MachineSuite.TearDownTest | 10:27 |
| rogpeppe | ... Panic: unauthorized db:presence ns:presence.presence.beings lock type:0 client:127.0.0.1 (PC=0x414321) | 10:27 |
| fwereade_ | TheMue, saying "the value is nil, and that's the default" is redundant/meaningless because a nil value is only possible when there's no default set | 10:27 |
| fwereade_ | TheMue, so a trivial CL stripping "default: true" out of settings where the value is nil would be cool | 10:28 |
| TheMue | fwereade_: ok, will do | 10:28 |
| fwereade_ | bbiab | 10:28 |
| jam | natefinch: you have https://code.launchpad.net/~natefinch/juju-core/005-azure-address/+merge/181117 still pending though it has my approval. just poking for you to think about landing it. | 10:33 |
| natefinch | jam: thanks, meant to talk to you about that. It's LGTM pending live tests, but the values aren't actually used anywhere in the branches, so I sorta can't live test | 10:34 |
| jam | natefinch: I meant "test against Azure" | 10:34 |
| davecheney | natefinch: nice work in the win32 installer | 10:34 |
| jam | as in actually running it live | 10:34 |
| davecheney | i hope you get some feedback | 10:35 |
| davecheney | but given the self selection population in the channel | 10:35 |
| davecheney | please don't take it too hard if you don't get a lot of feedback | 10:35 |
| natefinch | davecheney: my message got caught by the size limit on the mailing list, so no one but mods have seen it I suspect :) | 10:35 |
| natefinch | jam: I guess I can insert some temporary code to call instance.Addresses[]... my point was that nothing calls that method in the branch I'm on | 10:36 |
| natefinch | at least as far as I could tell | 10:37 |
| davecheney | natefinch: i saw it | 10:38 |
| === andre____ is now known as andrew_w_deane | ||
| davecheney | i am not a nod | 10:38 |
| davecheney | mod | 10:38 |
| natefinch | davecheney: oh, ok, so maybe it was let through and there wasn't a message back to me about it... and since I don't think I get my own messages from the list... makes it hard to tell what's going on. | 10:39 |
| davecheney | natefinch: you do | 10:40 |
| davecheney | but gmail doesn't show them too you | 10:40 |
| natefinch | davecheney: oohh... ok, that could be it | 10:40 |
| davecheney | you can also check the listserv directlu | 10:40 |
| natefinch | Ahh yeah, I always forget I can do that | 10:41 |
| natefinch | well, good. | 10:42 |
| * TheMue => lunchtime | 10:58 | |
| jam | natefinch: talking about the windows installer. should we just make a 32 bit one and not worry about it? juju-the-client won't ever really benefit from >4GB addressable space. Though I realize go itself works better in 64-bit because of how it does virtual adress | 10:59 |
| jam | addressing | 10:59 |
| jam | juju-the-client isn't long lived enough to really matter. | 10:59 |
| jam | when we get to 'jujud-the-daemon' we'll want to focus more on 64-bit | 10:59 |
| natefinch | jam: yeah, that's probably a good point. There's very little benefit from 64 bit as you said. And 32 bit is more portable, which is a lot more valuable | 11:00 |
| jam | (I originally started in 64-bit, and then went to 32-bit when we had C dependencies, because 64-bit mingw wasn't very good, and was thinking to switch back but maybe we just want to stay 32-bit for juju-the-client for now) | 11:00 |
| davecheney | does anyone know where cloud-init writes the original text of the user data ? | 11:05 |
| mgz | dave, sec | 11:10 |
| davecheney | i can never find the bugger | 11:10 |
| mgz | /var/lib/cloud/instance/user-data.txt | 11:11 |
| davecheney | mgz: ta muchly | 11:11 |
| davecheney | bloody hell, cloud-init has everything that opens and shuts | 11:11 |
| davecheney | oh shit, its base64 encoded | 11:11 |
| davecheney | well, that is one terminal i wont be using again | 11:11 |
| natefinch | lol | 11:12 |
| mgz | dave : in the same folder are the decoded bits | 11:12 |
| davecheney | mgz: last time I c&p something blindly from you | 11:13 |
| mgz | I included not cat :) | 11:13 |
| natefinch | it's like the CLI equivalent of rick rolling :) | 11:15 |
| fwereade_ | guys, I'm afraid I won't make the standup, and I'll probably be working somewhat interrupted hours all week -- it emerges that the flat is actually literally falling apart and I need to find somewhere to live :/ | 11:20 |
| natefinch | fwereade_: wow, damn | 11:21 |
| fwereade_ | yeah, it kinda sucks | 11:21 |
| natefinch | fwereade_: yeah, I can see how that could be less than optimal | 11:21 |
| TheMue | fwereade_: iiirks, wishing you luck | 11:23 |
| natefinch | fwereade_: yeah, good luck, definitely. That's a crappy situation to be in. Do you own the flat, or are you renting? | 11:24 |
| jam | natefinch: https://plus.google.com/hangouts/_/f497381ca4d154890227b3b35a85a985b894b471 standup | 11:33 |
| jam | fwereade_: ^^ | 11:33 |
| rogpeppe | mramm: standout, if you want to | 11:34 |
| rogpeppe | standup even | 11:34 |
| mgz | standover | 11:34 |
| jam | standarounder | 11:35 |
| davecheney | stand to the right | 11:38 |
| davecheney | https://bugs.launchpad.net/juju-core/+bug/1202163/comments/4 | 11:49 |
| _mup_ | Bug #1202163: openstack provider should have config option to ignore invalid certs <juju-core:Triaged> <https://launchpad.net/bugs/1202163> | 11:49 |
| davecheney | jpds needs love on this one | 11:49 |
| mgz | that's sadly not as easy to do in gojuju as it was in pyjuju | 11:54 |
| davecheney | mgz: could we make it always on ? | 11:55 |
| davecheney | mgz: sorry, was jumping to the soluont | 11:55 |
| mgz | as in, never check certs? | 11:55 |
| davecheney | yes | 11:55 |
| davecheney | i know how to create a client that doens't check certs | 11:55 |
| mgz | that seems like a bad thing from a security perspective :) | 11:56 |
| davecheney | mgz: sure | 11:56 |
| davecheney | is the issue in passing down the 'please don't check' flag | 11:56 |
| davecheney | or implementing ' please don't check ' | 11:56 |
| mgz | passing through to all the points that use an endpoint seems like the hard part | 11:57 |
| davecheney | mgz: what if it was at a per environment level ? | 11:57 |
| mgz | I'm just assuming go has a flag in the http library somewhere for not checking certs | 11:58 |
| davecheney | yup | 11:58 |
| mgz | davecheney: so, what happens in pyjuju, is we have the silly er... ssh-hostname-verify or something env config variable | 11:58 |
| davecheney | yup | 11:58 |
| mgz | that used to default to false, but got switched to true for openstack | 11:58 |
| mgz | and that gets used but the openstack provider client, | 11:59 |
| * davecheney remembers ssh-hostname-verify | 11:59 | |
| mgz | *and* also the curl for looking up the instance id on machine startup | 11:59 |
| davecheney | cocking nora | 12:00 |
| mgz | we should be able to do roughly that for gojuju... but we use the swift container for many more things, like tools, | 12:00 |
| mgz | and without checking, I'm not totally certain all the places where we might access provider storage we actually have the environment config | 12:01 |
| mgz | because some of that is now initiated from each machine, whereas it used to all be from the provisioner | 12:02 |
| davecheney | i guess the 'just turn it off everywhere' option is not acceptable ? | 12:03 |
| mgz | given the elling that made us flip the default for python, I think not | 12:04 |
| jam | davecheney: I would think the self-signed cert thing is High + papercut vs Critical | 12:04 |
| mgz | *yelling | 12:04 |
| jam | Critical == block the next release until fixed | 12:04 |
| davecheney | jam: feel free to change it | 12:04 |
| jam | davecheney: just running it by you rather than having an edit war on LP :) | 12:04 |
| davecheney | jam: don't care | 12:04 |
| jam | natefinch: I sent what is hopefully a point-by-point discussion about the installer. Mostly it seems like "good enough for now". | 12:05 |
| natefinch | jam: thanks, the feedback is very much appreciated | 12:05 |
| jam | I'll check with Mramm if it is appropriate to upload it to juju-core proper today. | 12:05 |
| jam | at least, if he shows up to my 1:1 this morning. | 12:06 |
| mramm | jam: seems reasonable to upload it | 12:06 |
| natefinch | mgz: I moved my cards around so they're up to date. I'll poke the red squad about azure creds so I can do a sanity check on the azure code and land that today | 12:09 |
| mgz | ace, and added an intaller one, great | 12:10 |
| === natefinch is now known as natefinch-afk | ||
| natefinch-afk | going to be in and out some today, kids stuff. | 12:18 |
| davecheney | TheMue: https://docs.google.com/a/canonical.com/document/d/1aEvcmxSJaj1i9zNjGy48yKF-SPlTFwW-NiKfoO_Ygo4/edit | 12:30 |
| davecheney | could you please document juju unset in the release notes | 12:30 |
| davecheney | ta | 12:30 |
| TheMue | davecheney: yep, will do | 12:30 |
| jam | natefinch-afk: quick comment, we probably won't release 1.13.2 as a windows installer because of the known bug against azure with public tools. (fixed in trunk) | 12:33 |
| davecheney | also, 1.13.3 is coming at the end of the week | 12:34 |
| jam | davecheney: right, so we'll probably wait for 1.13.3 to do a public -setup.exe | 12:34 |
| davecheney | jam: and then we get to have the discusion about 1.14 again :) | 12:34 |
| jam | davecheney: to be fair we have chatted about it a few times now :) | 12:35 |
| natefinch-afk | davecheney: gotcha | 12:37 |
| davecheney | jam: next time's the charm | 12:37 |
| wallyworld | fwereade_: hiya, if you have time, would be great if you could look at this to ensure it matches our discussions. i hope to land it tomorrow so i can progress the simplestreams stuff with andrew https://codereview.appspot.com/13278043 | 12:57 |
| jam | TheMue: in talking with mramm he mentioned you'd likely be interested in being at the doc sprint on Friday. Is that still true? | 12:58 |
| jam | I can add you to the invite list so you get a reminder. | 12:59 |
| jam | hey wallyworld, how's your day been? | 12:59 |
| wallyworld | busy :-) | 13:00 |
| wallyworld | i know understand how horrible and hard charms are to write :-( | 13:00 |
| wallyworld | we so need to fix that | 13:00 |
| wallyworld | the mental model is difficult and there are so many ways to subtley shoot yourself in the foot | 13:01 |
| davecheney | warning: juju may contain traces of footgun | 13:01 |
| wallyworld | traces = copious quantities | 13:01 |
| TheMue | jam: i'm interested in supporting the documentation, yes. i only have some troubles on this friday as i would have to leave at about 8pm local time. | 13:02 |
| jam | TheMue: I don't think you are expected to be there the whole time | 13:02 |
| jam | but if you can be there during your work day that is still a great advantage. | 13:02 |
| TheMue | jam: yep, absolutely no prob. sounds fine to me | 13:03 |
| * rogpeppe goes for lunch | 13:25 | |
| TheMue | hmmmpf | 14:45 |
| TheMue | my software does not like me. first two missed tests i forgot but now a panic on the bot while i have a never seen failing test (but no panic) here. *sigh* | 14:52 |
| TheMue | and now it passes again. | 14:54 |
| TheMue | fu.. | 15:07 |
| TheMue | running the failing suite isolated it works, but with go test ./... it fails | 15:07 |
| mgz | that is not uncommon | 15:08 |
| TheMue | mgz: but also not nice | 15:14 |
| fwereade_ | TheMue, added a couple of comments to your review | 15:19 |
| TheMue | fwereade_: thx | 15:20 |
| TheMue | fwereade_: who's best to contact in the GUI team? | 15:21 |
| * TheMue has to leave, bank appointment | 15:41 | |
| === tasdomas is now known as tasdomas_afk | ||
| hazmat | fwereade_, the relationcount, unitcount on respective states (service, and relation) are primarily used for txn condition guards ? | 16:01 |
| natefinch-afk | arosales, mgz: either of you guys know how to set up juju with azure? I don't see anything on juju.ubuntu.com | 16:09 |
| mgz | not really, but you can just read the comment in the source/autogenerated config | 16:11 |
| mgz | provider/azure/config.go | 16:12 |
| natefinch-afk | mgz: cool, thanks | 16:12 |
| natefinch-afk | man, the SSO for azure is buggy. It only works if I log in from particular websites | 16:14 |
| natefinch-afk | evidently I should "contact your admin and report the following error: 80045C17" | 16:15 |
| === weblife is now known as web-brandon | ||
| rogpeppe | hmm, how is it possible that provider.StartInstance and provider.StartBootstrapInstance have gone in without any tests? | 18:03 |
| jamespage | anyone from the juju team care to join the 'deliverying juju 2.0 into saucy' session? | 18:04 |
| jamespage | rogpeppe, fwereade_, mramm ^^ | 18:04 |
| rogpeppe | jamespage: if that's happening now, i'm afraid i'm just reaching end of day, so can't without domestic consequences... | 18:05 |
| jamespage | mgz, ^^ | 18:06 |
| === natefinch-afk is now known as natefinch | ||
| rogpeppe | if anyone fancies a largish but pretty mechanical code review: https://codereview.appspot.com/13269045 | 18:43 |
| rogpeppe | natefinch: ^ | 18:43 |
| rogpeppe | and with that, i *must* leave! | 18:43 |
| rogpeppe | g'night all | 18:43 |
| natefinch | rogpeppe: I'll take a look. Gnight | 18:44 |
| kvt | surprising number of tests work on osx (+ mongodb w/ ssl) nice | 18:47 |
| natefinch | kvt: that's awesome. not entirely surprising, but it would be great if we could get all the tests passing on OSX. Ideally, we'd be able to run the tests on any platform juju supports. | 18:50 |
| hazmat | natefinch, there are a couple of lxc tests that make it barf | 18:56 |
| hazmat | the mongodb manual compilation for ssl was a bit unfortunate/time consuming, required some patching of scons config files. | 18:57 |
| natefinch | hazmat: yeah. lxc is kind of a problem child for cross compatibility. Still, if we can isolate that with OS compilation restrictions, that's ok. | 18:57 |
| * kvt nods | 18:58 | |
| kvt | kvt == hazmat + osx ;-) | 18:58 |
| thumper | morning | 21:02 |
| thumper | fwereade_: you around? | 21:02 |
| fwereade_ | thumper, heyhey | 21:02 |
| thumper | fwereade_: got time for a chat | 21:02 |
| thumper | ? | 21:02 |
| fwereade_ | thumper, sure, just a mo, would you start one please? | 21:02 |
| thumper | sure | 21:03 |
| bigjools | hullo | 22:54 |
| thumper | hi bigjools | 23:00 |
| bigjools | hey man | 23:01 |
| mramm | thumper: bigjools: hey all | 23:18 |
| bigjools | g'day mramm | 23:18 |
| thumper | hi mramm | 23:18 |
| davecheney | axw: hey | 23:19 |
| davecheney | off da phone | 23:19 |
| davecheney | gonna check out of my room and i'll see you in mitchel | 23:19 |
| axw | davecheney: okey dokey. I'm down there now | 23:19 |
| axw | davecheney: they took the wifi off when I mentioned we had a meeting room | 23:19 |
| davecheney | fuckers | 23:20 |
| axw | davecheney: ? | 23:20 |
| axw | davecheney: I mean they didn't charge me | 23:20 |
| mramm | davecheney: that is good at least | 23:21 |
| davecheney | oh right | 23:21 |
| davecheney | i thought they *turned it off* | 23:21 |
| axw | hah :) no sorry | 23:21 |
| axw | hey mramm, how's it going? | 23:22 |
| mramm | axw: pretty good | 23:22 |
| mramm | busy with all kinds of stuff for the other teams the last couple of weeks | 23:22 |
| mramm | seems like juju core land is running pretty well | 23:22 |
| axw | cool :) | 23:23 |
| sidnei | mramm: if i can get one bug in your prioritization radar, it'd be https://bugs.launchpad.net/juju-core/+bug/1190985 it's biting us madly :/ | 23:41 |
| _mup_ | Bug #1190985: Confusing update-charm and deploy -u behavior <juju-core:Triaged> <https://launchpad.net/bugs/1190985> | 23:41 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!