| wallyworld | Conn was an artifact of the old days when agents used to connect directly to mongo | 00:00 |
|---|---|---|
| wallyworld | perrito666: what's trhe use case, is this for restore? | 00:00 |
| perrito666 | wallyworld: yup, same part, new restore | 00:00 |
| wallyworld | perrito666: do you need state or apiState? | 00:01 |
| wallyworld | there's NewAPIState to get you an api connection | 00:01 |
| wallyworld | that takes an environment | 00:01 |
| perrito666 | I was using conn.State | 00:02 |
| jcw4 | https://github.com/juju/names/pull/18 <-- ActionResultTag added to complement the exisiting ActionTag in the names package | 00:07 |
| davecheney | menn0: tmpfs /tmp tmpfs nodev,nosuid,mode=1777 0 0 | 00:18 |
| davecheney | ^ put that in /etc/fstab | 00:18 |
| wallyworld | perrito666: conn.State was just made from a call to state.Open(), using info extracted from the environment | 00:19 |
| wallyworld | if you really did need access to state.State rather than api.State, you could add a helper method to state/open.go | 00:20 |
| davecheney | jcw4: done with review | 00:20 |
| davecheney | OOPS: 139 passed, 114 FAILED, 4 PANICKED, 5 MISSED | 00:28 |
| davecheney | --- FAIL: TestPackage (264.62s) | 00:28 |
| davecheney | god damnit | 00:28 |
| menn0 | fwereade: https://github.com/juju/juju/pull/353 | 00:33 |
| thumper | tasdomas: https://github.com/juju/juju/pull/354 | 00:35 |
| jcw4 | davecheney: thanks! | 00:43 |
| jcw4 | davecheney: I addressed the issues you commented on; I'm stepping away for the evening but if you get a chance to respond that will be appreciated | 00:52 |
| jcw4 | Cheers! | 00:52 |
| axw | wallyworld: the core work for #1271144 was done in 1.17.6, so it's already in 1.20 | 01:45 |
| _mup_ | Bug #1271144: br0 not brought up by cloud-init script with MAAS provider <canonical-is> <cloud-installer> <landscape> <local-provider> <lxc> <maas> <regression> <juju-core:Fix Released by axwalk> <juju-core 1.20:Triaged> <juju-core (Ubuntu):Fix Released> <juju-core (Ubuntu Trusty):Confirmed> <https://launchpad.net/bugs/1271144> | 01:45 |
| axw | just need to do the other one I think | 01:45 |
| axw | #1337091 | 01:45 |
| _mup_ | Bug #1337091: maas provider: allow users to specify network bridge interface. <canonical-is> <juju-core:Fix Committed by niedbalski> <juju-core 1.20:Triaged> <https://launchpad.net/bugs/1337091> | 01:45 |
| wallyworld | axw: great, ok | 01:46 |
| axw | ah, I think I added that didn't I. doh | 01:49 |
| axw | removing it from the milestone | 01:49 |
| davecheney | apiclient_test.go:127: c.Assert(err, gc.ErrorMatches, `unable to connect to "wss://.*/"`) | 01:51 |
| davecheney | ... error string = "unable to connect to \"wss://127.0.0.1:42509/environment/6e4a12fc-b1c1-4fc7-8401-6cb66fce31b2/api\"" | 01:52 |
| davecheney | ... regex string = "unable to connect to \"wss://.*/\"" | 01:52 |
| davecheney | [LOG] 0:00.249 INFO juju.provider.dummy reset environment | 01:52 |
| davecheney | oh | 01:52 |
| davecheney | how is this a failure ?!? | 01:52 |
| davecheney | i see | 01:52 |
| davecheney | thumper: waigani https://github.com/juju/juju/pull/356 | 02:01 |
| axw | wallyworld: what else would you like me to do for 1.20.2? | 02:10 |
| wallyworld | axw: i'm still working through the i/o timeout issues getting passing tests. like hitting gophers :-( perhaps take a peek at the ec2 health failures to see if there's anything we can do. might just be an ec2 issue but i'm not sure | 02:15 |
| axw | wallyworld: cool, that's what I started doing. | 02:15 |
| wallyworld | great :-) | 02:15 |
| davecheney | wss://127.0.0.1:42509/ | 02:15 |
| davecheney | but not wss://127.0.0.1:42509/api | 02:15 |
| davecheney | and not wss://127.0.0.1:42509/environment/xxxx/api | 02:15 |
| wallyworld | axw: also, we've been told to reduce the max oplog file size from 50GB. I'm thinking 10GB might be reasonable | 02:24 |
| axw | wallyworld: I think it could be lower, and concur with jam that we ought to do a size-to-capacity analysis | 02:31 |
| axw | not sure how to go about that tho | 02:32 |
| wallyworld | yeah, me either | 02:33 |
| wallyworld | fwereade is -1 on reducing the size from the manufacturer's recommnedation so 10GB is sort of a compromise | 02:34 |
| fwereade | wallyworld, I'm -0.5 at most, it's really just "let's analyse this, not just pull a number out of our collective ass" | 02:34 |
| wallyworld | yeah, need to figure out ow to do that | 02:35 |
| wallyworld | we can adjust the size later if needed | 02:36 |
| wallyworld | if we make a wrong decision now | 02:36 |
| axw | wallyworld: the problem is that changing existing installations is difficult | 02:36 |
| axw | not impossible though | 02:37 |
| wallyworld | we can do it as an upgrade step can't we? | 02:37 |
| axw | wallyworld: probably. not sure how current/correct this is, but: http://www.kchodorow.com/blog/2011/02/22/resizing-your-oplog/ | 02:38 |
| axw | http://docs.mongodb.org/v2.2/tutorial/change-oplog-size/ | 02:38 |
| wallyworld | axw: i was looking at a more recent version, but yeah, it does involve some steps http://docs.mongodb.org/manual/tutorial/change-oplog-size/ | 02:40 |
| fwereade | wallyworld, can you remember what the launchpad project with the functional tests is called? | 02:43 |
| fwereade | wallyworld, IIRC sinzui sent round a mail saying where it was and a bit about how to write new ones, but I can't find it | 02:43 |
| wallyworld | hmmm. i can't recall, but i can look | 02:44 |
| axw | I thought it was under ~juju-qa, but I don't see the branch name I'm looking for | 02:45 |
| axw | ci-cd2 something something | 02:45 |
| axw | ci-cd-scripts2 -- that might just be the support scripts tho | 02:45 |
| fwereade | axw, yeah, all I could remember was there was a "2" in the name | 02:45 |
| wallyworld | lp:juju-ci-tools | 02:46 |
| wallyworld | i think | 02:46 |
| axw | yeah, that matches what I have on disk for ci-cd-scripts2 | 02:47 |
| axw | must've been renamed | 02:47 |
| fwereade | axw, am I being dumb? I don;t see any actual *tests* in there | 02:51 |
| axw | fwereade: I think they're just mixed together with support code. I'll follow through from jenkins and find one | 02:52 |
| axw | fwereade: the aws-deploy-trusty-amd64 job just runs this: http://paste.ubuntu.com/7833744/ | 02:54 |
| axw | fwereade: backup/restore runs test_recovery.py | 02:54 |
| axw | with some args | 02:55 |
| fwereade | axw, yeah, I just found that one, but I feel it's likely that that's not the only one... | 02:55 |
| axw | fwereade: it's not. are you looking for something in particular, or you just want to know what our set of functional tests is? | 02:56 |
| fwereade | axw, yeah, I just want to tell ericsnow to write big new tests as FTs directly, and to do it in <insert-place-here> instead of juju-core | 02:58 |
| axw | I see. probably best off getting sinzui's advice then | 02:59 |
| axw | or maybe abentley | 03:00 |
| davecheney | just in case anyone was confused, i really hate JujuConnSuite | 03:00 |
| fwereade | davecheney, I too have a deep and abiding loathing for that thing | 03:03 |
| davecheney | fwereade: i want to cleeve it in two | 03:04 |
| davecheney | JujuClientConnSuite and JujuServerConnSuite | 03:05 |
| davecheney | then at least for the moment, recombine them into some horrific lovecroftian monster as JujuConnSuite | 03:05 |
| davecheney | menn0: % df -h /tmp | 03:06 |
| davecheney | Filesystem Size Used Avail Use% Mounted on | 03:06 |
| davecheney | tmpfs 3.9G 1016M 2.9G 26% /tmp | 03:06 |
| davecheney | this is what a day of running juju tests looks like | 03:06 |
| axw | I'd rather we just didn't use full end-to-end testing for our unit tests | 03:09 |
| axw | why is shit getting left behind in tmp now? I don't think it used to? | 03:09 |
| davecheney | axw: always leaks like a mofo | 03:11 |
| axw | wallyworld: I can't reproduce the ec2 failures :/ | 03:11 |
| davecheney | especially if your fixtures panic | 03:11 |
| davecheney | axw: note, i wasn't complaining | 03:11 |
| davecheney | i'm used to it | 03:11 |
| wallyworld | axw: yeah, i suspect it's on the ec2 side | 03:12 |
| axw | davecheney: hmmok. I'm complaining, because I actually ran out of tmpfs the other day ;) | 03:12 |
| axw | wallyworld: according to the AWS docs, it just means the server is overloaded. I guess it's more likely to happen in a US region during US-friendly hours | 03:15 |
| axw | AWS status page has nothing much to say | 03:15 |
| wallyworld | yep. can you comment on the bug, so that curtis knows we've looked at it? | 03:15 |
| axw | sure | 03:16 |
| wallyworld | axw: so, state of session copying branch. i have got a passing set of unit tests (with occasional replicaset failure). but bootstrapping a real env fails - 2014-07-22 03:18:28 ERROR juju.cmd supercommand.go:323 failed to initialize state: cannot create state servers document: not authorized for insert on juju.txns | 03:20 |
| wallyworld | it will be something simple, so i should be able to propose something soon | 03:21 |
| axw | hmm ok | 03:21 |
| wallyworld | not sure just right now exactly what the issue is though | 03:21 |
| axw | is this related to the thin michael emailed about? and that jam suggested we might be able to drop? (the mongo password dance) | 03:21 |
| axw | thing* | 03:22 |
| wallyworld | could be, but I think that has been changed in this branch, i need to go look into it | 03:22 |
| axw | ok | 03:22 |
| wallyworld | i'll get you to review later, but here's the current diff https://github.com/wallyworld/juju/compare/copy-session | 03:22 |
| davecheney | fwereade: can you explain the rational for this signature and comment ? | 03:50 |
| davecheney | func APIInfo(env Environ) (*api.Info, error) { | 03:50 |
| davecheney | bugger | 03:50 |
| davecheney | // APIInfo returns an api.Info for the environment. The result is populated | 03:50 |
| davecheney | // with addresses and CA certificate, but no tag or password. | 03:50 |
| waigani | thumper: https://github.com/juju/juju/pull/346 question in user_test.go:106 | 03:51 |
| mattyw | thumper, http://paste.ubuntu.com/7833982/ | 04:00 |
| axw | wallyworld: #1344857 <- this was fixed for 1.20, but not in the 1.18 branch | 04:36 |
| _mup_ | Bug #1344857: Crash deploying landscape-dense on MAAS <juju-core:Triaged> <juju-core 1.20:Triaged> <juju-deployer:New> <https://launchpad.net/bugs/1344857> | 04:36 |
| axw | fixed some time during 1.19 anyway | 04:36 |
| axw | added a comment | 04:36 |
| axw | about to propose a fix for the machine-agent suciding bug | 04:37 |
| wallyworld | axw: so, using session.Copy() results in all sorts of permission errors, whereas session.Clone() doesn't | 05:17 |
| axw | Clone doesn't create a new socket | 05:18 |
| wallyworld | the different at the mgo level is that Copy() makes a Refresh() cal lon the session which releases the socket | 05:18 |
| tasdomas | thumper, https://github.com/juju/juju/pull/358/files | 05:19 |
| wallyworld | yes, so now the question is - why does acuiring a new socket cause mongo to reject collection queries etc as unauthorised | 05:19 |
| axw | wallyworld: I can only guess that it has to do with Login is being called after the session is copied | 05:20 |
| axw | so other sessions don't inherit the credentials | 05:20 |
| wallyworld | could be | 05:21 |
| wallyworld | yeah, looks like i need to rearrange some code | 05:24 |
| davecheney | fwereade: https://github.com/juju/juju/pull/359 | 05:25 |
| davecheney | a semi seroius attempt to reduce the horror | 05:25 |
| davecheney | for comment, not review | 05:25 |
| wallyworld | hmmm, maybe not. Login() is called first | 05:30 |
| axw | wallyworld: getting anywhere? | 06:11 |
| wallyworld | axw: sorta. state.Open() is first called with no credentials, and that fucks up everything from that point onwards | 06:12 |
| wallyworld | because there are no credentials, db.Login() is never called | 06:12 |
| wallyworld | so credentials are never set on the session | 06:13 |
| axw | I see | 06:13 |
| wallyworld | but it tries to use the session via session.Copy() | 06:13 |
| wallyworld | using the session directly (which also has no creds) works | 06:13 |
| axw | huh | 06:13 |
| wallyworld | so i thin we must be calling Open() twice | 06:14 |
| wallyworld | once without creds and once with | 06:14 |
| wallyworld | well, that's my theory | 06:14 |
| wallyworld | i think it's all tied up in how we initially create the admin user | 06:17 |
| wallyworld | ah, it's in agent/bootstrap | 06:21 |
| wallyworld | we create an initial state as part of the bootstrap process, but without user or password | 06:21 |
| * wallyworld bbiab, raining, goota get kid from train startion | 06:21 | |
| axw | right, cos it's the first connection to mongo and there are no users yet | 06:21 |
| wallyworld | yeah | 06:21 |
| wallyworld | so have to special case that somehow | 06:22 |
| === eagles05- is now known as eagles0513875 | ||
| TheMue | morning | 07:13 |
| === uru_ is now known as urulama | ||
| perrito666 | good morning everyone, please remember dimitern and I will be your on call reviewers today :) | 10:11 |
| mgz | mornin' | 10:17 |
| TheMue | perrito666: morning | 10:17 |
| TheMue | perrito666: dimiterm isn’t available, diving holidays | 10:18 |
| perrito666 | TheMue: then its only me :p | 10:19 |
| TheMue | perrito666: but I can support you, currently only fighting a bit with my machine after upgrading | 10:22 |
| axw | wallyworld: I've got some things to do after dinner, so may be late for / miss the standup | 10:40 |
| wallyworld | sure, np | 10:40 |
| wallyworld | thanks for doc, haven't read it yet | 10:40 |
| wallyworld | i think i'm going to revert michael's changes to txn runner so we can control when session is closed | 10:41 |
| axw | mk | 10:41 |
| wallyworld | i should say Copy()ied | 10:41 |
| wallyworld | cause now, it copies inside txn runner even when it's not appropriate to do so | 10:42 |
| wallyworld | the caller (state) should be in control of when a session is copied | 10:42 |
| wallyworld | as it knows if the credentials have been set | 10:42 |
| wallyworld | it's a bit messy | 10:43 |
| jam | TheMue: i realized I just went through our standup time, I had an interview I was conducting. Want to touch base in about 5 min? | 11:17 |
| TheMue | jam: ah, hey, just back from lunch. after you didn’t came I thought that watching the games has been too long. :D | 12:22 |
| natefinch | morning all | 12:24 |
| perrito666 | natefinch: morning | 12:24 |
| TheMue | natefinch: morning | 12:26 |
| * perrito666 uses a rather inefficient method to find out code to be reviewed | 12:26 | |
| jam1 | perrito666: looking through the entrails and chicken bones for URL fragments? | 12:27 |
| TheMue | perrito666: which one? | 12:27 |
| perrito666 | TheMue: the one jam1 just mentioned | 12:28 |
| perrito666 | TheMue: I open all the repos of gh/juju/ and then click on the ones that actually have pull requests | 12:28 |
| TheMue | perrito666: yeah, I’m looking at the PRs and use also the number of comments as a hint if nobody has looked at a PR so far | 12:29 |
| * perrito666 senses a script in the near future | 12:29 | |
| perrito666 | btw, what is dave cheney's nicnkane on irc? | 12:30 |
| TheMue | davecheney | 12:31 |
| TheMue | ;) | 12:31 |
| * TheMue is happy enough that his local provider is working again. :) somehow it yesterday didn’t wanted to start the API server :( | 12:37 | |
| perrito666 | can anyone shed some light here? https://github.com/juju/utils/pull/7 I think axw and I are right and I am inclined to merge this | 12:37 |
| TheMue | perrito666: will take a look | 12:38 |
| perrito666 | tx | 12:38 |
| TheMue | perrito666: hmmm, I do understand dave, it hurts a bit to see those two Close so near to each other. the defer relies on the fact that Close only may return an error but doesn’t hurt more (eg. with a panic in case of an already closed file) | 12:45 |
| TheMue | perrito666: just looking for a way check if the file is not yet closed, so defer would use a func() { if f.IsOpen() { … } } | 12:46 |
| TheMue | perrito666: sadly it doesn’t exist | 12:46 |
| perrito666 | I guess that could be split into a couple of functions but I believe it is a bit overkill | 12:47 |
| TheMue | perrito666: so yeah, we have to act this way, otherwise ReplaceFile would have to be changed to use a os.File instead of prep and path | 12:47 |
| natefinch | TheMue: I think it's a lot clearer to use two closes rather than defer and close. Defer and close looks like a bug. two closes looks like we're choosing not to use defer. With a comment as to why we have to close before replacing, it'll be totally clear and ok. | 12:47 |
| TheMue | natefinch: yep, the comment would help to not struggle about it later | 12:49 |
| natefinch | TheMue: yeah. I can just totally see someone coming by and deleting the f.Close because it's extraneous due to the defer (or convert both closes back to a defer in the case of the two close form) | 12:50 |
| TheMue | natefinch: hehe, death by refactoring | 12:51 |
| natefinch | that's half the reason I comment stuff... so the guy coming into the code 6 months from now won't go "wtf is this? man, what an idiot, I'll just fix it...." and then break stuff (P.S. that guy in 6 months is often oneself ;) | 12:52 |
| mgz | heh, that's a funny pr | 12:53 |
| perrito666 | mgz: it has an amazing review/changes ratio | 12:53 |
| mgz | nate's solution seems the best, f.Close twice after f.Write, on the error and non error branches | 12:53 |
| mgz | or put Write and the error check on seperate lines so you can Close between them (and comment, of course) | 12:54 |
| hackedbellini | natefinch: I just noticed in the log after the workaround thumper asked us to try this: 2014-07-22 13:12:37 ERROR juju.worker runner.go:218 exited "state": failed to add "machine-0" to admin database: cannot set admin password: system.users entry must not have both 'roles' and 'readOnly' fields | 13:15 |
| hackedbellini | after that, the db service stopped. I'm thinking if fixing this could maybe fix the ha | 13:15 |
| natefinch | hackedbellini: hmm weird | 13:16 |
| hackedbellini | do you or anyone here knows how to workaround that? | 13:18 |
| TheMue | jam1: ping | 13:19 |
| jam1 | TheMue: pong | 13:20 |
| TheMue | jam1: the LXC card tells that an lxc instance has no IPv6 address. who’s the source of this info? | 13:21 |
| natefinch | hackedbellini: looking | 13:21 |
| jam1 | TheMue: when I've tried the local provider, and then run "sudo lxc-ls —fancy" I see it list an IPv4 address but "—" for ipv6, has this changed? | 13:21 |
| TheMue | jam1: when I’m starting LXC manually as well as via juju (using the local provider) it has an IPv6 address | 13:21 |
| TheMue | jam1: hmm, interesting, using lxc-ls it doesn’t show the IPv6, yes | 13:23 |
| natefinch | hackedbellini: it seems like the problem is that we're trying to modify the user, but that's not really how you're supposed to change passwords on users anymore | 13:23 |
| TheMue | jam1: but when visit an instance with juju ssh 1 and do a ifconfig -a I do see it, bound to eth0 with the same address lxc-ls show | 13:24 |
| TheMue | shows | 13:24 |
| hackedbellini | natefinch: that error was new after setting the "upgradedToVersion" = 1.18.4. Do you think that this error is a side effect that happened after the agent was trying to "fix" the ha migration? | 13:25 |
| natefinch | niemeyer: ^^ It looks like juju is using mgo.Database.UpsertUser to change passwords, but mongo really wants us to use changeUserPassword: http://docs.mongodb.org/v2.4/tutorial/change-user-password/ | 13:25 |
| TheMue | jam1: ah, the IPv4 can be pinged, the IPv6 not, interesting | 13:26 |
| natefinch | hackedbellini: I think you're hitting a weird upgrade path where you're half upgraded, because you're really on 1.19 and not 1.18, so some of the upgrade code is failing because it expects the user not to exist | 13:27 |
| hackedbellini | natefinch: hrm, so this is actually good. It's trying to migrate from 1.18 to 1.20, that's what we wanted. Any way to "skip" those already migrated code? | 13:29 |
| jam1 | TheMue: so I think the ubuntu that comes up in the LXC is auto-assigning something, but it isn't being made routable by the LXC outer machinery. | 13:30 |
| TheMue | jam1: looks like, the standard address is link-local | 13:32 |
| katco | good morning all | 13:38 |
| TheMue | katco: heya | 13:39 |
| mgz | hey katco | 13:39 |
| katco | TheMue: mgz: hi :) | 13:39 |
| niemeyer | natefinch: What's the actual error, and what is the version of mgo and of MongoDB | 13:42 |
| niemeyer | natefinch: FWIW, db.changeUserPassword is just a wrapper, which just updates the user using one method or the other depending on which version of MongoDB you're using | 13:46 |
| niemeyer | natefinch: In theory, the latest version of mgo does exactly the same.. | 13:46 |
| niemeyer | natefinch: Unless there's a bug, which I'd be happy to know about | 13:47 |
| natefinch | niemeyer: we're getting "system.users entry must not have both 'roles' and 'readOnly' fields" | 13:49 |
| niemeyer | natefinch: What is the version of mgo and MongoDB you are using? | 13:50 |
| niemeyer | natefinch: This sounds like a really old behavior.. even an older mgo should be able to cope with that, unless you are actually trying to modify the database yourself, in which case you are of course on your own | 13:51 |
| natefinch | hackedbellini: I presume you have mongodb 2.4.6? | 13:52 |
| natefinch | niemeyer: we're just using UpsertUser: https://github.com/juju/juju/blob/1.20/mongo/admin.go#L114 | 13:52 |
| niemeyer | natefinch: Okay, so that should be working fine | 13:53 |
| natefinch | niemeyer: mgo is labix.org/v2/mgo 20140331185009-fhnh3xzfdpicup0j | 13:53 |
| niemeyer | natefinch: Testing.. | 13:53 |
| natefinch | niemeyer: the question is what would happen if you ran that code twice? I have a feeling that's what happening. hackedbellini's state server is in a weird state - he got upgraded to 1.19 from 1.18, and now we're trying to get him to 1.20... my guess is that the upseruser code got called from 1.18-> 1.19 and now is getting called again from 1.19->1.20 | 13:54 |
| niemeyer | natefinch: Ah, hmm.. | 13:56 |
| niemeyer | natefinch: If you try to upgrade a user from a password-less state to a password-rich one, yeah, you might have issues | 13:56 |
| niemeyer | natefinch: Erm, sorry | 13:56 |
| niemeyer | natefinch: If you try to upgrade a user from a role-less state to a role-rich one, yeah, you might have issues | 13:56 |
| niemeyer | natefinch: There's no "readOnly" field in there | 13:57 |
| niemeyer | natefinch: So I assume that field is in the database already, from an ancient run | 13:57 |
| niemeyer | natefinch: So the error is correctly saying you cannot do that | 13:57 |
| natefinch | niemeyer: yeah, that's what I was thinking. I wonder if we should just override readonly | 13:58 |
| niemeyer | natefinch: This was unfortunately quite fiddled with during MongoDB 2.2 => 2.4 => 2.6 | 13:59 |
| niemeyer | natefinch: I did my best to sanitize the upgrade path for driver users, but the server did change significantly and certain cases are just not doable | 13:59 |
| niemeyer | natefinch: You can try dropping readOnly, yes, but I suggest testing it both in 2.4 and 2.6 | 14:00 |
| niemeyer | natefinch: The manual dropping code will not work on 2.6, as user changes are now done via commands rather than directly fiddling with the db | 14:00 |
| natefinch | niemeyer: yeah, we're working on 2.6 compatibility.... but so much changed, it's quite a process. | 14:01 |
| niemeyer | natefinch: So probably "drop on 2.4" + "check for that one error on 2.6 and ignore" | 14:01 |
| niemeyer | natefinch: Yeah, user managed, specifically, did change both on 2.4 and on 2.6 | 14:01 |
| niemeyer | natefinch: I'm hopeful the fiddling is done now. | 14:02 |
| perrito666 | wwitzel3: stdup? | 14:03 |
| natefinch | niemeyer: thanks for the help. I'll make that fix, in case anyone else hits this (this is a pretty bizarre case, but shows it's not impossible) | 14:03 |
| niemeyer | natefinch: No problem | 14:04 |
| natefinch | perrito666, wwitzel3: brt, one minute | 14:05 |
| hackedbellini | natefinch: sorry I was helping a workmate here. The mongodb version is 2.4 here. | 14:20 |
| hackedbellini | I saw that you and niemeyer were discussing some things regarding my problem. Is there anything you want me to try? My installation is totally broken, so I have nothing to loose (well, I cannot loose in any case the data on the lxc machines, since they are on production, but I don't think that will happen) | 14:20 |
| natefinch | hackedbellini: we could have you twiddle the DB manually, but I think I'd rather get the fix into production and have you retry the update after the fix | 14:27 |
| hackedbellini | natefinch: hrm, perfect! When do you think that update will be available? I will probably have to download the tools by hand, right? Since I can't do an "upgrade-juju" | 14:36 |
| jcw4 | what is the normal approach to testing internal helper functions? | 15:36 |
| jcw4 | should we export the internal functions so that the tests can see and test them directly, or do we rely on the helper functions being implicitly tested by the public functions being tested? | 15:36 |
| jcw4 | perrito666, mgz, natefinch, et. al. ^^^ | 15:37 |
| perrito666 | jcw4: in that case I usually add a test in the package | 15:40 |
| jcw4 | perrito666: i.e. in the actual package rather than in package_test | 15:40 |
| perrito666 | yup | 15:40 |
| perrito666 | jcw4: but dont take my word as law I am far from elder here | 15:41 |
| jcw4 | :) | 15:41 |
| jcw4 | I've heard strong opinions against that, but possibly there might be exceptions for internal helper methods | 15:41 |
| mgz | jcw4: I pretty much always prefer the export_test.go method | 15:42 |
| perrito666 | mgz: @more | 15:42 |
| jcw4 | @more? | 15:42 |
| meetingology | jcw4: Error: "more?" is not a valid command. | 15:42 |
| mgz | lol | 15:43 |
| jcw4 | ah, | 15:43 |
| jcw4 | lol | 15:43 |
| perrito666 | mgz: meh, you could very well support basic irc bot commands | 15:43 |
| mgz | for any non-trivial internal function, having dedicated unit tests makes sense, rather than just testing at pubic interfaces (which some prefer) | 15:43 |
| mgz | so, just have a SomeInternalFunction = someInternalFunction in export_test.go in that package | 15:44 |
| mgz | and test at that level | 15:45 |
| jcw4 | mgz +1 | 15:48 |
| jcw4 | perrito666, mgz I'll add some tests and then ask for a review again | 15:48 |
| perrito666 | btw, there are a bunch of prs from 26 days ago about cloud sigma that no one commented on, is there a reason for that? | 15:50 |
| mgz | perrito666: congratulations, you get to review them I think | 15:50 |
| jcw4 | hehe | 15:50 |
| perrito666 | mgz: I am on it, just wondering if there was a deeper reason for them to be ignored for almost a month | 15:51 |
| mgz | perrito666: not that I'm aware of, apart from some obvious reviewers being enveloped with other things | 15:53 |
| katco | mgz: hey, i'm unsure of where to put this test for including the simple-streams URL in the container start. my options are juju/juju/container/kvm/kvm_test.go and live_test.go. live_test.go looks like it has some container-ish tests in it, but the change will be in kvm.go... thoughts? | 16:04 |
| mgz | katco: I'd try to put in kvm_test.go even if it means adding a new suite there | 16:05 |
| katco | mgz: k that's what i was thinking initially; thank you :) | 16:06 |
| perrito666 | I just burned frozen burgers... that must be some sort of special skill | 16:14 |
| mgz | perrito666: I thought you were the chef in the house... | 16:14 |
| perrito666 | mgz: I am, I just got distracted by a code review | 16:15 |
| jcw4 | riveting code review | 16:15 |
| mgz | scorching code review... | 16:15 |
| perrito666 | mgz: well this burned food will definitely affect the outcome of the review | 16:17 |
| katco | haha | 16:17 |
| katco | "this is the worst object instnatiation i've ever seen. the formatting is crap, and i'm hungry. dnflgtm" | 16:18 |
| perrito666 | lol | 16:18 |
| rogpeppe1 | jam, jam1: ping | 16:22 |
| rogpeppe1 | cmars: ping | 16:28 |
| katco | i need a second opinion on a design decision; anyone available to PM for a few minutes? | 16:35 |
| mgz | katco: bug perrito666, he's in a good mood and not otherwise occupied | 16:38 |
| katco | rofl | 16:38 |
| katco | perrito666: if that's true, got a sec? or are you munching on some burnt lunch? ;) | 16:38 |
| mgz | katco: more seriously, you can bug me if you like :) | 16:39 |
| katco | mgz: that would probably be best since we're on the same team :) | 16:39 |
| natefinch | jcw4: for what it's worth, I think export_test.go is an anti-pattern. I think it is doubly so when you're doing it so you can test an implementation detail like an unexported method . Just make a new testing file that is in the same package as the test, and write your unit test there. There's a reason both test forms exist. | 16:53 |
| perrito666 | mgz: mm I see you have managerial talent | 16:59 |
| perrito666 | katco: I am in a good mood, excepting for that incident with the chickenburger, so fire away | 17:00 |
| katco | perrito666: mgz helped me out, but thank you! | 17:00 |
| mgz | perrito666: I can be useful, as well as wind people up about their burnt burgers :) | 17:02 |
| jcw4 | natefinch: thanks | 17:24 |
| natefinch | jcw4: as others said, not everyone agrees with me.... but I don't think anyone will stop you from writing more tests :) | 17:25 |
| jcw4 | haha | 17:25 |
| perrito666 | I have had in-package tests merged so I guess no one has very strong objections | 17:26 |
| jcw4 | I'll clear it with mgz, and decide between a separate .go file and the export_test.go file, but either way I have to make perrito666 happy | 17:26 |
| jcw4 | ;) | 17:26 |
| perrito666 | jcw4: well, you will not make me happy per se (not after my burguers charred) but you might get a lgtm from me if you add tests :p | 17:27 |
| jcw4 | lol | 17:28 |
| jcw4 | from my perspective that's the same thing | 17:28 |
| jcw4 | ;) | 17:28 |
| perrito666 | I need someone with managerial skills for a quick question | 17:31 |
| natefinch | perrito666: you can't expense the heroin, I told you that | 17:32 |
| perrito666 | natefinch: maan, whyyy? | 17:32 |
| natefinch | perrito666: whazzup? | 17:32 |
| perrito666 | natefinch: I pmd you | 17:33 |
| katco | perrito666: jcw4: i am thinking about the same thing. i think the idiomatic thing to do is to keep the tests in the same package. they will only get included in the test binary, not the production binary. | 17:53 |
| katco | perrito666: jcw4: and it avoids the export dance for tests | 17:53 |
| jcw4 | katco: okay, I haven't heard from mgz and it sounds like a bit of a quorum so I'll go with in-package tests | 17:54 |
| katco | jcw4: mind you i am brand new, so my opinion probably doesn't count for much :) | 17:54 |
| jcw4 | katco: haha - well change is easy. But, when I see fairly divergent opinions regularly on the project I think it just means a clear standard has yet to emerge | 17:55 |
| katco | well then i'll throw my hat into the "keep in the package" ring. if it's good enough for the standard lib, it's good enough for me :) | 17:56 |
| jcw4 | :) | 17:56 |
| ericsnow | FWIW, last I saw, niemeyer favored the *_test package approach, which somewhat explains our use of it :) | 18:07 |
| ericsnow | see https://groups.google.com/d/msg/Golang-nuts/dkk0X1tIs6k/nO3CKFqbIxYJ | 18:08 |
| niemeyer | +1 | 18:08 |
| perrito666 | sounds to me like: Public API in package foo_test and Private in _test.go file of foo package but I did not read the whole link from ericsnow :) | 18:10 |
| perrito666 | testing things as they are intended to be used | 18:11 |
| niemeyer | Yep | 18:11 |
| niemeyer | In many cases, it's also easy to export the things that are internal so that they can be touched by tests, using the export_test.go trick in use in many packages | 18:11 |
| niemeyer | Also like that extra mile.. encourages people to be more honest about testing what is meant to be tested, rather than implementation details | 18:12 |
| jcw4 | hmm; the quorum is crumbling.. since I report to mgz and niemeyer is an elder in the community I'm going to flip flop and go with export_test.go :) | 18:16 |
| niemeyer | jcw4: It's hard to give reasonable advice without knowing what the case is, though.. what are you testing? | 18:17 |
| jcw4 | niemeyer: helper functions in the actions and actionresults watchers | 18:17 |
| niemeyer | jcw4: URL to the code? | 18:17 |
| jcw4 | niemeyer: https://github.com/juju/juju/pull/351 ... specifically : | 18:18 |
| jcw4 | https://github.com/juju/juju/pull/351/files#diff-5a822a48b96dd79a90eda5ba852495c4R1673 | 18:19 |
| jcw4 | niemeyer: I'm actually about to refactor them down to one watcher with a parameter for which collection to watch... | 18:19 |
| niemeyer | jcw4: I'd be on the fence on that one.. it's all internal, and it's a relevant abstraction that deserves testing | 18:21 |
| niemeyer | jcw4: It's previously existent code, so I'd probably just go with what is already in place unless I had an actual reason to change it | 18:22 |
| niemeyer | jcw4: (other than purism) | 18:22 |
| jcw4 | niemeyer: meaning the refactor to one watcher? | 18:22 |
| jcw4 | or the helper functions | 18:22 |
| niemeyer | jcw4: No, meaning the subject of our conversation | 18:23 |
| jcw4 | :D | 18:23 |
| jcw4 | okay | 18:23 |
| jcw4 | niemeyer: I'll see what in-place testing there is for helper functions and if there is none then I'll export the couple helper functions we want to test... | 18:24 |
| katco | ericsnow: niemeyer: ty for the context :) | 18:33 |
| === gsamfira1 is now known as gsamfira | ||
| natefinch | thumper: got a minute to talk about jujud logging? | 20:58 |
| thumper | natefinch: sure, a few | 20:59 |
| natefinch | thumper: just as easy on irc..... basically, there's the log rotation bug that has come up again. I wanted to use a Go package for rolling Jujud logs so it'll work when we support windows.... but the relationship between the commands and logging and stuff is kinda twisty | 21:00 |
| thumper | natefinch: oh... kay.. | 21:01 |
| thumper | natefinch: what needs to be done is this: | 21:01 |
| thumper | 1) at the startup of jujud, configure the logging to use the special writers you have | 21:01 |
| thumper | 2) change the upstart scripts so it writes out to a file, not to stdout/stderr | 21:01 |
| thumper | or at least, logging goes to the file | 21:01 |
| thumper | I don't think we write anything else out | 21:02 |
| thumper | yeah….. | 21:02 |
| thumper | just that... | 21:02 |
| thumper | changing the upstart scripts are the biggest thing | 21:02 |
| natefinch | thumper: so, we have a few different commands, bootstrap, machine agent, unit agent, juju-run | 21:02 |
| natefinch | thumper: to get the right directory in which to configure loggo's output on startup, we need the agentConf, which everything except juju-run has access to right now. | 21:03 |
| thumper | don't worry about juju-run | 21:04 |
| thumper | it sends stdout, stderr back to the client | 21:04 |
| natefinch | ok, that was sort of my question | 21:04 |
| thumper | and shouldn't log | 21:04 |
| natefinch | right, that make sense | 21:04 |
| natefinch | ok... so I think the only thing we have to do is change cloud-init to modify the upstart script, but I guess we'll need an upgrade step to modify the upstart script of existing installations | 21:05 |
| natefinch | so... three, three things we have to do: configure loggo for the commands, change cloud-init, and update the existing upstart scripts of running machines | 21:06 |
| thumper | right | 21:08 |
| natefinch | thumper: and how do I set the output on loggo? | 21:10 |
| natefinch | thumper: there's the register writer thing, but it takes a loggo.Writer, and to make one of those from an io.Writer wants a formatter and...... I just want to say "here, loggo, write to this io.Writer" | 21:13 |
| thumper | natefinch: change the writers | 21:13 |
| * thumper looks | 21:13 | |
| natefinch | https://godoc.org/github.com/juju/loggo | 21:13 |
| thumper | NewSimpleWriter | 21:14 |
| thumper | with a default formatter | 21:14 |
| natefinch | do I replace the "Default" logger or no? | 21:14 |
| thumper | yes | 21:14 |
| thumper | otherwise it will continue to write to stderr | 21:14 |
| natefinch | thumper: can I overwrite, or do I have to delete and then register? | 21:15 |
| natefinch | thumper: the package example seems to say delete and register | 21:16 |
| thumper | loggo.ReplaceDefaultWriter | 21:16 |
| thumper | probably an old example | 21:16 |
| natefinch | awesome | 21:16 |
| natefinch | thanks. | 21:16 |
| thumper | np | 21:16 |
| natefinch | thoughts on max log size? | 21:16 |
| thumper | 5 meg? | 21:17 |
| * thumper shrugs | 21:17 | |
| natefinch | wow really? I was going to say like 200 | 21:17 |
| natefinch | (meg) | 21:17 |
| natefinch | I dunno | 21:17 |
| thumper | me neither | 21:17 |
| thumper | ask an admin type person | 21:17 |
| natefinch | I'll PR it and let people duke it out | 21:17 |
| natefinch | Thanks again. Gotta run' | 21:17 |
| === liam_ is now known as Guest15796 | ||
| fwereade | perrito666, ping | 21:34 |
| katco | gofmt sure does make for messy diffs when adding longer variable names to structs | 21:34 |
| perrito666 | fwereade: pong | 21:35 |
| fwereade | perrito666, the semi-readable PRs for cloudsigma are here: https://github.com/Altoros/juju-cloudsigma/pulls | 21:36 |
| perrito666 | fwereade: so the ones that are in my email are not the semi readable ones? | 21:36 |
| fwereade | perrito666, those ones I think are kinda unbearably yucky to read | 21:37 |
| fwereade | perrito666, I got upset with them, jam pointed me at the others | 21:37 |
| perrito666 | fwereade: you have a lower tolerance level than I | 21:37 |
| perrito666 | they are ugly, not unbearable | 21:37 |
| perrito666 | fwereade: we should perhaps close those ones? for what I see in an answer I got in priv I am not the first to ask these questions | 21:40 |
| perrito666 | s/those/the ones I pointed | 21:40 |
| fwereade | perrito666, yeah, sgtm, please close them with a note pointing at the others | 21:46 |
| fwereade | perrito666, I got all upset with the 18 changed files, 20 changed files, 22 changed files progression | 21:46 |
| perrito666 | fwereade: mm I will email the proposer guy | 21:48 |
| perrito666 | I am not sure which is which and I fear I might close the wrong ones | 21:48 |
| perrito666 | the link you passed me contains the ones I mentioned plus other | 21:49 |
| fwereade | perrito666, that sounds reasonable -- and, sorry I'm not completely up to date with it, jam has been liaising with them primarily | 21:55 |
| davecheney | mattyw: http://i.imgur.com/IODqYZN.jpg | 22:49 |
| cmars | waigani, http://sourcegraph.com/blog/ipfs-the-permanent-web-by-juan-benet-talk | 22:49 |
| mattyw | davecheney, http://irclogs.ubuntu.com/2014/07/22/%23juju-dev.html | 22:51 |
| davecheney | http://i.imgur.com/x6DVenz.jpg | 22:51 |
| bodie_ | anyone have thoughts on how to format a descriptor of optional flags to a command? | 23:00 |
| bodie_ | it's simply a map index which can also have lists | 23:00 |
| bodie_ | action-get outfile.format for example | 23:01 |
| bodie_ | in that example, outfile is the key to an inner map | 23:01 |
| bodie_ | format is a key in that inner map | 23:01 |
| bodie_ | picture JSON | 23:01 |
| bodie_ | however, there could also be a list index | 23:02 |
| bodie_ | action-get outfile.files[1] | 23:02 |
| bodie_ | right now, the help displays something like: [<key>[.<key>.<key>....]] | 23:03 |
| bodie_ | which doesn't seem optimal | 23:03 |
| bodie_ | but adding the list index option is much much uglier | 23:03 |
| bodie_ | jcw4 any thoughts? | 23:03 |
| jcw4 | bodie_: wouldn't the pattern extend to outfile.files.1 ? | 23:04 |
| jcw4 | bodie_: you'd have to work a bit to make sure you parsed integer indexes at the right time instead of strings | 23:05 |
| bodie_ | I figured traditional list index would be easier to parse | 23:06 |
| bodie_ | right now it indexes by the key parsed from the list, so '1' would have to be a string key to another map | 23:06 |
| bodie_ | or uh | 23:06 |
| bodie_ | files would be a map | 23:06 |
| bodie_ | '1' would be one of the keys in it | 23:06 |
| bodie_ | whereas if we have [1] we clearly know it's an int, plus it's more familiar to the user | 23:07 |
| bodie_ | it almost seems like this isn't something that is really the job of the action-get command | 23:08 |
| bodie_ | but lists are clearly supported by JSON-Schema as JSON elements | 23:09 |
| bodie_ | perhaps I'll simply PR it as WIP and query for responses on this topic | 23:09 |
| jcw4 | bodie_: agree, this is potentially common utility code.. I don't know if it's implemented anywhere though | 23:09 |
| bodie_ | hmm, good thought | 23:09 |
| bodie_ | it's not hard to recurse a map on a list of keys, but adding list indexing could be a -bit- more tedious, though I imagine it would just be an interface{} list instead of a string list, and we'd either have int keys or string keys in the list | 23:10 |
| === meetingology` is now known as meetingology | ||
| bodie_ | this would be vastly simpler in a functional language with .apply() ... :P | 23:23 |
| bodie_ | oh well, this is fine too | 23:23 |
| perrito666 | fwereade: ping | 23:53 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
