jcastro | cjohnston, man, thanks for that | 00:26 |
---|---|---|
cjohnston | :-) | 00:27 |
menn0 | review please: https://github.com/juju/juju/pull/51 | 00:29 |
menn0 | really keen to get this one landed | 00:30 |
waigani | thumper: I'm writing the cmd branch, is there an example of mocking out the server api that you can point me to? | 01:20 |
thumper | debug-log mocks out the api | 01:20 |
waigani | thumper: cheers | 01:21 |
thumper | menn0: just one comment on the PR | 01:22 |
menn0 | thumper: thanks. I'll add some comments along those lines. | 01:24 |
thumper | menn0: the guts of the problem is that they looked like they weren't needed, and if they are for some reason, there should be a comment explaining why | 01:25 |
wallyworld | axw: morning. do you know if any of the recent mongo related session fixes may address bug 1305014? | 01:26 |
_mup_ | Bug #1305014: panic: Session already closed in TestManageEnviron <intermittent-failure> <test-failure> <juju-core:Triaged by rogpeppe> <https://launchpad.net/bugs/1305014> | 01:26 |
axw | wallyworld: don't think so | 01:26 |
menn0 | thumper: understood | 01:26 |
wallyworld | ok, a | 01:27 |
wallyworld | ta | 01:27 |
axw | still seeing them in CI | 01:27 |
menn0 | thumper: if API versioning lands soon then we might be able to avoid these backwards compatibility shenanigans altogether | 01:28 |
* thumper nods | 01:29 | |
davecheney | thumper: https://github.com/juju/names/pull/2 | 01:36 |
thumper | davecheney: hmm, weren't we going to squish commits before proposing? | 01:42 |
thumper | axw, wallyworld: was there a definitive answer? | 01:42 |
thumper | also, can someone tell me how to do it again? | 01:42 |
wallyworld | thumper: sort of optional, devs call | 01:42 |
wallyworld | rebase -i | 01:43 |
davecheney | thumper: i have no idea | 01:43 |
davecheney | is this documented ? | 01:43 |
wallyworld | iow, i don't think there's necessarily definitive agreement | 01:43 |
wallyworld | yes, in the contributing doc | 01:43 |
axw | thumper davecheney: https://github.com/juju/juju/blob/master/CONTRIBUTING.md#proposing | 01:44 |
davecheney | which one do I pick ? | 01:44 |
davecheney | seriously | 01:44 |
davecheney | i don't care about this rebase stuff | 01:44 |
davecheney | i opt to not do it | 01:44 |
wallyworld | there's no one correct answer | 01:45 |
davecheney | % git rebase -i --autosquash master | 01:45 |
davecheney | Successfully rebased and updated refs/heads/103-introduce-tags-type. | 01:45 |
davecheney | oh, i said don't do it an it did it anyway | 01:45 |
davecheney | cest la vie | 01:45 |
wallyworld | some people like to rebase stuff out and throw away history, others don't | 01:45 |
axw | use "fixup" things like "go fmt" | 01:45 |
wallyworld | i din't think we could get agreement on the ist | 01:45 |
davecheney | i'm in the latter category | 01:46 |
wallyworld | me too | 01:46 |
wallyworld | bzr makes it so you do't need to care | 01:46 |
bodie_ | thumper, it started making more sense to me when I realized rebase is actually for replaying your commits on top of another branch (such as master) after pulling latest upstream content | 01:46 |
wallyworld | github's lack of support for pipelines really makes me sad | 01:46 |
wallyworld | totally screws up how i work | 01:46 |
thumper | bodie_: sure, with bzr I just used to merge trunk | 01:46 |
thumper | the git way seems to be rebase | 01:46 |
wallyworld | so now i can't propose more than 1 branch at a time | 01:47 |
wallyworld | so have all this stuff left pending | 01:47 |
bodie_ | so, when you rebase, if you pass the -i (interactive) flag, it will also allow you to condense your noisy fix commits into a few more meaningful commits, for git log purposes | 01:47 |
bodie_ | as well as replaying them on top of master | 01:47 |
wallyworld | thumper: git way = throw away history, no thanks :-( | 01:47 |
davecheney | bodie_: so you have to have a commit, at the bottom of the list | 01:47 |
wallyworld | one person's noise is another person's history | 01:47 |
davecheney | which has the description that you want ? | 01:47 |
davecheney | then you chuck all the others ? | 01:48 |
bodie_ | not necessarily at the bottom, it will just take your commit history that deviates from master out of the way, then apply master's latest changes, and stick your changes at the end | 01:48 |
davecheney | that sounds insane | 01:48 |
wallyworld | yep :-( | 01:49 |
wallyworld | welcome to the New World | 01:49 |
axw | davecheney: the rebase commands describe what they do. some discard messages, others squash them together, others you can amend | 01:49 |
bodie_ | I think it has to do with the way Git thinks of history chronologically. that way, your commits appear when you apply them, instead of scattered through a pack of other people's commits | 01:49 |
bodie_ | also, condensed into a meaningful log entry or two | 01:49 |
bodie_ | s/apply/rebased/ | 01:50 |
davecheney | i don't understand | 01:50 |
davecheney | if I want to 'condense' something | 01:50 |
davecheney | i'll just take a diff | 01:50 |
davecheney | make a new branch | 01:50 |
davecheney | apply it and propose | 01:50 |
rick_h_ | menn0: ty for the pull request, landed | 01:50 |
davecheney | this whole rebase nonsense sounds like a solution that was looking for a problem | 01:50 |
bodie_ | that's one way | 01:50 |
bodie_ | that would accomplish basically the same thing, I suppose | 01:50 |
davecheney | thumper: ty | 01:50 |
thumper | np | 01:51 |
bodie_ | if you're a visual person, http://git-scm.com/book/en/Git-Branching-Rebasing has some really good diagrams that exlpain it | 01:51 |
davecheney | big branch coming on juju/juju | 01:51 |
bodie_ | explain* | 01:51 |
rick_h_ | davecheney: but you can reorder commits, perform work across commits, remove them. imo nice for a lot of things once you get used to it | 01:51 |
bodie_ | ^ | 01:51 |
davecheney | rick_h_: why do I need that | 01:51 |
bodie_ | the --interactive stuff can get interesting | 01:51 |
davecheney | reorder commits ? | 01:51 |
davecheney | change source code, but not inside an editor ? | 01:51 |
rick_h_ | but I went from bzr -> hg -> git and have been using it outside for a couple of years so maybe I've learned some brain damage :P | 01:52 |
davecheney | maybe juggle some chainsaws blindfolded at the same time for extra dexterity points | 01:52 |
bodie_ | http://git-scm.com/figures/18333fig0339-tn.png speaking of juggling chainsaws | 01:52 |
davecheney | this all sounds like a list of things you "could" do, but not a list of things that you "should" do | 01:52 |
rick_h_ | davecheney: you're not willing to admit that perhaps the rest of the world might know a little something and the 'different' is biting some devs around here a bit? :P | 01:52 |
davecheney | rick_h_: i'm sure it's my closed minded attitude | 01:53 |
axw | rick_h_: do you have any suggestions for people used to bzr pipelines? | 01:53 |
bodie_ | the thing with Git is, if you just merge over and over, your log history turns into a giant nightmarish sea of mini forks and chronologically meaningless noise | 01:53 |
rick_h_ | not enough haskell in the script for you? :) | 01:53 |
davecheney | but I've never come across any reason do any of the things that bodie_ proposed | 01:53 |
rick_h_ | axw: I never got into pipelines. I'd just do branches off branches and use cherry-pick to move commits among the branches of work | 01:53 |
axw | mk | 01:54 |
bodie_ | I bet if you brought it up with the folks in #git they'd be thrilled to have some cultural challenge | 01:54 |
bodie_ | ;) | 01:54 |
bodie_ | it's actually a strangely kind arena | 01:54 |
wallyworld | rick_h_: that sounds error prone and a pain inthe arse | 01:54 |
rick_h_ | davecheney: oh, the 90% use case for me is just 'feature branch...work work...commit so I don't lose crap...work work...rebase to make it all clean with nicer commit messages, pull request, make tweaks, rebase that clean, and then land | 01:54 |
rick_h_ | davecheney: so reordering commits and such tends to just be when someone keeps a long running branch going and needs a better merge to go down | 01:55 |
bodie_ | yep, that's the usual way. when done properly and with very clear commit messages, it lends itself to a really great log | 01:55 |
wallyworld | with rewritten hisotry :-( | 01:55 |
rick_h_ | yea, have to get past that. I find no value in "lint lint lint" | 01:56 |
rick_h_ | in 1.5yrs of doing launchpad bzr work I only looked at the nested commits one time in a merge with trunk | 01:56 |
rick_h_ | I think that "every little thought the dev had in his head" is supremely over valued | 01:56 |
wallyworld | that's the point, bzr handles that for you | 01:56 |
rick_h_ | but I admit that's me | 01:56 |
rick_h_ | wallyworld: right, but to what end? I mean really. bzr came out of the box in the way that no one ever used it. | 01:57 |
wallyworld | huh? | 01:57 |
wallyworld | no one? | 01:57 |
rick_h_ | and then you had to explain crap like lightweight checkouts and pipelines and colo and all that | 01:57 |
bodie_ | speaking of good commit messages -- https://github.com/erlang/otp/wiki/Writing-good-commit-messages | 01:57 |
wallyworld | rick_h_: so how do you do stuff like propose 2 branches where branch 2 depends on branch 1, you have done branch 1, waiting for review, then want to also put up branch 2 | 01:57 |
rick_h_ | yes, you had to know all kinds of bzr voodoo to actually have it not suck and spend 30min branching | 01:57 |
rick_h_ | wallyworld: git co featurebranch1 | 01:58 |
wallyworld | i feel the same about git voodoo | 01:58 |
rick_h_ | git push origin featurebranch1 | 01:58 |
rick_h_ | git co -b featurebranch2 | 01:58 |
rick_h_ | worky worky worky | 01:58 |
rick_h_ | "oh, a change for feature branch 1 is there" | 01:58 |
wallyworld | rick_h_: sure but how do you propose branch 2 without the diff for branch 1 being included in the pr | 01:58 |
rick_h_ | git co feature branch one; vim file.py; git push origin...; git co branch2; git rebase branch1 | 01:58 |
wallyworld | since github has no pre-req support | 01:59 |
rick_h_ | wallyworld: that's the issue with github reviews. reviewboard supports that | 01:59 |
bodie_ | see also http://git-scm.com/book/ch5-2.html for more ugly merge history diagrams | 01:59 |
wallyworld | is that what gui uses? | 01:59 |
wallyworld | should we switch to review board? | 01:59 |
rick_h_ | wallyworld: no, I'm trying it out, but find it doesn't match up well to the github workflow. So it's a fundamental change I'm not sure I want to go down | 01:59 |
axw | I think we should switch to *something* eventually, GitHub review is a bit sucky | 01:59 |
thumper | axw: I looked at stg stacked git briefly | 02:00 |
thumper | and discarded it as something I wanted to do | 02:00 |
* thumper will work manually for a while | 02:00 | |
axw | ah yeah | 02:00 |
wallyworld | stacked git is for patches | 02:00 |
rick_h_ | https://rbcommons.com/s/jujuui/dashboard/ | 02:00 |
rick_h_ | wallyworld: happy to invite in a ccouple of people if you want to test it out | 02:00 |
wallyworld | rick_h_: so right now, i'm really hamstrubg but how sucky the tooling is compared with bzr + lp | 02:00 |
rick_h_ | paying for it for a month to try it out and see if it's something we want to use | 02:00 |
wallyworld | have you tried gerrit? | 02:00 |
thumper | davecheney: do we not handle merges to juju/names through the lander? | 02:00 |
rick_h_ | wallyworld: definitely, bzr + lp was built for our workflow and our workflow was built to bzr + lp | 02:00 |
axw | thumper: only juju/juju atm | 02:01 |
axw | more later | 02:01 |
thumper | axw: ok, ta | 02:01 |
rick_h_ | wallyworld: but the rest of the world doesn't agree unfortunately. I know hg has some other concepts, but even that is having issues gaining ground | 02:01 |
wallyworld | rick_h_: well, i wouldn't have thought our workflow of several peieces in place at once was unique | 02:01 |
wallyworld | i mean, do git people realy only work on one thing at once? | 02:02 |
wallyworld | surely not | 02:02 |
rick_h_ | wallyworld: well we're unique in a lot of ways. Others just deal with the tool, or use things like quilt/etc I think. | 02:02 |
thumper | ah fark... | 02:02 |
rick_h_ | wallyworld: most of them aren't doing code reviews and landings and gated lands and all that | 02:02 |
* thumper needs to dive deeper | 02:02 | |
rick_h_ | wallyworld: they put it up and it lands | 02:02 |
wallyworld | rick_h_: wtf? so they just land stuff unreviewed and untested? | 02:02 |
wallyworld | you have to be kidding | 02:02 |
rick_h_ | wallyworld: why do you think I work here? :) | 02:03 |
wallyworld | seriously? | 02:03 |
rick_h_ | even big projects don't do a CI run post-review and pre-land like we do. | 02:03 |
wallyworld | i'm shocked | 02:03 |
davecheney | rick_h_: none of atlassian's products handle pre commit CI | 02:03 |
* wallyworld is just astonished | 02:03 | |
rick_h_ | wallyworld: when I brought it up to travisci they didn't feel it was their ball of wax. No one else I could find doing it. | 02:03 |
davecheney | land change, send it to ci | 02:03 |
davecheney | add it to the review queue for someone to ignore | 02:03 |
bodie_ | I don't hate the git review | 02:03 |
bodie_ | but, that may just be me | 02:03 |
rick_h_ | davecheney: yea, we're pretty unique in the overall workflow | 02:04 |
rick_h_ | which is kind of sad once you live in our world for a bit | 02:04 |
davecheney | the martini framework does precommit ci with werker | 02:04 |
rick_h_ | bodie_: yea, honestly a lot of it is that you don't know what you don't have until you use something better | 02:04 |
axw | bodie_: Rietveld had some nice things for long on-going reviews. you could see the difference between two patchsets for example. I miss that | 02:04 |
rick_h_ | but I never got into bzr. I did a happy dance when we got to move to git | 02:04 |
wallyworld | rick_h_: or worse for me, using something great and being forced to use something crap | 02:04 |
bodie_ | yeah, I enjoyed what I saw of Rietveld | 02:04 |
rick_h_ | maybe it just fits my brain better, I even wrote two bzr articles for python magazine back in the day | 02:05 |
rick_h_ | wallyworld: with cherry pick running two branches dependent on each other isn't that bad tbh | 02:05 |
rick_h_ | <3 cherry pick and bisect | 02:05 |
bodie_ | not familiar w/ bisect | 02:05 |
wallyworld | rick_h_: sure, but it's way more manual than bzr pipelines and you can't get stff reviewed | 02:05 |
rick_h_ | those two are worth their weight in gold | 02:05 |
rick_h_ | wallyworld: but you can do a pull request with any parent | 02:06 |
* rick_h_ wonders if you can git pr from another git pr | 02:06 | |
wallyworld | rick_h_: i did but it doesnt go against juju/juju | 02:06 |
rick_h_ | I think it has to be a real branch though | 02:06 |
wallyworld | it went against the parent | 02:06 |
rick_h_ | wallyworld: right, but wtf is the point anyway. If branch 1 isn't ok you can't get signoff on branch 2 | 02:06 |
wallyworld | rick_h_: sure you can | 02:07 |
wallyworld | we did it all the time in lp | 02:07 |
rick_h_ | is it really really an issue that blocks you for a significant amoutn of time. | 02:07 |
wallyworld | yes :-( | 02:07 |
wallyworld | cause if it takes a day for branch 1 to get reviewed and landed, i'm blocked on putting up branch 2 | 02:07 |
wallyworld | so the reviews are serialised | 02:07 |
wallyworld | and i have to manage it all locally without nice bzr pipeline support | 02:08 |
rick_h_ | It seems to me they should be. An ok on branch 2 while changes are still made to branch 1 that could effect branch 2 seems scary to me. | 02:08 |
rick_h_ | but yea, your killer feature is a bzr plugin, time to get a git plugin :P | 02:08 |
rick_h_ | or use a diff review tool that can take the diff vs a github pull request | 02:08 |
wallyworld | depends right. if branch 1 is api calls used by branch 2, then branch 1 can change and branch 2 doesn't care | 02:09 |
wallyworld | sadly right now we are stuck with what github offers out of the box | 02:09 |
rick_h_ | how can the api calls not effect the second one? | 02:09 |
rick_h_ | you're assuming there's no bikeshedding around those calls :P | 02:09 |
rick_h_ | wallyworld: right, but your beef is with github not git in this case. So move beyond github. | 02:10 |
wallyworld | if branch 1 gets a comment saying fix this implementation bit, then branch 2 doesn't care | 02:10 |
wallyworld | i hate git too for other reasons | 02:10 |
rick_h_ | wallyworld: but as the reviewer on branch 2 I don't want to LGTM until I'm sure branch 1 won't change | 02:10 |
rick_h_ | wallyworld: :) | 02:10 |
wallyworld | see the canonical tech thread for comments by others that some u the issues | 02:10 |
rick_h_ | I don't doubt it | 02:10 |
wallyworld | by why not lgtm branch 2? it is correct as is | 02:10 |
wallyworld | we did it all the time in lp | 02:11 |
rick_h_ | wallyworld: because nothing says the branch I reviewed will actually make it. | 02:11 |
rick_h_ | if there is a bikeshed over the api itself, and branch 2 has to change, but I already gave it a LGTM | 02:11 |
wallyworld | the author can ask for another review if stuff changes significantly as a result of branch 1 changing | 02:11 |
rick_h_ | wallyworld: yea, I don't say one is better than the other, but that you lose some gain some by the different tools | 02:11 |
wallyworld | we have lost significcant velocity | 02:12 |
davecheney | lucky(~/src/github.com/juju/juju) % git rebase -i --autosquash master | 02:12 |
davecheney | Cannot 'squash' without a previous commit | 02:12 |
davecheney | lucky(~/src/github.com/juju/juju) % git rebase -i --autosquash master | 02:12 |
davecheney | It seems that there is already a rebase-merge directory, and | 02:12 |
davecheney | I wonder if you are in the middle of another rebase. If that is the | 02:12 |
davecheney | case, please try git rebase (--continue | --abort | --skip) | 02:12 |
davecheney | If that is not the case, please rm -fr "/home/dfc/src/github.com/juju/juju/.git/rebase-merge" | 02:12 |
davecheney | and run me again. I am stopping in case you still have something | 02:12 |
davecheney | valuable there. | 02:12 |
davecheney | welk fuck | 02:12 |
davecheney | really hate this | 02:12 |
rick_h_ | git works sans plugin management, fast, and gives me multiple remotes, cherry-pick, bisect (priceless for chasing bugs) | 02:13 |
rick_h_ | davecheney: git rebase --abort is safe | 02:13 |
rick_h_ | then git diff master to see what's up. If master was updated you'll have to rebase it with your feature branch | 02:14 |
rick_h_ | git rebase master && git rebase -i | 02:14 |
rick_h_ | I tend to leave off the autosquash just so I can see what it's doing. | 02:14 |
davecheney | sorry folks, this is insane | 02:15 |
davecheney | if someone wants to teach the bot to squish things go for it | 02:15 |
davecheney | but i want no part of this | 02:15 |
rick_h_ | I thought you all weren't going to use rebase? | 02:15 |
wallyworld | rick_h_: git forces all of these extra mental steps to understand the internals, kinda like outside classes looking into a something implementation. yuk | 02:16 |
rick_h_ | wallyworld: I'll admit I didn't care for git until I read the oreilly book that got into internals and it 'clicked' a bit for me | 02:16 |
rick_h_ | but then bzr never 'clicked' for me so I admit to being strange | 02:16 |
wallyworld | but why should anyone have to undertand the internal implementtion details | 02:16 |
rick_h_ | if I build a house of wood I should understand how wood work | 02:17 |
wallyworld | i don't have to understand how an engine works to drive my car | 02:17 |
rick_h_ | works | 02:17 |
rick_h_ | imo | 02:17 |
rick_h_ | you need to know enough to debug 'does it have gas or is the oil low' | 02:17 |
rick_h_ | so I argue that's not quite right | 02:17 |
bodie_ | it helps to know why you shouldn't drive with your foot on the clutch | 02:17 |
wallyworld | looking at some dashlights is not the same as having to understand how a crankshatft works | 02:18 |
wallyworld | bzr alllowed the former | 02:18 |
wallyworld | git forces the latter | 02:18 |
wallyworld | way too many footguns | 02:18 |
rick_h_ | http://www.reviewboard.org/docs/rbtools/dev/rbt/commands/post/#tracking-branches wallyworld for the reviewboard ability to submit a review that's a diff of another branch | 02:18 |
wallyworld | ta | 02:18 |
rick_h_ | wallyworld: the great thing is that there's the reflog so no matter how well you aim the gun you can get your foot back | 02:19 |
bodie_ | perhaps we should be doing merge --squash on the bot? | 02:19 |
bodie_ | that way it doesn't really matter how the PR's come in. I don't like it, but it's an option | 02:19 |
wallyworld | rick_h_: good to know | 02:19 |
wallyworld | bodie_: but does that throw away history | 02:19 |
rick_h_ | bodie_: the issue is that they often have longer running branches and I'm not sure they're rebasing those feature branches vs merging from trunk so they'll get a lot more conflicts and crappy intermingled commits that are impossible to rebase easily | 02:19 |
bodie_ | wallyworld, yes, but things that throw away history are the subject matter, really | 02:20 |
wallyworld | rick_h_: so rebase vs merge. which the f*ck do i use. in bzr it just worked without having to worry | 02:20 |
rick_h_ | I thinki roger has had to update from conflicts with trunk trying to land his last 4 branches in a row. Hard to deal with using auto tools | 02:20 |
rick_h_ | wallyworld: rebase will roll back your commits, update the fork point to the current head, and then reapply your commits one at a time, resolving any conflicts | 02:21 |
rick_h_ | wallyworld: but it appears in the history that your first commit wasn't until the after the latest one in the trunk branch | 02:21 |
bodie_ | wallyworld, depends what you're trying to do. generally speaking, when opening a PR, you want to rebase -i against master to condense your potentially noisy commit log into something more atomic and useful to the master log | 02:21 |
rick_h_ | wallyworld: merge just says "take X and git it into Y and interweaving of commits be damn" | 02:21 |
rick_h_ | wallyworld: then that gets into why you might want to move commits around and reorder them/etc | 02:21 |
wallyworld | whereas bzr handed that nicely by keeping your commits off to the side | 02:22 |
bodie_ | (after pulling your latest master from upstream) | 02:22 |
rick_h_ | wallyworld: it's a sign of using merge vs rebase | 02:22 |
rick_h_ | wallyworld: right, git doesn't have that. Never will. That's the one thing you can't have. :) | 02:22 |
wallyworld | which is why i hate it sooo much | 02:22 |
rick_h_ | wallyworld: have to move past that one thing. It's the big blocker I know. | 02:22 |
bodie_ | we-e-e-lll... you could always keep your user branches around, and clone them for rebasing. | 02:22 |
wallyworld | i guess rebase is the right workflow then for eeping your feature beanch up to date | 02:22 |
rick_h_ | just like I can't have C speeds in python and have to use Go, which means I lose exceptions. Have to get over it. | 02:22 |
wallyworld | it's more than just that one thing | 02:23 |
bodie_ | sorry, s/user branches/feature dev branches/ | 02:23 |
rick_h_ | different tools provide different + and different - | 02:23 |
wallyworld | that's just the current topic :-) | 02:23 |
davecheney | lucky(~/src/github.com/juju/juju) % gb ./... | 02:23 |
davecheney | state/apiserver/apiserver.go:16:2: cannot find package "github.com/bmizerany/pat" in any of: /home/dfc/go/src/pkg/github.com/bmizerany/pat (from $GOROOT) | 02:23 |
davecheney | umm | 02:23 |
davecheney | how did this dependency creep into the code ? | 02:23 |
rick_h_ | lol | 02:23 |
wallyworld | nfi | 02:23 |
davecheney | not cool | 02:23 |
davecheney | has anyone checked the licence on it ? | 02:23 |
rick_h_ | speaking of features...dep management wtf | 02:24 |
wallyworld | you mean in go projects? | 02:24 |
bodie_ | oy, don't get me started | 02:24 |
rick_h_ | wallyworld: yea, every time I try to get my head into go and start trying to use it I hit that and my brain shuts right down | 02:24 |
wallyworld | rick_h_: if you think i feel strongly about bzr vs git ...... | 02:25 |
rick_h_ | wallyworld: anyway, sorry to poke the bear and stir up the angst. waaaay past my EOD and past my bed time. | 02:25 |
wallyworld | np, thanks for discussing | 02:25 |
rick_h_ | thanks menn0 again for the patch. | 02:25 |
axw | wallyworld: there's a good use-case for a rebase in https://github.com/juju/juju/pull/50 | 02:25 |
wallyworld | rick_h_: they'll be another patch coming to improve the initial bot message when a pr is picked up | 02:25 |
axw | wallyworld: do "git rebase -i" and just delete the lines with the commits you don't want in it | 02:26 |
rick_h_ | wallyworld: coolio, yea it was definitely a MVP and love seeing it improved by others as well. | 02:26 |
wallyworld | axw: but i want them all - theyre history | 02:26 |
wallyworld | rick_h_: we do appreciate having it, thank you :-) | 02:26 |
axw | wallyworld: eh? you've got a commit in there and a commit that undoes it | 02:26 |
rick_h_ | wallyworld: ugh, but the merge's to update with trunk and fix conflicts should go and be a rebase :P | 02:26 |
wallyworld | why? it's history | 02:27 |
bodie_ | but not necessarily history that someone who wants to understand juju's dev history needs to understand | 02:27 |
wallyworld | history should be read only | 02:27 |
rick_h_ | the two merges of master add no value to anyone at all. If you rebase, the branch appears just like you branched master, did 3 commits, and landed it | 02:27 |
bodie_ | I agree with that in theory, though. so I'm actually somewhat conflicted about rebase and never used it much before | 02:27 |
rick_h_ | and I don't care that you merged master and fixed conflicts I didn't ever know existed or cared to know | 02:27 |
axw | yeah, once committed. I see no value. difference of opinion I suppose... | 02:27 |
bodie_ | however, getting familiar with it right now has made me much comfier with the idea | 02:27 |
rick_h_ | seriously, what value is that merge and fixed conflicts in a branch no one else is working on with you? | 02:28 |
wallyworld | i guess if the general consensus is for rebase then i need to do it | 02:28 |
rick_h_ | if you want to say it's good because you and axw were both working on that branch at the same time, ok maybe... | 02:28 |
wallyworld | imagine if financial institutions could do that | 02:29 |
axw | wallyworld: IMO it is only confusing. I only want to see commits that I'm expected to review | 02:29 |
wallyworld | rewrite audit logs | 02:29 |
rick_h_ | juju core's source is not a financial institution :P | 02:29 |
rick_h_ | you work on him axw :) | 02:29 |
bodie_ | he has a point, rebasing can nuke useful blame data | 02:29 |
axw | heh :) | 02:29 |
wallyworld | axw: yiu review the diff, not the commits? | 02:29 |
rick_h_ | bodie_: any rebasing will lead to a commit. You can always bisect and find a blame point in a single chunk of diff | 02:30 |
wallyworld | the reviewer looks at the final diff, the commit are the audit log | 02:30 |
axw | wallyworld: depending on the size of the review, that can be quite onerous | 02:30 |
waigani | thumper: I'm blocked on all my branches now. Waiting on final reviews for https://github.com/juju/juju/pull/35 and https://github.com/juju/juju/pull/22 | 02:30 |
wallyworld | which is why we keep diffs small | 02:30 |
wallyworld | and use pipelines | 02:30 |
thumper | waigani: ok, will look shortly | 02:30 |
wallyworld | large diffs are very bad, cause if tere's one issue, the whole lot gets blocked | 02:30 |
rick_h_ | wallyworld: +1000 finally we can agree on something :P | 02:31 |
wallyworld | with pipelines, the downstream ones can stil get lgtm | 02:31 |
wallyworld | independent of the upstream | 02:31 |
axw | wallyworld: agree on that. can you explain to me why I care about seeing you do something and then revert ithough? | 02:31 |
rick_h_ | plus large diffs means long running branches which means lots of conflicts and wasted time and loss of scope, wasted energy, and lack of potential to parallize work | 02:31 |
wallyworld | yep | 02:31 |
wallyworld | so who cares about the commits then | 02:31 |
wallyworld | just look at the diff | 02:31 |
wallyworld | and let the commits be as they may | 02:31 |
rick_h_ | wallyworld: then why do you care if they go anywhere :P | 02:31 |
axw | people may want to cherry pick things | 02:32 |
waigani | wallyworld: any plans for a pipeline workflow with git? | 02:32 |
axw | they should be independent units of work for that purpose | 02:32 |
wallyworld | rick_h_: so i can use them later if needed | 02:32 |
rick_h_ | for what? that's what I mean. what use is https://github.com/wallyworld/juju/commit/1a799afdd8133c5d63031df225e7f5f4b63a0218 ? | 02:32 |
wallyworld | axw: in lp+bzr, we tended to cherry pick the final revision merged in | 02:32 |
axw | sure, but we don't have that luxury here | 02:32 |
wallyworld | why not? | 02:33 |
wallyworld | we can cherry pick a single merge? | 02:33 |
axw | well I suppose you can pick out the merge one | 02:33 |
axw | yeah ok, you can do that. | 02:33 |
wallyworld | rick_h_: i may not want to look at that particular one, but it's still hisotry | 02:33 |
wallyworld | and shows the thought process and evolution of the branch | 02:34 |
rick_h_ | wallyworld: stop moving your argument on me. First commits aren't important, then you might want the commit, then "it's history" | 02:34 |
wallyworld | rick_h_: they're not important for doing the review per se (necessarily) | 02:34 |
wallyworld | just se the diff for that | 02:34 |
wallyworld | use | 02:34 |
wallyworld | but, they are important for hisotry | 02:34 |
rick_h_ | and git bisect can narrow any bug/issue down to a single commit | 02:34 |
rick_h_ | which can get you the diff that introduced the issue | 02:35 |
wallyworld | well, i think of the diff that merged to trunk as that | 02:35 |
wallyworld | which may be composed of several working commits | 02:35 |
rick_h_ | http://robots.thoughtbot.com/git-bisect | 02:35 |
rick_h_ | sure, but if the diff is small, and you keep a couple of the important commits in the history | 02:35 |
rick_h_ | it's not an issue at all | 02:35 |
rick_h_ | after all, you just said we'd agree to keep the diffs small :) | 02:36 |
wallyworld | the whole review generally should be small | 02:36 |
wallyworld | < 800 lines was the lp guidelne | 02:36 |
rick_h_ | +1 we use c400 as the initial guide | 02:36 |
wallyworld | if i want to bisect something, i'll do it at the granularity of the dev's final commit to trunk | 02:37 |
rick_h_ | over that you need 2 reviews | 02:37 |
axw | wallyworld: there's actually a tool, "git bisect" that runs a command to find a commit that introduces a bug | 02:37 |
axw | I don't think you can choose | 02:38 |
wallyworld | ok, well in that case it can find the commit. i don't care if the commit is for a go fmt or whatever | 02:38 |
wallyworld | it just finds the commit | 02:38 |
rick_h_ | wallyworld: well anyway. Try to let go of what's lost, look forward to what's gained, and don't let things make you so cranky :P | 02:39 |
wallyworld | rick_h_: sure, i'm still im mourning and trying to transition | 02:40 |
wallyworld | and discussing these things is healthy cause the new tooling sucks and we do need to fix it or else veloxity wil suffer | 02:40 |
rick_h_ | wallyworld: there are things we can do better, but there's a certain amount of wasted energy in trying to stick too hard to what 'used to work' | 02:41 |
wallyworld | just at the moment i'm having trouble figuring out what's gained, apart from "everyone else uses it" | 02:41 |
rick_h_ | http://devopsdays.org/events/2013-telaviv/proposals/Jenkins%20and%20automatic%20multibranch%20GIT%20support/ is interesting | 02:41 |
wallyworld | rick_h_: we just discussed above gated commits etc etc - that's what 'used to work' and we needed to (and need to) get to the same place | 02:42 |
rick_h_ | wallyworld: definitely, that's something we needed to find a way to do better. | 02:42 |
menn0 | rick_h_: sorry, just saw your messages. Thanks for getting the change in. Do I need to deploy it now or have you done that? | 02:42 |
rick_h_ | wallyworld: but the nested commit history isn't coming from anywhere. | 02:42 |
wallyworld | and so i would argue the other stuff like pipelines etc falls into the same cathory | 02:42 |
rick_h_ | menn0: I don't have access to your jenkins install so that's up to someone on your end | 02:42 |
rick_h_ | wallyworld: well I said pipelines can be done using a diff review tool | 02:43 |
rick_h_ | wallyworld: that's not a git issue | 02:43 |
menn0 | rick_h_: ok cool | 02:43 |
axw | menn0: I think mgz has some changes to push up to trunk too, so check with him | 02:43 |
wallyworld | rick_h_: there's discagreement on the list about throwing away hisotry, so it's not just me pushing this line :-) | 02:43 |
rick_h_ | wallyworld: definitely, fortunately there's more than one way to do it and it's completely possible to not throw away history | 02:43 |
wallyworld | rick_h_: yeah, i'm not just talking about git, but the whole toling set up | 02:43 |
rick_h_ | wallyworld: I'll not argue that there's more than one way to do the history stuff | 02:43 |
menn0 | axw: ok np. I'll wait. | 02:44 |
wallyworld | rick_h_: we need to do this oer a beer :-) | 02:44 |
wallyworld | over | 02:44 |
wallyworld | not irc :-) | 02:44 |
rick_h_ | wallyworld: another +1000 :P in germany see you there | 02:44 |
wallyworld | looking forward to it | 02:44 |
rick_h_ | hmm, maybe I'll learn to like beer if I drink german beer | 02:45 |
rick_h_ | it's supposed to be all kinds of awesome right? | 02:45 |
* wallyworld dosn't like beer either :-) | 02:46 | |
wallyworld | we'll discuss over a red | 02:46 |
wallyworld | rick_h_: so if i want to do a 3rd branch in my pipeline off branch 2, i should "git checkout branch2; git rebase master" first? and then git checkout -b branch3 ? | 02:47 |
rick_h_ | wallyworld: the rebase is just to sync with trunk before you put it up for review. | 02:48 |
wallyworld | what if i want to ensure trunk is all there before i start branch 3 | 02:48 |
rick_h_ | wallyworld: you can do it whenever, but I just sync my feature branch before review to make sure it's clean before CI takes it and tests it | 02:48 |
rick_h_ | wallyworld: you can do it at any point in time | 02:48 |
wallyworld | cool, cause i may need stuff in trunk etc | 02:48 |
wallyworld | or want to avoid conflicts later | 02:49 |
rick_h_ | rgr | 02:49 |
wallyworld | actually, i think i need git rebase upstream master | 02:49 |
wallyworld | or origin master | 02:49 |
rick_h_ | I find that it works well to create my feature branch from trunk, work on feature, get it all through and working/test passing. Then catch up with trunk, fix stuff, and then put it up for review | 02:49 |
wallyworld | or something | 02:49 |
rick_h_ | wallyworld: heh, yea we have a shortcut for that | 02:49 |
rick_h_ | wallyworld: https://github.com/juju/juju-gui/blob/develop/HACKING.rst#syncing-your-feature-branch-with-develop-trunk | 02:50 |
wallyworld | rick_h_: i can't create my next feature branch from trunk cause it has to come off branch 2 | 02:50 |
rick_h_ | wallyworld: it uses the sync-juju alias (we use develop as our master) | 02:50 |
wallyworld | rick_h_: i looked at that but from memory you guys use a develop branch also | 02:50 |
rick_h_ | wallyworld: but you can rebase branch 2 from trunk, then create branch 3 from branch 2 | 02:50 |
wallyworld | we don't do that | 02:50 |
rick_h_ | wallyworld: right, so s/develop/master | 02:50 |
wallyworld | ok, ta | 02:50 |
rick_h_ | master is just sacred in our world. No one touches master but on release day | 02:50 |
wallyworld | i'm getting there | 02:50 |
rick_h_ | ok, seriously bed time night | 02:52 |
wallyworld | night | 02:53 |
menn0 | thumper: updated agent status branch pushed. LGTY? | 02:59 |
* thumper looks | 02:59 | |
thumper | um... pushed where? | 02:59 |
thumper | the PR looks the same | 02:59 |
menn0 | look again... it seems like the GH web interface took a little while to see the change (the git push from my machine was done) | 03:01 |
menn0 | thumper: ^^ | 03:01 |
wallyworld | axw: do you know how i make it so that "git push" will magically know to push to "https://github.com/wallyworld/juju/<foo>" without needing to type in the full path? | 03:10 |
axw | wallyworld: push.default=simple -- see menn0's email from a couple of days ago | 03:13 |
axw | sorry, =current | 03:13 |
wallyworld | axw: ah, i had simple | 03:13 |
wallyworld | i wanted to use the non deprecated baheviour | 03:14 |
wallyworld | there must be a way of doing it with the new behaviour you would think | 03:14 |
wallyworld | axw: ah balls, that just pushed to github.com/juju/foo :-) | 03:16 |
axw | wallyworld: what's deprecated? | 03:16 |
wallyworld | not github.com/wallyworld/juju/foo | 03:16 |
axw | wallyworld: oops.. is your origin not set correctly? | 03:16 |
wallyworld | [environ-resource-catalogs]ian@wallyworld:~/juju/go/src/github.com/juju/juju$ git remote -v | 03:17 |
wallyworld | origin https://github.com/wallyworld/juju (fetch) | 03:17 |
wallyworld | origin https://github.com/wallyworld/juju (push) | 03:17 |
wallyworld | upstream https://github.com/juju/juju.git (fetch) | 03:17 |
wallyworld | upstream https://github.com/juju/juju.git (push) | 03:17 |
axw | hm | 03:17 |
wallyworld | looks like it went to upstream not origin | 03:17 |
axw | git branch -vv | 03:17 |
axw | what's that say? | 03:17 |
wallyworld | when i didn't have push.default set, it said i think current was deprecated | 03:18 |
axw | for the current branch | 03:18 |
wallyworld | [environ-resource-catalogs]ian@wallyworld:~/juju/go/src/github.com/juju/juju$ git branch -vv | 03:18 |
wallyworld | * environ-resource-catalogs c2001f6 [upstream/master: ahead 6] Add tests for put/remove races | 03:18 |
wallyworld | master 4e7b047 [origin/master] Merge pull request #38 from axw/remove-instance-dnsname | 03:18 |
wallyworld | new-txn-package 1a799af [environ-resource-catalogs: ahead 5, behind 9] Merge trunk and fix conflicts | 03:18 |
axw | yeah, it's tracking upstream | 03:18 |
wallyworld | sigh | 03:18 |
axw | the bit in [] | 03:18 |
wallyworld | so how did it do that? and how do i fix it? | 03:19 |
wallyworld | i didn't ask it to i don't thin | 03:19 |
wallyworld | k | 03:19 |
axw | dunno how you did that... I htink you just want to "git --unset-upstream" | 03:19 |
axw | did you set up branch.autosetupmerge? | 03:20 |
axw | I think that's what does that | 03:20 |
wallyworld | yeah, i did set that. i unset upstream and set it to the right thing and it seems happier | 03:23 |
waigani | thumper, menn0 thanks :) | 03:24 |
wallyworld | axw: so with branch.autosetupmerge=true, i just did a checkout -b to create a new branch off an upstream one in my pipeline, and branch -vv shows no tracking. i guess it will wait till i've done the first push? | 03:28 |
axw | wallyworld: you can either set it now, or pass "-u" to "git push" to set it to whatever you push to | 03:29 |
axw | wallyworld: e.g. git push -u origin | 03:29 |
wallyworld | ok, ta. so that does autosetupmerge do then? | 03:29 |
wallyworld | what | 03:30 |
axw | like --remember | 03:30 |
axw | umm | 03:30 |
axw | I don't actually know | 03:30 |
wallyworld | i'd push to origin/foo right? | 03:30 |
wallyworld | cause origin is my fork | 03:30 |
axw | yes | 03:30 |
davecheney | /away lunch | 03:33 |
waigani | wallyworld, axw: I came across git-flow (https://github.com/nvie/gitflow) >9000 stars, don't know anything about it | 03:42 |
wallyworld | looks interesting | 03:43 |
axw | seems master vs. develop isn't necessary if you have gated commits. am I missing something? | 03:46 |
axw | gated merge I mean | 03:46 |
wallyworld | axw: that's what i think too | 03:48 |
wallyworld | hence why for juju we only have master | 03:48 |
wallyworld | i can't see the need for develop. just adds complexity for no good reason afaict | 03:49 |
thumper | menn0, davecheney: will we need to provide explicit json and bson serialisation of the new tags? | 03:59 |
* thumper watched many tests panic | 03:59 | |
waigani | thumper: do we only want to get a list of envs from jenvs and discard environments.yaml? | 04:03 |
thumper | no | 04:03 |
waigani | thumper: okay, do we look in both and merge the list? | 04:05 |
thumper | I think so, but give preference to the jenvs | 04:05 |
waigani | will do | 04:05 |
thumper | hmm... | 04:12 |
* thumper thinks | 04:12 | |
thumper | we are currently logging the passwords of all the login connections... | 04:13 |
thumper | that seems a bit suspicious | 04:14 |
thumper | I'm at that stage of looking at code where I'm thinking "how could that ever have worked" | 04:25 |
* thumper goes to make a coffee | 04:25 | |
jam1 | waigani: I think you need to set your membership in the Juju github team to public | 04:25 |
waigani | jam1: oh | 04:25 |
jam1 | waigani: https://github.com/orgs/juju/members?page=2 | 04:26 |
jam1 | set your entry to Public | 04:26 |
jam1 | otherwise the Bot won't accept your $$merge$$ requests. | 04:26 |
waigani | jam1: done. thanks for that | 04:27 |
waigani | jam1: do I need to recomment? | 04:27 |
jam1 | waigani: I *think* it will see an old request, but give it a sec and see | 04:27 |
jam1 | If it doesn't post "queued" then vote again. | 04:27 |
waigani | jam1: okay | 04:29 |
=== menn0 is now known as menn0-afk | ||
jam1 | waigani: just looking through the merge queue, I'm pretty sure fwereade is clear that all API calls should talk in terms of Tags | 04:31 |
jam1 | so we don't pass raw Machine Ids, or Unit names, or Users | 04:32 |
jam1 | but "user-admin" and "machine-0" and "unit-mysql-1" | 04:32 |
jam1 | waigani: so for consistency, I'm pretty sure AddServerAPI method UserInfo should take Entities | 04:32 |
jam1 | waigani: you can assert that they are all UserTagKind early on and convert them into a list of usernames | 04:32 |
jam1 | but the *API* should be tags | 04:33 |
waigani | jam1: ah right. I'll do a follow up branch to fix that. | 04:34 |
thumper | waigani: wait, I'd like to talk to fwereade about that | 04:38 |
thumper | particularly when we are talking about users and only users | 04:38 |
waigani | thumper: no problem | 04:38 |
waigani | thumper: I've got plenty to go on | 04:38 |
thumper | jam1: I disagree on the premise that we should have ONLY tags for ALL the api | 04:38 |
jam1 | thumper: you can do so, but it should be debated, because it *has* been the API design that William has pushed heavily | 04:39 |
thumper | I can see where that makes sense for general connections, or actions | 04:39 |
thumper | but when you are talking to a user end point, and only dealing with users | 04:39 |
thumper | it is asinine to convert to and from user tags. | 04:39 |
=== vladk|offline is now known as vladk | ||
jam1 | thumper: I disagree on the asinine part. I can see a point about "I'm in a particular context, I shouldn't need to wrap my requests with more context", but I can also agree with consistency across the API. | 04:45 |
jam1 | And fwereade did specifically ask for Entity here | 04:45 |
jam1 | and so we should have the discussion | 04:45 |
thumper | I'm happy to have the discussion | 04:46 |
thumper | but I hold my asinine position :) | 04:46 |
* wallyworld gets some popcorn | 04:56 | |
lifeless | thumper: what flavor :) | 04:58 |
lifeless | bah | 04:58 |
lifeless | wallyworld: ^ | 04:58 |
wallyworld | um | 04:58 |
wallyworld | caramel | 04:58 |
thumper | salted caramel? | 04:59 |
thumper | jam1: we have a meeting with fwereade in about three hours anyway | 05:00 |
thumper | I propose claiming some of that time to discuss this | 05:00 |
* thumper is beating the tests into submission | 05:07 | |
thumper | wallyworld: you'll like this branch | 05:07 |
wallyworld | yeah? | 05:07 |
thumper | wallyworld: or better put, if you don't like it, it will make me very sad | 05:07 |
thumper | wallyworld: trying to get some object factory concepts into out test suite | 05:07 |
wallyworld | .me wonders if he should make thinper sad or not | 05:07 |
wallyworld | yay | 05:07 |
thumper | time to go and make dinner | 05:10 |
thumper | before meeting later | 05:10 |
=== thumper is now known as thumper-afk | ||
wallyworld | axw: is running rebase again on a branch meant to remember merge resolutions from last time it was run? | 05:41 |
axw | wallyworld: it's stateless. it just takes out your changes, and replays on top of the thing you're rebasing on. if it needs merging again, you have to do it again | 05:42 |
wallyworld | sigh | 05:42 |
wallyworld | so each time i change an upsream branch and rebase, i have to redo all the same conflict resolution :-( | 05:43 |
wallyworld | there has to be a better way | 05:43 |
axw | wallyworld: what are you doing? | 05:45 |
axw | a pipeline-ish thing? | 05:45 |
wallyworld | yeah | 05:45 |
wallyworld | i fixed the defers you mentioned | 05:45 |
wallyworld | and now i want to pump those changes to the downstream branches | 05:45 |
wallyworld | so i checkout to branch B | 05:46 |
wallyworld | and rebase branch A | 05:46 |
axw | ah, you want to pick up the conflict resolution you did in branch A? | 05:46 |
wallyworld | but it comes up with all the same merge conflicts i fixed last time when i did a rebase | 05:46 |
wallyworld | the conflict resolution in branch b | 05:46 |
wallyworld | i was in branch B ( the downstream branch) and wanted to pull in latest changes from branch A | 05:47 |
wallyworld | i did it once and resolved things | 05:47 |
wallyworld | now having changed branch A i need to do it again | 05:47 |
wallyworld | but it is conflicting in the same places as earlier | 05:47 |
axw | wallyworld: I think because it replays the commits in order | 05:48 |
wallyworld | branchB is the one I can't propose yet because github doesn't support dependent branches | 05:48 |
axw | so the fixup you did comes later down the line | 05:48 |
axw | although.. hmm no, it should apply to the commit you're processing at the time | 05:49 |
axw | I dunno. I'm still a novice | 05:49 |
wallyworld | yeah, i'm even more of a noob | 05:49 |
wallyworld | i shouldn't be this hard to be productive | 05:49 |
wallyworld | do git people just do things serially | 05:50 |
axw | github people | 05:51 |
wallyworld | well, git rebase seems broken too | 05:53 |
wallyworld | i ended up having to cherrypick the rev | 05:53 |
wallyworld | not very desirable if there's more than one change to bring forward | 05:54 |
axw | gotta get my daughter and pick up a cake, bbl | 05:55 |
jam1 | TheMue: just a reminder that you're On Call Reviewer today. | 05:55 |
dimitern | morning all | 06:15 |
dimitern | fwereade, jam, https://github.com/juju/juju/pull/49 | 06:15 |
jam1 | morning dimitern | 06:16 |
vladk | dimitern: morning, https://github.com/juju/juju/pull/54 | 06:18 |
jam1 | dimitern: first comment on your proposal, do we want it as "environs/network" or just "network/" ? | 06:47 |
jam1 | it would be nice to not have stuff like State depend on things in Environment | 06:48 |
jam1 | which I thought is why we moved stuff into the top level 'instance/' | 06:48 |
jam1 | like 'agent/' shouldn't really depend on 'environs' IMO | 06:48 |
dimitern | jam1, fwereade suggested that originally | 06:50 |
dimitern | jam1, i also thought of network/ at first | 06:50 |
jam1 | dimitern, sorry was switchnig rooms and my laptop went to sleep. was there a "but" there? last I saw was "i also thought of network/ first" | 06:57 |
dimitern | jam, sorry, no | 07:16 |
dimitern | jam, I did make it network/ first, but IIRC fwereade asked me to change it to environ/network/, as it better describes its intent | 07:17 |
jam1 | dimitern: k, I think we should run it by him, since instance was intentionally to pull it out of environs, it seems odd to put it back in | 07:18 |
fwereade | dimitern, did I? that sounds kinda like I was blithering at the time, sorry | 07:18 |
dimitern | fwereade, that's what I recall :) | 07:18 |
fwereade | dimitern, network/ is fine by me, I apologise for telling you to do stupid things :) | 07:19 |
dimitern | fwereade, but now's the time to move it to network/ instead of environ/network/ | 07:19 |
dimitern | fwereade, sure, np ;) | 07:19 |
=== thumper-afk is now known as thumper | ||
* thumper scowls at the code | 07:55 | |
thumper | I have no idea why this test is failing... | 07:55 |
voidspace | morning all | 07:57 |
=== menn0-afk is now known as menn0 | ||
mattyw | morning folks, I was thinking of making a start on this: thought/ comments? https://bugs.launchpad.net/juju-core/+bug/1216644 | 08:11 |
_mup_ | Bug #1216644: allow open-port to expose several ports <addressability> <improvement> <strategy> <usability> <juju-core:Triaged> <https://launchpad.net/bugs/1216644> | 08:11 |
axw | mattyw: it would be awesome, but it may be a deceptively large task | 08:14 |
axw | there's also status that needs fixing | 08:15 |
axw | condensing port ranges | 08:15 |
axw | and possibly security groups changes needed | 08:15 |
mattyw | axw, I'm wonder if those tasks could be split up (we can allow open-port to operate on ranges) and we change status later - although I guess specifiying a large range would make status unreadable | 08:17 |
axw | mattyw: we could yeah, but the concern is that if we allow people to do it, then we're recommending something that's going to immediately lead to sucky UX | 08:18 |
axw | it's sucky either way though I suppose | 08:18 |
TheMue | jam1: just came online, so starting reviews | 08:24 |
TheMue | morning btw | 08:24 |
voidspace | rebooting | 10:15 |
wallyworld | fwereade: i have done the work to extract out a separate txn package if you wanted to look at some time. it's used in a downstream branch but i can't propose that yet because github doesn't support dependent branches https://github.com/juju/juju/pull/50 | 10:45 |
jam | dimitern: standup ? | 10:46 |
jam | fwereade: wallyworld: I was thinking about dependent branches, one option is that you could propose the branch against the one it is dependent upon, and then propose it again against trunk but ask to have it reviewed against the other proposal | 10:47 |
jam | that would give you just the increment diff to review | 10:47 |
jam | but still have something targeted to trunk when we are ready to land | 10:47 |
wallyworld | jam: i did propose against the unlanded one in my fork, but then people don't get emails etc and it doesn't show inthe juju/juju pull requests | 10:48 |
wallyworld | i guess i could also propose against juju/juju | 10:48 |
jam | wallyworld: right,that is why I suggested *also* proposing against master, and in that PR you say "please review the one over here: http://..." | 10:48 |
wallyworld | a bit clunky. there's got to be a better solution. sure we aren't the only ones wantint to do this | 10:49 |
jam | wallyworld: in the github world, you rebase it all down to individual commits and they review incremental work that way | 10:49 |
jam | so commit 1 is the first feature, commit 2 is using that, etc. | 10:49 |
dimitern | jam, sorry, brt | 10:49 |
wallyworld | what? all in one branch? | 10:50 |
jam | wallyworld: AIUI, yes | 10:50 |
wallyworld | :-( | 10:50 |
wallyworld | that makes me sad | 10:50 |
jam | wallyworld: hence why people like to rewrite their commits because that is how they get reviewed | 10:50 |
wallyworld | that makes me even sadder | 10:50 |
wallyworld | let's just alter history as we see fit | 10:50 |
mgz | it's the way of the world, wallyworld | 10:56 |
wallyworld | it may be. i just don't get how people are comfortable changing history and making all that extra work for themselves in having to do it | 10:56 |
wallyworld | it must kill velocity | 10:57 |
natefinch | talking about rebasing I guess? | 10:58 |
wallyworld | yeah, and that github doesn't support dependent branches | 10:58 |
wallyworld | so you can't propose more than one branch at a time | 10:59 |
wallyworld | if they are in a pipeline | 10:59 |
wallyworld | so you get blocked | 10:59 |
wallyworld | and rebasing i did today kept tripping over the same merge conflicts again and again | 10:59 |
wallyworld | so i had to cherry pick stuff i wanted form by upstream branch | 11:00 |
wallyworld | way too painful comapred with bzr pump :-( | 11:00 |
natefinch | I think there has to be a way to do it, we just don't know how yet. And that's the problem. We have a ton of bzr experts, but no git experts. | 11:01 |
natefinch | I cab;t believe that we're the only ones who have ever stumbled on this problem and that no one has ever solved it. | 11:01 |
dimitern | wallyworld, you can propose multiple branches | 11:01 |
jam | natefinch: so the github overview is to use the individual commits to review (at least, you can do inline comments on the step-by-step commits) | 11:01 |
jam | dimitern: you can, but the dependent branch contains the diff of everything | 11:01 |
wallyworld | sure, but if they depend on each other the diffs are screwed | 11:01 |
dimitern | wallyworld, and rebasing into your fork just makes two things much better: 1) linear history, 2) much easier conflict resolution | 11:02 |
jam | dimitern: I strongly question (2) | 11:02 |
dimitern | wallyworld, because you're rebasing your changes on top of upstream | 11:02 |
jam | and a linear, but false history is... something :) | 11:02 |
dimitern | wallyworld, hence, you resolve conflicts as you rebase, rather than the other option - the merge workflow | 11:02 |
wallyworld | dimitern: in my case i had branch A and branch B in a pipeline. changes to branch A i then switch to branch B and rebased. conflicts. so i fixed them. but then i had to make more changes and rebase again. and the same conflicts ahd to be resolved again | 11:03 |
dimitern | wallyworld, which *forces* you to resolve conflicts out-of-context (i.e. a huge merge commit combining a lot of changes will need to be resolved *every time* you merge upstream) | 11:03 |
natefinch | I wonder if the way to do a pipeline is to branch from juju to your branch A, then branch from A to B. PR A -> juju, PR B -> A | 11:03 |
wallyworld | dimitern: bzr doesn't have the issue | 11:04 |
jam | dimitern: a good merge tool uses a new base when you merge upstream | 11:04 |
dimitern | wallyworld, agreed | 11:04 |
jam | bzr certainly always does, and I definitely expect git to do so as well | 11:04 |
perrito666 | good morning everyone | 11:04 |
wallyworld | natefinch: that's what i did. i did a checkout -b when i was in branch A | 11:04 |
dimitern | git is not a DVCS, it's a platform to build your own DVCS on top of :) | 11:04 |
jam | natefinch: you can certainly do that, but mixing that model with rebasing is ... hard | 11:05 |
jam | stacked git is another option, though it *clearly* is favoring 1-commit == 1 feature | 11:05 |
wallyworld | git i find is fircing me to try and understand all the internals and it's got sooo many footguns | 11:05 |
voidspace | perrito666: morning | 11:06 |
wallyworld | jam: i get the impression stacked git is for producing patches for mailing llists | 11:06 |
natefinch | so, again, I'm not a git expert. It sounds like we need to do some research on how to get this workflow done. | 11:06 |
jam | wallyworld: it is for evolving a series of patches | 11:06 |
jam | that can be viewed as a series of git commits | 11:06 |
perrito666 | are we bashing git again? | 11:06 |
jam | and being able to pop them off, and make a change to an earlier one, and then pop them back on again | 11:06 |
dimitern | stacked git is great only locally | 11:06 |
voidspace | perrito666: once you start a backup with mongodump can you cancel it - or is the only way to kill the running process? (And how would mongo cope with that?) | 11:07 |
dimitern | but it doesn't have the nice bzr pump equivalent | 11:07 |
wallyworld | perrito666: no, trying to figure out how to deal with its limitations compared to bzr | 11:07 |
natefinch | Although, it sounds like maybe we just need to get stuff reviewed faster, if wallyworld can create a whole new CL before someone reviews his old one | 11:07 |
voidspace | perrito666: natefinch has defined the backup api with cancellable backups - and I wonder if that is even possible | 11:07 |
wallyworld | natefinch: well, i often have 3 or 4 branches on the go in a pipeline | 11:07 |
jam | natefinch: it often makes a lot of sense to do things as a series of "introduce the API, use the API" rather than comingle those changes | 11:08 |
voidspace | natefinch: ah, hi - I didn't realise you were around | 11:08 |
wallyworld | and can put up 1 or 2 at once | 11:08 |
wallyworld | jam: yep exactly | 11:08 |
jam | but you want to be writing the "use the API" at the same time, because you want to know the API you have is correct | 11:08 |
voidspace | right, I often want to split a feature into a series of branches that depend on one another | 11:08 |
perrito666 | voidspace: seems to me that you need to kill the process | 11:08 |
natefinch | jam: right, so you write the API, propose... someone reviews the API. If reviewing is a bottleneck, we should fix that | 11:08 |
voidspace | perrito666: would mongo be ok with that? | 11:08 |
natefinch | jam: I guess so. | 11:08 |
jam | natefinch: but you develop them concurrently, as I mentioned | 11:08 |
wallyworld | natefinch: no, doesn't work. because the api evolves as you write the next branh to use it | 11:09 |
perrito666 | voidspace: absolutely, the new backup way behaves pretty much as a client | 11:09 |
voidspace | natefinch: it's not even so much that - sometimes you want a series of branches, and you want to be able to see the diffs of branch two from branch one | 11:09 |
voidspace | natefinch: not from branch two against master | 11:09 |
wallyworld | yep, that too | 11:09 |
voidspace | natefinch: I've been looking at your backup api | 11:09 |
jam | natefinch: and the reviews from github that I've run across in the past have asked for rebased commits so that they can review each change in isolation, which is what multiple branches and pipeline model is | 11:09 |
voidspace | natefinch: you propose an asynchronous api that returns the name of the new backup immediately | 11:10 |
voidspace | natefinch: with a cancel call to cancel one that's in progress | 11:10 |
voidspace | natefinch: how do you know whether or not it's finished? | 11:10 |
natefinch | I think the way you do that in git is.... you write it all in one branch, then, once you're satisfied, you split up the branch into multiple PRs. | 11:10 |
voidspace | yuck :-) | 11:10 |
wallyworld | double yuck :-( | 11:10 |
wallyworld | what a lot of extra work | 11:10 |
natefinch | no way, you don't have to juggle branches that way | 11:10 |
voidspace | this is a github problem not a git one though, right? | 11:10 |
wallyworld | a bit of both i reckon | 11:11 |
natefinch | who wants to constantly switch branches? Just write your code in one branch and THEN decide how it makes sense for it to go in | 11:11 |
jam | voidspace: tools are tools, though. and we're doing code review on github. but yes, it is mostly our code review tool that we're trying to figure out how to use. | 11:11 |
wallyworld | switching branche sis easy | 11:11 |
voidspace | that means you *have* to make monolithic changes that are harder to reason about | 11:11 |
natefinch | that's true voidspace | 11:11 |
voidspace | that's the use case for dependent branches | 11:11 |
jam | natefinch: voidspace: well you can write it all together, and then rebase it into incremental changes | 11:12 |
natefinch | wallyworld: switching is easy, but syncing them can get annoying | 11:12 |
wallyworld | not with bzr | 11:12 |
wallyworld | it's so easy | 11:12 |
jam | new history, but you do have "introduce here, use there" as a sequence | 11:12 |
jam | wallyworld: fwiw "git checkout branch" is cheaper than even bzr | 11:12 |
jam | just missing pipelines' pump to correlate them | 11:12 |
wallyworld | yep, that's the whole pont | 11:12 |
jam | wallyworld: going further, trying to rebase with multiple branches is going to just bite you all over | 11:13 |
jam | so I certainly wouldn't go there | 11:13 |
jam | but you don't *have* to rebase | 11:13 |
wallyworld | as i am finding out :-( | 11:13 |
wallyworld | well, how else do i pump changes from A into B downstream | 11:13 |
wallyworld | like with bzr pump | 11:13 |
natefinch | what's the difference between pump and merge? | 11:14 |
wallyworld | pump pushes changes all the way down a pipeline | 11:14 |
voidspace | natefinch: I've been reading through your proposed backup API | 11:14 |
wallyworld | from A->B->C->D etc | 11:14 |
voidspace | natefinch: I have some questions *but* | 11:14 |
wallyworld | and merges along the way | 11:14 |
natefinch | wallyworld: and conflicts along the way? | 11:14 |
natefinch | voidspace: yep | 11:14 |
voidspace | natefinch: perrito666 doesn't think we can start work on the actual code behind the api until he's ready to merge his work | 11:14 |
voidspace | natefinch: so we can't really parallelise this yet | 11:15 |
jam | wallyworld: "git checkout A", "git pull upstream", "git checkout B", "git merge A", "git checkout C", "git merge B" | 11:15 |
wallyworld | natefinch: if it finds a conflict is stops, you resolve and repump | 11:15 |
jam | it is what pump is doing for you | 11:15 |
natefinch | voidspace: we can start the work to download the backups | 11:15 |
perrito666 | voidspace: I can merge that now :D | 11:15 |
voidspace | ah, cool | 11:15 |
voidspace | natefinch: I'm not sure an asynchronous backup api makes sense | 11:15 |
voidspace | natefinch: how do you tell whether a backup is completed or not | 11:15 |
natefinch | voidspace: the API tells you it's done | 11:15 |
voidspace | natefinch: where? | 11:15 |
wallyworld | jam: i think we need a git pipeline tool :-) | 11:16 |
natefinch | voidspace: in list backups there's an in-progress backup listed, if one's not there, it's done | 11:16 |
jam | wallyworld: if I actually knew git plugins a bit better, I have the feeling it would be pretty easy | 11:16 |
voidspace | natefinch: ah, ok | 11:16 |
jam | the only trick is figuring out how/where to store what the previous/next pointers are | 11:16 |
voidspace | natefinch: by the way, typo | 11:16 |
voidspace | natefinch: // NewBackupAPI creates a new server-side FirewallerAPI facade. | 11:16 |
wallyworld | if i kew git and pluguns better maybe it woud be | 11:16 |
perrito666 | natefinch: voidspace perhaps a quick hangout to sort this? | 11:16 |
natefinch | see? It's not a git problem, it's a "we don't know git well enough" problem | 11:16 |
natefinch | so stop complaining about the problem and start figuring out a solution | 11:16 |
wallyworld | jam: and then we just need a better code review tool than github | 11:17 |
voidspace | perrito666: natefinch: sure - I'm not around for standup today so it could be good | 11:17 |
natefinch | voidspace: haha | 11:17 |
wallyworld | maybe gerrit? or reviewboard? | 11:17 |
jam | wallyworld: or reitveld :) | 11:17 |
wallyworld | noooooooo | 11:17 |
voidspace | let me grab coffee | 11:17 |
perrito666 | voidspace: natefinch going to the channel | 11:18 |
natefinch | perrito666, voidspace: ok, I don't have much time until I have to go help with the kiddos, but I can pop on | 11:18 |
perrito666 | and by chanel I meant our regular hangout | 11:19 |
davecheney | are /win12 | 11:33 |
perrito666 | meh, I get no video on the tablet for hangouts | 11:35 |
* dimitern likes fast reviews, we need more of those :P | 11:35 | |
perrito666 | voidspace: natefinch where do you think this thing should live? I have it in cmd/plugin/juju-backup, but only because in the beginning it was a replacement for the current juju-backup | 11:36 |
natefinch | perrito666: state/backup sounds good to me | 11:39 |
natefinch | backup API for general comments: https://github.com/juju/juju/pull/56 jam, fwereade, anyone else? | 11:40 |
natefinch | gotta run | 11:40 |
=== natefinch is now known as natefinch-afk | ||
TheMue | natefinch-afk: will review it next | 11:46 |
rogpeppe | frankban, dimitern, perrito666, jam: trivial PR to improve isolation a little more: https://github.com/juju/juju/pull/57 | 11:56 |
jam | rogpeppe: lgtm | 11:57 |
rogpeppe | jam: thanks | 11:57 |
voidspace | perrito666: I would leave it there for the moment | 12:01 |
voidspace | perrito666: it's only when we actually create the backup commands / api that we are able to move it | 12:01 |
voidspace | ah | 12:01 |
voidspace | perrito666: or do as natefinch-afk says :-) | 12:02 |
natefinch-afk | perrito666, voidspace: no reason to put it once place then move it to another place later, might as well create it in the correct place. adding a package no one uses yet is perfectly ok. | 12:07 |
* natefinch-afk is not really here ;) | 12:07 | |
=== BradCrittenden is now known as bac | ||
voidspace | heh | 12:12 |
jam | mgz: you around? | 12:13 |
mgz | jam: yeah | 12:13 |
jam | mgz: just wondering if you realized you're OCR today | 12:13 |
mgz | I'm also reviewer today, so poke... | 12:14 |
jam | clearly you do :) | 12:14 |
axw | fwereade: this may interest you: https://github.com/juju/juju/pull/55 | 12:18 |
fwereade | axw, you and wallyworld are distracting me by doing awesome things I want to review ;p | 12:19 |
axw | heh :) | 12:19 |
axw | feel free to ignore it | 12:19 |
mgz | axw: can I land your goose branch btw? | 12:21 |
wallyworld | fwereade: i've replied to some of your comments, thanks for looking :-) | 12:21 |
axw | mgz: if you're fine with the latest changes, then yes please | 12:21 |
mgz | axw: done | 12:26 |
axw | mgz: thanks! | 12:26 |
jam | I see an awful lot of red on the CI dashboard :( | 12:28 |
dimitern | fwereade, ping | 12:34 |
fwereade | dimitern, pong | 12:35 |
dimitern | fwereade, re default public/private networks - they shouldn't be shown in status, right? | 12:35 |
dimitern | fwereade, cause after we create them there will always be at least 2 networks in state right after bootstrap | 12:35 |
fwereade | dimitern, I think we should show them | 12:36 |
fwereade | dimitern, they're part of the model and special-casing those ones will be really odd | 12:36 |
dimitern | fwereade, ok, but for now we don't know what their CIDR is | 12:36 |
dimitern | fwereade, so we'll show something like: networks:\n private:\n provider-id: juju-private\n public:\n provider-id: juju-public | 12:38 |
fwereade | dimitern, assuming there *is* a public network, yeah, and assuming those are really what the provider calls those networks | 12:39 |
dimitern | fwereade, they are juju-specific, hence the "juju-" prefix of the provider-id | 12:40 |
fwereade | dimitern, not sure I follow there | 12:40 |
dimitern | fwereade, hmm.. i think it looks like a bit deeper than I thought | 12:40 |
dimitern | fwereade, so you're saying we should get them from the provider, not just create them in state? | 12:41 |
fwereade | dimitern, isn't the provider-id the equivalent of instance-id? ie it's what the provider would call that network even if juju weren't around | 12:41 |
fwereade | dimitern, yeah | 12:41 |
fwereade | dimitern, it's what you use to identify what's really going on if you know/care about both levels | 12:41 |
fwereade | dimitern, I'd be fine calling those networks "juju-private" and "juju-public" though, it might be a good idea to reserve the juju-* namespace for our own use | 12:42 |
dimitern | fwereade, so then: 1) add Environ.GetDefaultNetworks() - returns the info, 2) change all providers to implement it, 3) call it at bootstrap to save them in state | 12:42 |
fwereade | dimitern, yeah, think so | 12:42 |
dimitern | fwereade, should we also mark them as IsDefault: true in state? | 12:43 |
fwereade | dimitern, sorry, I'm not up to date on that field's meaning | 12:43 |
dimitern | fwereade, that's some amount of special handling, but it might pay off as we progress with the model | 12:43 |
dimitern | fwereade, i'm proposing to add that field to the networkDoc | 12:44 |
fwereade | dimitern, ah ok | 12:44 |
dimitern | fwereade, and similarly expose it as IsDefault() on *state.Network | 12:44 |
fwereade | dimitern, I'm not wholeheartedly +1 on that tbh... convince me? | 12:45 |
dimitern | fwereade, as for the naming | 12:45 |
dimitern | fwereade, we could always name them "public" and "private" (their juju name) | 12:45 |
fwereade | dimitern, I guess my worry is clearer in the public case | 12:46 |
dimitern | fwereade, well, i thinking wrt selecting a default network a relation is on when not specified and things like that - knowing which is the default might help | 12:46 |
fwereade | dimitern, it only makes sense to have one default public network, I think | 12:46 |
dimitern | fwereade, and one private at least | 12:46 |
dimitern | fwereade, to model what we currently do anyway | 12:47 |
fwereade | dimitern, yeah, I'm not quibbling about the notion of defaults | 12:47 |
fwereade | dimitern, I'm quibbling about having as IsDefault field | 12:47 |
fwereade | dimitern, once you have 2 docs with IsDefault:true things become confusing | 12:48 |
dimitern | fwereade, well, if we stretch it further, we can just have a Environ.AllNetworks() instead an just call it at bootstrap and record whatever the provider knows | 12:48 |
fwereade | dimitern, that's ideal tbh | 12:49 |
dimitern | fwereade, that way we'll have the info already in state by the time you want to deploy a service and perform better (earlier) sanity checks on names | 12:49 |
fwereade | dimitern, although it constrains us a bit | 12:49 |
fwereade | we need to give them good names | 12:49 |
dimitern | fwereade, the provider will give us its id for each, but we can name them anyway we like in juju | 12:50 |
fwereade | dimitern, we can either accept config at bootstrap time (I want these (provider-id) networks and I want to give them these (juju) names (including mapping a particular network to juju-private etc) | 12:50 |
dimitern | fwereade, we can call them "public#" and "private#" # increasing from 1, and using the CIDR range to determine if it's public or private | 12:51 |
dimitern | fwereade, hmm.. yeah, it'll be nice to be able to remap names as part of the bootstrap config | 12:51 |
fwereade | dimitern, yeah, but I want users to be able to name them | 12:51 |
fwereade | dimitern, (and pick which one *is* juju-private for that matter) | 12:52 |
dimitern | fwereade, but if that's not the case, what should we pick as names? | 12:52 |
fwereade | dimitern, well, the other option is *not* to pick names until the user asks | 12:52 |
fwereade | dimitern, that's the add-network --existing case | 12:53 |
fwereade | dimitern, ok, baseline case is that the user specifies nothing | 12:53 |
dimitern | fwereade, i don't like that so much | 12:53 |
fwereade | dimitern, we have to guess what the default private/public ones should be, and we name them | 12:53 |
dimitern | fwereade, we'll already have added the network in state | 12:53 |
fwereade | dimitern, (assuming there *is* a public one ofc) | 12:54 |
dimitern | fwereade, for ec2 there is a way to get both the private (subnet(s)) and the public one | 12:54 |
fwereade | dimitern, can we plausibly store proto-networks? ie not full network docs, but kinda notifications that there *is* a network that will become available for use once it's named? | 12:55 |
dimitern | fwereade, for local and manual is easy - either no public or it needs to be explicit | 12:55 |
fwereade | dimitern, then in the super-shiny case the user defines them all in bootstrap config | 12:55 |
fwereade | dimitern, and has them all available and named from the beginning | 12:55 |
dimitern | fwereade, i suppose we can have a pendingnetworks collection for these | 12:56 |
fwereade | dimitern, yeah, something like that | 12:56 |
dimitern | fwereade, identified only by provider id | 12:56 |
dimitern | fwereade, and what other info we can get from the provider | 12:56 |
fwereade | dimitern, if we don't have guidance, we pick a private and (maybe) a public to promote at bootstrap time | 12:56 |
fwereade | dimitern, sgtm | 12:56 |
dimitern | fwereade, then, either using bootstrap config (mapping) or the CLI to "add" them as proper networks | 12:56 |
fwereade | dimitern, yeah, exactly | 12:57 |
fwereade | dimitern, figuring out how to expose them tastefully might be interesting | 12:57 |
rick_h_ | bodie_: sorry for the delay but commented on the docs. Great stuff! Let me know if any of my questions don't make sense. | 12:57 |
fwereade | dimitern, the unnamed nets probably shouldn't be part of the default status output | 12:57 |
dimitern | fwereade, ok, so we need 1) Environ.ListNetworks(), which is called at bootstrap and 2) populates the pending networks (hidden for all intents and purposes except for ->), 3) CLI/API add-network --existing to promote them to real networks | 12:58 |
fwereade | dimitern, yeah, sgtm | 12:59 |
fwereade | dimitern, down the road we'll also want to poll the provider for network changes I think | 12:59 |
dimitern | fwereade, and 4) a way to define the mapping before bootstrapping | 12:59 |
fwereade | dimitern, yeah | 12:59 |
fwereade | dimitern, definitely talk to me in detail before starting on (4) | 12:59 |
dimitern | fwereade, right, we can have even a CLI like update-networks to do it manually at first | 13:00 |
fwereade | dimitern, I think that'd be just as hard as a worker that polls them tbh | 13:00 |
dimitern | fwereade, ok, i'll start with proposing the subbed-out ListNetworks and then we should decide which providers should get an implementation | 13:00 |
fwereade | dimitern, cool | 13:01 |
dimitern | s/subbed/stubbed | 13:01 |
dimitern | fwereade, ok, thanks! | 13:01 |
sinzui | The removal of instance dns-name broke CI. We need to rethink how to make the tests work without it | 13:03 |
mgz | sinzui: what's the breakage exactly? | 13:04 |
sinzui | mgz get_machine_dns_name helper never gets the info it needs to to verify the local stack is ready to confirm the charms are in a sane state | 13:05 |
mgz | oh, that's actual breakage | 13:07 |
sinzui | mgz, the tests need to learn the real host name so that we can ssh in even when juju ssh is busted | 13:07 |
mgz | andrews pr also removed the dns-name field from status? | 13:07 |
mgz | did no one object to that in review? | 13:07 |
sinzui | mgz, it was intentionally | 13:07 |
* mgz goes back and looks | 13:07 | |
mgz | I'd be surprised if we broke status compat intentionally | 13:10 |
mgz | and it doesn't seem like that... | 13:10 |
* sinzui needs to bootstrap to see what the status really look like | 13:10 | |
sinzui | mgz, the specific call is status['machines'][str(machine)] | 13:10 |
mgz | right, I'm looking at that | 13:10 |
sinzui | we are looking for the bootstrap nodes data | 13:11 |
mgz | the line after is .get('dns-name') | 13:11 |
mgz | which should absolutely still work | 13:11 |
sinzui | the joyent failure was joyent cloud's fault | 13:12 |
* sinzui disables CI | 13:14 | |
sinzui | mgz, I recall we use dns-name because aws wont let us use the ip address. | 13:15 |
axw | my change was only removing dead code. there have been some changes to "juju status", but I didn't think they had landed yet | 13:16 |
bodie_ | rick_h_, cool | 13:16 |
mgz | sinzui: do you have blame on a specific revision? | 13:16 |
sinzui | mgz Merge pull request #38 from axw/remove-instance-dnsname | 13:17 |
mgz | axw: I'll investigate, go to bed :) | 13:17 |
sinzui | mgz, 3 revs ago 4e7b0471 | 13:17 |
perrito666 | back | 13:18 |
bodie_ | rick_h_, I actually made a separate PR against the juju/juju/docs docs (as opposed to the web docs) | 13:18 |
bodie_ | https://github.com/juju/juju/pull/46 | 13:18 |
sinzui | only local lxc is affected. kvm is fine | 13:18 |
mgz | o_O | 13:18 |
mgz | no such request "FullStatus" on client - we also broke status compat? | 13:19 |
mgz | or was this a really old deploy I have here... | 13:19 |
bodie_ | anyone have any thoughts on this test issue? I'm trying to hit an external URL to get my schema definition: http://paste.ubuntu.com/7623520/ | 13:23 |
bodie_ | why would juju testing runs be disallowed external access? | 13:24 |
natefinch-afk | bodie_: tests that depend on the internet are a bad idea | 13:24 |
mgz | bodie_: SERIOUSLY? | 13:24 |
=== natefinch-afk is now known as natefinch | ||
mgz | er, too much caps | 13:24 |
natefinch | haha | 13:24 |
mgz | but, seriously? | 13:24 |
bodie_ | *wipes sweat off brow* | 13:25 |
natefinch | man, for a minute, I thought mgz was super mad | 13:25 |
mgz | you want your test runs to require an internet connection to pass? | 13:25 |
bodie_ | lol, when you put it that way... | 13:25 |
wwitzel3 | natefinch: haha, me too | 13:25 |
mgz | somehow nick-tab-shift-for-colon makes me it capslock way too often | 13:26 |
bodie_ | it's not the test itself that depends on the internet connection -- one of the features of json-schema is to reference schema specs by URL | 13:27 |
bodie_ | so, I'm thinking that's not one of the features we want to make use of ;) | 13:27 |
mgz | bodie_: ANYWAY, PRACTICAL ... suggestions, override the url for the tests | 13:27 |
mgz | but having us hit their schema url at all seems like a bad thing | 13:28 |
bodie_ | practically every json-schema is supposed to have a $schema key with a URL value | 13:29 |
alexisb | natefinch, call? | 13:30 |
mgz | sinzui: this is something local-specific, but what exactly I'm not sure yet | 13:30 |
mgz | sinzui: can you log the status output perhaps? | 13:30 |
mgz | bodie_: sure, but why are we resolving the url? | 13:31 |
sinzui | mgz, I see a rouge machine left behind that could be the cause | 13:31 |
mgz | this is like the old w3c's issue with dtd urls | 13:31 |
sinzui | mgz, the ppc64 machine was victim of another test | 13:31 |
sinzui | arm64 failed because the machine is too slow | 13:31 |
mgz | sinzui: well, that'd be a relief | 13:32 |
bodie_ | mgz, well, the idea was to load the json-schema spec itself, which is json-schema, in order to validate our params schemae | 13:32 |
bodie_ | that would be simple enough to inline, I suppose, it just feels ugly. but I guess it's not as ugly as requiring external HTTP GET-ability :) | 13:33 |
mgz | bodie_: http://www.w3.org/blog/systeam/2008/02/08/w3c_s_excessive_dtd_traffic/ | 13:33 |
bodie_ | Hahaha, that's sort of awesomely bad | 13:34 |
bodie_ | nearly all-caps scream-worthy! | 13:34 |
rick_h_ | bodie_: oops, comments still valid? | 13:35 |
bodie_ | rick_h_, awesome comments, I realized in conversation with jcw4 and mgz yesterday that our docs probably have separate scope -- my PR is against the juju/docs/actions.md file so it's not totally clear how much the scope of our docs overlaps | 13:39 |
bodie_ | the public-facing stuff might need to be less technical and more UX oriented, while the ~/docs stuff might need to be more hacker / architecturally oriented | 13:39 |
rick_h_ | bodie_: definitely | 13:40 |
rick_h_ | bodie_: but I <3 that you've got docs on both ends in mind. | 13:40 |
bodie_ | :D | 13:40 |
bodie_ | I really want to get a hackers section pushed up to the public zone, I'd have gotten involved much sooner if I'd realized it was participation-friendly | 13:41 |
bodie_ | whether that's a high-level overview, or just a link to a highly usable github docs made of readable, interlinked markdown | 13:41 |
bodie_ | rick_h_, I do have a few more specific questions oriented to the frontend team that I'd love some input on, in that PR | 13:42 |
bodie_ | but take your time, it'll be there for a while probably | 13:42 |
rick_h_ | bodie_: sure thing, pointers? | 13:42 |
rick_h_ | bodie_: or just want to do a hangout? | 13:42 |
bodie_ | just specifically item 3, frontend hackers, it's all laid out there | 13:43 |
bodie_ | primary questions are, do you guys want a json-schema getter, and any requests for tech specs or API endpoints for you guys | 13:43 |
bodie_ | anything you need to know | 13:43 |
bodie_ | not even totally sure all that belongs in the backend /docs, really just wanted to get conversation started for those issues | 13:44 |
rick_h_ | bodie_: cool yea. I mean basically we need whatever you give the cli user except in a really nice machine readable form. One thing might be to provide a watcher api to the actions as they will be done async always, like bundles. | 13:44 |
bodie_ | so, something like a polling / push mechanism to open a connection so you guys can make a progress bar or some such? | 13:45 |
bodie_ | it might be pretty complicated to get insight into the status of the hook (i.e. the actual action on the unit) while it's running | 13:46 |
bodie_ | but, perhaps we could set up a hook-env level call so the charm author could add such a thing to his action script | 13:47 |
rick_h_ | bodie_: progress bars are evil :P | 13:47 |
bodie_ | heh | 13:47 |
rick_h_ | bodie_: so it's mainly so that we request an action and get back the UUID, then we request a watcher on that UUID so that when it changes we get that notificiation over the websocket | 13:47 |
bodie_ | most of ours at DO were completely fake... ^_^ | 13:47 |
bodie_ | hmm, ok | 13:47 |
rick_h_ | bodie_: and that way we can make sure to get a notification to the user | 13:48 |
rick_h_ | bodie_: that it's completed, here's the status info, or it errored, more info, or it bombed out and crashed | 13:48 |
bodie_ | right | 13:48 |
rick_h_ | but we want it over the websocket vs polling in the GUI | 13:48 |
bodie_ | instead of just polling the status query | 13:48 |
rick_h_ | :( | 13:48 |
rick_h_ | bodie_: polling makes everyone sad when we've got such a nice always on websocket connection sending us messages we can process async | 13:49 |
* bodie_ socks that away in his ponderment satchel | 13:49 | |
rick_h_ | bodie_: but we only want to worry about it while the connection is alive and someone's at the browser listening, so we ask for a watcher on it | 13:49 |
bodie_ | I'm going to ask for the favor of putting some of that in a brief comment on that doc just so there's something for us to look at and be reminded by | 13:50 |
bodie_ | s/doc/pr/ | 13:50 |
bodie_ | however, it's in my awareness | 13:50 |
rick_h_ | bodie_: definitely, I've made a todo to try to spell out in more formal doc form what you've asked for there. | 13:51 |
bodie_ | awesome1 | 13:51 |
bodie_ | ! | 13:51 |
rick_h_ | bodie_: it'll be a bit for me to get down to writing it out, but will try to get it today/tomorrow | 13:51 |
bodie_ | yeah, again this is a longer-term thing I'd like to have people jabbering over, no rush whatsoever | 13:51 |
bodie_ | (ideal goal state, anyway) | 13:52 |
rick_h_ | bodie_: ok, sounds like a plan. Just feel free to get stabby if you don't have the info you need. Don't want to be a blocker on anything | 13:52 |
bodie_ | sure, this is more like a long polling thing | 13:52 |
dimitern | jam, vladk, fwereade, there it is - https://github.com/juju/juju/pull/58 - Environ.ListNetworks() call, PTAL | 13:54 |
bodie_ | mgz -- regarding that schema validation, I'd like to validate our param schemas against the actual json-schema schema itself. do you think it would be best just to inline it in the charm/actions file? | 13:54 |
bodie_ | that seems so ugly | 13:54 |
rogpeppe | frankban, dimitern, mgz: another trivial patch, avoiding the need for charm to import environs/config: https://github.com/juju/juju/pull/59 | 13:55 |
frankban | rogpeppe: looking | 13:55 |
rogpeppe | frankban: thanks | 13:56 |
dimitern | rogpeppe, what's the IsolationSuite? | 13:56 |
wwitzel3 | fwereade: https://github.com/juju/juju/pull/2 | 13:56 |
rogpeppe | dimitern: it's what everything is moving towards using - it isolates all environment variables amongst other things | 13:57 |
rogpeppe | dimitern: it's the new BaseSuite | 13:57 |
rogpeppe | dimitern: but with a slightly more useful name | 13:57 |
dimitern | rogpeppe, right, LGTM then | 13:57 |
rogpeppe | s/useful/self-explanatory | 13:57 |
rogpeppe | dimitern: thanks | 13:57 |
=== rharper_ is now known as rharper | ||
TheMue | jam: made a PR for the planning doc, see https://github.com/juju/juju/pull/60. could you LGTM it so that we can merge it and Nick can add it to j.u.c | 13:58 |
dimitern | rogpeppe, I'd appreciate if you look at a similarly trivial https://github.com/juju/juju/pull/58 | 13:58 |
TheMue | rogpeppe: will review PR 59 | 13:58 |
rogpeppe | dimitern: looking | 13:58 |
TheMue | rogpeppe: oh, already reviewed | 13:59 |
rogpeppe | jeeze, it sometimes takes sooo long to add a github comment | 14:01 |
rogpeppe | ah, finally! | 14:01 |
wwitzel3 | ericsnow: ping if you're around for standup | 14:05 |
rogpeppe | dimitern: reviewed | 14:07 |
dimitern | rogpeppe, thanks! | 14:08 |
jcw4 | bodie_, mgz I think the point of mgz's link to the w3 article is that the uri for the schema is intended as an identifier not as a validation lookup... i.e. that uri refers to a static published schema thats not gonna change... just use your local copy for validation. | 14:12 |
jcw4 | I don't know if we literally do store files like this locally, but it may be worth doing so if we *really* want to validate against that schema for every test run | 14:13 |
bodie_ | right, I get that, I'm trying to separately validate the schema against the json-schema spec, which is encoded as json-schema | 14:14 |
TheMue | anyone else taking a look at PR 60 for me? | 14:14 |
bodie_ | so that we don't accept bad charm schemas that happen to be meaningless but parseable as jsonschemadocuments | 14:14 |
bodie_ | (which are basically just required to be json) | 14:14 |
bodie_ | maybe I'm barking up the wrong tree, though | 14:15 |
mgz | TheMue: that's big | 14:15 |
mgz | is [TOC} | 14:15 |
mgz | *[TOC] at the top actually meant to do anything? | 14:15 |
jcw4 | bodie_: agreed... not sure if 'accepting bad charm schemas' is what we test in the tests though... | 14:15 |
bodie_ | I think [TOC] isn't parsed by github markdown | 14:16 |
TheMue | mgz: will be replaced by a table of contents by the processor | 14:16 |
mgz | TheMue: are we also sure we want all this in the tree? natefinch linked something earlier and had to private-ize it after | 14:16 |
TheMue | mgz: nick has a toolchain to generate HTML for juju.ubuntu.com out of it | 14:16 |
bodie_ | jcw4, I guess the question I'm asking is whether charms can define bad schemas which we then try to use for validation and never work | 14:17 |
mgz | TheMue: I hit render and github didn't do any magic | 14:17 |
TheMue | mgz: it is sanitized | 14:17 |
mgz | k | 14:17 |
bodie_ | I'm not certain this kind of critter exists at all though, since validation is going to be in the state scope | 14:17 |
rogpeppe | bodie_: i wouldn't inline the schema, but i'd suggest putting it into a file and reading it at test time | 14:17 |
TheMue | mgz: nick uses the python markdown | 14:17 |
jcw4 | rogpeppe: +1 | 14:18 |
rogpeppe | bodie_: if you wanted, you could even set up a trivial local http server and change urls to point to that | 14:18 |
bodie_ | hmm, that's not a bad idea | 14:18 |
rogpeppe | bodie_: it's an approach we already use in several places | 14:19 |
bodie_ | rogpeppe, well, it's not for the testing, it's actually a validation step at the time the charm loads its Actions schema from YAML | 14:19 |
rogpeppe | bodie_: surely your code should be able to verify that the schema is valid? | 14:20 |
bodie_ | it happened to be breaking the tests because it wanted http access for that method | 14:20 |
rogpeppe | bodie_: or are we allowing charms to reference arbitrary schemas over the internet? | 14:20 |
bodie_ | well, that's part of the JSON-Schema spec | 14:20 |
rogpeppe | bodie_: if so, i might suggest that's perhaps not a great idea | 14:21 |
bodie_ | yeah, hehe. got that input from mgz as well, and it makes sense | 14:21 |
sinzui | mgz, local host breaks were NOT juju's fault. each test was broken by something else, and the report of what broke co-incidentally matched test in the commit under test. | 14:21 |
rogpeppe | bodie_: i think it's reasonable to restrict schemas to vanilla schemas that don't require further dereferences | 14:21 |
rogpeppe | bodie_: and... surely... it's possible to generally transform a schema-with-references to a schema-without-references? | 14:22 |
bodie_ | so you're saying that if there's a $schema key in the schema, it should be rejected? | 14:22 |
bodie_ | or simply stripped | 14:22 |
sinzui | mgz: killing the rogue mongo was hard. I had to delete all the directories it used to get kill to really stop the service | 14:22 |
rogpeppe | bodie_: probably rejected is better | 14:22 |
mgz | sinzui: yeah, it's really painful | 14:23 |
mgz | sinzui: and one of the main issues with using a persistent machine for tests | 14:23 |
bodie_ | rogpeppe, I don't think the $schema key is actually loaded, rather used as an identifier for the schema version -- as mgz mentioned, much like the WC3 DTD's referenced in the DOCTYPE | 14:24 |
sinzui | joyent is still ill. I don't want it to curse this revision | 14:24 |
TheMue | mgz: thx for review, will talk to nick how we can handle title and toc better in future | 14:24 |
rogpeppe | bodie_: so schemas can't reference arbitrary other sub-schemas that define parts of the schema? | 14:25 |
bodie_ | well, no, they can... | 14:25 |
bodie_ | the reason I'm hitting the issue is that I'm actually separately attempting to validate the loaded schema against json-schema itself; making references to the inner json-schema keys is not problematic, and I don't think external references are necessarily loaded (my test cases were passing with $schema URL keys defined) | 14:26 |
bodie_ | however, since we haven't gotten to the point where we're actually using the Validate method on any legit JSON objects, it's not totally clear yet since I haven't deeply grokked the spec itself yet | 14:27 |
bodie_ | it's possible that when Validate is run, those URLs try to resolve | 14:28 |
bodie_ | the gojsonschema stuff I wrote doesn't touch that part, it's more to do with properly breaking apart reference URLs into constituent bits | 14:28 |
bodie_ | right now, the reason my code is GETting is because I'm actually attempting to retrieve the remote definition for JSON-Schema itself, which I understand is a bad idea and I should probably store it as a resource in juju if I want to do that | 14:30 |
dimitern | rogpeppe, https://github.com/juju/juju/pull/58#discussion_r13596383 | 14:30 |
rogpeppe | dimitern: hmm | 14:31 |
rogpeppe | dimitern: what code does the public/private detection? | 14:31 |
dimitern | rogpeppe, network.SelectPublicAddress | 14:32 |
dimitern | rogpeppe, but it will need improvement | 14:32 |
dimitern | rogpeppe, it's all part of the "we should automatically do amazing things" approach :) | 14:33 |
rogpeppe | dimitern: SelectPublicAddress doesn't look at the IP address itself | 14:33 |
rogpeppe | dimitern: just having any real IP addresses makes me think "potential isolation issue" | 14:34 |
dimitern | rogpeppe, it's hidden in a couple of helpers below - internalAddressMatcher etc. | 14:34 |
dimitern | rogpeppe, this ip range is isolated by design | 14:35 |
dimitern | rogpeppe, and since it won't reach anything, if we happen to use them inadvertently we'll notice at once | 14:35 |
rogpeppe | dimitern: i bet i can route to those addresses on my machine | 14:35 |
bodie_ | rogpeppe, thoughts on that? I'd really like to either ditch the validation or make a final call here so I can move on to the watcher/uniter stuff | 14:36 |
rogpeppe | dimitern: BTW when would anything be legimitately using network.NewAddress on the addresses returned from ListNetworks? | 14:37 |
dimitern | rogpeppe, it won't be used directly | 14:37 |
bodie_ | if rogpeppe / mgz / etc think I need to enforce that JSON-Schema specs not include $schema keys, I can do that, but this is like the 5th thing that's pushed me back into this single file that should have been done ages ago -- I'm perfectly happy to make it flawless but I'm also antsy to get a product built here | 14:37 |
dimitern | rogpeppe, it will be stored in state as "pending" or "potential" networks, and the user then can say - i want to use this as my private/public default network | 14:38 |
rogpeppe | dimitern: i don't quite get it then | 14:38 |
dimitern | rogpeppe, or further down the line, we'll use ListNetworks to update what we know about | 14:39 |
rogpeppe | bodie_: so the issue is you need the schema for JSON-Schema in your tests? | 14:39 |
pindonga | hi, anyone around to help troubleshoot some issues with the local provider? | 14:39 |
mgz | pindonga: have you tried turning it off and on again? | 14:40 |
pindonga | several times (per hour) | 14:40 |
mgz | pindonga: (more seriously, try dimitern's script for wiping out left over junk, it's helped before) | 14:40 |
pindonga | mgz, do you have a link to it? | 14:40 |
rogpeppe | dimitern: i think that for testing specific logic that automatically derives public/privateness from network CIDR addresses, it's fine to use addresses that tweak that logic. but for the default networks returned by the dummy provider, i don't think so | 14:41 |
dimitern | pindonga, http://blog.naydenov.net/2014/03/remove-juju-local-environment-cleanly/ | 14:41 |
dimitern | pindonga, you might need to tweak it a bit | 14:41 |
pindonga | dimitern, thx, will take a look | 14:41 |
bodie_ | rogpeppe, negative, I'm attempting to validate the parsed schemas from YAML as useful JSON-Schema, by using gojsonschema's Validate method against the spec doc, which was at the remote URL -- so that's getting triggered by the tests and failing, which makes perfect sense. I can simply ditch that bit, or keep the doc as a local file; I was under the impression you were saying I should enforce a constraint against references in the schemas themselves | 14:41 |
dimitern | rogpeppe, so how's 0.1.2.0/8 better than 203.0.113.0/24 ? | 14:41 |
cmars | jam, i'm wondering if I should just defer Machine.Destroy in Unit.Destroy, and let the machine's advanceLifecycle checks sort out whether the machine is clean (re: https://github.com/juju/juju/pull/52) | 14:42 |
rogpeppe | dimitern: because it gets immediately rejected by the network stack | 14:42 |
rogpeppe | dimitern: try it | 14:42 |
bodie_ | i.e. oneTrueJsonSchemaSpec.Validate(userDefinedSchema) | 14:42 |
cmars | seems a simpler way to go about it, what do you think? | 14:42 |
rogpeppe | dimitern: e.g. try "telnet 203.0.113.3" and "telnet 0.1.2.3" | 14:42 |
bodie_ | where oneTrueJsonSchemaSpec is loaded from a URL now, which is causing the issue; rather, it should be loaded from a local resource, which I get | 14:43 |
jam1 | rogpeppe: of course the downside is that it fails with a different error | 14:43 |
rogpeppe | jam1: it *should* never be actually used | 14:43 |
rogpeppe | jam1: but better that it fails immediately, regardless of the error | 14:43 |
rogpeppe | jam1: rather than timing out (possibly, depending on external network state) after a minute or so | 14:44 |
jam1 | cmars: so… I probably prefer checking before calling Destroy, because otherwise you end up with something like errors/warnings because destroy realizes it can't be destroyed | 14:45 |
rogpeppe | bodie_: i am +1 on avoiding references in the schemas themselves (and i assume that gojsonschema does currently have code in it to go out to fetch stuff from remote URLs) | 14:45 |
dimitern | rogpeppe, ok, i'm not sure i want to push for it now, i'll change both cidrs to 0.1.2.0/8 and 0.4.3.0/24 for example | 14:45 |
rogpeppe | bodie_: but i don't think it needs to be done now | 14:46 |
dimitern | rogpeppe, if i need to use actual public ips in tests, will find a workaround | 14:46 |
bodie_ | rogpeppe, I'll know more once we get some Validate() tests written in State | 14:46 |
rogpeppe | dimitern: sgtm. maybe 0.10.0.0 and 0.203.0.0 to be reminiscent of the real things? | 14:46 |
bodie_ | but I definitely see the point of what you're saying | 14:46 |
cmars | jam1, ack. i'll try to set it on a reasonable path to success, without duplicating too many checks | 14:47 |
rogpeppe | bodie_: are you still planning to replace gojsonschema? | 14:47 |
bodie_ | I'm just getting ridiculously stir-crazy here in charm/actions.go | 14:47 |
rogpeppe | bodie_: ha ha | 14:47 |
jam1 | cmars: I'm happy to use a common helper that does the checks | 14:47 |
bodie_ | ah | 14:47 |
rogpeppe | dimitern: thanks | 14:47 |
bodie_ | rogpeppe, I went through that some with fwereade -- basically, there were some really hackish workarounds in gojsonreference / gojsonpointer | 14:48 |
rogpeppe | bodie_: it's not nice code :-) | 14:48 |
bodie_ | it was glossing over some test issues and simply faking good results on others | 14:48 |
bodie_ | so, I gutted that stuff and replaced it with much more idiomatic go | 14:48 |
rogpeppe | bodie_: it looks to me as if it's vulnerable to being crashed too | 14:48 |
bodie_ | gojsonschema itself looks pretty solid | 14:48 |
bodie_ | it was its dependencies that were more half-assed | 14:49 |
bodie_ | afaict, anyway | 14:49 |
rogpeppe | bodie_: i was pretty dubious about gojsonschema actually, but i haven't actually tried to implement it myself, so i guess it may be ok really... | 14:49 |
bodie_ | we're now using the rewritten deps from my personal github, I haven't opened a PR against xeipuuv's codebase since he was unresponsive to an Issue on the topic | 14:50 |
rogpeppe | bodie_: if we want to just mutate gojsonschema, i'd be happy to send a review pointing out some obvious ways that i think it could look a bit better | 14:50 |
bodie_ | I'm definitely open to that, right now I'm just getting anxious about actually pushing product on Actions since we're getting pretty overdue on our expected timeline | 14:51 |
bodie_ | but, obviously we also don't want to deliver half-baked code :) | 14:51 |
rogpeppe | bodie_: fair enough | 14:52 |
rogpeppe | bodie_: it should be an easy-enough dependency to change later | 14:52 |
bodie_ | yeah, I think so | 14:52 |
* ericsnow starts day 2 of the firehose | 14:53 | |
rick_h_ | ericsnow: hope you're thirsty! | 14:54 |
bodie_ | in the meantime, what's the best place for storing resource type files like jsonschema-draft4.json? | 14:54 |
bodie_ | i.e., is there a canon location for resources, or should it just go in the package folder | 14:55 |
rogpeppe | frankban, dimitern: another ultra-trivial PR: https://github.com/juju/juju/pull/62 | 14:55 |
dimitern | rogpeppe, No description provided? | 14:56 |
rogpeppe | dimitern: isn't the subject line enough? | 14:56 |
dimitern | rogpeppe, LGTM | 14:56 |
rogpeppe | dimitern: ta! | 14:56 |
natefinch | see, dimitern gets it ;) | 14:56 |
dimitern | rogpeppe, it's nice to know what it is about :) | 14:56 |
dimitern | (i.e. more than a one-liner) | 14:57 |
rogpeppe | dimitern: sorry, thought it was obvious | 14:57 |
rogpeppe | dimitern: will provide a description too | 14:57 |
dimitern | thanks! | 14:57 |
ericsnow | natefinch: were you able to locate that meeting calendar? | 14:59 |
ericsnow | BTW, does this channel have public logs somewhere? | 15:00 |
natefinch | I think freenode logs stuff, but I don't know where | 15:00 |
natefinch | ericsnow: crud, no I forgot about the calendar. alexisb - do you know how to get eric access to the team calendar? | 15:01 |
perrito666 | heh https://github.com/search?l=go&q=if+err+!%3D+nil&type=Code | 15:03 |
jcw4 | ericsnow: http://irclogs.ubuntu.com/ | 15:03 |
ericsnow | jcw4: thanks | 15:04 |
natefinch | ericsnow: would you like to jump on a hangout? might be helpful for questions and stuff | 15:09 |
ericsnow | sure, if you have a minute | 15:10 |
natefinch | ericsnow: https://plus.google.com/hangouts/_/canonical.com/moonstone?authuser=1 | 15:11 |
pindonga | mgz, I've run dimitern's cleanup script, and I'm still getting the same error over and over again... when I deploy the charm, I get | 15:12 |
pindonga | ERROR juju runner.go:220 worker: exited "deployer": exec ["start" "--system" "jujud-unit-click-appstore-api-0"]: exit status 1 (start: Unable to connect to system bus: Failed to connect to socket /var/run/dbus/system_bus_socket: No such file or directory) | 15:12 |
pindonga | and the unit never finishes starting | 15:12 |
mgz | pindonga: sounds like a privileges issue | 15:13 |
pindonga | well, I shouldn't run juju as root, should I ? :) | 15:14 |
pindonga | and that file is owned by root | 15:14 |
pindonga | mhh, in any case, the juju and lxc processes *are* running as root | 15:15 |
mgz | pindonga: you are on what release, which what kernel and lxc versions? | 15:15 |
pindonga | trusty | 15:15 |
pindonga | 3.13.0-29-generic | 15:15 |
pindonga | 1.0.3-0ubuntu3 | 15:15 |
bodie_ | PR: Charm now validates YAML-loaded Actions schema https://github.com/juju/juju/pull/63 | 15:16 |
pindonga | juju is 1.18.1-trusty-amd64 | 15:16 |
natefinch | ericsnow: are you joining? | 15:16 |
bodie_ | imo, you guys should have the channel bot call out fresh PR's | 15:16 |
bodie_ | :) | 15:16 |
ericsnow | natefinch: I'm on | 15:16 |
alexisb | natefinch, let me see if I can add him, jam1/thumper has always added folks | 15:16 |
natefinch | alexisb: thanks | 15:16 |
natefinch | ericsnow: hmm... me too. try refreshing? | 15:17 |
mgz | pindonga: http://askubuntu.com/questions/399382/juju-local-failed-with-var-run-dbus-system-bus-socket-no-such-file-or-director | 15:17 |
ericsnow | natefinch: hang on a sec | 15:18 |
rogpeppe | frankban: so, the charm package now has no external dependencies other than inside charm itself (charm/hooks and charm/testing). i'm going to create a new package github.com/juju/charm, and move charm there, then change core to use it. | 15:19 |
rogpeppe | frankban, fwereade, natefinch, mgz, jam1: does that sound reasonable? | 15:19 |
frankban | rogpeppe: sounds good | 15:19 |
rogpeppe | bodie_: this will affect you | 15:20 |
bodie_ | cool | 15:20 |
mgz | rogpeppe: yurp | 15:20 |
fwereade | rogpeppe, +1 to that, and thanks for warning bodie_ | 15:20 |
rogpeppe | fwereade: thanks | 15:20 |
pindonga | mgz, thx but sorry, dbus was installed :) | 15:20 |
rogpeppe | fwereade: one possibly controversial thing is that it will involve moving the testing charm directory out of core, but i think that should be ok | 15:21 |
fwereade | rogpeppe, one thought, I think there's a subtle interdependency between charm and names | 15:21 |
mgz | pindonga: did you try purging and reinstalling it like that guys did? | 15:21 |
fwereade | rogpeppe, that's fine by me | 15:21 |
rogpeppe | fwereade: names is already external | 15:21 |
fwereade | rogpeppe, ah, fantastic, I hadn't spotted that | 15:21 |
bodie_ | rogpeppe, I did just open a PR against charm | 15:21 |
bodie_ | it's a pretty simple change... | 15:21 |
pindonga | mgz, purging dbus? no I haven't yet... although this is a clean trusty image... it shouldn't change anything (I will test it anyway) | 15:21 |
rogpeppe | bodie_: cool. i'll merge that in before factoring out everything | 15:22 |
rogpeppe | bodie_: link? | 15:22 |
bodie_ | https://github.com/juju/juju/pull/63 | 15:22 |
bodie_ | ergh, it probably still has some crappy comments and mess left | 15:23 |
bodie_ | maybe you should just go ahead and I'll reopen it once you're finished | 15:23 |
bodie_ | there, corrections are in | 15:25 |
mgz | pindonga: otherwise you'll need to dive into permissions, this seems like it is mostly outside of juju itself | 15:31 |
pindonga | mgz, am re-running everything now after reinstalling dbus... thx | 15:32 |
pindonga | will keep looking | 15:33 |
bodie_ | rogpeppe, fixes are in | 15:38 |
rogpeppe | bodie_: still reviewing, sorry... | 15:38 |
bodie_ | oops, didn't see some other comments, heh | 15:38 |
rogpeppe | bodie_: (there's no way to send all the comments at once, unfortunately) | 15:38 |
bodie_ | no worries, I can commit as you go I think | 15:38 |
rogpeppe | bodie_: i don't quite see why you want to verify against the schema definition - shouldn't gojsonschema be doing that when it parses it? | 15:39 |
bodie_ | rogpeppe, I don't think it is, I think it's just defining a well-typed map (i.e. JSON) and assuming it's json-schema, but I could be wrong | 15:41 |
rogpeppe | bodie_: well, IMO, it *should* verify. but i can see that you might want an additional verification step for the time being. | 15:41 |
rogpeppe | bodie_: i'm sorry, i was under the impression that you needed the json schema doc for testing purposes only | 15:42 |
bodie_ | there was some talk Monday with Jeremy (JSON-Schema guy) about how / where we're going to do validation and how we need to think about schemas -- I realized most of the sample schemas we were using weren't really proper json-schema at all | 15:42 |
rogpeppe | bodie_: reading the schema from a file isn't really acceptable in prod | 15:42 |
bodie_ | gotcha | 15:42 |
rogpeppe | bodie_: so, i'd suggest just embedding the text | 15:43 |
bodie_ | all righty | 15:43 |
rogpeppe | bodie_: in a file | 15:43 |
rogpeppe | bodie_: sorry, | 15:43 |
rogpeppe | bodie_: in a Go file | 15:43 |
rogpeppe | bodie_: also, you don't want to be parsing it every time | 15:43 |
rogpeppe | bodie_: so perhaps have an init-time statement which parses it | 15:43 |
rogpeppe | bodie_: and stores it in a global var | 15:44 |
rogpeppe | bodie_: alternatively (and perhaps better) use a sync.Once to do that once, the first time it's required | 15:44 |
alexisb | ericsnow, I shared the team calendar with you | 15:45 |
rogpeppe | bodie_: i'll add a comment suggesting that last | 15:45 |
ericsnow | alexisb: thanks! | 15:45 |
bodie_ | rogpeppe, all righty :) | 15:45 |
bodie_ | rogpeppe, brb, taking the girls down to the car | 15:46 |
rogpeppe | bodie_: k | 15:47 |
rogpeppe | bodie_: i've gone ahead with creating the new repo | 16:24 |
rogpeppe | bodie_: you should find it quite easy to patch your changes onto it | 16:24 |
bodie_ | great, just finishing up with mgz and jcw4 here | 16:24 |
rogpeppe | frankban, fwereade, bodie_: initial commit of new charm repo: https://github.com/juju/charm/pull/1 | 16:24 |
frankban | rogpeppe: done | 16:31 |
frankban | mgz: https://github.com/juju/juju/pull/61 passed CI tests but did not get merged | 16:32 |
mgz | frankban: looking | 16:32 |
frankban | mgz: thanks | 16:32 |
mgz | gah, it should have got reported back... | 16:33 |
frankban | mgz: so, I guess I need to merge trunk, right? | 16:34 |
mgz | frankban: I'm still not sure on that | 16:34 |
mgz | sometimes it just seems to work the second time | 16:34 |
frankban | mgz: should I just try $$merge$$ again? | 16:35 |
mgz | frankban: I'll sort it | 16:35 |
frankban | mgz: thanks | 16:35 |
mgz | does the lander not return a non-zero exit code whe it gets an exception or summat... | 16:36 |
mgz | apparently not | 16:37 |
mgz | ret = ... print ret | 16:37 |
* mgz fixes | 16:37 | |
rogpeppe | frankban: added testing repo: https://github.com/juju/charm/pull/2 | 16:40 |
rogpeppe | pwd | 16:40 |
frankban | rogpeppe: why not preserving history in this case? not worth the effort? | 16:43 |
rogpeppe | frankban: yeah | 16:43 |
rogpeppe | frankban: it's in github.com/juju/juju anyway, if we need it | 16:43 |
rogpeppe | pwd | 16:43 |
frankban | rogpeppe: pwd exit status: LGTM | 16:45 |
rogpeppe | frankban: :-) | 16:45 |
mattyw | fwereade, do you have a moment? | 16:59 |
=== natefinch is now known as natefinch-afk | ||
alexisb | mattyw, I think it is late for fwereade | 17:25 |
alexisb | although it is late for you too :) | 17:25 |
mattyw | alexisb, I was watching some of the UDS sessions to I totally lost track of time | 17:25 |
mattyw | alexisb, but I don't think I need him at the moment - but I don't have a way of cancelling the ping :) | 17:26 |
alexisb | :) | 17:26 |
alexisb | there should be an unping | 17:26 |
jcw4 | are the UDS sessions recorded and/or publicly available? | 17:40 |
mgz | jcw4: they are | 17:41 |
alexisb | jcw4, http://summit.ubuntu.com/uos-1406/all/ | 17:42 |
jcw4 | cool, tx alexisb | 17:42 |
alexisb | yep | 17:42 |
TheMue | mgz: any idea why my branch doesn’t merge with $$merge$$? | 17:56 |
=== BradCrittenden is now known as bac | ||
fwereade | mattyw, I'm briefly here if yu need me | 18:19 |
voidspace | hey folks, I gotta EOD | 18:28 |
voidspace | g'night all | 18:28 |
ericsnow | voidspace: bye :) | 18:29 |
mattyw | fwereade, it can wait till tomorrow, thanks anyway | 18:35 |
=== natefinch-afk is now known as natefinch | ||
natefinch | ericsnow: how goes? | 19:43 |
ericsnow | natefinch: trying to figure out why your change to CONTRIB. isn't showing up in git log for me | 19:44 |
natefinch | no idea | 19:45 |
natefinch | do you actually have the change locally? | 19:45 |
ericsnow | no...but it shows up if I git show the hash | 19:46 |
ericsnow | I'm sure it's a git workflow thing I'm missing | 19:47 |
natefinch | ericsnow: probably. Did you pull or fetch? | 19:47 |
ericsnow | natefinch: fetch (and then checkout) | 19:48 |
jcw4 | ericsnow: checkout what? | 19:49 |
natefinch | I think it's because you have to fetch and merge, not fetch and checkout. pull is fetch + merge | 19:49 |
natefinch | if you're not on the same branch as trunk | 19:50 |
natefinch | master... whatever they call it in git | 19:50 |
ericsnow | then I'm misunderstanding merge | 19:50 |
menn0 | morning ppl | 19:51 |
jcw4 | menn0: o/ | 19:51 |
natefinch | morning menn0 | 19:51 |
menn0 | jcw4: \o | 19:51 |
bodie_ | can someone explain to me what rogpeppe was thinking with the underscore-named var here? https://github.com/juju/juju/pull/63#discussion_r13601842 | 19:52 |
bodie_ | I think I've f'd something up with the way I implemented this, because it's going into an unterminated recursion and exploding | 19:53 |
natefinch | bodie_: the underscore variable is a special variable that says "throw away this data" | 19:53 |
ericsnow | yep, it was just the whole merge thing | 19:53 |
natefinch | bodie_: so in this case, whatever data the function returns, he doesn't care, he just wants to see the error | 19:54 |
jcw4 | in this case it's an underscore prefix, not just underscore | 19:54 |
natefinch | oh sorry, I was looking at the code not the comment | 19:54 |
natefinch | no, that's horrible, don't do that | 19:55 |
bodie_ | natefinch, er, it's not the _ pattern matching variable | 19:55 |
natefinch | bodie_: you mean _jsonMetaSchema | 19:55 |
bodie_ | yeah | 19:55 |
bodie_ | I'll be back in 60 seconds | 19:55 |
natefinch | I think you need to replace _jsonMetaSchema with metaSchema and the code makes sense | 19:57 |
natefinch | I think he changed his variable name halfway through and didn't fix it up at the top | 19:58 |
bodie_ | I see | 19:59 |
bodie_ | natefinch, I'm getting what looks to be a stack explosion and I'm not clear why | 20:00 |
natefinch | does it involve a String() function? | 20:00 |
bodie_ | I don't think so, why? | 20:01 |
bodie_ | I'm really hoping it's not a recursion caused by this schema definition referencing itself | 20:01 |
natefinch | common mistake is to do fmt.Printf("%s", myVal) inside myVal's String() function, which then calls myVal's String() funciton.... | 20:01 |
natefinch | bam, infinite recursion and stack blowupedness | 20:02 |
bodie_ | hmmm | 20:02 |
bodie_ | I don't think so | 20:03 |
natefinch | it was just a stab in the dark | 20:03 |
bodie_ | natefinch, my best thought is that somehow, the Do is triggering itself | 20:03 |
natefinch | is it in the code that you just linked to, or somewhere else? | 20:03 |
jcw4 | bodie_: do you have the crash stack? | 20:03 |
bodie_ | http://golang.org/pkg/sync/#Once.Do | 20:03 |
bodie_ | jcw4, I have | 20:03 |
bodie_ | https://github.com/binary132/charm/blob/actions-validation-fixes/json_schema.go | 20:04 |
bodie_ | (jcw4, natefinch) | 20:04 |
jcw4 | bodie_: panic / stack trace? | 20:07 |
bodie_ | http://paste.ubuntu.com/7625313/ fwiw, jcw4 | 20:08 |
jcw4 | bodie_: it looks to me like you're validating a json doc, which references the json schema at "http://json-schema.org/draft-04/schema#", and your trying to validate that, which references itself | 20:12 |
jcw4 | the stack trace shows recursive calls between parseReference and parseSchema | 20:13 |
bodie_ | I see, yeah | 20:16 |
bodie_ | jcw4, well, I'm not trying to validate the "meta-schema" | 20:17 |
bodie_ | just to load it into a JsonSchemaDocument | 20:17 |
=== hatch__ is now known as hatch | ||
bodie_ | so, it shouldn't even be referencing itself.... referencing the remote document wasn't ideal, but it shouldn't be recursive | 20:18 |
bodie_ | it was working fine as a file:// reference | 20:19 |
bodie_ | sigh | 20:19 |
jcw4 | :) | 20:25 |
jcw4 | or maybe it should be :( | 20:25 |
bodie_ | https://github.com/juju/juju/pull/63#discussion_r13601842 -- I think this is my EOD, so.... take care gentlemen. I'll see you in the morning jcw4 | 20:28 |
jcw4 | ttyl bodie_ | 20:28 |
bodie_ | correction, https://github.com/juju/charm/pull/3 | 20:50 |
bodie_ | (rogpeppe) | 20:50 |
=== vladk is now known as vladk|offline | ||
=== vladk|offline is now known as vladk | ||
=== vladk is now known as vladk|offline | ||
=== vladk|offline is now known as vladk | ||
=== vladk is now known as vladk|offline | ||
* thumper realises he left irc connected all night | 21:25 | |
rick_h_ | thumper: isn't that how it works? | 21:29 |
thumper | rick_h_: I try to actually disconnect | 21:29 |
* thumper needs to pull out of this refactoring dive RSN™ | 21:45 | |
cmars | thumper, i need to restart chrome, just a minute | 22:01 |
thumper | cmars: ack | 22:01 |
perrito666 | has anyone used archive/tar? | 22:08 |
ericsnow | perrito666: a bit | 22:09 |
perrito666 | I have implemented a tarGz that suits my current needs but I find out that I do not know how to add folders, I add the header but then in the body part I am not sure what goes | 22:10 |
ericsnow | perrito666: using the tar command? | 22:11 |
perrito666 | ericsnow: nope, te archive/tar builtin from go | 22:19 |
ericsnow | perrito666: sorry then | 22:19 |
perrito666 | heh no prob | 22:19 |
perrito666 | how are you going so far? | 22:20 |
ericsnow | good | 22:20 |
menn0 | thumper: what's the easiest way I can set up a HA env to mess with? Canonistack, AWS or is it possible on my own machine? | 22:20 |
ericsnow | working up a patch to CONTRIBUTING | 22:20 |
menn0 | thumper: I want to experiment with some mongo backup/restore ideas in relation to schema upgrades | 22:20 |
ericsnow | it's very meta, following the workflow while working on the doc on the workflow :) | 22:21 |
perrito666 | menn0: for me aws has been the easiest | 22:22 |
thumper | menn0: otp | 22:34 |
menn0 | menn0: no worries. perrito666 is helping and we can discuss further in the standup. | 22:35 |
wallyworld | fwereade: not sure if you're still around - wrt TransactionRunner embedded in State struct. The new storage stuff does indeed maintain an instance of a txn.TransactionRunner interface. That though is orthogonal as to whether State struct embeds the interface or not. It is embedded as of now because previously the methods that have been moved to the TransactionRunner previously were on State itself, so the refactoring is simplified . | 22:47 |
wallyworld | I can make it an attr though if you prefer | 22:47 |
waigani | http://juju-ci.vapour.ws:8080/job/github-merge-juju/88/console tests pass but my pull request "is not mergeable"? | 22:53 |
waigani | I'll merge upstream and see if there are any conflicts | 22:55 |
davecheney | waigani: that is 'cos you don't have permission | 22:56 |
davecheney | use $$merge$$ and the bot will do it | 22:56 |
davecheney | i think that is the answer | 22:56 |
waigani | davecheney: the bot is doing it | 22:56 |
davecheney | waigani: so everything is ok ? | 22:56 |
waigani | davecheney: no, I mean the bot is TRYING to do it: https://github.com/juju/juju/pull/22 | 22:57 |
ericsnow | I've put up a patch for cleaning up the CONTRIBUTING doc (PR #65, Bug #1328716) | 23:03 |
_mup_ | Bug #1328716: CONTRIBUTING should be cleaned up a bit <juju-core:New for ericsnowcurrently> <https://launchpad.net/bugs/1328716> | 23:03 |
davecheney | ericsnow: link to PR > | 23:05 |
davecheney | ? | 23:05 |
ericsnow | davecheney: https://github.com/juju/juju/pull/65 | 23:05 |
ericsnow | davecheney: thanks for taking a look :) | 23:20 |
ericsnow | doc changes are always such an objective affair <wink> | 23:21 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!