/srv/irclogs.ubuntu.com/2014/06/13/#juju-dev.txt

davecheneythumper: menn0 waigani https://github.com/juju/names/pull/400:11
menn0davecheney: I'm done00:35
davecheneymenn0: ta00:37
davecheneyjust responding now00:37
davecheneywon't take long00:37
davecheneychanges to juju/juju are surprisingly minimal00:37
davecheneyhttps://github.com/juju/juju00:38
davecheneygah00:38
davecheneyhttps://github.com/juju/juju/pull/88/files00:38
davecheneyif you have time to review those i can get stuck into the final change00:38
axwwallyworld_: need to restart, back for 1:1 on a minute00:59
wallyworld_ok01:00
axwwallyworld_: mind if we defer for 10 mins or so?01:05
wallyworld_sure01:05
axwwallyworld_: ready when you are01:12
davecheneyaxw: http://juju-ci.vapour.ws:8080/job/github-merge-juju/118/console01:18
davecheney^ this test failed, but the bot marked it as a success01:18
axwwallyworld_: sorry did I cut you off?01:37
axwsounded like you were saying something when I closed the window01:37
wallyworld_not all all :-)01:37
axwcool01:37
wallyworld_nah, i have to learn to shut up more01:37
wallyworld_i talk too much01:38
thumperwallyworld_: what? no.01:38
wallyworld_go away01:38
* axw grrs at his graphics stack01:39
axwwhen I change res, sometimes the screen craps itself and flickers01:39
axwgotta restart...01:39
davecheneyERROR: Failed to merge: {u'documentation_url': u'https://developer.github.com/v3/pulls/#merge-a-pull-request-merge-button', u'message': u'Pull Request is not mergeable'}01:42
davecheney+ /home/ubuntu/jenkins-github-lander/bin/lander-merge-result --ini /home/ubuntu/jenkins-github-lander/development.ini '--failure=Merging failed' --pr=88 --job-name=github-merge-juju --build-number=11901:42
davecheneyhttps://api.github.com/repos/juju/juju/issues/comments/4596716701:42
davecheney++ date01:42
davecheney+ echo Finished: Fri Jun 13 01:27:54 UTC 201401:42
davecheneyFinished: Fri Jun 13 01:27:54 UTC 201401:42
davecheney+ exit 001:42
davecheneyDescription set: davecheney 105-introduce-tags-type-iii01:43
davecheneyFinished: SUCCESS01:43
davecheneyfailed: SUCCESS01:43
davecheney+ bash scripts/pre-push.bash01:43
davecheneymongo/prealloc.go:141: missing argument for Debugf verb %q: need 1, have 001:43
davecheney+ bzr whoami 'Jenkins bot'01:43
davecheneyhow come this didn't fail the build01:43
davecheneyhttps://bugs.launchpad.net/juju-core/+bug/132957801:44
_mup_Bug #1329578: build: pre-build.bash failures do not fail the build <juju-core:New> <https://launchpad.net/bugs/1329578>01:44
axwdavecheney: sorry missed your message before. it failed once, then passed01:45
axwhuh01:45
axwdidn't see that01:45
axwhow come my pre-push script didn't pick it up ...01:46
axweh, because apparently I never set it. oops01:47
axwdavecheney: apparently "go tool vet" returns rc 0 even if it finds that01:49
davecheney:(01:50
axwperhaps we should check for empty output?01:51
davecheneyhow are you involing it01:52
davecheneygo vet $PKG01:52
davecheneygives the right rc01:52
davecheneybut we have to use go tool vet to tweak the flags01:53
axwdavecheney: it's just a copy of the old .lbox.check, it calls "go tool vet" directly because it wants to set the print funcs01:53
davecheneythat is bizarre01:53
davecheneyhow does go vet know that go tool vet failed01:53
axwnfi01:53
axwoh what01:56
axwdavecheney: if you do "go tool vet ." it does something different to "go tool vet *.go"01:56
davecheney:crying:01:56
thumperaxw: haha02:03
davecheneyok  github.com/juju/juju/environs/sync143.376s02:20
davecheney^ why does this test take so long to run ?02:20
wwitzel3just a mere 143 seconds02:20
davecheneyit must be one of the only tests that _doesn't_ spin up several mongodb's02:20
wwitzel3oh I see what you did there02:20
wwitzel3just to be clear, I have nothing helpful to add .. just peanut gallery02:21
davecheneyhttp://juju-ci.vapour.ws:8080/job/github-merge-juju/121/console02:22
davecheneyyay, i crashed mongo02:22
davecheneyreplica sets, adding instability since 201202:22
axwactually looks related to SSL from here02:23
axwmaybe both02:23
waiganithumper: sweet! thanks02:36
waiganithumper: I also thought about the DateCreated AND LastConnection being mockable via the user factory02:36
davecheneyaxw: the results of go test -p 1 ./...02:42
davecheneyappear to be ignored by the bot ?02:42
davecheneyis that correct ?02:42
axwdavecheney: erm, I don't think so? lemme see...02:44
axwdavecheney: the script just does "go test ./... || go test -p 2 ./..."02:44
davecheneyso if the build fails02:45
davecheneytry again faster ?02:45
axwno, try again slower02:46
axw"go test ./..." uses #CPU, which is 402:46
davecheneyoh, indeed02:46
davecheneyok02:46
davecheneyaxw: where is the script for the build02:56
davecheneyi want to turn off all the -v styel output02:56
davecheneyie, we don't need to spend 2,000 lines on tar xvfz02:56
axwdavecheney: embedded in jenkins02:57
davecheneyi don't understand why your build failed02:57
davecheneythe branch applied and the build worked02:57
davecheneybut then when the same merge is done for real02:57
davecheneyit fails ?02:57
axwGitHub flaking02:57
axwsometimes the merge just mysteriously fails, then you retry and it works02:58
axwdespite the fact that the lander managed to merge the branches fine02:58
axwlocally I mean02:58
davecheneyexactly02:58
davecheneyo_O!02:58
davecheneyits happened twice this morning02:59
davecheneyi don't accept that git hub is that flakey02:59
axwdavecheney: I've taken out the -v from tar extraction02:59
davecheneyi think it's a procedural error02:59
davecheneyaxw: thank you02:59
axwentirely possible, I'm not really familiar with the code that does the merge.. .will take a look and see if it looks suss03:00
* axw shrugs03:01
axwit's just doing a PUT to the merge URL03:01
davecheneyso, there must be something different about the way we make the local branch and apply the PR03:02
davecheneyand the way that github is trying to merge the PR back onto master03:03
axwmgz and I briefly discussed just doing pushing the merge directly03:04
axwmgz pointed out that we could then "go fmt" the code before landing03:04
axwwe could also add coverage data to the repo in that way03:05
davecheney+1 tasty03:05
axwgithub and/or jenkins must hate me03:29
* thumper EOWs04:33
=== vladk|offline is now known as vladk
fwereadewallyworld_, couple more comments on the PR -- still not convinced by those refreshes05:56
wallyworld_fwereade: do you agree that they were called previously?05:56
fwereadewallyworld_, no05:56
fwereadewallyworld_, I think they were only called on ErrAborted05:56
fwereadewallyworld_, the usual pattern is if err := runtxn(); err != ErrAborted { return err }05:57
wallyworld_fwereade: oh, hang on, maybe i'm being totally stupid05:57
wallyworld_sigh, i think i am05:58
wallyworld_ffs05:58
fwereadewallyworld_, no worries, those constructs are a bit tangled05:58
wallyworld_sorry05:58
* wallyworld_ goes to kick the dog05:59
fwereadewallyworld_, it just means the code can get a bit simpler05:59
* fwereade feels like an accessory to animal cruelty05:59
wallyworld_yeah, still doesn't absolve my stupidity05:59
fwereadewallyworld_, oh bollocks05:59
fwereadewallyworld_, the benefit of factoring that package out absolves far worse05:59
wallyworld_i am looking forward to being able to use it06:00
wallyworld_fwereade: was the bollocks for a bad reason?06:01
fwereadewallyworld_, I was expostulating violently at you beating yourself up for imo inadequate reasons06:02
wallyworld_well, fsvo inadequate :-)06:02
wallyworld_thanks for the review though, was a bigun06:03
fwereadewallyworld_, quickly searching through I think we're actually behaving noticeably better now because all the refreshes seem to be in `if attempt != 0` branches now06:04
fwereadewallyworld_, (once the others are droped anyway)06:05
wallyworld_yup06:05
fwereadewallyworld_, one quick thought (don't do it now)06:05
fwereadewallyworld_, we're not really using attempt, just attempt != 006:05
wallyworld_um, i think there's a place where we are06:06
fwereadewallyworld_, gut feel on (firstAttempt bool) vs (attempt int)?06:06
fwereadewallyworld_, ah ok then06:06
wallyworld_attempt ==1 is somewhere06:06
wallyworld_only one place06:06
fwereadewallyworld_, must have missed it06:06
fwereadewallyworld_, disregard :)06:06
wallyworld_ok :-)06:06
wallyworld_fwereade: i'm currently fixing a nasty goose json bug which fucks up our floating ip usage for hp cloud. so i'll land soon06:06
fwereadewallyworld_, sweet06:07
wallyworld_and i got a meeting soonish as well06:07
fwereadewallyworld_, no rush06:08
wallyworld_yeah, i'm the only one who needs it right now :-)06:08
fwereadewallyworld_, I'm just happily imagining all the nice stuff we'll be able to do more easily in future06:08
wallyworld_yep06:08
fwereadewallyworld_, backoff from failed txns06:08
wallyworld_like internal storag06:08
wallyworld_yep, that too06:09
fwereadewallyworld_, even a txn type so we can detect and reject ops for the same doc -- or at least combine them all06:09
wallyworld_would be nice06:09
fwereadewallyworld_, not to mention cleanly adding metrics like txn spread, time to execute, number of retries06:10
wallyworld_indeed06:10
fwereadeaxw, ping06:10
axwfwereade: pong06:13
fwereadeaxw, maas 1.5 supports zones, we should do that too -- did I miss it?06:16
axwfwereade: just haven't gotten to it yet06:17
axwwasn't sure if it was ranked as important yet06:17
fwereadeaxw, cool06:17
fwereadeaxw, I think it is06:17
axwokey dokey, I shall add a card06:17
fwereadeaxw, maas is pretty much the flagship provider for a whole bunch of use cases06:18
axwI'm sure it'll be important to people building OpenStack clouds06:18
fwereadeaxw, yeah06:18
axw(on top of MAAS)06:18
fwereadeaxw, another thing, btw -- and this is probably a bit big to charge to the azs work, but we need to think about it06:20
fwereadeaxw, azs, instance types, and networks all affect constraint validity06:21
fwereadeaxw, it would be really good if we could expose an api that lists the choices for those things06:21
axwfwereade: heh :)  was talking about this with wallyworld before06:22
fwereadeaxw, so that the gui can have dropdowns/checkboxes/whatever06:22
axwthere's a card for instance types already06:22
wallyworld_fwereade: there's a plan for it06:22
axwI figured we'd want it for placement directives like AZs too06:22
fwereadeaxw, wallyworld_: you rock :)06:22
wallyworld_fwereade: i've discussed with gui guys06:22
axwfwereade: not sure how AZs affect constraints though?06:22
wallyworld_fwereade: there will be a new client API call returning a struct containing inst types, availability zones, and possibly env constraints etc06:23
fwereadeaxw, ha, not constraints for azs, you're right06:23
axwregardless, it'll be nice to be able to list them06:23
fwereadeaxw, although... hmm... why *shouldn't* we do az constraints?06:23
axwI thought you didn't want to :)06:24
fwereadeaxw, I know this is feature creep, I'm not necessarily asking for us to do it now06:24
axwfwereade: hazmat wants them though06:24
fwereadeaxw, just trying to figure out if it's a good idea and we should pre-emptively add a lets-do-it-one-day bug for it06:24
fwereadeaxw, ha, ok, that's generally good enough for me06:24
fwereadeaxw, am I right in thinking that joyent doesn't support azs?06:25
axwfwereade: IIRC, CloudFoundry has some components that want to be partitioned across AZs06:25
axwI have no idea, haven't looked06:25
* axw looks06:25
fwereadeaxw, I just poked around quickly, couldn't see anything06:25
axwokey dokey06:25
fwereadeaxw, re cloudfoundry -- so we're ideally looking for something like 2 services, 4 azs, non-overlapping spread?06:33
fwereadeaxw, ha, now I remember why I don't want zone constraints06:33
fwereadeaxw, I want near/far, not explicit zones06:33
axwfwereade: I don't know the specifics. I was under the impression that services were tied to a single AZ06:33
fwereadeaxw, oh ok, that's interesting06:33
* fwereade probably needs to know more about cloudfoundry :/06:34
axwi.e. all units of one service in one AZ, all units of another service in another06:34
axwneed to chat with hazmat more about it06:34
davecheneyurgh06:35
davecheneyFAIL: run_test.go:269: RunSuite.TestAllMachines06:35
davecheneyrun_test.go:293: c.Check(testing.Stdout(context), gc.Equals, string(jsonFormatted)+"\n")06:35
davecheney... obtained string = "[{\"Error\":\"command timed out\",\"MachineId\":\"1\",\"Stdout\":\"\"},{\"MachineId\":\"0\",\"Stdout\":\"megatron\\n\"}]\n"06:35
davecheney... expected string = "[{\"MachineId\":\"0\",\"Stdout\":\"megatron\\n\"},{\"Error\":\"command timed out\",\"MachineId\":\"1\",\"Stdout\":\"\"}]\n"06:35
davecheneyOOPS: 233 passed, 1 FAILED06:35
davecheney--- FAIL: TestPackage (155.82 seconds)06:35
davecheneyFAIL06:35
davecheneyFAIL    github.com/juju/juju/cmd/juju   155.991s06:35
wallyworld_axw: can i bother you for a review to fix one of the 1.19.4 bugs? i'll update and land the dependencies fix after goose is sorted https://codereview.appspot.com/10518004306:44
axwno worries06:44
axwwallyworld_: reviewed06:49
wallyworld_thanks06:49
wallyworld_axw: is there a branch for the azure card?06:50
axwwallyworld_: which one?06:51
wallyworld_bug 132491006:51
_mup_Bug #1324910: azure destroy-environment does not complete <azure-provider> <destroy-environment> <juju-core:Triaged> <https://launchpad.net/bugs/1324910>06:51
axwwallyworld_: no branch, I only just started looking06:51
wallyworld_ah ok, it was iin the review lane06:51
axwoops06:51
wallyworld_:-)06:51
axwthanks06:51
voidspacemorning all07:13
axwmorning07:14
voidspaceo/07:17
mattywmorning all07:19
TheMuemorning all o/07:35
=== psivaa-afk is now known as psivaa
wallyworld_axw: trivial https://github.com/juju/juju/pull/9008:20
axwlooking08:20
TheMuehehe, really trivial08:20
axwwallyworld_: done08:21
wallyworld_thanks :-)08:21
wallyworld_axw: i have no soccer tonight so if mgz  is around we can do a standup in 40 mins if you are free08:21
axwwallyworld_: sure, I think I will be08:22
wallyworld_ok08:22
* fwereade off in a minute for a swim and a think, does anyone need me for anything?08:25
wallyworld_nah08:27
TheMueenjoy swimming08:27
wallyworld_axw: looking at the bot, there's a fucktonne of blue dots :-)08:28
axw:)08:28
axwwallyworld_: I'm getting a fucktonne of spurious "cannot merge" errors though :(08:28
wallyworld_oh :-(08:28
wallyworld_let's talk to mgz at standup08:29
voidspacebloodearnest: morning09:20
bloodearnestbloodearnest: heya09:20
bloodearnestvoidspace: heya, even :)09:21
mattywaxw, ping?09:21
axwmattyw: yo09:21
voidspacebloodearnest: :-)09:21
mattywaxw, I've added the charm link https://github.com/juju/juju/pull/80. Shall we land it or think of a better summary?09:22
axwmattyw: land it, hopefully someone will think of something better after seeing it ;)09:23
mattywaxw, I seem to remember there was a thread on one of the mailng lists 18 months ago that tried to come up with a decent summary, but I couldn't find it09:25
axw*shrug* I wasn't here 18 months ago :)09:26
dimiternmattyw, hey09:39
dimiternmattyw, do we still need to discuss port ranges tasks?09:40
mattywdimitern, sorry totally forgot09:41
mattywdimitern, yeah lets do it09:41
dimiternmattyw, ok, i'm joining09:41
mattywfwereade, https://github.com/juju/juju/pull/64 is this what you had in mind?10:32
TheMuedimitern: standup10:48
perrito666morning10:49
dimiternTheMue, brt, sorry10:50
perrito666rogpeppe1: tx again :)11:15
voidspaceperrito666: morning11:16
rogpeppe1perrito666: np - it's looking loads better11:16
rogpeppe1perrito666: BTW i introduced some folks to backgammon at the pub last night...11:16
perrito666axw: btw, I noticed the 2013/2014 issue, I let 2013 because lots of new code seems to have 2013 perhaps this is something we should address too?11:17
perrito666rogpeppe1: and backgammon is not an euphemism for your fist being drunk right? :p11:17
perrito666rogpeppe1: you are a bg evangelist11:17
rogpeppe1perrito666: i kind of am, but i actually haven't played in months. i just saw that the pub had a set.11:18
axwperrito666: if it's code that started in 2013 then that's fine, but this is all new isn't it?11:18
perrito666axw: it is11:18
voidspaceactually enjoying writing tests11:19
voidspaceit turns out this particular piece of code I'm working on is testable11:19
perrito666voidspace: lol11:19
voidspace:-)11:19
voidspaceand I'm getting to use the os / ioutil / path libraries11:19
voidspacealways a good part of a language's standard library to be familar with11:19
perrito666rogpeppe1: the strip parameter is to emulate -C in gnutar it is stripped from the h.Name11:20
rogpeppe1perrito666: but you don't use it, right?11:20
perrito666I do, if I dont I screwed the merge11:20
* perrito666 checks11:20
perrito666line 5911:21
rogpeppe1perrito666: that's just passing the strip parameter through, though AFAICS11:21
rogpeppe1perrito666: but it's always empty11:21
* perrito666 finds a bit odd that there is path.Join and filepath.Join11:21
rogpeppe1perrito666: path is forward-slash-separated paths11:22
perrito666rogpeppe1: nope, when building the tar.gz it strips the tempdir path11:22
rogpeppe1perrito666: filepath is OS-dependant11:22
perrito666mm, that is a useful piece of advice11:22
* perrito666 tries the hard task of finding a movie theater that broadcasts x-men in English11:24
rogpeppe1perrito666: ah, got it. in which case, why not get rid of line 78?11:24
rogpeppe1perrito666: and pass "/" in where currently you pass ""11:25
perrito666rogpeppe1: yup, makes a lot of sense11:25
rogpeppe1perrito666: one other thing - you should probably use filepath.ToSlash for the file name you store in the tar11:26
rogpeppe1perrito666: so that even if we're generating a tar file on windows, the files will look normal11:26
perrito666rogpeppe1: will that work when untaring on windows?11:26
rogpeppe1perrito666: if it didn't, you'd never be able to untar any tar file on windows :-)11:27
rogpeppe1perrito666: so, yes, i think so11:27
perrito666rogpeppe1: to be honest.. I never tried to untar on windows11:27
perrito666:p11:27
rogpeppe1perrito666: i probably have :-)11:27
rogpeppe1perrito666: tbh none of the /var/lib/juju paths will work in windows, so it's all pretty academic11:28
perrito666I have been using either osx or linux since .. well since like a ton of years, I recall using xp as my desktop when It jut came out for a few months and then that was it11:28
voidspaceI think the tests are done for the backup downloading (server side)11:29
perrito666rogpeppe1: I know, that is why the pat is set up there, I guess that if we try to run in windows we will have some convenience getter for that11:29
voidspacenow for the client side implementation11:29
rogpeppe1perrito666:  yeah, you could have the getFiles implementation forked, one for each platform in two files11:29
perrito666rogpeppe1: from the tests of archive/tar on the go source I cannot find a clear way to actually untar those into the hd, specially directories, have you ever done it?11:32
rogpeppe1perrito666: you don't have to untar to disk11:33
rogpeppe1perrito666: just read the contents (for files)11:33
rogpeppe1perrito666: see io.ReadAll11:33
perrito666rogpeppe1: well, the restore part wich Ill write after I fix your comments says otherwise :p11:34
rogpeppe1perrito666: ah, i see11:34
rogpeppe1perrito666: i thought you were talking about checking contents11:34
perrito666rogpeppe1: nope, that is pretty much clear, I just need to add contents to these files11:35
perrito666:p11:35
rogpeppe1perrito666: the current restore plugin shows how you can extract files from a tar file in go11:37
perrito666rogpeppe1: really?11:38
rogpeppe1perrito666: well, it doesn't bother to actually extract them to disk, it's true11:39
rogpeppe1perrito666: it should be relatively straightforward to write a tar file extraction function11:39
rogpeppe1perrito666: there might be one already around somewhere11:40
rogpeppe1perrito666: but it's just a matter of iterating through the contents, creating and writing each file in turn11:40
perrito666rogpeppe1: I am not sure how to handle directories, I did google a bit but found nothing, everyone seems to be happy enough with buffer11:40
rogpeppe1perrito666: if there's an explicit dir entry, mkdir it, otherwise MkdirAll the file's directory before writing the file11:41
perrito666ok, tx11:42
rogpeppe1does anyone know anything about the juju local plugin?12:12
rogpeppe1i can't see that it does anything12:12
rogpeppe1the only substantive logic in there is runAsRoot, but AFAICS that never gets called12:13
=== vladk is now known as vladk|offline
rogpeppe1i guess it might be a place holder for future stuff12:17
rogpeppe1fwereade, dimitern, jam: any idea?12:17
jam1rogpeppe1: thumper was working on it as a namespace for the local provider tweaks he wanted to do12:18
jam1IIRC he wanted to be able to refresh the LXC templates12:18
jam1though now we cache on all providers12:18
jam1which means just having it in "local" isn't good enough12:18
jam1rogpeppe1: so either it isn't complete yet, or its functionality got moved out12:18
jam1(I'm personally not a fan of putting that into a plugin vs core functionality, but thumper likes it)12:19
rogpeppe1jam1: ok, thanks12:20
rogpeppe1jam1: one other thing: do you know why it was split into two packages rather than just having one main package like all the other commands?12:21
jam1no idea12:21
rogpeppe1jam1: ta12:22
fwereaderogpeppe1, I would guess that it's because he shares my opinion that the single-main-package thing is a horrible antipattern :)12:30
rogpeppe1fwereade: i don't see how factoring out the one-line main() function helps12:30
rogpeppe1fwereade: if the package being imported was actually useful to be used from Go, i might agree12:31
rogpeppe1fwereade: but the difference between func Main and func main seems trivial to me12:31
fwereaderogpeppe1, it means that (1) all the code is easily accessible from elsewhere without weirdness, so it's a good habit independent of whether the code in a given command is currently obviously useful from elsewhere; and (2) it turns in-package testing into a shameful fallback, as it should be, rather than a necessity ;p12:33
rogpeppe1fwereade: if Main didn't call os.Exit, i might possibly agree to (1). but it does - packages should be written to be useful to call from Go. just separating main from Main doesn't help anyone.12:35
fwereaderogpeppe1, fair point there, sure12:35
fwereaderogpeppe1, I think that's just an argument that part of it is in the wrong package, rather than an argument against spltting packages, though12:36
rogpeppe1fwereade: and about in-package testing... really, who cares? for main packages, all the stuff you're going to be testing is likely to be internal anyway12:36
rogpeppe1fwereade: i'm totally up for factoring out useful functionality from main packages12:37
rogpeppe1fwereade: but splitting them just "because" just adds obfuscation12:37
fwereaderogpeppe1, if you need internal testing, your packages are probably too big ;)12:37
rogpeppe1fwereade: i don't think all the grubby implementation details of a package are always worth factoring out. it's nice to hide things. but they may very well be worth testing.12:38
fwereaderogpeppe1, I dunno, the ideal code is lasagna, but given a choice between spaghetti and ravioli I've generally found the latter to be more palatable12:39
mattywdoes anyone know why this might be failing? http://juju-ci.vapour.ws:8080/job/github-merge-juju/138/console12:40
rogpeppe1fwereade: we can bake lasagna within packages too :-)12:40
fwereaderogpeppe1, IME that doesn't work -- without the language helping you to enforce the boundaries, it invariably degrades spaghettiwards12:41
fwereaderogpeppe1, even super-strong convention like python's _field members doesn't seem to actually help12:41
fwereaderogpeppe1, it does help a *little* but the temptation to violate layers is strong for the programmer in a hurry12:42
rogpeppe1fwereade: if people want to violate layers, they'll do it regardless of package boundaries. hiding implementation details inside a well defined package boundary is good for avoiding needless reliance of external code on internal details12:43
=== psivaa is now known as psivaa-afk
fwereaderogpeppe1, right, but without package boundaries people are often not aware that the layers even exist to be violated12:45
rogpeppe1fwereade: internal types are a pretty good indication12:45
fwereaderogpeppe1, even the careful and conscientious programmer-in-a-hurry is susceptible to perpetrating that sort of breakage12:45
rogpeppe1fwereade: there are many forms of breakage that we can do at any time :-)12:46
fwereaderogpeppe1, sure, I'm arguing from anecdotal evidence: I have spent a lot of my life rolling my eyes at many forms of bad code -- not to mention writing plenty of it myself -- and ISTM that failing to take advantage of the encapsulation constructs available is the root of an awful lot of avoidable evil12:49
fwereaderogpeppe1, that go only offers package boundaries is not *necessarily* a strike against go, but it does constrain the forms that long-term-robust code can actually take12:50
rogpeppe1fwereade: it's a trade-off, as usual. making packages with no decently explainable reasons for existing is another way to make spaghetti code that's really hard to fathom IME.12:50
fwereaderogpeppe1, that's the ravioli code metaphor I think12:50
fwereaderogpeppe1, hundreds of tiny things with hardly any meat in12:50
fwereaderogpeppe1, long-term, it's *much* easier to deal with than spaghetti is12:51
fwereaderogpeppe1, possibly this is just because all the boundaries make it harder to spaghettify12:51
fwereaderogpeppe1, so after N years of maintenance there's still *some* structure discernible12:52
rogpeppe1fwereade: i think it just makes for spaghetti at another level12:52
rogpeppe1fwereade: for me, the key thing is strong, useful abstractions12:52
rogpeppe1fwereade: then you can understand why a package exists and how you might change it to do what you want without compromising external clients.12:53
rogpeppe1fwereade: in all honesty, i don't think we suffer much from violation of intra-package boundaries in juju. the most egregious violations of boundaries that i've seen have been cross-package.12:56
bodie_if I could get a LGTM on this, it would be fantastic! https://github.com/juju/charm/pull/413:01
bodie_also, good morning all13:01
mattywhas anyone ever seen the 'Pull Request is not mergeable' error from the github-lander?13:09
mattywI can't see a reason why the request isn't landable13:09
rogpeppe1anyway, i strongly disagree that if it's worth testing a function, it's worth exporting it from another package. a package is often built from quite a few functions that make sense only within the context of that package and operate on types and concepts that are really internal implementation details. an example is desiredPeerGroup in worker/peergrouper. it's very useful to be able to test it directly, but it only makes sense in the13:09
rogpeppe1context of what the rest of the package is doing.13:09
fwereaderogpeppe1, I'm not saying internal tests are *never* a good idea13:10
rogpeppe1[13:37:39] <fwereade> rogpeppe1, if you need internal testing, your packages are probably too big ;)13:11
natefinchit's often way way WAY easier to test a single internal function than it is to test a huge external function.   A failure from a test against a very small internal function is a hell of a lot easier to understand than a failure from a huge external function.13:11
fwereaderogpeppe1, I would say they *are* a code smell, though -- an indication that something's probably a bit off13:11
fwereaderogpeppe1, closer inspection may reveal that everything's really fine13:11
rogpeppe1fwereade: the only thing that's "off" about them is IMO is that it makes it harder to refactor the code because your tests are dependant on internal implementation details to some extent.13:12
natefinchfwereade: I disagree completely.  There are lots of individual bits of logic that you can test in isolation which can help you when you're editing/refactoring later.13:12
fwereadenatefinch, yeah, agreed there -- I'd just usually rather put a couple of helper packages behind the outward-facing one, and have properly segregated tests against theose13:12
rogpeppe1fwereade: but that's not made any better by factoring out the code13:12
rogpeppe1fwereade: because they're implementation details whether they're in a separate package or not13:13
natefinchfwereade: except that then you have exposed logic that should only be an implementation detail, but now other developers will be tempted to use it13:13
fwereadenatefinch, you've written a package which is the language in which another package is written13:13
fwereadenatefinch, ratherthan embedding the implementation language in the implementation13:13
rogpeppe1fwereade: that makes sense *if* that language is coherent in itself13:14
fwereaderogpeppe1, and so it should be..?13:14
rogpeppe1fwereade: but if it's just an implementation detail, it often makes sense only in the package that's doing the implementation13:15
natefinchfwereade: it just breaks encapsulation... and many times, the implementation details are not something you want anyone else to get access to, so you can freely refactor later.  If they're exposed, then you have to worry about maintaining compatibility with other packages that use them13:15
rogpeppe1natefinch: fwereade is concerned about inappropriate access to parts of that implementation from within the package.13:15
fwereadenatefinch, I'd rather take the risk code-sharing than giant overgrown packages ;p13:15
fwereadenatefinch, yes, it demands a bit of extra effort to build the layers in the right places13:16
rogpeppe1natefinch: and i can see that p.o.v. too - for instance (to continue with worker/peergrouper) some other part of the package *could* call one of the other functions inside desired.go.13:16
fwereadenatefinch, but IME the more clients you have the better-factored the code generally is, but the causal relationship is not entirely clear13:17
rogpeppe1natefinch: but tbh i don't see that as a significant risk13:17
bodie_at the last place I worked, we used to have 4-hour yelling arguments in the conference room.  I much prefer this style ^_^13:17
fwereadebodie_, haha13:17
wwitzel3my love of corkscrew pasta has doomed me to always writing poor code.13:18
fwereadewwitzel3, lol13:18
rogpeppe1lol13:18
rogpeppe1using Go packages like java name spaces is not a great fit IMO.13:20
fwereadenatefinch, rogpeppe1: anyway, I'm worried that I'm not making myself clear -- I understand the risks you cite, I'm just saying that I'd rather [err towards small packages and risk inappropriate reuse of incomplete abstractions] than [err towards large packages and risk spaghetti behind clean, but large, boundaries] because I have generally found it easier to unfuck systems that made the former mistake13:21
fwereadenatefinch, rogpeppe1: I'm certainly not saying my approach can't go wrong13:22
rogpeppe1personally i'd prefer to err towards packages with small boundaries that encapsulate useful functionality, even if that internal implementation might get big sometimes13:22
fwereadenatefinch, rogpeppe1: everything goes wrong13:22
fwereaderogpeppe1, natefinch: https://twitter.com/DEVOPS_BORAT/status/22417183277173964813:23
rogpeppe1i think one can easily have a package with a significantly sized implementation that nonetheless has a small API13:23
bodie_fwereade, on that note, I now have a reasonably good JSON-Schema validator and YAML grinder-into-suitable-format and I'm thinking about how it might be useful for charm Config13:23
fwereadebodie_, I heartily endorse that direction, but not at the moment :)13:24
bodie_right now, those things are members of charm Actions, the grinder is an unexported method13:24
rogpeppe1many things in the stdlib are a good example of that13:24
bodie_was thinking about whether that stuff needs to be its own bit in Charm, or where it would belong13:24
gnuoyHi, I'm having my first go at compiling juju (1.18) from src. After "go get -v launchpad.net/juju-core/1.18/..." I'm moving the content of the 1.18 dir up one into juju-core otherwise go install seems unable to find anything. After doing that install fails due to missing src/github.com/juju/testing/logging, so I grabbed those files from old revision. After doing that install fails with "utils/ssh/ssh_gocrypto.go:77: undefined: ssh.ClientConn". I can't13:25
gnuoyhelp but feel I'm doing something fundamentally wrong here13:25
fwereadebodie_, I would personally consider each of those pieces to be a viable small package13:25
bodie_gnuoy, I think what you want to do is rm -rf launchpad.net/juju-core and go get -u -v github.com/juju/juju/...13:26
bodie_it's moved to github :)13:26
fwereadegnuoy, if you need 1.18, moving the branch up a level is right; but you need to run `godeps -u dependencies.tsv` as well to get the right packages13:26
bodie_although, I'm not totally certain where to go for the versioned historic branches13:26
bodie_pay attention to fwereade13:27
gnuoybodie_, achk :)13:27
gnuoyfwereade, thanks13:27
fwereadegnuoy, most of what you need to know should be in CONTRIBUTING and README13:28
mattywmgz, ping?13:28
gnuoyfwereade, ok, thank you13:28
bodie_fwereade, on a side note, I've updated some code in my gojsonschema dependency -- does that mean I need to push out a tweak to dependencies.tsv and tell people they need to re-up on their deps?13:28
mgzmattyw: hey13:29
fwereadebodie_, yeah, although it probably doesn't demand a specific message, running godeps is just a habit one needs to develop, I think13:29
mgzgnuoy: you just need to run godeps on the branch, which is in CONTRIBUTING13:32
mgzgnuoy: 1.18 is on launchpad still13:32
gnuoymgz, ta13:33
mattywmgz, for the purposes of this conversation you're responsible for github and jenkins - ok?13:33
mgzmattyw: okay :)13:34
mattywmgz, any idea what the pull request not mergable error is all about? http://juju-ci.vapour.ws:8080/job/github-merge-juju/138/console13:34
mattywmgz, only it seems mergable to me - it's only a trivial change to the readme13:34
mgzmattyw: as best I can tell it's the github api being weird, just retrying often seems to work13:34
mattywmgz, I'll try again - thanks :)13:35
mgzmattyw: the only other thing to check is if it merges cleanly into trunk, but I expect it does13:35
mattywmgz, just in case my git foo isn't good enough - I did a git merge master on my local branch - and master is up to date with trunk - and that all worked13:36
bodie_mgz / fwereade -- so, I have some new code in juju/charm that has an updated dependency requirement, but I noticed the dependencies.tsv is in juju/juju13:36
bodie_I just became aware that that could pose a problem13:37
mgzrogpeppe1: ^13:37
mgzwhat's our solution to new subpackages that have dep requirements?13:37
bodie_I'm guessing I just need to put the updated dep in juju/juju13:37
bodie_but, it seems a little clunky13:37
bodie_then again, dep management in go seems... a little clunky ;)13:38
mgzmattyw: the bot did successfully merge your branhc into tip, so it's an ongoing mystery why github is failing with that error sometimes13:38
voidspaceperrito666: how far off merging is your branch?13:38
rogpeppe1bodie_: yeah, in general only main specifies deps13:38
voidspaceperrito666: I think mine (well - the first of mine) is ready to be integrated with yours13:38
rogpeppe1bodie_: otherwise you can get conflicting deps13:38
bodie_ah13:39
bodie_that makes sense13:39
rogpeppe1bodie_: the real solution to this particular problem is to export stable APIs13:39
mattywmgz, I can't see it in tip13:39
mattywmgz, and https://github.com/juju/juju the readme is still old13:39
rogpeppe1bodie_: then you can go get -u with impunity13:39
mgzmattyw: I'm not saying it landed13:39
mattywmgz, oh right13:40
mgzI'm saying the *bot* managed to do the merge in order to run the tests, so the merge itself shouldn't be the reason github failed the merge itself at the end13:40
rogpeppe1bodie_: that's something we should really do for all our packages under github.com/juju, perhaps excepting juju core itself13:40
mattywmgz, nice, well I tried it again, 3rd time's the charm I guess13:40
mgzone option if this continues to be a pain is to drop using the github api here and get the bot to commit/push/etc13:40
bodie_rogpeppe1, not sure I'm following -- I made a tweak to gojsonschema to get a subset of functionality working properly, so the API I'm exporting definitely depends on that fix13:41
bodie_now I'm worrying about whether there will be more such cases13:41
bodie_obviously I don't want to keep forcing dep updates13:41
rogpeppe1bodie_: it sounds like this is basically a bug fix to gojsonschema, right?13:42
rogpeppe1bodie_: not a backwardly incompatible change13:42
bodie_yeah, but tests won't pass on charm without the fix13:43
bodie_so... not totally backward compatible with our test base13:44
bodie_s/totally/13:44
bodie_ //13:44
rogpeppe1bodie_: that's fine. go get -u will fix it, and because it's backwardly compatible, won't break anything else that's importing gojsonschema13:44
rogpeppe1bodie_: in general, we should test against the latest version of a package13:45
gnuoymgz, assuming the "no version control" messages are just warning the dep process completed without error http://paste.ubuntu.com/7638931/ but the undefined: ssh.ClientConn error is still present when trying to do an install13:46
bodie_rogpeppe1, okay, so I can basically update the dependency willy-nilly as long as I'm not changing the API significantly / in such a way that builds will fail?13:46
rogpeppe1bodie_: yup13:46
mgzgnuoy: you're doing it the wrong way round :)13:46
bodie_rogpeppe1, but, shouldn't that be in dependencies.tsv?13:46
mgzyou *have* a dependencies.tsv in the juju-core branch, you want to make the other branches match that, which is `godeps -u ...`13:47
mgznot - t13:47
rogpeppe1bodie_: see http://labix.org/gopkg.in for an explanation of how to maintain backward compatibility13:47
gnuoymgz, ah, ok13:47
mgzso, un-overwrite the deps, and just do the -u13:47
rogpeppe1bodie_: if every repo has its own dependencies.tsv, which one do you believe?13:48
bodie_heh13:48
mgzyou probably still need a go get first, use the -d flag on that13:48
rogpeppe1bodie_: IMO, the only decent place to specify dependencies is at the root (i.e. in main packages)13:48
perrito666voidspace: working a re-reproposal13:49
mgzgnuoy: so, `cd juju-core && go get -v -d ./... && godeps -u dependencies.tsv`13:49
voidspaceperrito666: heh, you'll be happy to finally get this in!13:50
gnuoytbh it looked to me like go get was doing all the dependency handling13:50
bodie_rogpeppe1, good link re. gopkg :)13:50
rogpeppe1bodie_: and if you combine that with maintaining stable APIs, we should find that a) all dependencies should build and test ok against (at least) the latest version of their deps and b) we can have reproducible builds of our binaries13:50
gnuoymgz, all compiled and happy, thank yo13:57
* fwereade needs to be away, on later tonight13:58
mgzgnuoy: ace14:00
voidspaceericsnow: this is the initial prototype of the api client Backup method14:14
voidspaceericsnow: https://github.com/voidspace/juju/compare/backup-client#diff-e3e783960401dd1c43cf368383b4992eR57714:14
ericsnowvoidspace: yeah, plugging that into the CLI shouldn't be a problem14:31
voidspaceericsnow: cool14:32
voidspaceericsnow: it occurs to me that I can just leave you to test it... ;-)14:32
voidspaceonly mostly joking...14:33
ericsnowvoidspace: we'll see :)14:33
ericsnowvoidspace: such a kidder14:33
rogpeppe1frankban, dimitern_, fwereade, voidspace, mgz, perrito666: factor cmd package out of juju-core: https://github.com/juju/juju/pull/9314:36
dimitern_rogpeppe1, why?!14:36
rogpeppe1dimitern_: because we want stuff outside of juju-core to be able to use it14:36
rogpeppe1dimitern_: specifically, the store commands are moving out of juju-core too14:37
dimitern_rogpeppe1, oh brother..14:37
dimitern_explosion of deps and repos :)14:37
mgzdimitern_: most of these pacakge splits are so rog can use 'em from elsewhere14:37
rogpeppe1dimitern_: and really, it should not be juju-specific14:37
rogpeppe1dimitern_: i thought you liked external repos :-)14:37
mgzit's quite a lot of short term pain though...14:37
mgzwell, and some ongoing pain, given the non-smoothness of dep management14:38
voidspacefor every new repo I have to create a new mail rule...14:38
rogpeppe1voidspace: can't you create one mail rule that matches all of 'em?14:38
dimitern_rogpeppe1, I'll like them even more when we stop moving stuff around in 5000+ line diffs :)14:38
rogpeppe1dimitern_: most of the diff there is deletion14:38
rogpeppe1dimitern_: i'm sure i remember you arguing that more stuff should be factored out of juju-core, ages ao14:39
rogpeppe1ago14:39
voidspacerogpeppe1: well I could do - so long as I never work with any non-juju git repos14:39
dimitern_rogpeppe1, even the agents are not juju-specific?14:40
voidspacerogpeppe1: and I don't yet, but I don't want to assume I never will14:40
natefinchvoidspace: on your canonical account?  Probably safe assumption14:40
rogpeppe1dimitern_: the agents haven't moved14:40
voidspacenatefinch: I only have one github account14:40
rogpeppe1dimitern_: it's the cmd package only14:40
natefinchvoidspace: you can give it multiple email addresses and address notifications on a per-team basis14:40
rogpeppe1dimitern_: (and cmd/testing)14:40
voidspacenatefinch: ah, I didn't know that14:40
natefinchvoidspace: so my juju notifications all go to canonical, everything else to my gmail14:40
dimitern_rogpeppe1, so there will be juju/cmd repo and juju/juju/cmd/jujud ?14:40
voidspacenatefinch: I will do that immediately - thanks!14:40
natefinchvoidspace: np14:41
voidspaceyep, that's exactly what I want14:41
rogpeppe1dimitern_: yup14:41
* voidspace lunches14:41
dimitern_rogpeppe1, ah, i see - just the root cmd package then14:41
rogpeppe1dimitern_: yup14:41
rogpeppe1dimitern_: which is pretty general tbh (and should probably be more so, but i didn't want to break everything and take ages doing that)14:42
dimitern_rogpeppe1, lgtm14:42
rogpeppe1dimitern_: thanks!14:42
rogpeppe1dimitern_: there were a couple of non-mechanical pieces14:43
rogpeppe1dimitern_: specifically, the new functions NewSuperCommand and NewSubSuperCommand which are now in github.com/juju/juju/cmd14:43
rogpeppe1dimitern_: which wrap cmd.NewSuperCommand in a juju-specific way (to do the expected logging)14:44
frankbanrogpeppe1: LGTM thanks14:45
dimitern_rogpeppe1, right, still lgtm14:48
bodie_odd14:53
bodie_I tagged my repo as .v114:53
bodie_http://gopkg.in/binary132/gojsonschema.v114:53
bodie_when adding it to imports( ... ) as gopkg.in/binary132/gojsonschema.v1, I think gofix is altering it to point to the repo it was forked from14:54
bodie_never mind, I'd had the original repo alongside my forked one14:55
bodie_in $GOPATH14:55
bodie_removed and it works14:55
rogpeppe1bodie_: awesome, good plan14:55
bodie_yeah, gofix was messing with my head though, heh.14:56
bodie_anyway, hopefully this is the last we have to think about gojsonschema for quite some time.14:56
* rogpeppe1 hopes so too14:56
bodie_rogpeppe1, since I now am using the gopkg.in dep, I need to alter dependencies.tsv too, don't I.14:57
rogpeppe1bodie_: yup14:57
bodie_very well then.  I think this is prepared for review other than the addition of the dep.  I'll add that in a separate PR momentarily14:59
bodie_https://github.com/juju/charm/pull/414:59
bodie_rogpeppe1, mgz, fwereade14:59
=== psivaa-afk is now known as psivaa
bodie_might be cool to add tagged releases to github juju15:19
bodie_that way if someone wanted a stable build from source, they could check out and build a specific version15:21
rick_h_we use master vs develop for that :)15:25
rick_h_in UI land15:25
bodie_okay, I'm having some confusion here with the dependencies.tsv stuff15:29
bodie_I made sure I had the latest juju/juju with go get -u -v, and then ran godeps -u dependencies.tsv in juju/juju15:30
bodie_but, it looks like a bunch of my versions are different in my new dependencies.tsv15:30
bodie_er, the one I just generated with godeps -t $(go list github.com/juju/juju/...) > dependencies.tsv.new15:30
bodie_http://www.diffchecker.com/3en2efa815:32
bodie_okay, I didn't have the correct pinned deps due to an issue with the new ones I got.  looks good now.15:34
bodie_THERE we go.15:39
bodie_okay, I need to get this PR in so that I can update dependencies.tsv properly.  can anyone vet this?  https://github.com/juju/charm/pull/415:39
bodie_it's not terribly complicated15:39
bodie_however, juju/juju/dependencies.tsv has an incorrect dep on a dev branch for charm, so I need to get that merged into master in order to update deps correctly.15:40
bodie_fwereade, mgz, rogpeppe1, anyone?  I guess you guys are probably in a meeting15:40
rogpeppe1bodie_: oops15:41
rogpeppe1i wonder how that got merged15:41
bodie_it didn't15:42
bodie_rogpeppe1, I'm building against a dev branch right now15:42
bodie_the master doesn't have my changes yet15:42
bodie_so, my generated dependencies.tsv doesn't have the correct version of charm15:42
bodie_slowly going insane15:43
bodie_lol15:43
rogpeppe1bodie_: i'm not sure exactly what you're asking me to vet15:43
rogpeppe1bodie_: unfortunately github doesn't make it easy to see what changes have been made in response to what comments15:44
bodie_rogpeppe1, basically I nuked most of that PR, fixed a bunch of code, and it should be thought of as a new PR15:44
rogpeppe1bodie_: you rebased?15:45
bodie_rebased, amended commit message15:45
rogpeppe1bodie_: hmm, it would have been nice to have been able to diff from the comments15:46
rogpeppe1bodie_: because now i have no idea what's changed15:46
bodie_rogpeppe1, right....  so, when I alter something that you commented on, the history hides the comment15:47
rogpeppe1bodie_: it doesn't look totally different15:47
rogpeppe1bodie_: i can see the old comments15:47
rogpeppe1bodie_: but i want to find out how the code has changed since i made the comment15:47
bodie_it's not that different, I mostly addressed your issues and beefed up the tests, stripped out some useless tests15:47
rogpeppe1bodie_: so i don't have to continually cross-refer15:47
bodie_hmmmm15:47
bodie_there must be a way to do that15:47
rogpeppe1bodie_: perhaps you could reply to the comments saying which ones you've done or not done15:48
rogpeppe1bodie_: it's something i found indispensable in rietveldt, and i'm a bit at a loss without it15:48
bodie_perhaps the issue is my rebase15:49
bodie_rogpeppe1, basically I addressed the concerns exactly, except for a couple where I left comments15:49
bodie_https://github.com/juju/charm/pull/4#discussion_r1370137115:49
bodie_https://github.com/juju/charm/pull/4#discussion_r1370464415:49
bodie_I don't see why they don't have a feature to address exactly your concern, that does seem really silly15:51
rogpeppe1bodie_: hmm, looks like i never published one comment15:55
natefinchhow github gets new features is really quite a mystery given the very obvious deficiencies they've had for a long long time.15:55
rogpeppe1bodie_: the cleanse function doesn't properly give an error on all bad keys15:56
rogpeppe1bodie_: (AFAICS)15:56
bodie_rogpeppe1, in the case of type map[i{}]i{}, it just coerces the keys to strings and then runs through it again in the m[s]i{} case15:57
rogpeppe1bodie_: ah, i missed that16:02
rogpeppe1bodie_: BTW if prohibitedSchemaKeys was a map[string]bool (or a set.Strings) then the lookup would not require a loop16:04
bodie_ah, good point rogpeppe116:09
rogpeppe1godeps: cannot parse "/mnt/jenkinshome/jobs/github-merge-juju/workspace/tmp.gduuL94JJe/RELEASE/src/github.com/juju/juju/dependencies.tsv": cannot find directory for "github.com/binary132/gojsonpointer": not found in GOPATH16:12
rogpeppe1mgz: doesn't the 'bot automatically get new deps?16:12
rogpeppe1mgz: if not, then how can i fix it (FWIW, i don't *think* i added that dependency)16:13
mgzrogpeppe1: it does, and that's not a new dep16:13
rogpeppe1hmm, weird16:13
mgzis it not actually imported in the branch?16:13
mgzwe still rely of go get for fetch16:13
bodie_rogpeppe1, I'm actually going to revert the gopkg changes16:14
rogpeppe1mgz: ah...16:14
bodie_I think mgz had some good points that we're already pinning to a version via dependencies.tsv16:14
rogpeppe1mgz: it's quite possible we don't have that as a dep any more16:14
rogpeppe1mgz: hmm, no, we must do16:15
mgzrogpeppe1: I think it's just gopkg weirdness we don't need to deal with for now16:16
rogpeppe1mgz: gopkg?16:16
mgzrogpeppe1: gopkg.in16:17
rogpeppe1mgz: oh, i see16:17
rogpeppe1pwd16:18
rogpeppe1bodie_: but the branch i'm committing doesn't have any gopkg.in deps16:18
bodie_rogpeppe1, come again?16:19
bodie_rogpeppe1, I'm going to push a couple of tweaks to the PR, so we don't have to deal with this gopkg stuff right now16:19
rogpeppe1bodie_: ok, fair enough.16:20
rogpeppe1bodie_: sorry, i thought you were linking it to my 'bot failure16:20
mgzrogpeppe1: it is related16:22
mgzsee the go get higher up in the console log16:22
mgzhaving tip of gojsonschema pointing at gopkg.in borked us16:22
mgzI'll requeue the proposals when that gets sorted16:23
mgzbodie_: poke me when we're back to normality please :)16:23
rogpeppe1ah, i get it16:23
rogpeppe1we to go get, which has no dependencies; then we update the deps, and that means we really need to go get again16:24
wwitzel3fwereade: updated https://github.com/juju/juju/pull/2 when you have a moment to review. thanks.16:28
bodie_rogpeppe1, mgz https://github.com/juju/charm/pull/4 ready16:36
rogpeppe1bodie_: thanks for not rebasing :-)16:37
rogpeppe1bodie_: i think a load of my comments must've got lost in the ether16:45
rogpeppe1bodie_: did you see a comment about var swap = unmarshaledActions.ActionSpecs[name]16:46
rogpeppe1?16:46
rogpeppe1bodie_: i suggested: spec := unmarshaledActions.ActionSpecs[name]16:46
rogpeppe1bodie_: because nothing is being actually swapped16:46
mgzrogpeppe1: there's a possibly valid error on your 010 merge16:46
rogpeppe1mgz: link?16:47
mgzjob 144, should be on the pr16:47
mgz(sorry, can't easily paste url into irc)16:47
bodie_rogpeppe1, one moment, looking16:48
rogpeppe1mgz: ok, looking16:48
rogpeppe1mgz: ah, perhaps i haven't pushed the latest version of cmd16:49
* rogpeppe1 juggles dependencies16:49
bodie_rogpeppe1, https://github.com/binary132/charm/blob/actions-validate/actions.go#L77-L9416:50
bodie_I'm not seeing the problem here16:51
rogpeppe1bodie_: the "swap" variable isn't actually swapping anything. it's just a temporary mutable store for the struct.16:51
rogpeppe1bodie_: it confused me for a few moments16:51
bodie_ah, so just the naming16:51
rogpeppe1yeah16:51
mgzyeah, I'd generally name those kinds of things temp16:52
bodie_that syntax is a little kludgy, imo16:52
mgz(or more commonly, tempSomething)16:52
rogpeppe1bodie_: also, we can do this later, but i'm inclined to think that bundling the bad-key checking inside the cleanse function isn't great16:52
bodie_I get why it's there, since map resolution happens at runtime, and things need to be good at compile time16:52
rogpeppe1bodie_: why what's there?16:53
rogpeppe1i generally name it after the thing that it is16:53
rogpeppe1in this case "spec" should work fine16:53
bodie_er, the need to use a temp stuct16:53
bodie_struct*16:53
bodie_rather than assigning to the struct member via map resolution ActionSpecs[name]16:53
rogpeppe1bodie_: there have been murmurings about allowing the assignment of fields inside by-value structs in maps16:53
rogpeppe1bodie_: it's a slightly awkward language spec change to make though16:54
rogpeppe1bodie_: as values in maps are deliberately not addressable16:54
bodie_there we go16:54
rogpeppe1bodie_: and that's currently the rule for allowing mutation of struct members16:54
bodie_new bit up16:54
bodie_I think it's nice that Go is oriented towards usefulness without total depth of knowledge ;)16:55
natefinchbodie_: I like that total depth of knowledge is not too hard to obtain16:56
natefinch(especially compared to other languages)16:56
bodie_:)16:56
bodie_I always find myself comparing everything to C in my head16:56
bodie_it doesn't get much simpler than that, really16:57
natefinchbodie_: there's a lot of memory management stuff that you need to know about C and using pointers as arrays & strings16:57
bodie_you mean, like chunking for the cpu and caching behavior, heap vs stack?16:58
natefinchlike malloc etc16:58
bodie_yeah, the mechanics can be kind of klutzy, for sure, especially starting out16:59
natefinchoh yeah, and data structures getting created with completely random data... that's an awesome feature16:59
bodie_makes perfect sense, though16:59
natefinchlots of foot guns :)16:59
bodie_what's really fun is when you manage to mangle your stored code, and things start blowing up in weird ways that are impossible to debug "the normal way"17:00
bodie_because you're actually altering your program's behavior in unexpected, unpredictable ways....17:01
bodie_wheee17:01
bodie_rogpeppe1 / mgz is that pr good to land?17:01
mgzbodie_: is from my perspective17:02
bodie_I changed the name of the variable, I'd like to get this dependency stuff over with so I can fix the deps.tsv stuff and get on with my life (heh)17:02
rogpeppe1bodie_: yeah, land it. i might fix up a couple of trivials later.17:02
bodie_rogpeppe1, it's meant to be landed with $$merge$$ .... right?17:08
rogpeppe1bodie_: no, just click merge17:09
bodie_ah17:09
rogpeppe1bodie_: we haven't got CI set up yet on any of the external repos17:09
rogpeppe1bodie_: make sure that the tests pass tho'!17:09
bodie_:)17:10
bodie_I'm not seeing a merge button.17:11
bodie_I'm sure that now that I've said so, it will become immediately obvious17:12
bodie_probably just need to rebase on master17:14
bodie_nope17:15
bodie_rogpeppe1, I'm sorry for all the constant interrupt polling I'm doing here, do I just not have permissions or what's going on here?  I just want to get this finished.17:17
rogpeppe1bodie_: hmm, you *should* have perms, assuming you're a member of juju-hackers. i'll just check.17:18
bodie_I am a member17:18
bodie_I think...?17:18
jcw4And you're marked public I think bodie_ right?17:18
bodie_I'm a member of juju17:18
rogpeppe1bodie_: ah, try again17:18
rogpeppe1bodie_: i hadn't added juju-hackers to the collabs17:18
bodie_I see17:19
bodie_well, that's nice to know, at least I wasn't doing something daft17:19
bodie_and merged.17:19
bodie_thank god.17:19
rogpeppe1yay, merged!17:22
* rogpeppe1 high fives bodie_17:22
bodie_this doesn't look right17:35
bodie_https://github.com/binary132/juju/compare/dependencies-charm-gojsonschema?expand=117:35
bodie_ugh17:35
bodie_okay, fixing17:35
bodie_https://github.com/juju/juju/pull/9617:40
bodie_rogpeppe1, mgz.... that should be all17:41
* rogpeppe1 is done for the day17:47
voidspaceg'night folks18:03
voidspaceEOW18:03
voidspacesee you all on Monday18:03
ericsnowvoidspace: have a nice one18:04
natefinchsee ya voidspace18:04
perrito666btw, adding this to your ~/.gitconfig might easy your lives http://pastebin.ubuntu.com/7640097/18:08
jcw4perrito666: which part in particular, or all of it?18:09
natefinchperrito666: looks like there's some duplicate data in there18:10
natefinchlike color for status18:10
perrito666natefinch: might be some dupes I have ben tweaking it for some time18:10
jcw4perrito666: I see now.. I just saw a wall of text and didn't read enough to see it was all related to color coding git status info18:10
perrito666jcw4: I get colors on status, a few useful shortcuts18:10
perrito666jcw4: yup, basically it all makes to display git data into better ways18:11
natefinchit's funny, my biggest gripe with git is that git difftool won't open more than one diff at a time.  I like being able to go through all the diffs in different tabs and see how they relate to one another18:11
perrito666jcw4: looking it more in depth, I might have two files pasted there :p I just over cated18:12
natefinchheh18:12
jcw4:)18:15
perrito666but that makes very clear the output of my git :) just needs a bit of cleaning18:18
perrito666jcw4: natefinch http://pastebin.ubuntu.com/7640139/18:22
perrito666there better18:22
jcw4sweet18:22
perrito666I find the aliases at the bottom to be specially life improving18:23
perrito666as you can see some of those come from when I used svn :p18:24
natefinchheh... yeah... I wish all the vcs people could just agree on a naming scheme. It would make things so much simpler18:27
jcw4perrito666: I like git aliases too...http://paste.ubuntu.com/7640151/18:27
perrito666natefinch: oh git agrees on the naming, they just have the commands do different stuff18:28
natefinchhaha, exactly18:29
bodie_so, any chance I could get that dep update landed?  do I need a LGTM for this?18:38
bodie_https://github.com/juju/juju/pull/9618:38
natefinchwyou always need a LGTM :)18:39
natefinchnot that I can tell at all if you screwed up or not ;)18:40
bodie_hehe, yep...  seemed like a pretty simple thing to get landed18:40
jcw4bodie_: LGTM18:40
jcw4;018:40
jcw4;) even18:40
bodie_I'm really anxious to move on to the action stuff...18:40
natefinchjcw4: you gotta do it for realz in the PR, so it's recorded that it's your head on the line if there's a problem :)18:40
jcw4er... umm18:41
jcw4well...18:41
bodie_what, natefinch, you can't eyeball the difference between 1be916ca1fee152004f27cf83df96b130411ea9f and dbe10f58fcb80669aa3972574f51d92fdd20ee47?18:41
jcw4my head is now on the line18:41
bodie_from a certain perspective, it's just moved from one line to another18:41
natefinchyou can actually just go look at the ids of the heads of those repos and eyeball that they're the same as in the dependencies file18:42
natefinchLGTM to me too18:43
bodie_thanks gents18:43
bodie_and this is where I need to do the $$merge$$ bit?18:43
jcw4bodie_: yep $$merge$$18:45
perrito666mm, just the kind of mail to get from curtis a friday afternoon18:48
natefinchahh, the weekly "everything is totally screwed up!" email :/18:50
jcw4what does PTAL mean?19:04
natefinchplease take a look19:04
jcw4ah... thanks!19:04
natefinchtook me a while to figure that one out too19:04
jcw4I got SGTM, LGTM, WTF (j/k) but I couldn't get that one19:05
lazyPowerhave you guys had a chance to look at https://bugs.launchpad.net/ubuntu/+source/juju-core/+bug/132942919:06
_mup_Bug #1329429: local bootstrap fails <amd64> <apport-bug> <trusty> <juju-core (Ubuntu):New> <https://launchpad.net/bugs/1329429>19:06
lazyPoweri'm seeing more users show up with this null pointer dereference problem, so far about 319:06
lazyPowerhowever, the user being affected by this particular issue is here now, ali1234  is the filer of 132942919:09
natefinchlazyPower: you shouldn't need to sudo for bootstrap... in fact that may mess things up (obviously a nil pointer panic is not really an acceptable handling of that)19:09
ali1234hi19:09
lazyPowernatefinch: is that the root of the issue, using sudo?19:09
lazyPowerlet me update that AU answer - since juju has evolved beyond requiring sudo to run the bootstrap for local.19:09
natefinchI don't know.  juju bootstrap with local will work without sudo (if it needs sudo, it'll prompt you for creds)19:10
natefinchso... it may need sudo, but running it preemptively with sudo is not necessary nor recommended19:10
lazyPowernatefinch: good point. I've got an edit filed for that AU answer, i didn't realize it wasp ointing to still using sudo19:11
lazyPowerthat's been corrected for a little over what, 4 months now?19:11
natefinchsomething like that yeah19:12
natefinchwe wanted it to be completely sudo-less but there were some userspace-lxc things that I think either didn't make it in or weren't available on all platforms19:13
lazyPowerali1234: when you run juju bootstrap without prefixing with sudo, do you still get the nil pointer problem?19:14
ali1234yes19:14
ali1234~/.juju/environments/ is owned by root, this probably doesn't help19:15
natefinchI figured that would be too much to hope for19:15
ali1234i only get the null pointer problem when running without sudo19:16
ali1234i don't get it when running with sudo, i get a different error telling me not to use sudo :)19:16
natefinchoh yeah, sorry, I see that now in the bug description. well, it's good to get the documentation correct anyway19:17
ali1234yeah. i expect if i hadn't made that mistake i wouldn't be in this situation19:17
natefinchI'm trying to dredge up the 1.18.1 code so I can make sure I'm looking at the right code for the tracebacks in the bug19:21
lazyPowerThanks for taking a look natefinch19:22
ali1234~/.juju/environments/ was empty so i deleted it... now i don't get the error19:24
natefinchhuh19:24
natefinchlike.... bootstrap worked?19:24
ali1234yes19:24
ali1234it recreated the dir and now i have a local.jenv inside it19:25
natefinchI wonder if that got created with root credentials and then it couldn't be written to19:25
ali1234yes, that is exactly what happened19:25
natefinchI think we've seen that.... I think we fixed it... there was another bug for it, let me check19:25
natefinchHave you tried this with 1.18.4?19:29
ali1234no19:30
ali1234i just wanted to try out juju, i have never used it before19:30
ali1234i have no idea how to do that19:30
natefinchapologies for such a bad first impression19:31
ali1234np, i've seen worse19:31
jcw4[miracle max's wife]:  you think it'll work?19:31
jcw4[miracle max]:  it'll take a miracle19:32
ali1234so my question now is are there going to be more root owned files? where does it keep all the local stuff?19:34
natefinchThere should not be any root owned files.  We only need root for local environments because we start lxc containers.19:34
natefinchthe local stuff is all kept under $HOME/.juju/19:34
ali1234yeah, where are those containers stored? in ~/.juju?19:34
ali1234~/.juju/local/ has a load of root owned files19:35
natefinchali1234: the containers are stored in the standard lxc place, I believe, /var/lib/lxc?  something like that.19:35
ali1234/var/lib/lxc/ is empty19:36
ali1234should i just nuke ~/.juju and start again?19:36
natefinchali1234: that's probably not a bad idea.  I was wrong, it looks like there are some root owned files in .juju, at least under the .juju/local/ directory19:37
ali1234now i get "ERROR cannot use 37017 as state port, already in use"19:43
ali1234apparently mongodb is running on that port19:44
natefinchali1234: so the mongo server from the first bootstrap is evidently still running.  Assuming you're not running mongod on your local box for anything else, you can just do sudo killall mongod19:44
ali1234start: Job is already running: juju-agent-al-local Bootstrap failed, destroying environment19:45
natefinchtry running    juju destroy-environment local --force19:46
ali1234ERROR open /home/al/.juju/environments.yaml: no such file or directory19:46
natefinchheh, oops, ok, so we've been blowing stuff away so it's all confused19:47
ali1234i did init, switch, destroy and that seems to have worked19:47
natefinchahh, cool19:47
ali1234bootstrap appears to have worked19:47
ali1234i was under the impression it had to download a lot of stuff, but it ran very quickly19:48
natefinchthe local provider is super quick19:54
natefinchgreat for demos and proof of concepts19:54
ali1234so the askubuntu page says "The first time this runs it might take a bit, as it's doing a netinstall  for the container, it's around a 300 megabyte download. Subsequent  bootstraps should be much quicker. "19:57
ali1234it definitely didn't download 300mb...19:57
ali1234looks like it's working now, thanks!20:08
=== Ursinha is now known as Ursinha-afk
=== Ursinha-afk is now known as Ursinha
bodie_does anyone happen to know much about juju run?20:51
bodie_I'm trying to figure out where to look for the bits where it builds the hook context20:51
natefinchbodie_: thumper wrote that, but it's saturday where he is20:51
bodie_:)20:51
bodie_on that note, if fwereade sees this -- https://github.com/juju/juju/blob/master/doc/charms-in-action.txt#L80-L9220:55
bodie_I think this content is outdated, but I want to be sure20:55
=== Ursinha is now known as Ursinha-afk
perrito666ok, EOD | EOW for me, cheers21:32
=== Ursinha-afk is now known as Ursinha

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