| * thumper is losing the will to finish this bit of work | 01:18 | |
| * thumper takes a look at that critical bug that fwereade suggested | 01:19 | |
| thumper | time to step back and look at the pending reviews... | 01:46 |
|---|---|---|
| axw | wallyworld_, thumper: what are your thoughts on doing `strings.HasSuffix(os.Args[0], ".test")` for changing default behaviour when running tests? | 02:00 |
| thumper | ?! | 02:00 |
| axw | my gut feeling is that it's bad, because it's adding test logic into non-test code | 02:00 |
| thumper | horrible | 02:00 |
| wallyworld_ | context? | 02:00 |
| * thumper school run | 02:00 | |
| axw | wallyworld_: disabling the finish-bootstrap stuff | 02:00 |
| axw | I was thinking we could just disable it always when running tests by doing something hackish like that | 02:01 |
| wallyworld_ | yeah, go to agree with thumper on that one. i hate adding TestFoo() stuff but Go sucks in that it doesn't allow a better way to do it | 02:01 |
| wallyworld_ | TestingFoo() even | 02:01 |
| axw | wallyworld_: how would you do it with, say, Python? | 02:02 |
| axw | I guess you'd just monkey patch | 02:03 |
| wallyworld_ | yeah | 02:03 |
| axw | but that's what I'm doing here... | 02:03 |
| wallyworld_ | but the test code has to be in the production code | 02:03 |
| axw | well, we could expose the finishBootstrap var | 02:04 |
| axw | the TestingX thing isn't strictly necessary | 02:04 |
| wallyworld_ | we could, and that has been done elsewhere eg var FinishBootstrap = finishBootstrap | 02:04 |
| wallyworld_ | i'd prefer the above | 02:05 |
| axw | which above? | 02:05 |
| axw | :) | 02:05 |
| wallyworld_ | with a suitable comment, saying "exposed for testing" | 02:05 |
| wallyworld_ | var FinishBootstrap = finishBootstrap | 02:05 |
| axw | hmmkay. I didn't do that to cut down on duplication | 02:05 |
| wallyworld_ | what duplication? | 02:05 |
| axw | defining a replacement function. I guess I could just put that in another package tho | 02:06 |
| wallyworld_ | maybe i'm missing something | 02:06 |
| wallyworld_ | i'm just suggesting introducing anj exported var | 02:07 |
| wallyworld_ | so the testing stuff can live inside the _test file | 02:07 |
| axw | wallyworld_: and then remove the Testing functions, right? | 02:07 |
| wallyworld_ | yes | 02:07 |
| axw | wallyworld_: so the contents of those are either duplicated, or moved | 02:07 |
| axw | no big deal I suppose | 02:08 |
| wallyworld_ | there's also a pattern where a testing package is introduced | 02:08 |
| axw | yeah, I'll probably go with that | 02:08 |
| wallyworld_ | and all the callers over various other packages call the stuff in the testing package | 02:08 |
| axw | provider/common/testing or so | 02:08 |
| wallyworld_ | yeah | 02:08 |
| wallyworld_ | still messy compated with python etc al | 02:08 |
| thumper | not entirely surprising that | 02:10 |
| thumper | given that go is compiled and strictly typed | 02:11 |
| wallyworld_ | and it doesn't have friends like C++ or access to "private" methods like python | 02:13 |
| axw | I never liked friends in C++ | 02:15 |
| thumper | I didn't like friends in C++ either | 02:15 |
| thumper | friendship isn't really that useful | 02:16 |
| thumper | because you have to give it | 02:16 |
| thumper | and it tightly couples things together | 02:16 |
| thumper | it would be nice though if you had the ability to say "i know what I'm doing" in go | 02:16 |
| thumper | anyway... | 02:17 |
| abentley | axw: For a minute I thought you were using "like" and "friends" in the Facebook sense. My mind may be permanently warped. | 02:31 |
| axw | abentley: :) I don't Like my facebook Friends either | 02:32 |
| axw | thumper: you added a whole lot of stuff to the CL you just updated | 03:05 |
| axw | the local provider rsyslog one | 03:05 |
| thumper | really? | 03:05 |
| * thumper grunts | 03:05 | |
| thumper | I merged trunk | 03:05 |
| thumper | and lbox shits itself | 03:06 |
| axw | ah | 03:06 |
| axw | because of the prereq maybe? | 03:06 |
| thumper | in all honesty there is nothing interesting in that diff, just questions around the interface... | 03:06 |
| thumper | maybe | 03:06 |
| axw | thumper: I mean the other one - but anyway, LGTM | 03:07 |
| axw | I didn't realise rsyslog was "Important" | 03:07 |
| thumper | :) | 03:07 |
| axw | should've checked | 03:07 |
| thumper | even on my vanilla precise box we have /etc/rsyslog.d | 03:07 |
| thumper | I was trying to do a reverse depends on rsyslog | 03:08 |
| thumper | but not sure how to drive dpkg | 03:08 |
| axw | not sure, but the Priority=Important, so it's in the base system | 03:09 |
| thumper | axw: ok, so ubuntu-minimal depends on rsyslog | 03:09 |
| axw | okey dokey | 03:09 |
| thumper | although | 03:09 |
| thumper | technically, we should add it as a depends to juju-local | 03:10 |
| thumper | apt-cache rdepends pkg-name FWIW | 03:10 |
| axw | thumper: thanks for the comment on tagOffset, the 1-indexing was the bit that confused me | 03:11 |
| thumper | :) | 03:11 |
| thumper | yeah, who indexes from 1? | 03:11 |
| axw | not sure, maybe the same people who say Monday is the first day of the week | 03:11 |
| axw | anyway... /me gets back to synchronous bootstrap | 03:12 |
| axw_ | wallyworld_: I've updated the synchronous bootstrap CL. There's now no upper limit on DNS/SSH wait, and it can be cancelled with SIGINT. Also added the output as you suggested, and better progress reporting | 06:05 |
| axw_ | haven't tested against the other providers yet | 06:06 |
| wallyworld_ | axw_: ok, looking, thanks | 06:09 |
| wallyworld_ | axw_: you were going to introduce a testing package for those test helpers? | 06:25 |
| axw_ | doh | 06:25 |
| axw_ | I was | 06:25 |
| axw_ | wallyworld_: I'll update again later with that | 06:27 |
| wallyworld_ | yeah, i'm not going ti blokc on that | 06:27 |
| * wallyworld_ -> walk the dog | 06:48 | |
| rogpeppe | axw__, wallyworld_, TheMue: mornin' all | 07:21 |
| axw__ | morning rogpeppe | 07:22 |
| rogpeppe | wallyworld_: thanks for the review | 07:22 |
| wallyworld_ | rogpeppe: np. nice to see that add machine stuff get some lovin' | 07:26 |
| rogpeppe | wallyworld_: glad you're ok with it | 07:27 |
| wallyworld_ | why wouldn't i be? :-) | 07:27 |
| rogpeppe | wallyworld_: it took me a full day to understand the old code :-) | 07:27 |
| wallyworld_ | i have had a had in it but a lot of the code predated my time :-) | 07:27 |
| wallyworld_ | /had/hand | 07:27 |
| rogpeppe | wallyworld_: i ended up manually simulating each path through it and writing down the trace | 07:28 |
| wallyworld_ | i sorta just added to it, without refactoring | 07:28 |
| wallyworld_ | i like it that the top level methods reflect each business case | 07:28 |
| rogpeppe | wallyworld_: you're right about the machine doc duplication BTW - i'm just adding a machineDocForTemplate function | 07:28 |
| wallyworld_ | \o/ thanks :-) | 07:28 |
| rogpeppe | wallyworld_: i plan on losing AddMachineParams entirely BTW and exporting machineTemplate | 07:29 |
| wallyworld_ | any code that takes a day to understand needs refactoring :-) | 07:29 |
| rogpeppe | wallyworld_: well, perhaps it was just my small brain... | 07:29 |
| wallyworld_ | that would be good cause that but of duplication will disappear with it | 07:30 |
| wallyworld_ | bit | 07:30 |
| wallyworld_ | i have a small brain too, smaller than most :-) | 07:30 |
| rogpeppe | wallyworld_: yeah. and the code that digs around in it to work out what the caller is actually trying to do. | 07:30 |
| wallyworld_ | i think the final outcome will be good | 07:31 |
| rogpeppe | wallyworld_: i hope so. i feel that it should be straightforward to do the next modifications that i need to do. | 07:35 |
| wallyworld_ | i'm always up for a good refactoring :-) | 07:35 |
| rogpeppe | wallyworld_: BTW the logger.Info lines about transaction hooks will only ever happen in tests - transaction hooks are explicitly only there for testing | 07:38 |
| wallyworld_ | ah ok | 07:38 |
| wallyworld_ | maybe a comment in the code so the reader knowns the hooks are only run in tests? | 07:39 |
| wallyworld_ | just as an fyi? | 07:39 |
| rogpeppe | wallyworld_: will do | 07:39 |
| wallyworld_ | thanks :-) | 07:40 |
| rogpeppe | fwereade, axw__: your reviews of https://codereview.appspot.com/30390043/ would be appreciated too, as it's a significant change | 08:00 |
| axw__ | rogpeppe: ok, will take a look | 08:00 |
| rogpeppe | axw__: thans | 08:00 |
| rogpeppe | ks | 08:00 |
| fwereade | rogpeppe, looking | 08:00 |
| fwereade | axw__, I haven't followed up on your CL -- did what I said yesterday make sense? | 08:01 |
| axw__ | fwereade: are you talking about the state changes? | 08:01 |
| fwereade | axw__, yeah, the environment life stuff | 08:02 |
| axw__ | fwereade: yup, makes sense - I checked with frankban about the tag usage | 08:02 |
| axw__ | I sent you an email about it. but yes, they do use it | 08:02 |
| fwereade | axw__, great, thanks | 08:02 |
| axw__ | so I guess we'll have to not validate them for now | 08:02 |
| * fwereade tries to keep up to date with mail :/ | 08:03 | |
| axw__ | fwereade, rogpeppe: if you're interested, I've got a synchronous bootstrap CL up for review here: https://codereview.appspot.com/30190043/ | 08:03 |
| rogpeppe | axw__: looking | 08:03 |
| axw__ | wallyworld_ has given it a once over, but this is a non-trivial change, so more eyes the better | 08:04 |
| axw__ | well, a thrice over I think | 08:04 |
| axw__ | :) | 08:04 |
| axw__ | fwereade: with the environment life CL, I still have a question about how we signal state servers to kill themselves | 08:06 |
| axw__ | fwereade: that was the reason for Remove (and EnsureDead before it) | 08:06 |
| fwereade | axw__, off the top of my head: would it make sense to have a path that ties it into the Remove of a manually provisioned machine? current schema might not accommodate that, though | 08:09 |
| fwereade | axw__, might require per-provider provisioned-machine refcounts | 08:09 |
| fwereade | axw__, so forget that for now :( | 08:09 |
| axw__ | fwereade: refcounts for what sorry? | 08:10 |
| fwereade | axw__, the number of manually provisioned machines in existence | 08:10 |
| axw__ | I don't understand how that'd be used here | 08:11 |
| fwereade | axw__, possibly broken down by role -- we can kill the environment (or the manual state servers) once all the manual normal nodes have been killed | 08:12 |
| axw__ | fwereade: ok. I think we're thiinking the same thing, but you want transactional guarantees on mongo? | 08:12 |
| fwereade | axw__, yes | 08:12 |
| fwereade | axw__, they're tedious to work with but they're a lot better than nothing | 08:13 |
| axw__ | fwereade: sure. I was going to put all the logic inside apiserver/client, but you probably gathered that already | 08:13 |
| axw__ | (hence the env Dying -> prevents machine addition) | 08:14 |
| fwereade | axw__, I feel that an env being set to Dying should set things in motion in itself | 08:16 |
| fwereade | axw__, the dying preventing machine addition is necessary regardless, isn't it? | 08:16 |
| fwereade | axw__, assume multiple clients | 08:16 |
| fwereade | axw__, it's a little bit thin, but have you read doc/hacking-state.txt? and the various other lifecycle related docs? | 08:17 |
| axw__ | fwereade: no, but I will now :) | 08:18 |
| fwereade | axw__, I'm afraid the lifecycle stuff has not entirely kept up with reality (I might get around to fixing them in my Copious Free Time) but things haven't diverged that much -- most of them should still be actually true | 08:19 |
| rogpeppe | fwereade: how strongly do you feel about Machine.doc.TxnRevno, and in particular how important is it that AddMachine do a refresh immediately after creation to pick up an accurate txn-revno? | 08:22 |
| rogpeppe | fwereade: i've just realised that it's racy, and it seems unnecessary. | 08:23 |
| fwereade | rogpeppe, IIRC it's necessary for the watcher -- but I think I'd be fine with Machine.Watch grabbing a fresh TxnRevno before we start watching | 08:24 |
| fwereade | rogpeppe, Ican't think of any cases where the API server calls Watchon a freshly-added machine | 08:25 |
| rogpeppe | fwereade: it's not really necessary for the watcher tbh | 08:25 |
| fwereade | rogpeppe, fwiw the point is not that it be accurate, specifically, but that it have a value that corresponds with reality | 08:25 |
| rogpeppe | fwereade: you'll get an extra event if txn-revno is too low, but otherwise the watchers work fine | 08:26 |
| rogpeppe | fwereade: and that extra event might happen anyway, if someone's made a change since you last refreshed | 08:26 |
| fwereade | rogpeppe, I recall niemeyer got really irritated when I tried to write watchers that just treated the last-known-revno as -1, so I internalised it as something necessary, but I don't recall the details-- yeah, that had been my initial supposition | 08:26 |
| rogpeppe | fwereade: i'm thinking that just leaving it as zero when adding a machine should be fine | 08:27 |
| fwereade | rogpeppe, ha! | 08:27 |
| fwereade | rogpeppe, I factored that stuff out anyway | 08:27 |
| fwereade | rogpeppe, have a look around, see if *anyone* uses that field | 08:27 |
| fwereade | rogpeppe, I'd love it if we just dropped it | 08:27 |
| rogpeppe | fwereade: the machineUnits watcher does | 08:27 |
| rogpeppe | fwereade: but tbh i don't think it needs to | 08:28 |
| rogpeppe | fwereade: the only problem is the test that specifically tests that txnrevno is correct after AddMachine (TestAddAndGetEquivalence) | 08:30 |
| fwereade | rogpeppe, I think that really I'd prefer not to have watchers that potentially start with double-events signifying no change | 08:31 |
| rogpeppe | fwereade: i don't think any extra event will be emitted, but... let me check | 08:32 |
| fwereade | rogpeppe, but equally that txn-revno lookups should probably be secreted away inside the watcher code | 08:32 |
| fwereade | rogpeppe, the TxnRevno fields then become useful only when doing ultra-pedantic txn assertions and can in generalbe dropped | 08:32 |
| rogpeppe | fwereade: i finally gave in and did it. what do you think of this? https://codereview.appspot.com/30770043 | 10:19 |
| fwereade | rogpeppe, if it's what we were talking about above, I love it already, but I've got to go to laura's school right now :( | 10:20 |
| fwereade | (not emergency but appointment) | 10:20 |
| rogpeppe | fwereade: it's not quite | 10:20 |
| rogpeppe | fwereade: but it was prompted by it | 10:20 |
| rogpeppe | fwereade: it's a DeepEquals checker that treats empty slices == nil slices | 10:20 |
| rogpeppe | axw_, dimitern, wallyworld_: would appreciate your thoughts too | 10:21 |
| dimitern | rogpeppe, looking | 10:22 |
| * wallyworld_ has had several drinks, being friday evening and all | 10:22 | |
| wallyworld_ | but in general i don't agree empty == nil | 10:22 |
| wallyworld_ | well, maybe go likes to think they are equal, but that i disagree with | 10:23 |
| wallyworld_ | since nil should be a distinct value with a different semantic meaning to empty | 10:23 |
| wallyworld_ | at least that's the case in all other languages i have used | 10:24 |
| wallyworld_ | and it allows you to distinguish between "i don't know" and "empty" | 10:24 |
| mattyw | fwereade, rogpeppe who would be best to talk to about the juju deploy command - how it works currently and how it might work in the future (I know there are some changes coming at some point) | 10:36 |
| rogpeppe | wallyworld_: go doesn't think they're equal, but in many cases we do treat them as the same | 10:37 |
| dimitern | rogpeppe, reviewed | 10:37 |
| rogpeppe | dimitern: thanks | 10:37 |
| wallyworld_ | rogpeppe: that i find an issue with | 10:38 |
| wallyworld_ | since nil is semantically different to empty | 10:38 |
| rogpeppe | wallyworld_: it's actually only an issue for DeepEquals | 10:38 |
| rogpeppe | wallyworld_: because otherwise you can't compare slices | 10:38 |
| rogpeppe | wallyworld_: (except against nil, which we almost never do) | 10:38 |
| wallyworld_ | i think is an unfortunate Go idiom or some such | 10:39 |
| wallyworld_ | usually, nil = "i don't have a value for this thing" | 10:39 |
| rogpeppe | mattyw: i'm happy to chat about it; fwereade probably has the best bird's eye view of where things might be going | 10:39 |
| rogpeppe | wallyworld_: luckily in go we don't need to use in-band signalling so much | 10:39 |
| mattyw | rogpeppe, ok - so the way it works now - it actually does a bzr branch locally (more or less) | 10:40 |
| rogpeppe | wallyworld_: lots of things *can't* be nil (ints, strings, etc) | 10:40 |
| wallyworld_ | rogpeppe: yeah, i'm not sure if that's a cause or a consequence | 10:40 |
| wallyworld_ | in a true oo world everything can be nil | 10:40 |
| wallyworld_ | which sadly go is not | 10:40 |
| rogpeppe | wallyworld_: i'm glad that we don't have nil everyewhere | 10:41 |
| rogpeppe | wallyworld_: value types are ace | 10:41 |
| rogpeppe | mattyw: go on | 10:41 |
| * TheMue thinks back of his smalltalk time *sigh* | 10:41 | |
| mattyw | rogpeppe, that's right is it - more or less a bzr branch happens on the machine you run juju deploy on? | 10:42 |
| rogpeppe | mattyw: i'm not sure it does; let me check | 10:42 |
| rogpeppe | mattyw: no i don't think it does | 10:43 |
| rogpeppe | mattyw: it pulls a charm bundle from the charm store (if the charm's not local) | 10:43 |
| rogpeppe | mattyw: the charm bundle is a tgz file | 10:43 |
| mattyw | rogpeppe, ok - and I believe that might be changing so that it happens on the state server instead of locally? | 10:44 |
| rogpeppe | mattyw: if you do a deploy through the api, that already implements that logic, so nothing is created locally | 10:44 |
| rogpeppe | mattyw: yes. except for local charms, which must be pushed up to the server | 10:44 |
| mattyw | rogpeppe, ok - that's probably all I need for know - thanks very much - don't leave the country though | 10:46 |
| * rogpeppe puts his passport down | 10:46 | |
| natefinch | fwereade: standup? | 10:48 |
| mattyw | rogpeppe, when would you have a spare few minutes for a quick hangout? | 10:58 |
| natefinch | mattyw: we're in a hangout right now, but should be done soon | 11:05 |
| mattyw | natefinch, no problem I thought that was the case - just getting my booking in early ;) | 11:07 |
| rogpeppe | mattyw: i'll just look at a review, have a bowl of cereal and then we'll do that | 11:11 |
| mattyw | rogpeppe, ok - no particular hurry | 11:15 |
| jamespage | anyone around with an azure account who could confirm https://bugs.launchpad.net/juju-core/+bug/1246320 with the juju-core package that's in saucy proposed? (details on how in the bug report) | 11:26 |
| _mup_ | Bug #1246320: Azure bootstrap fails: versioning header is not specified <azure> <bootstrap> <verification-needed> <Go Windows Azure Client Library:Fix Committed by wallyworld> <juju-core:Fix Committed by wallyworld> <juju-core 1.16:Fix Released by wallyworld> <juju-core trunk:Fix Committed by wallyworld> <juju-core (Ubuntu):Fix Released> <juju-core (Ubuntu Saucy):Fix Committed> <juju-core (Ubuntu Trusty):Fix Released> <https://launchpad.net/bugs/12 | 11:26 |
| rogpeppe | mattyw: you got a hangout location? | 11:28 |
| natefinch | jamespage: I have an azure account, but limited time. I could share my azure creds with you if that's helpful | 11:29 |
| jamespage | natefinch, please | 11:29 |
| jamespage | (its the last bug needing verficiation) | 11:29 |
| natefinch | jamespage: sent | 11:33 |
| rogpeppe | dimitern: responded to your review. | 11:49 |
| dimitern | rogpeppe, i'm not entirely convinced | 11:51 |
| dimitern | rogpeppe, if we're going to copy/paste stuff from the go source into our codebase, it should be consistent in formatting | 11:52 |
| rogpeppe | dimitern: if we're copy/pasting something, it should be as identical as possible with the original so that when the original changes there are as few conflicts as possible | 11:52 |
| dimitern | rogpeppe, if otoh we're going to follow possible go source changes and use/patch as needed when it changes, it doesn't belong in juju-core, but in an external package | 11:52 |
| rogpeppe | dimitern: we've already got one precedent, in thirdparty/pbkdf2 | 11:53 |
| rogpeppe | dimitern: i don't want to add an external dependency for this | 11:53 |
| dimitern | rogpeppe, which was done before we decided on the current rules of consistency | 11:53 |
| rogpeppe | dimitern: i don't think that vendored code should be subject to the same rules | 11:54 |
| dimitern | rogpeppe, I don't believe making our code messier is helpful for long-term maintenance | 11:54 |
| dimitern | ;) | 11:54 |
| rogpeppe | dimitern: it's not really our code | 11:54 |
| rogpeppe | dimitern: it's their code + minimal patches to make it do what we need | 11:54 |
| dimitern | rogpeppe, then what's wrong with an external dependency? | 11:54 |
| rogpeppe | dimitern: it seems overkill to make a whole new project just for this. and i don't think it solves anything. | 11:55 |
| dimitern | rogpeppe, call it code.google.com/rog/deepequals or something | 11:55 |
| rogpeppe | dimitern: i don't see what issue it solves | 11:55 |
| dimitern | rogpeppe, use an existing project then | 11:55 |
| dimitern | rogpeppe, consistency | 11:55 |
| dimitern | rogpeppe, that's what it solves | 11:56 |
| rogpeppe | dimitern: i don't believe that consistency is an iron rule, particular in this kind of case | 11:56 |
| natefinch | rogpeppe: I agree with dimitern... I think we put way too much under juju-core that should be independent projects | 11:56 |
| dimitern | natefinch, +1 | 11:56 |
| dimitern | rogpeppe, we should be going leaner and more consistent, not the other way around | 11:56 |
| natefinch | rogpeppe: this has nothing to do with juju specifically. It just happens to be useful for us. | 11:57 |
| rogpeppe | dimitern: i'd prefer us to have less dependencies not more | 11:57 |
| rogpeppe | natefinch: true, but i don't think that's a bad thing | 11:57 |
| dimitern | rogpeppe, complex projects like juju are composed of many dependencies | 11:57 |
| dimitern | rogpeppe, nothing wrong with that, it's a better design, cleaner | 11:57 |
| natefinch | rogpeppe: dependencies shouldn't be a problem. If they are, we should address that, but I don't think they are. | 11:57 |
| rogpeppe | dimitern: the less dependencies, the less fragile IMO | 11:57 |
| dimitern | rogpeppe, i disagree | 11:58 |
| natefinch | rogpeppe: true for dependencies not controlled by us. But we do control this one. | 11:58 |
| rogpeppe | dimitern: it's a fundamental truth | 11:58 |
| rogpeppe | dimitern: even if we do control the dependency, it's one more entity to manage | 11:58 |
| dimitern | rogpeppe, smaller, better organized chunks that are internally consistent and do best one or a few things is better than a monolithic whole | 11:59 |
| dimitern | rogpeppe, what's wrong with managing more? | 11:59 |
| rogpeppe | dimitern: it's not monolithic - we're organised into independent package within juju-core. i really don't see the problem here. | 11:59 |
| rogpeppe | dimitern: more work to do | 11:59 |
| jamespage | natefinch, thanks | 12:00 |
| dimitern | rogpeppe, we're coming the the old discussion of "why having a huge package like state with no internal substructure is worse than smaller, well-defined packages" | 12:00 |
| dimitern | rogpeppe, less is more in this case | 12:00 |
| rogpeppe | dimitern: i think that's a different issue actually | 12:01 |
| natefinch | rogpeppe: for this particular case in question... it's a very well-defined project that does one thing and does it well and is unlikely to see a lot of churn. in this case, the overhead of creating and maintaining a project should be minimal. | 12:02 |
| dimitern | rogpeppe, it's true we were not always following the principle of "what's in juju-core is juju related", but we can improve on that by not making it worse | 12:02 |
| rogpeppe | natefinch: that's an argument for moving all of testing/checkers into an external package | 12:03 |
| rogpeppe | natefinch: which might be a reasonable thing to do | 12:03 |
| natefinch | rogpeppe: I was actually just thinking that | 12:03 |
| dimitern | rogpeppe, i'm not arguing just for the sake of it - this specific case is a trivial example, but shows a fundamental design issue with many parts of the codebase, which have not place in the main code base | 12:03 |
| dimitern | rogpeppe, i agree to moving checkers outside | 12:03 |
| rogpeppe | dimitern: for me, it depends if some other project might want to use those parts | 12:04 |
| dimitern | rogpeppe, it doesn't really matter | 12:04 |
| dimitern | rogpeppe, what matters is to make our life easier by removing reusable chunks like that, not specific to juju, even if noone else uses them | 12:04 |
| rogpeppe | dimitern: there *is* overhead in maintaining an external project dependency | 12:04 |
| rogpeppe | dimitern: i don't see how it makes *anything* easier | 12:05 |
| dimitern | rogpeppe, and doing so will make them more likely to be reused, than if there are in juju | 12:05 |
| rogpeppe | dimitern: perhaps we don't want them to be reused | 12:05 |
| dimitern | rogpeppe, what's the overhead of maintaining a set of reusable, well-defined helpers, like the checkers, which certainly do not change often | 12:05 |
| rogpeppe | dimitern: because if something is reused by external code, then we're not nearly as free to change if if our requirements change | 12:06 |
| rogpeppe | dimitern: non-zero | 12:06 |
| dimitern | rogpeppe, we can't stop them from being reused, even now, but it's not obvious | 12:06 |
| dimitern | rogpeppe, not true - we still control the external project | 12:06 |
| rogpeppe | dimitern: that's true | 12:06 |
| natefinch | rogpeppe: that is an interesting point, about having to worry about external use.... keeping stuff in juju-core means we can hack at it any way we like, since no one in their right mind would link against anything under juju-core | 12:06 |
| dimitern | rogpeppe, by your logic goose should've been part of juju-core | 12:07 |
| rogpeppe | natefinch: well, i kinda hope that people will in time, but that's a different issue | 12:07 |
| dimitern | rogpeppe, or the gomaasapi - who else uses them? | 12:07 |
| rogpeppe | dimitern: goose should be useful by other people wanting to use openstack, just like goamz is useful by other people wanting to use aws | 12:07 |
| rogpeppe | dimitern: same there | 12:07 |
| natefinch | rogpeppe: only for juju-specific stuff. Anything else could break at any time, since it's so deep in our codebase | 12:07 |
| dimitern | rogpeppe, right, and a set of comprehensive checkers, that have nothing to do with juju in particular, also | 12:08 |
| natefinch | rogpeppe: I think this and the checkers are a good example of prime subjects for extraction.... because they're well defined and no going to have their interfaces change | 12:08 |
| rogpeppe | dimitern: except i don't want us to depend on 100 external projects, each created just because it was possible to factor them out | 12:08 |
| dimitern | rogpeppe, why? | 12:09 |
| dimitern | rogpeppe, that's the main issue of argument | 12:09 |
| rogpeppe | dimitern: because i don't think it helps anything | 12:09 |
| dimitern | rogpeppe, i don't think we're getting anywhere here | 12:09 |
| dimitern | rogpeppe, ;) | 12:10 |
| rogpeppe | dimitern: yeah, i guess | 12:10 |
| natefinch | dimitern: I think it's valid to say that there is an expectation of API compatibility in independent projects that we don't have under juju-core. Stuff under juju-core is sort of assumed to be part of a larger project and not intended for independent consumption | 12:10 |
| rogpeppe | natefinch: +1 | 12:10 |
| rogpeppe | natefinch: *particularly* for stuff under testing... | 12:10 |
| dimitern | rogpeppe, you're generalizing a specific case where using an external dep is a good thing | 12:11 |
| natefinch | rogpeppe: but that being the case, the checkers and your lax deepequals are still things that probably are fine and good to have as independent packages | 12:11 |
| dimitern | rogpeppe, i'm not saying "let's have 100 deps", i'm saying let's apply some common sense | 12:11 |
| rogpeppe | dimitern: i haven't seen a single argument as to *why* using more external deps is a Good Thing | 12:11 |
| dimitern | natefinch, exactly | 12:11 |
| natefinch | rogpeppe: it makes us better open source citizens and helps out fellow go programmers, including people from Canonical | 12:12 |
| dimitern | rogpeppe, code describes linked concepts and abstractions, all related | 12:12 |
| rogpeppe | natefinch: ok, that's a good argument | 12:12 |
| rogpeppe | dimitern: that's too abstract for me | 12:12 |
| dimitern | rogpeppe, if they're not related, they shouldn't be coupled together | 12:12 |
| rogpeppe | dimitern: that sounds like a statement of belief, not an actual practical reason | 12:13 |
| dimitern | rogpeppe, would you put a wordprocessor and a database in the same codebase, if they just happen to connect sometimes, but can operate totally independently? | 12:13 |
| natefinch | rogpeppe: definitely we have to be pragmatic. We can't sacrifice our own flexibility... but we're not saying that... just saying, if it's not going to cause much pain and someone else might want to use it, then make it an external project | 12:13 |
| rogpeppe | dimitern: quite possibly | 12:13 |
| dimitern | rogpeppe, ok, no point to argue further | 12:13 |
| dimitern | rogpeppe, you simply refuse to see my point :) | 12:14 |
| rogpeppe | dimitern: i see that you strongly believe in that principle | 12:14 |
| rogpeppe | dimitern: but i believe in pragmatism, not principles | 12:14 |
| dimitern | rogpeppe, yes, i do, because of experience (bad usually) - what happens with large projects with fuzzy boundaries, things that shouldn't be together but historically are kept like that and coupled even further, which makes refactoring difficult | 12:16 |
| rogpeppe | natefinch: i definitely buy that argument. by that argument, we could easily factor out the checkers, cloudinit, maybe the rpc package and some others too | 12:16 |
| rogpeppe | dimitern: i'm not sure those historical arguments apply here, as the go package system is strongly boundaried and non-circular | 12:16 |
| natefinch | rogpeppe: seems like the good open source thing to do | 12:17 |
| dimitern | rogpeppe, putting independent things together, just for the sake of convenience, in my experience ever so slightly tends to couple them more and make them less independent | 12:17 |
| rogpeppe | dimitern: there's nothing about having something in an external project that means that things are easier to refactor | 12:17 |
| rogpeppe | dimitern: decoupling is good, no question | 12:17 |
| rogpeppe | dimitern: but whether two packages are in the same project is orthogonal to how coupled they are | 12:18 |
| dimitern | rogpeppe, no it's not - because they can change together and that's a temptation too big to resist to couple them over time | 12:18 |
| dimitern | rogpeppe, anyway, we veered of way off track | 12:19 |
| rogpeppe | dimitern: goose is way too strongly coupled with the openstack provider | 12:19 |
| rogpeppe | dimitern: the fact that they're in separate projects doesn't really help in that refard | 12:19 |
| rogpeppe | regard | 12:19 |
| rogpeppe | dimitern: conversely the rpc package isn't strongly coupled with any other package outside rpc. we can maintain dependency hygiene inside juju-core too, and we really should. | 12:20 |
| dimitern | rogpeppe, lgtm, just to have it landed, even with these minor inconsistencies | 12:21 |
| rogpeppe | dimitern: thanks | 12:21 |
| * TheMue => lunch | 12:21 | |
| rogpeppe | dimitern: i hope you can see where i'm coming from, even if you might not agree :-) | 12:21 |
| dimitern | rogpeppe, I see, I hope you can see my point as well | 12:22 |
| * dimitern gets hungry when arguing like that :) | 12:23 | |
| * dimitern -> lunch as well | 12:23 | |
| rogpeppe | dimitern: i hear it, but i can't *feel* it in the same way as you... | 12:23 |
| rogpeppe | dimitern: enjoy! | 12:24 |
| dimitern | rogpeppe, thanks :) | 12:26 |
| rogpeppe | hmm, this has some test failures i've never seen before: https://code.launchpad.net/~rogpeppe/juju-core/467-checkers-deepequals/+merge/196247 | 12:53 |
| rogpeppe | ... Panic: Couldn't create temporary directory: mkdir /tmp/gocheck-5074209722772702441: file exists (PC=0x413D01) | 12:53 |
| rogpeppe | how could that happen?! | 12:53 |
| jam | rogpeppe: gocheck is broken | 12:58 |
| rogpeppe | jam: oh, that's not good | 12:59 |
| jam | rogpeppe: it doesn't seed its random number generator | 12:59 |
| jam | so after 100 temp directories get created, it "runs out" | 12:59 |
| rogpeppe | jam: ha ha | 12:59 |
| jam | rogpeppe: I'll clean it up, I would say gocheck should be fixed, but it keeps us from leaking *too* many | 12:59 |
| rogpeppe | jam: is that actually a problem with ioutil.MkTempDir() ? | 12:59 |
| jam | rogpeppe: so MkTempDir wouldn't run out after 100 tries like gocheck's internals | 13:00 |
| jam | but we would still be leaking dirs (I think) | 13:00 |
| jam | rogpeppe: and filling up /tmp with dirs is a good way to slowly kill your machine | 13:00 |
| jam | so I haven't "fixed" gocheck | 13:00 |
| rogpeppe | jam: ioutil.TempDir does seed properly anyway | 13:01 |
| rogpeppe | jam: could you clean up those tempories please then, before my branch dies again? (perhaps you have, in which case thanks!) | 13:02 |
| jam | rogpeppe: i did | 13:03 |
| rogpeppe | jam: cool | 13:03 |
| jam | rogpeppe: ideally we'd figure out why we're leaking dirs, fix that, end then fix gocheck to use ioutil.TempDir | 13:03 |
| rogpeppe | jam: that would indeed be good | 13:03 |
| rogpeppe | jam, dimitern, fwereade, TheMue, wallyworld_, mattyw: FYI i'm going to be travelling this afternoon. i'll be working on the train, but probably with only partial internet connectivity | 13:05 |
| fwereade | rogpeppe, cool, enjoy your travels | 13:06 |
| rogpeppe | i leave in about 2 hours | 13:06 |
| TheMue | rogpeppe: ok, enjoy | 13:06 |
| rogpeppe | fwereade: did you manage to take a look at https://codereview.appspot.com/30390043/ BTW? | 13:06 |
| rogpeppe | fwereade: i've also got https://codereview.appspot.com/30900043 which does the txn-revno changes we discussed | 13:07 |
| fwereade | rogpeppe, not enough of it yet | 13:07 |
| rogpeppe | fwereade: (please ignore the testing/checkers changes in that branch - i don't know why they're there, as i included the other branch as a prereq) | 13:07 |
| rogpeppe | fwereade: ok, np | 13:07 |
| fwereade | rogpeppe, no worries | 13:07 |
| === gary_poster|away is now known as gary_poster | ||
| mattyw | rogpeppe, going anywhere nice? | 13:40 |
| rogpeppe | mattyw: up to edinburhg | 13:40 |
| rogpeppe | gh | 13:40 |
| mattyw | rogpeppe, that is nice - enjoy | 13:40 |
| rogpeppe | mattyw: there's a fiddle festival on. lots and lots of trad tunes! | 13:40 |
| mattyw | rogpeppe, excellent! going for the weekend? | 13:41 |
| rogpeppe | mattyw: aye | 13:41 |
| mattyw | rogpeppe, scotsfiddlefestival.com? | 13:42 |
| rogpeppe | mattyw: that'll be the one | 13:42 |
| mattyw | rogpeppe, who are you looking forward to seeing most (which ones should listen to) | 13:43 |
| rogpeppe | mattyw: Frigg are excellent, though I've seen 'em before | 13:44 |
| rogpeppe | mattyw: mostly i'm going for the crack | 13:44 |
| rogpeppe | mattyw: lots of sessions and late nights | 13:44 |
| * rogpeppe goes to lunch | 13:47 | |
| rick_h_ | morning sinzui, can I get a few min hangout time today at your leisure? | 14:26 |
| sinzui | sure | 14:26 |
| sinzui | I am free at the top of the hour rick_h_ | 14:27 |
| rick_h_ | sinzui: sounds good, thanks | 14:27 |
| jcastro | sinzui, for your consideration sir! https://bugs.launchpad.net/juju-core/+bug/1254061 | 14:47 |
| _mup_ | Bug #1254061: Add bash completion for OSX <juju-core:New> <https://launchpad.net/bugs/1254061> | 14:47 |
| sinzui | oh, I might be able to to that mysel | 14:47 |
| sinzui | f | 14:47 |
| natefinch | niemeyer: just wanted to mention I got that code working yesterday. Thanks for the help. | 14:51 |
| rick_h_ | sinzui: hit me up with an invite when you're free. Thanks. | 15:06 |
| sinzui | rick_h_, https://plus.google.com/hangouts/_/7ecpigc90pv2nkor675dm9p9oo?hl=en | 15:10 |
| natefinch | hey rogpeppe, I don't think I can go through with making the public struct for replica set members and a private one for marshaling. It just complicates the code too much, plus we lose information in the translation about what values are actually set in the database versus which are being defaulted. The code is so much cleaner and easier to understand just using pointers to the optional values. | 15:19 |
| rogpeppe | natefinch: i actually think that not providing that information could be considered a positive thing | 15:31 |
| rogpeppe | natefinch: it means that the user doesn't have to know what a missing value means | 15:31 |
| rogpeppe | natefinch: but i do appreciate what you're saying. | 15:33 |
| rogpeppe | natefinch: i can't decide which comes first: a package that's easy and intuitive to use, or one that's simpler maps closely to the underlying transport | 15:34 |
| natefinch | rogpeppe: I actually think that the pointers are simpler to use. You don't have to use some default value struct to create new members | 15:38 |
| rogpeppe | natefinch: but you do have to create variables for all your fields so you can make pointers to them... | 15:39 |
| natefinch | rogpeppe: yes, but most of the optional fields aren't going to be used often, and it's not really that hard to make a f := false and use &f for the value | 15:40 |
| rogpeppe | natefinch: is it really that awkward to have a couple of functions for each type, doing the translation in each direction? | 15:43 |
| rogpeppe | natefinch: but if you really feel strongly, as it seems you do, then do it the way you feel like | 15:45 |
| natefinch | rogpeppe: it's not the translation that is awkward, it's the fact that we lose whether or not a value was intentionally set.... also the zero value of Member is then a generally broken value, so we need AddressToMembers and DefaultMember. All that can go away if we just make people deal with the fact that there are some optional values on Member | 15:45 |
| rogpeppe | natefinch: but the other side of the coin is that when reading a value, the package needs to know that a nil vote count actually means 1, for example | 15:47 |
| rogpeppe | natefinch: and it won't print out very nicely, because it'll just print pointer values for all the fields | 15:47 |
| natefinch | rogpeppe: we could make accessor methods that do the translation internally. I know it's not ideal. | 15:47 |
| rogpeppe | natefinch: and people will need to be careful with pointer aliasing | 15:48 |
| rogpeppe | natefinch: gotta go right now, back in 5 when i'm on the train | 15:48 |
| natefinch | rogpeppe: ok | 15:48 |
| rogpeppe1 | natefinch: sorry, intermittent connection here | 15:53 |
| rogpeppe1 | natefinch: last thing i saw was: | 15:53 |
| rogpeppe1 | [15:45:56] <natefinch> rogpeppe: it's not the translation that is awkward, it's the fact that we lose whether or not a value was intentionally set.... also the zero value of Member is then a generally broken value, so we need AddressToMembers and DefaultMember. All that can go away if we just make people deal with the fact that there are some optional values on Member | 15:53 |
| niemeyer | natefinch: Sweet | 15:56 |
| === bjf is now known as _bjf | ||
| natefinch | rogpeppe1: no problem. That was pretty much it... we could make accessor methods that return the right defaults, but you're right about printing it out | 16:03 |
| natefinch | rogpeppe2: (retype of my last message) no problem. That was pretty much it... we could make accessor methods that return the right defaults, but you're right about printing it out. Seems like there's no real great way to do it while maintaining information about values that are unset. | 16:08 |
| * rogpeppe3 has reached end of journey. | 17:16 | |
| rogpeppe3 | i can say definitively that east coast wi-fi is terrible... | 17:16 |
| rogpeppe3 | happy weekends all | 17:16 |
| hazmat | ? -> Copyright 2013 Joyent Inc. | 18:46 |
| natefinch | hazmat: ? | 18:47 |
| hazmat | natefinch, joyent provider has that, pretty sure that's not the intent for cla | 18:47 |
| hazmat | contributor license agreement | 18:48 |
| natefinch | hazmat: hah, I see. Yeah, probably just old habit for them | 18:48 |
| === gary_poster is now known as gary_poster|away | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!