wallyworldConn was an artifact of the old days when agents used to connect directly to mongo00:00
wallyworldperrito666: what's trhe use case, is this for restore?00:00
perrito666wallyworld: yup, same part, new restore00:00
wallyworldperrito666: do you need state or apiState?00:01
wallyworldthere's NewAPIState to get you an api connection00:01
wallyworldthat takes an environment00:01
perrito666I was using conn.State00:02
jcw4https://github.com/juju/names/pull/18 <-- ActionResultTag added to complement the exisiting ActionTag in the names package00:07
davecheneymenn0: tmpfs   /tmp    tmpfs   nodev,nosuid,mode=1777  0       000:18
davecheney^ put that in /etc/fstab00:18
wallyworldperrito666: conn.State was just made from a call to state.Open(), using info extracted from the environment00:19
wallyworldif you really did need access to state.State rather than api.State, you could add a helper method to state/open.go00:20
davecheneyjcw4: done with review00:20
davecheneyOOPS: 139 passed, 114 FAILED, 4 PANICKED, 5 MISSED00:28
davecheney--- FAIL: TestPackage (264.62s)00:28
davecheneygod damnit00:28
menn0fwereade: https://github.com/juju/juju/pull/35300:33
thumpertasdomas:   https://github.com/juju/juju/pull/35400:35
jcw4davecheney: thanks!00:43
jcw4davecheney: 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 appreciated00:52
axwwallyworld: the core work for #1271144 was done in 1.17.6, so it's already in 1.2001: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
axwjust need to do the other one I think01: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
wallyworldaxw: great, ok01:46
axwah, I think I added that didn't I. doh01:49
axwremoving it from the milestone01:49
davecheneyapiclient_test.go:127: c.Assert(err, gc.ErrorMatches, `unable to connect to "wss://.*/"`)01:51
davecheney... error string = "unable to connect to \"wss://\""01:52
davecheney... regex string = "unable to connect to \"wss://.*/\""01:52
davecheney[LOG] 0:00.249 INFO juju.provider.dummy reset environment01:52
davecheneyhow is this a failure ?!?01:52
davecheneyi see01:52
davecheneythumper: waigani https://github.com/juju/juju/pull/35602:01
axwwallyworld: what else would you like me to do for 1.20.2?02:10
wallyworldaxw: 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 sure02:15
axwwallyworld: cool, that's what I started doing.02:15
wallyworldgreat :-)02:15
davecheneybut not wss://
davecheneyand not wss://
wallyworldaxw: also, we've been told to reduce the max oplog file size from 50GB. I'm thinking 10GB might be reasonable02:24
axwwallyworld: I think it could be lower, and concur with jam that we ought to do a size-to-capacity analysis02:31
axwnot sure how to go about that tho02:32
wallyworldyeah, me either02:33
wallyworldfwereade is -1 on reducing the size from the manufacturer's recommnedation so 10GB is sort of a compromise02:34
fwereadewallyworld, 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
wallyworldyeah, need to figure out ow to do that02:35
wallyworldwe can adjust the size later if needed02:36
wallyworldif we make a wrong decision now02:36
axwwallyworld: the problem is that changing existing installations is difficult02:36
axwnot impossible though02:37
wallyworldwe can do it as an upgrade step can't we?02:37
axwwallyworld: probably. not sure how current/correct this is, but: http://www.kchodorow.com/blog/2011/02/22/resizing-your-oplog/02:38
wallyworldaxw: 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
fwereadewallyworld, can you remember what the launchpad project with the functional tests is called?02:43
fwereadewallyworld, IIRC sinzui sent round a mail saying where it was and a bit about how to write new ones, but I can't find it02:43
wallyworldhmmm. i can't recall, but i can look02:44
axwI thought it was under ~juju-qa, but I don't see the branch name I'm looking for02:45
axwci-cd2 something something02:45
axwci-cd-scripts2  -- that might just be the support scripts tho02:45
fwereadeaxw, yeah, all I could remember was there was a "2" in the name02:45
wallyworldi think02:46
axwyeah, that matches what I have on disk for ci-cd-scripts202:47
axwmust've been renamed02:47
fwereadeaxw, am I being dumb? I don;t see any actual *tests* in there02:51
axwfwereade: I think they're just mixed together with support code. I'll follow through from jenkins and find one02:52
axwfwereade: the aws-deploy-trusty-amd64 job just runs this: http://paste.ubuntu.com/7833744/02:54
axwfwereade: backup/restore runs test_recovery.py02:54
axwwith some args02:55
fwereadeaxw, yeah, I just found that one, but I feel it's likely that that's not the only one...02:55
axwfwereade: 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
fwereadeaxw, 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-core02:58
axwI see. probably best off getting sinzui's advice then02:59
axwor maybe abentley03:00
davecheneyjust in case anyone was confused, i really hate JujuConnSuite03:00
fwereadedavecheney, I too have a deep and abiding loathing for that thing03:03
davecheneyfwereade: i want to cleeve it in two03:04
davecheneyJujuClientConnSuite and JujuServerConnSuite03:05
davecheneythen at least for the moment, recombine them into some horrific lovecroftian monster as JujuConnSuite03:05
davecheneymenn0:  % df -h /tmp03:06
davecheneyFilesystem      Size  Used Avail Use% Mounted on03:06
davecheneytmpfs           3.9G 1016M  2.9G  26% /tmp03:06
davecheneythis is what a day of running juju tests looks like03:06
axwI'd rather we just didn't use full end-to-end testing for our unit tests03:09
axwwhy is shit getting left behind in tmp now? I don't think it used to?03:09
davecheneyaxw: always leaks like a mofo03:11
axwwallyworld: I can't reproduce the ec2 failures :/03:11
davecheneyespecially if your fixtures panic03:11
davecheneyaxw: note, i wasn't complaining03:11
davecheneyi'm used to it03:11
wallyworldaxw: yeah, i suspect it's on the ec2 side03:12
axwdavecheney: hmmok. I'm complaining, because I actually ran out of tmpfs the other day ;)03:12
axwwallyworld: 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 hours03:15
axwAWS status page has nothing much to say03:15
wallyworldyep. can you comment on the bug, so that curtis knows we've looked at it?03:15
wallyworldaxw: 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.txns03:20
wallyworldit will be something simple, so i should be able to propose something soon03:21
axwhmm ok03:21
wallyworldnot sure just right now exactly what the issue is though03:21
axwis this related to the thin michael emailed about? and that jam suggested we might be able to drop? (the mongo password dance)03:21
wallyworldcould be, but I think that has been changed in this branch, i need to go look into it03:22
wallyworldi'll get you to review later, but here's the current diff https://github.com/wallyworld/juju/compare/copy-session03:22
davecheneyfwereade: can you explain the rational for this signature and comment ?03:50
davecheneyfunc APIInfo(env Environ) (*api.Info, error) {03:50
davecheney // APIInfo returns an api.Info for the environment. The result is populated03:50
davecheney // with addresses and CA certificate, but no tag or password.03:50
waiganithumper: https://github.com/juju/juju/pull/346 question in user_test.go:10603:51
mattywthumper, http://paste.ubuntu.com/7833982/04:00
axwwallyworld: #1344857   <- this was fixed for 1.20, but not in the 1.18 branch04: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
axwfixed some time during 1.19 anyway04:36
axwadded a comment04:36
axwabout to propose a fix for the machine-agent suciding bug04:37
wallyworldaxw: so, using session.Copy() results in all sorts of permission errors, whereas session.Clone() doesn't05:17
axwClone doesn't create a new socket05:18
wallyworldthe different at the mgo level is that Copy() makes a Refresh() cal lon the session which releases the socket05:18
tasdomasthumper, https://github.com/juju/juju/pull/358/files05:19
wallyworldyes, so now the question is - why does acuiring a new socket cause mongo to reject collection queries etc as unauthorised05:19
axwwallyworld: I can only guess that it has to do with Login is being called after the session is copied05:20
axwso other sessions don't inherit the credentials05:20
wallyworldcould be05:21
wallyworldyeah, looks like i need to rearrange some code05:24
davecheneyfwereade: https://github.com/juju/juju/pull/35905:25
davecheneya semi seroius attempt to reduce the horror05:25
davecheneyfor comment, not review05:25
wallyworldhmmm, maybe not. Login() is called first05:30
axwwallyworld: getting anywhere?06:11
wallyworldaxw: sorta. state.Open() is first called with no credentials, and that fucks up everything from that point onwards06:12
wallyworldbecause there are no credentials, db.Login() is never called06:12
wallyworldso credentials are never set on the session06:13
axwI see06:13
wallyworldbut it tries to use the session via session.Copy()06:13
wallyworldusing the session directly (which also has no creds) works06:13
wallyworldso i thin we must be calling Open() twice06:14
wallyworldonce without creds and once with06:14
wallyworldwell, that's my theory06:14
wallyworldi think it's all tied up in how we initially create the admin user06:17
wallyworldah, it's in agent/bootstrap06:21
wallyworldwe create an initial state as part of the bootstrap process, but without user or password06:21
* wallyworld bbiab, raining, goota get kid from train startion06:21
axwright, cos it's the first connection to mongo and there are no users yet06:21
wallyworldso have to special case that somehow06:22
=== eagles05- is now known as eagles0513875
=== uru_ is now known as urulama
perrito666good morning everyone, please remember dimitern and I will be your on call reviewers today :)10:11
TheMueperrito666: morning10:17
TheMueperrito666: dimiterm isn’t available, diving holidays10:18
perrito666TheMue: then its only me :p10:19
TheMueperrito666: but I can support you, currently only fighting a bit with my machine after upgrading10:22
axwwallyworld: I've got some things to do after dinner, so may be late for / miss the standup10:40
wallyworldsure, np10:40
wallyworldthanks for doc, haven't read it yet10:40
wallyworldi think i'm going to revert michael's changes to txn runner so we can control when session is closed10:41
wallyworldi should say Copy()ied10:41
wallyworldcause now, it copies inside txn runner even when it's not appropriate to do so10:42
wallyworldthe caller (state) should be in control of when a session is copied10:42
wallyworldas it knows if the credentials have been set10:42
wallyworldit's a bit messy10:43
jamTheMue: 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
TheMuejam: ah, hey, just back from lunch. after you didn’t came I thought that watching the games has been too long. :D12:22
natefinchmorning all12:24
perrito666natefinch: morning12:24
TheMuenatefinch: morning12:26
* perrito666 uses a rather inefficient method to find out code to be reviewed12:26
jam1perrito666: looking through the entrails and chicken bones for URL fragments?12:27
TheMueperrito666: which one?12:27
perrito666TheMue: the one jam1 just mentioned12:28
perrito666TheMue: I open all the repos of gh/juju/ and then click on the ones that actually have pull requests12:28
TheMueperrito666: 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 far12:29
* perrito666 senses a script in the near future12:29
perrito666btw, what is dave cheney's nicnkane on irc?12:30
* TheMue is happy enough that his local provider is working again. :) somehow it yesterday didn’t wanted to start the API server :(12:37
perrito666can 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 this12:37
TheMueperrito666: will take a look12:38
TheMueperrito666: 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
TheMueperrito666: just looking for a way check if the file is not yet closed, so defer would use a func() { if f.IsOpen() { … } }12:46
TheMueperrito666: sadly it doesn’t exist12:46
perrito666I guess that could be split into a couple of functions but I believe it is a bit overkill12:47
TheMueperrito666: so yeah, we have to act this way, otherwise ReplaceFile would have to be changed to use a os.File instead of prep and path12:47
natefinchTheMue: 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
TheMuenatefinch: yep, the comment would help to not struggle about it later12:49
natefinchTheMue: 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
TheMuenatefinch: hehe, death by refactoring12:51
natefinchthat'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
mgzheh, that's a funny pr12:53
perrito666mgz: it has an amazing review/changes ratio12:53
mgznate's solution seems the best, f.Close twice after f.Write, on the error and non error branches12:53
mgzor put Write and the error check on seperate lines so you can Close between them (and comment, of course)12:54
hackedbellininatefinch: 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' fields13:15
hackedbelliniafter that, the db service stopped. I'm thinking if fixing this could maybe fix the ha13:15
natefinchhackedbellini: hmm weird13:16
hackedbellinido you or anyone here knows how to workaround that?13:18
TheMuejam1: ping13:19
jam1TheMue: pong13:20
TheMuejam1: the LXC card tells that an lxc instance has no IPv6 address. who’s the source of this info?13:21
natefinchhackedbellini: looking13:21
jam1TheMue: 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
TheMuejam1: when I’m starting LXC manually as well as via juju (using the local provider) it has an IPv6 address13:21
TheMuejam1: hmm, interesting, using lxc-ls it doesn’t show the IPv6, yes13:23
natefinchhackedbellini: 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 anymore13:23
TheMuejam1: 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 show13:24
hackedbellininatefinch: 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
natefinchniemeyer: ^^ 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
TheMuejam1: ah, the IPv4 can be pinged, the IPv6 not, interesting13:26
natefinchhackedbellini: 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 exist13:27
hackedbellininatefinch: 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
jam1TheMue: 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
TheMuejam1: looks like, the standard address is link-local13:32
katcogood morning all13:38
TheMuekatco: heya13:39
mgzhey katco13:39
katcoTheMue: mgz: hi :)13:39
niemeyernatefinch: What's the actual error, and what is the version of mgo and of MongoDB13:42
niemeyernatefinch: 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 using13:46
niemeyernatefinch: In theory, the latest version of mgo does exactly the same..13:46
niemeyernatefinch: Unless there's a bug, which I'd be happy to know about13:47
natefinchniemeyer: we're getting "system.users entry must not have both 'roles' and 'readOnly' fields"13:49
niemeyernatefinch: What is the version of mgo and MongoDB you are using?13:50
niemeyernatefinch: 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 own13:51
natefinchhackedbellini: I presume you have mongodb 2.4.6?13:52
natefinchniemeyer: we're just using UpsertUser: https://github.com/juju/juju/blob/1.20/mongo/admin.go#L11413:52
niemeyernatefinch: Okay, so that should be working fine13:53
natefinchniemeyer: mgo is labix.org/v2/mgo   20140331185009-fhnh3xzfdpicup0j13:53
niemeyernatefinch: Testing..13:53
natefinchniemeyer: 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.2013:54
niemeyernatefinch: Ah, hmm..13:56
niemeyernatefinch: If you try to upgrade a user from a password-less state to a password-rich one, yeah,  you might have issues13:56
niemeyernatefinch: Erm, sorry13:56
niemeyernatefinch: If you try to upgrade a user from a role-less state to a role-rich one, yeah,  you might have issues13:56
niemeyernatefinch: There's no "readOnly" field in there13:57
niemeyernatefinch: So I assume that field is in the database already, from an ancient run13:57
niemeyernatefinch: So the error is correctly saying you cannot do that13:57
natefinchniemeyer: yeah, that's what I was thinking.  I wonder if we should just override readonly13:58
niemeyernatefinch: This was unfortunately quite fiddled with during MongoDB 2.2 => 2.4 => 2.613:59
niemeyernatefinch: I did my best to sanitize the upgrade path for driver users, but the server did change significantly and certain cases are just not doable13:59
niemeyernatefinch: You can try dropping readOnly, yes, but I suggest testing it both in 2.4 and 2.614:00
niemeyernatefinch: The manual dropping code will not work on 2.6, as user changes are now done via commands rather than directly fiddling with the db14:00
natefinchniemeyer: yeah, we're working on 2.6  compatibility.... but so much changed, it's quite a process.14:01
niemeyernatefinch: So probably "drop on 2.4" + "check for that one error on 2.6 and ignore"14:01
niemeyernatefinch: Yeah, user managed, specifically, did change both on 2.4 and on 2.614:01
niemeyernatefinch: I'm hopeful the fiddling is done now.14:02
perrito666wwitzel3: stdup?14:03
natefinchniemeyer: 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
niemeyernatefinch: No problem14:04
natefinchperrito666, wwitzel3: brt, one minute14:05
hackedbellininatefinch: sorry I was helping a workmate here. The mongodb version is 2.4 here.14:20
hackedbelliniI 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
natefinchhackedbellini: 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 fix14:27
hackedbellininatefinch: 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
jcw4what is the normal approach to testing internal helper functions?15:36
jcw4should 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
jcw4perrito666, mgz, natefinch, et. al. ^^^15:37
perrito666jcw4: in that case I usually add a test in the package15:40
jcw4perrito666: i.e. in the actual package rather than in package_test15:40
perrito666jcw4: but dont take my word as law I am far from elder here15:41
jcw4I've heard strong opinions against that, but possibly there might be exceptions for internal helper methods15:41
mgzjcw4: I pretty much always prefer the export_test.go method15:42
perrito666mgz: @more15:42
meetingologyjcw4: Error: "more?" is not a valid command.15:42
perrito666mgz: meh, you could very well support basic irc bot commands15:43
mgzfor any non-trivial internal function, having dedicated unit tests makes sense, rather than just testing at pubic interfaces (which some prefer)15:43
mgzso, just have a SomeInternalFunction = someInternalFunction in export_test.go in that package15:44
mgzand test at that level15:45
jcw4mgz +115:48
jcw4perrito666, mgz I'll add some tests and then ask for a review again15:48
perrito666btw, 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
mgzperrito666: congratulations, you get to review them I think15:50
perrito666mgz: I am on it, just wondering if there was a deeper reason for them to be ignored for almost a month15:51
mgzperrito666: not that I'm aware of, apart from some obvious reviewers being enveloped with other things15:53
katcomgz: 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
mgzkatco: I'd try to put in kvm_test.go even if it means adding a new suite there16:05
katcomgz: k that's what i was thinking initially; thank you :)16:06
perrito666I just burned frozen burgers... that must be some sort of special skill16:14
mgzperrito666: I thought you were the chef in the house...16:14
perrito666mgz: I am, I just got distracted by a code review16:15
jcw4riveting code review16:15
mgzscorching code review...16:15
perrito666mgz: well this burned food will definitely affect the outcome of the review16:17
katco"this is the worst object instnatiation i've ever seen. the formatting is crap, and i'm hungry. dnflgtm"16:18
rogpeppe1jam, jam1: ping16:22
rogpeppe1cmars: ping16:28
katcoi need a second opinion on a design decision; anyone available to PM for a few minutes?16:35
mgzkatco: bug perrito666, he's in a good mood and not otherwise occupied16:38
katcoperrito666: if that's true, got a sec? or are you munching on some burnt lunch? ;)16:38
mgzkatco: more seriously, you can bug me if you like :)16:39
katcomgz: that would probably be best since we're on the same team :)16:39
natefinchjcw4: 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
perrito666mgz: mm I see you have managerial talent16:59
perrito666katco: I am in a good mood, excepting for that incident with the chickenburger, so fire away17:00
katcoperrito666: mgz helped me out, but thank you!17:00
mgzperrito666: I can be useful, as well as wind people up about their burnt burgers :)17:02
jcw4natefinch: thanks17:24
natefinchjcw4: as others said, not everyone agrees with me.... but I don't think anyone will stop you from writing more tests :)17:25
perrito666I have had in-package tests merged so I guess no one has very strong objections17:26
jcw4I'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 happy17:26
perrito666jcw4: 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 :p17:27
jcw4from my perspective that's the same thing17:28
perrito666I need someone with managerial skills for a quick question17:31
natefinchperrito666: you can't expense the heroin, I told you that17:32
perrito666natefinch: maan, whyyy?17:32
natefinchperrito666: whazzup?17:32
perrito666natefinch: I pmd you17:33
katcoperrito666: 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
katcoperrito666: jcw4: and it avoids the export dance for tests17:53
jcw4katco: okay, I haven't heard from mgz and it sounds like a bit of a quorum so I'll go with in-package tests17:54
katcojcw4: mind you i am brand new, so my opinion probably doesn't count for much :)17:54
jcw4katco: 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 emerge17:55
katcowell 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
ericsnowFWIW, last I saw, niemeyer favored the *_test package approach, which somewhat explains our use of it :)18:07
ericsnowsee https://groups.google.com/d/msg/Golang-nuts/dkk0X1tIs6k/nO3CKFqbIxYJ18:08
perrito666sounds 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
perrito666testing things as they are intended to be used18:11
niemeyerIn 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 packages18:11
niemeyerAlso like that extra mile.. encourages people to be more honest about testing what is meant to be tested, rather than implementation details18:12
jcw4hmm; 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
niemeyerjcw4: It's hard to give reasonable advice without knowing what the case is, though.. what are you testing?18:17
jcw4niemeyer: helper functions in the actions and actionresults watchers18:17
niemeyerjcw4: URL to the code?18:17
jcw4niemeyer: https://github.com/juju/juju/pull/351 ... specifically :18:18
jcw4niemeyer: I'm actually about to refactor them down to one watcher with a parameter for which collection to watch...18:19
niemeyerjcw4: I'd be on the fence on that one.. it's all internal, and it's a relevant abstraction that deserves testing18:21
niemeyerjcw4: 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 it18:22
niemeyerjcw4: (other than purism)18:22
jcw4niemeyer: meaning the refactor to one watcher?18:22
jcw4or the helper functions18:22
niemeyerjcw4: No, meaning the subject of our conversation18:23
jcw4niemeyer: 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
katcoericsnow: niemeyer: ty for the context :)18:33
=== gsamfira1 is now known as gsamfira
natefinchthumper: got a minute to talk about jujud logging?20:58
thumpernatefinch: sure, a few20:59
natefinchthumper: 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 twisty21:00
thumpernatefinch: oh... kay..21:01
thumpernatefinch: what needs to be done is this:21:01
thumper1) at the startup of jujud, configure the logging to use the special writers you have21:01
thumper2) change the upstart scripts so it writes out to a file, not to stdout/stderr21:01
thumperor at least, logging goes to the file21:01
thumperI don't think we write anything else out21:02
thumperjust that...21:02
thumperchanging the upstart scripts are the biggest thing21:02
natefinchthumper: so, we have a few different commands, bootstrap, machine agent, unit agent, juju-run21:02
natefinchthumper: 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
thumperdon't worry about juju-run21:04
thumperit sends stdout, stderr back to the client21:04
natefinchok, that was sort of my question21:04
thumperand shouldn't log21:04
natefinchright, that make sense21:04
natefinchok... 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 installations21:05
natefinchso... three, three things we have to do: configure loggo for the commands, change cloud-init, and update the existing upstart scripts of running machines21:06
natefinchthumper: and how do I set the output on loggo?21:10
natefinchthumper: 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
thumpernatefinch: change the writers21:13
* thumper looks21:13
thumperwith a default formatter21:14
natefinchdo I replace the "Default" logger or no?21:14
thumperotherwise it will continue to write to stderr21:14
natefinchthumper: can I overwrite, or do I have to delete and then register?21:15
natefinchthumper: the package example seems to say delete and register21:16
thumperprobably an old example21:16
natefinchthoughts on max log size?21:16
thumper5 meg?21:17
* thumper shrugs21:17
natefinchwow really?  I was going to say like 20021:17
natefinchI dunno21:17
thumperme neither21:17
thumperask an admin type person21:17
natefinchI'll PR it and let people duke it out21:17
natefinchThanks again.  Gotta run'21:17
=== liam_ is now known as Guest15796
fwereadeperrito666, ping21:34
katcogofmt sure does make for messy diffs when adding longer variable names to structs21:34
perrito666fwereade: pong21:35
fwereadeperrito666, the semi-readable PRs for cloudsigma are here: https://github.com/Altoros/juju-cloudsigma/pulls21:36
perrito666fwereade: so the ones that are in my email are not the semi readable ones?21:36
fwereadeperrito666, those ones I think are kinda unbearably yucky to read21:37
fwereadeperrito666, I got upset with them, jam pointed me at the others21:37
perrito666fwereade: you have a lower tolerance level than I21:37
perrito666they are ugly, not unbearable21:37
perrito666fwereade: 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 questions21:40
perrito666s/those/the ones I pointed21:40
fwereadeperrito666, yeah, sgtm, please close them with a note pointing at the others21:46
fwereadeperrito666, I got all upset with the 18 changed files, 20 changed files, 22 changed files progression21:46
perrito666fwereade: mm I will email the proposer guy21:48
perrito666I am not sure which is which and I fear I might close the wrong ones21:48
perrito666the link you passed me contains the ones I mentioned plus other21:49
fwereadeperrito666, that sounds reasonable -- and, sorry I'm not completely up to date with it, jam has been liaising with them primarily21:55
davecheneymattyw: http://i.imgur.com/IODqYZN.jpg22:49
cmarswaigani, http://sourcegraph.com/blog/ipfs-the-permanent-web-by-juan-benet-talk22:49
mattywdavecheney, http://irclogs.ubuntu.com/2014/07/22/%23juju-dev.html22: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 lists23:00
bodie_action-get outfile.format for example23:01
bodie_in that example, outfile is the key to an inner map23:01
bodie_format is a key in that inner map23:01
bodie_picture JSON23:01
bodie_however, there could also be a list index23: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 optimal23:03
bodie_but adding the list index option is much much uglier23:03
bodie_jcw4 any thoughts?23:03
jcw4bodie_: wouldn't the pattern extend to outfile.files.1 ?23:04
jcw4bodie_: you'd have to work a bit to make sure you parsed integer indexes at the right time instead of strings23:05
bodie_I figured traditional list index would be easier to parse23:06
bodie_right now it indexes by the key parsed from the list, so '1' would have to be a string key to another map23:06
bodie_or uh23:06
bodie_files would be a map23:06
bodie_'1' would be one of the keys in it23:06
bodie_whereas if we have [1] we clearly know it's an int, plus it's more familiar to the user23:07
bodie_it almost seems like this isn't something that is really the job of the action-get command23:08
bodie_but lists are clearly supported by JSON-Schema as JSON elements23:09
bodie_perhaps I'll simply PR it as WIP and query for responses on this topic23:09
jcw4bodie_: agree, this is potentially common utility code.. I don't know if it's implemented anywhere though23:09
bodie_hmm, good thought23: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 list23:10
=== meetingology` is now known as meetingology
bodie_this would be vastly simpler in a functional language with .apply() ... :P23:23
bodie_oh well, this is fine too23:23
perrito666fwereade: ping23:53

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!