| jam | wallyworld_: I figured out what broke the test suite for the "no such variable hostname". | 04:19 |
|---|---|---|
| jam | The issue is that "hostname" is defined in service_test.go, but used in "service_http.go". | 04:19 |
| wallyworld_ | ah ok, i looked but didn't see how i broke it | 04:19 |
| jam | So everything works fine when you run tests in novaserviec | 04:19 |
| wallyworld_ | i found that too | 04:19 |
| jam | because it imports the test files to run the test suite. | 04:20 |
| jam | but when "nova" tests import "service_http" | 04:20 |
| jam | it doesn't import the test files | 04:20 |
| jam | (because it shouldn't) | 04:20 |
| wallyworld_ | makes sense | 04:20 |
| jam | so it is one of those cases where test definitions leak and make real code pass | 04:20 |
| jam | when it is broken if you try to use it directly. | 04:20 |
| wallyworld_ | too bad go fmt doesn't check compilation | 04:21 |
| wallyworld_ | there's also another breakage | 04:21 |
| jam | wallyworld_: right, so "go build" doesn't work in the novaservice directory | 04:21 |
| wallyworld_ | to do with nova something or other not implementinng HTTPServe | 04:21 |
| jam | so we should be using: go build ./... && go test ./... | 04:21 |
| wallyworld_ | go test ./... fails if there is a compile error | 04:21 |
| wallyworld_ | so perhaps go build is redundant there | 04:22 |
| jam | and, as I've said before, global state is terrible, we shouldn't have it in the first place. | 04:22 |
| wallyworld_ | yeah, agree | 04:22 |
| jam | wallyworld_: As I just said, cd testservice/novaservice ; go test passes | 04:22 |
| jam | go build fails | 04:22 |
| jam | we didn't *notice* until we used novaservice in nova testing. | 04:22 |
| wallyworld_ | hmm, i've never seen go test not have an error if go build fails | 04:22 |
| jam | because nova doesn't import the novaservice test package | 04:22 |
| jam | wallyworld_: try it right now with trunk | 04:23 |
| wallyworld_ | i'm prototyping in trunk, so it's a bt hard atm | 04:23 |
| wallyworld_ | i need to create a new branch to work in | 04:23 |
| jam | wallyworld_: essentially, we had a latent build failure that we didn't see at r40, in r41 you started importing the code, and triggered the failure. | 04:23 |
| jam | wallyworld_: "bzr shelve" ? | 04:23 |
| wallyworld_ | yeah, but i don't want to disrupt my train of thought just atm | 04:24 |
| jam | wallyworld_: np | 04:24 |
| wallyworld_ | thanks for looking into that further, i was going to mention itagain at the standup | 04:24 |
| hazmat | so for updating the juju-core src tree w. goose .. what do folks do.. go get -u ? | 04:25 |
| jam | hazmat: yes | 04:26 |
| jam | its not perfect, but it seems to be the best we have right now | 04:26 |
| hazmat | jam.. with what value/pkg name? | 04:26 |
| jam | hazmat: "go get -u launchpad.net/juju-core/..." | 04:26 |
| jam | should also update all dependencies | 04:26 |
| hazmat | jam, thanks | 04:26 |
| * hazmat always forgets where to put ... | 04:27 | |
| hazmat | i'm sure its easy once your used to it | 04:28 |
| * hazmat wonders if roger published his api callee tool | 04:30 | |
| hazmat | aha http://godoc.org/code.google.com/p/rog-go/exp/cmd/gosym | 04:32 |
| === _mup__ is now known as _mup_ | ||
| jam | wallyworld_: poke if you are still around | 12:01 |
| wallyworld_ | hi | 12:02 |
| jam | wallyworld_: so it might just be naming, but I'm looking over the code you pushed up, and it doesn't quite sit right. | 12:03 |
| wallyworld_ | which bit? | 12:03 |
| jam | "unauthenticatedClient": IMO unauthenticated is a state that can be remedied by authenticating | 12:03 |
| wallyworld_ | ok, i am happy to change names | 12:04 |
| jam | and having an Authenticator that doesn't authenticate... | 12:04 |
| wallyworld_ | it's a means to allow code to be shared | 12:04 |
| wallyworld_ | so the core logic to send requests can be ignorant of if it is running with somwthing that needs authenticatin | 12:05 |
| wallyworld_ | so the authentication stuff is just noops | 12:05 |
| jam | so, nonAuthenticatingClient seems to me like a client that wouldn't try to authenticate (naming), but I don't know that it would be an Authenticator. | 12:05 |
| jam | wallyworld_: but is that better than just having a boolean about whether it *needs* to authenticate? | 12:05 |
| wallyworld_ | hmmm. i could look at that for sure | 12:06 |
| wallyworld_ | the AUthentication interface though also provides MakeServiceURL | 12:06 |
| wallyworld_ | since that is different depending on the type of client | 12:07 |
| wallyworld_ | and go doesn't support virtual methods :-( | 12:07 |
| wallyworld_ | i really wanted to do it differently, like I would have in Java or C++, but alas | 12:07 |
| jam | wallyworld_: I agree that we likely need an interface at that level, I'm just trying to sort out how the pieces fit. And at least the naming of things meant it didn't quite fit well in my head. | 12:08 |
| wallyworld_ | i did struggle with the naming | 12:08 |
| wallyworld_ | the auth client needs to provide Authernticate(), UserId(), Token() etc | 12:09 |
| wallyworld_ | and the non auth client needs to provide noops for those if we want to allow the same client interface to be used everywhere | 12:10 |
| wallyworld_ | unless I cast when needed | 12:10 |
| wallyworld_ | which I can do | 12:10 |
| jam | wallyworld_: since we won't have any of that actual data, it seems like we should be doing it differently. | 12:11 |
| jam | we shouldn't return fake data for each one. | 12:11 |
| jam | like, if we don't have an authenticator, we try to do the request without authentication | 12:11 |
| jam | vs we have one that may just not have auth'd yet | 12:11 |
| wallyworld_ | i was trying to avoid type casts but i think i will need to do that | 12:11 |
| wallyworld_ | i'll rework it and resubmit | 12:11 |
| jam | c.auth != nil doesn't need a type cast :) | 12:12 |
| wallyworld_ | i wasn't 100% happy with the result, but I was happy that it all worked | 12:12 |
| wallyworld_ | no not there, in other places | 12:12 |
| wallyworld_ | like tests | 12:12 |
| wallyworld_ | but let me rework it - the 2nd version should be better | 12:13 |
| Aram | > go doesn't support virtual methods | 12:13 |
| wallyworld_ | yeah i know :-( | 12:13 |
| Aram | what? dynamic dispatch is an extremely used idiom in Go. | 12:13 |
| jam | Aram: go doesn't support subclassing in the standard sense | 12:13 |
| jam | is I think more what he was talking about | 12:14 |
| wallyworld_ | methods are staically bound to classes | 12:14 |
| Aram | use an interface. | 12:14 |
| wallyworld_ | it's a limitation Java and C++ refugees initially have to learn to deal with | 12:14 |
| wallyworld_ | yes, the zgo idiom is an interface but sometimes it would be easier not to have to do that | 12:15 |
| Aram | you don't have to do anything. you define an interface and type i := c somewhere instead of typing C implements I somewhere else. | 12:15 |
| wallyworld_ | you do have to rearrange your business logic sometimes | 12:16 |
| wallyworld_ | if you are used to virtual methods and expect a subclass to invoke the right method, you need to extract the business logic when you discover it doesn't work | 12:17 |
| wallyworld_ | to get it off the class and into an interface created just because virtual methods aren't supported | 12:17 |
| Aram | i.m() always invokes the right method if you set up i correctly (equivalent to setting a subclass correctly) | 12:17 |
| Aram | s/ui i/it up/ | 12:18 |
| wallyworld_ | yes, but it's a different arrangement of business logic and sometimes it means that stuff which belongs together on a class has to be split | 12:18 |
| wallyworld_ | because of non support for virtual methods | 12:19 |
| wallyworld_ | it's not such a big issue, just different to other languages which support such things | 12:19 |
| Aram | have to go buy some bread. | 12:19 |
| Aram | rogpeppe: ^ ^ | 12:19 |
| jam | wallyworld_: if you can set the MP to WIP (I can do it, but I'd rather not do that to other people's MPs) | 12:19 |
| wallyworld_ | jam: sure, thanks for the input, i'm glad you brought up the same issues i was having when i hit submit :-) | 12:20 |
| jam | wallyworld_: I'm glad you felt the same way, too. :) | 12:20 |
| wallyworld_ | jam: yeah, it was EOD and I just wanted to feel i had something done, so i thought it would be worth getting input | 12:21 |
| rogpeppe | jam, wallyworld: i'll have a look at this | 12:21 |
| wallyworld_ | rogpeppe: thanks, although i'm setting it back to wip to clean up some stuff, so don't look too hard just yet | 12:22 |
| rogpeppe | jam: in my experience, when these kinds of issues are resolved, the result is better structured. | 12:22 |
| rogpeppe | s/jam/wallyworld/ :-) | 12:22 |
| wallyworld_ | which issues? the virtual method dicussion? | 12:23 |
| rogpeppe | wallyworld_: yes | 12:23 |
| rogpeppe | wallyworld_: presumably we're talking about https://codereview.appspot.com/6962052/ here? | 12:23 |
| rogpeppe | wallyworld_: (i haven't looked at it yet) | 12:24 |
| wallyworld_ | yes | 12:24 |
| wallyworld_ | i do like virtual methods, but if they are no supported, then stuff needs to be done differently | 12:24 |
| rogpeppe | wallyworld_: i'm not sure what you mean. all interface methods are virtual | 12:26 |
| wallyworld_ | i mean on classes | 12:27 |
| wallyworld_ | stucts | 12:27 |
| wallyworld_ | structs | 12:27 |
| rogpeppe | wallyworld_: ah, you mean a class hierarchy isn't supported. that's true. | 12:27 |
| wallyworld_ | yeah, so logic in a bases class calls method foo() and if it is a subclass, the subclass's foo() get invoked | 12:28 |
| wallyworld_ | since that's not supported, you are forced to rip logic out of a class where it might be considered to belong | 12:28 |
| wallyworld_ | and put it elsewhere | 12:28 |
| rogpeppe | wallyworld_: this is actually a good thing, although it's probably difficult to see that currently... | 12:30 |
| wallyworld_ | yeah, difficult to see :-) since if everything else does it, it can't be that bad :-) | 12:30 |
| rogpeppe | wallyworld_: each type does what it does completely. | 12:31 |
| wallyworld_ | and it causes issues when you loose access to class state when the logic is removed | 12:31 |
| wallyworld_ | it's not necessarily bad, just different | 12:31 |
| rogpeppe | wallyworld_: in the end, IMHO, you get a better separation of concerns and more modular code. | 12:31 |
| wallyworld_ | i'll take your word for it, i'm still to come around to that way of thinking :-) not enough Go experience yet | 12:32 |
| wallyworld_ | i guess many people new to go have to unlearn past habits | 12:33 |
| rogpeppe | wallyworld_: it's certainly a different way of thinking. i think perhaps the crux is moving from thinking, when designing, "what *is* this?" to "what does this *do*?" | 12:34 |
| rogpeppe | wallyworld_: Go doesn't do "is a" relationships, other than when implementing interfaces | 12:34 |
| wallyworld_ | sure, i guess at the moment i don't see that "what does this do" way of thinking is mutually exclusive to having virtual methods. but maybe i'll get there | 12:35 |
| wallyworld_ | i'm actually having more of an issue with go's dependency version management | 12:36 |
| rogpeppe | wallyworld_: AFAICS, virtual methods are about defining something that is *almost* something else (but for the virtual methods). | 12:36 |
| rogpeppe | wallyworld_: yes, that's unfortunate - i have an idea for a little tool that might help with that. | 12:37 |
| wallyworld_ | rogpeppe: it seems that many go people will defend the current state of affairs with go get etc, even though it just seems so wrong | 12:38 |
| wallyworld_ | mixing what you are importing with the version you are import in the url which is then spread through every source file seems crazy to me | 12:39 |
| wallyworld_ | violates the dry principal big time | 12:39 |
| rogpeppe | wallyworld_: i'm not sure. it doesn't seem too different from having the version in some metadata file somewhere in the same source tree | 12:40 |
| wallyworld_ | big difference - you change the meta file in one place | 12:40 |
| wallyworld_ | not update each and every import | 12:40 |
| rogpeppe | wallyworld_: sure, it's repeated info, but what if it was a single two-word command to change all the paths (and check consistency)? | 12:40 |
| wallyworld_ | it introduces revision noise for starters | 12:41 |
| rogpeppe | wallyworld_: in the CL diffs? | 12:41 |
| wallyworld_ | yes, and source revision history | 12:41 |
| rogpeppe | wallyworld_: i'm not sure that's a big issue. you're going to have a revision change there anyway (for the metadata) - the only question is the size of the diff. | 12:42 |
| wallyworld_ | when reviewing or looking at past revisions, any excess noise distracts from trying to get to the heart of any changes | 12:43 |
| wallyworld_ | and why repeat something all throughout the source code? | 12:44 |
| rogpeppe | wallyworld_: because it makes the version dependency clear in every piece of code that has it? | 12:44 |
| rogpeppe | wallyworld_: i agree it's verbose, but i'm not sure that i see a better alternative currently. | 12:45 |
| wallyworld_ | that is true but i feel the cost outweighs the benefit | 12:45 |
| wallyworld_ | there's lots of better alternatives :-) | 12:45 |
| wallyworld_ | other languages/environments have solutions used successfully | 12:46 |
| rogpeppe | wallyworld_: there are quite a few benefits from having the package path as a universal currency that works independently of anything else. | 12:47 |
| wallyworld_ | perhaps if it weren't copy and pasted throughout the source code | 12:47 |
| rogpeppe | wallyworld_: honestly, i think that's a benefit. each piece of source code completely describes its own dependencies. | 12:48 |
| wallyworld_ | each peice of source code doesn't need to do that though - it's surperfulous boiler plate | 12:49 |
| wallyworld_ | humans make mistakes and i would be surprised if someone didn't miss changing one import somewhere, or worse, 2 people committed at around the same time with different imports | 12:49 |
| wallyworld_ | since the imports are repeated, someone might write a new source file with the "old" import version | 12:50 |
| wallyworld_ | and commit that not realising someone else has committed a change which updates the imports elsewhere | 12:50 |
| rogpeppe | wallyworld_: ok, here's my plan for a tool | 12:50 |
| rogpeppe | wallyworld_: called, say, godep | 12:51 |
| rogpeppe | wallyworld_: usage: godep pkg.... | 12:51 |
| rogpeppe | wallyworld_: it will look in all the source files under the current directory, and find any that have a version-independent matching package path | 12:52 |
| rogpeppe | wallyworld_: and change it to the specified package path | 12:52 |
| dimitern | wallyworld_, jam, etc. PTAL - just some comments and improved docs - https://codereview.appspot.com/6968051 | 12:52 |
| rogpeppe | wallyworld_: where a versioned package path is defined to be anything with an intermediate path name matching v[0-9]+ | 12:52 |
| rogpeppe | s/intermediate path name/intermediate path element/ | 12:53 |
| wallyworld_ | rogpeppe: so the first time it is run, version independent paths get changed, but what about the next time? | 12:53 |
| rogpeppe | wallyworld_: it will also check all dependencies and make sure that they all import the same version | 12:53 |
| * dimitern >> lunch | 12:53 | |
| rogpeppe | wallyworld_: all paths are versioned | 12:53 |
| rogpeppe | wallyworld_: just that labix.org/v2/mgo will match labix.org/v3/mgo, for example | 12:54 |
| wallyworld_ | ah ok | 12:54 |
| rogpeppe | actually, govers was the name i was thinking of originally | 12:54 |
| wallyworld_ | rogpeppe: that all sounds reasonable. it's the go equivalent of editing versions.cfg and changing mgo v2 to mgo v3 and comitting :-) | 12:55 |
| rogpeppe | wallyworld_: so then, to make juju-core use a new version of goose, you'd cd to juju-core root, and do govers launchpad.net/goose/v2 | 12:55 |
| rogpeppe | wallyworld_: yes, but without the commit | 12:56 |
| wallyworld_ | right | 12:56 |
| wallyworld_ | rogpeppe: late here, off to bed, thanks for the interesting discussion. too bad it wasn't at a pub over some beers | 12:58 |
| rogpeppe | wallyworld_: too right | 12:58 |
| wallyworld_ | next UDS maybe :-) | 12:58 |
| rogpeppe | wallyworld_: lets! | 12:58 |
| rogpeppe | wallyworld_: sweet dreams :-) | 12:58 |
| wallyworld_ | be warned, i start dancing on the table after i have 1 or 2 pints | 12:58 |
| rogpeppe | wallyworld_: lol | 12:59 |
| rogpeppe | Aram, dimitern, fwereade_: i'd be interested to know what you think of the above idea | 13:30 |
| hazmat | rogpeppe, would you have any time today or tomorrow to discuss api | 14:58 |
| rogpeppe | hazmat: definitely | 14:58 |
| rogpeppe | hazmat: could do it now if you'd like | 14:59 |
| hazmat | rogpeppe, sounds good | 14:59 |
| rogpeppe | hazmat: https://plus.google.com/hangouts/_/2887f22fb3213cc9e9ccd07cd3569c8e06a255ee?authuser=0&hl=en-GB | 15:00 |
| fwereade__ | rogpeppe, I hate to be a bore, but I think that https://codereview.appspot.com/6944058/ is uncontroversial in the light of our discussions online, and so I hope it should be an easy re-review | 17:01 |
| rogpeppe | fwereade__: ok. i thought you were on hols! :-) | 17:02 |
| rogpeppe | fwereade__: am on it | 17:02 |
| fwereade__ | rogpeppe, I am, but, y'know -- laura is painting, cath's out buying booze, I am otherwise at a loose end ;p | 17:02 |
| rogpeppe | fwereade__: here's a little one for you (preparatory to losing flags from jujud): https://codereview.appspot.com/6972048/ | 17:03 |
| fwereade__ | rogpeppe, cool | 17:03 |
| fwereade__ | rogpeppe, except that CL apparently does not exist | 17:03 |
| rogpeppe | fwereade__: oh bugger, one mo | 17:04 |
| rogpeppe | fwereade__: https://codereview.appspot.com/6963050/ | 17:04 |
| rogpeppe | fwereade__: (i proposed it against a badly named branch originally) | 17:04 |
| rogpeppe | fwereade__: LGTM | 17:08 |
| fwereade__ | rogpeppe, thanks -- and likewise :) | 17:09 |
| fwereade__ | rogpeppe, I won;t merge it for a bit yet, so go ahead with yours if it's convenient | 17:09 |
| rogpeppe | fwereade__: it's fine - i'm actually knocking up a quick tool to try to ease the goose versioning pains | 17:10 |
| rogpeppe | fwereade__: i'd be interested in your views on the idea actually | 17:10 |
| * rogpeppe is off for the night. | 18:33 | |
| rogpeppe | g'night anyone who's around | 18:33 |
| arosales | jcastro_: ping | 20:03 |
| hazmat | arosales, aren't you on holiday | 20:13 |
| arosales | hazmat: sudo holiday :-) | 20:16 |
| jcastro_ | that should be pseudo holiday, heh | 20:49 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!