| === StevenK_ is now known as StevenK | ||
| cjwatson | apt by-hash is going to need at least https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=798919 fixing before we can use it | 07:15 |
|---|---|---|
| cjwatson | I believe I have all the LP changes done for InRelease, but I need to get some internal mirroring etc. fixes landed first. | 07:16 |
| cjwatson | (All proposed, so I'll bug a webop once I catch one) | 07:16 |
| wgrant | Can we actually deploy mirror fixes atm? | 07:17 |
| wgrant | There was that RT that Adam said might have been progressed or something. | 07:17 |
| cjwatson | Is this https://rt.admin.canonical.com/Ticket/Display.html?id=83847 ? | 07:18 |
| cjwatson | Ha, I think I missed the existence of that charm | 07:18 |
| cjwatson | Is canonical-magic-mirror not being used any more? | 07:18 |
| wgrant | magic-mirror is used for the split | 07:18 |
| wgrant | But we also need to verify that the charm syncs InRelease last. | 07:19 |
| wgrant | As it wasn't doing with Translations-* until that ticket. | 07:19 |
| cjwatson | Right | 07:20 |
| wgrant | The charm doesn't do any weird stuff like magic-mirror, just the two-stage sync. | 07:20 |
| wgrant | cjwatson: Did you look into Tarmac git support in any depth? | 07:21 |
| wgrant | I don't intend to excise bzrlib's command processing etc. at this stage. | 07:21 |
| cjwatson | wgrant: Only to the extent of satisfying myself that it isn't hard and doing some extra API exports for it | 07:23 |
| cjwatson | zyga said he was working on it, but I think we should time out on that soon. | 07:23 |
| cjwatson | I wonder where https://pastebin.canonical.com/139395/ comes from | 07:25 |
| cjwatson | There are similar files in puppet, but not ports-sync. | 07:25 |
| wgrant | cjwatson: I also intend to add BugBMP tomorrow. | 07:38 |
| wgrant | Initially populated by BugBranches being added, but also manageable explicitly. | 07:38 |
| wgrant | Eventually the diff generator should ask turnip to regexp the new commits to link bugs. | 07:39 |
| wgrant | I think that's the only reasonable approach for git, do you agree? | 07:39 |
| wgrant | cjwatson: Hm, I didn't realise ports-sync was different. | 07:40 |
| wgrant | cjwatson: It may be manually maintained; there are few ports mirrors. | 07:40 |
| cjwatson | BugBMP etc.> Agreed. | 07:41 |
| cjwatson | wgrant: Yeah, appears to be (see #is) | 07:41 |
| wgrant | Sorry, busy collecting popcorn and reading too many channels. | 07:41 |
| wgrant | Haven't caught up. | 07:41 |
| * cjwatson checks whether cdimage's two-stage sync handles InRelease and finds he did that in 2011 | 07:43 | |
| wgrant | Heh | 07:44 |
| wgrant | That's an easy one, then. | 07:44 |
| cjwatson | Also I checked that I couldn't reproduce anything like the original apt InRelease security problems with precise and trusty | 07:47 |
| cjwatson | (Indeed, precise just has InRelease disabled with a big hammer) | 07:47 |
| wgrant | Oh, I didn't realise it was disabled. | 07:47 |
| wgrant | Since Debian was using it by that point. | 07:47 |
| wgrant | Maybe it was disabled while the sensible gpgv usage was fixed. | 07:47 |
| cjwatson | I guess our apt was a while out of sync. | 07:48 |
| cjwatson | Right. | 07:48 |
| wgrant | Rather than trying to parse it out manually. | 07:48 |
| cjwatson | In trusty, if you try tricks like appending an InRelease file, then it ignores the unsigned portions of the file | 07:48 |
| cjwatson | Which feels a bit odd, but not exploitably so | 07:49 |
| wgrant | That sounds like it's doing the right thing. | 07:51 |
| wgrant | It is a little odd, and there is an argument to be made for failing on non-whitespace outside the sig, but it doesn't really worry me. | 07:51 |
| wgrant | I don't remember how I found it the first time, but I'll have a poke around the code too this week. | 07:53 |
| cjwatson | Yeah. Thanks. | 08:01 |
| cjwatson | On apt by-hash, I think we probably can in fact do most of it without the bug above being fixed, just not the Acquire-By-Hash flag in Release. | 08:02 |
| wgrant | Yep | 08:02 |
| wgrant | The fallback is rather unpleasant, but we have little choice for at least a couple of years. | 08:03 |
| cjwatson | It'll need an apt-ftparchive backport (or we could do it in LP directly, but I don't think it's worth it). Fortunately a small one. | 08:03 |
| wgrant | Oh, I didn't realise a-f had support. | 08:03 |
| wgrant | How does it handle GC? | 08:03 |
| cjwatson | Keeps three by default. | 08:04 |
| wgrant | I'd be doing it in LP. | 08:04 |
| wgrant | Because we need to do it for NMAF anyway. | 08:04 |
| cjwatson | True. | 08:04 |
| wgrant | It's not dissimilar from the Release code which we already reimplement. | 08:04 |
| cjwatson | I think we want finer-grained control than a-f offers; it only lets you control the count. | 08:04 |
| wgrant | We currently let a-f write directly to dists, but that's only because it's convenient. | 08:04 |
| cjwatson | (APT::FTPArchive::By-Hash-Keep) | 08:04 |
| wgrant | It would be entirely reasonable to have it write to a temporary directory, then we grab the files and do whatever we want with them. | 08:05 |
| wgrant | I prefer to keep our dependency on a-f to a minimum. | 08:05 |
| cjwatson | Yeah, I guess so. | 08:05 |
| lifeless | cjwatson: since you're hacking on email notifications | 09:34 |
| lifeless | cjwatson: there is one bug (I don't have it to hand) that would make me so so so super happy | 09:34 |
| lifeless | cjwatson: which is that LP always sends mail to *me* even if I'm only indirectly subscribed | 09:35 |
| lifeless | cjwatson: which breaks GMail 'mute' | 09:35 |
| wgrant | lifeless: Just bugmail? | 10:00 |
| wgrant | There's a bug about that, let me find it. | 10:00 |
| lifeless | wgrant: bugmail is ~99% of my LP mail | 10:10 |
| lifeless | wgrant: so even if its not *just*. | 10:10 |
| lifeless | would still be happy happy joy joy schnaaaake time. | 10:11 |
| lifeless | Night all. | 10:11 |
| wgrant | lifeless: Oh sure, just checking there were no other cases. | 10:11 |
| wgrant | If we fix one source and you still whine, that's not good :) | 10:11 |
| wgrant | https://bugs.launchpad.net/launchpad/+bug/138592 | 10:14 |
| mup | Bug #138592: Bug notifications have personal To: header but aren't personal <email> <lp-bugs> <notifications> <Launchpad itself:Triaged> <https://launchpad.net/bugs/138592> | 10:14 |
| cjwatson | lifeless: Mm, that wouldn't be too hard, I guess. Some code review stuff already does something kind of similar (well, it actually does a weird thing where it tries to work out which of the people the notification's being sent to that you're allowed to know about, but I see even less justification for that for bugs than for code) | 11:24 |
| cjwatson | Code at least includes the MP address | 11:24 |
| rpadovani | hey all :-) There is a way to login to local installation with a non-admin user? | 11:31 |
| cjwatson | rpadovani: Use utilities/make-lp-user | 11:31 |
| rpadovani | ty! | 11:31 |
| cjwatson | (You'll want to start a local keyserver first, probably - utilities/start-dev-soyuz.sh is overkill for that but works) | 11:31 |
| cjwatson | wgrant: Answers should behave the same way, I think. | 11:33 |
| wgrant | Hm, indeed. | 12:06 |
| rpadovani | cjwatson, o/ I found a very trivial bug to fix, so I wrote the one line change needed, but I'm a bit confused about how writing the related test. To don't bother you right now, I wrote all in the description of the MR - when you have some spare time, could you please take a look? Thanks so much :-) | 12:41 |
| rpadovani | https://code.launchpad.net/~rpadovani/launchpad/unhide-comments/+merge/270950 | 12:41 |
| cjwatson | rpadovani: You'll want to add an optional comment_owner argument that's passed to makeBugComment as owner=, like TestBugHideCommentControls.getContext just below. Then extend TestMessageVisibilityMixin with a test saying that the comment owner can see the comment. | 12:45 |
| rpadovani | cjwatson, okay, thanks, I try | 12:46 |
| rpadovani | well, I pushed the test, but lp doesn't want to update my branch information. Time to study I think, I'll take a look later | 13:18 |
| cjwatson | rpadovani: I'll keep rescanning it until it works. | 13:18 |
| rpadovani | thanks | 13:19 |
| cjwatson | rpadovani: In fact, it's there now. | 13:21 |
| rpadovani | oh, great | 13:24 |
| rpadovani | hi cjwatson, thanks for your time :-) I have a couple of questions about your review: you wrote hidden comments need admin style - is that the one with gray background? If yes, it's already present, without having to edit BugComment.show_for_admin | 15:49 |
| rpadovani | Another question: how stories are supposed to work? I write what is expected and then write some code that should demonstrate that? | 15:50 |
| cjwatson | rpadovani: Oh, right, the implementation of show_for_admin is different from how I remember. | 15:56 |
| cjwatson | rpadovani: Stories *are* tests. | 15:57 |
| cjwatson | The indented stuff is run. | 15:57 |
| cjwatson | https://docs.python.org/2/library/doctest.html | 15:57 |
| rpadovani | oooh, I see! How much things I have to learn, thanks :-) | 15:57 |
| cjwatson | (They are mostly very irritating and we prefer not to write more of them, but it's OK to extend existing ones a little bit if there isn't an immediate alternative. Rewriting as unit tests is usually better.) | 15:58 |
| cjwatson | rpadovani: I think a pagetest (== .../stories/...) addition is a good idea here even if you don't need to change show_for_admin. | 15:58 |
| rpadovani | ok, great, I add it, thanks | 15:59 |
| mapreri | JOOI (and OT): do you think you'll be able to move to py3 in only 5 years? | 15:59 |
| cjwatson | mapreri: Heh, hopefully. Every so often we shave off another bit of the problem. | 16:00 |
| mapreri | looks like lot of pain from an outsider pov | 16:00 |
| mapreri | good luck :) | 16:01 |
| cjwatson | After we move off buildout, we'll be able to upgrade to a current ZTK, and then it might actually be possible to see how terrible the situation is. | 16:01 |
| cjwatson | Right now it's only possible to attack in the abstract, which isn't very rewarding. | 16:02 |
| cjwatson | Also, we're still mostly deploying on 12.04, and Python 3.2 wasn't a brilliant porting target. | 16:02 |
| mapreri | ic | 16:03 |
| cjwatson | Why do you ask? | 16:05 |
| rpadovani | sorry to bother again, how could I run the story test? Using python -m doctest -v lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.txt doesn't work | 16:05 |
| mapreri | cjwatson: just curiosity (sometimes i really act like a kid, with my curiosity) :) | 16:07 |
| cjwatson | rpadovani: bin/test -vvct lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.txt | 16:08 |
| cjwatson | (or a regex that matches that) | 16:08 |
| rpadovani | thanks | 16:08 |
| cjwatson | mapreri: OK, just wondering about the choice of five years | 16:08 |
| mapreri | cjwatson: py2 is doomed for EOL around 2020, you know... | 16:09 |
| cjwatson | Oh right | 16:09 |
| cjwatson | It's not a timeframe I've been worrying about much | 16:10 |
| cjwatson | Too many closer ones :) | 16:10 |
| mapreri | eheh | 16:11 |
| cjwatson | I did go through recently and get rid of most of our use of class advice, which was a multi-thousand line patch needed for py3 | 16:12 |
| cjwatson | So we're not ignoring it :) | 16:12 |
| mapreri | nice :) | 16:14 |
| rpadovani | cjwatson, how can I use user_browser.open() to log in with a different user? My edits broke test in xx-bug-hidden-comments.txt at line 40 because now the comment is visible - to test if a ordinary user doesn't see the hidden comment, I need to change user - how? (or, what can I read to understand the solution?) | 16:20 |
| cjwatson | rpadovani: You'll need to get hold of a different browser instance that's authenticated as a different user. The pagetests often rely on the sample data shipped with Launchpad, so the easiest way is probably something like >>> test_browser = setupBrowser(auth="Basic test@canonical.com:test") | 16:28 |
| cjwatson | Then you'll want to test that the comment appears for the non-admin user that posted the comment (with basically the same set of tests we perform for admin users), and doesn't appear for a different non-admin user. | 16:29 |
| rpadovani | okay, thanks | 16:30 |
| cjwatson | lib/lp/testing/pages.py is the file that sets up things like user_browser | 16:30 |
| rpadovani | cjwatson, I pushed new stories - thanks for your suggestions, I hope to become more independent with time ;-) | 16:39 |
| mapreri | rpadovani: maybe assign yourself the bug? | 16:41 |
| cjwatson | mapreri: not required | 16:42 |
| cjwatson | (I mean, neater perhaps, but the QA bot is about to do so anyway) | 16:43 |
| cjwatson | rpadovani: LGTM, thanks, landing | 16:43 |
| mapreri | goody bot | 16:43 |
| rpadovani | cjwatson, thanks, I'll take a look to another bug then - usually I assign bugs to myself only when I'm sure I know how to fix them, to don't block any other user that wants to fix it | 16:47 |
| cjwatson | Reasonable. | 16:48 |
| === Guest39525 is now known as anthonyjf | ||
| === anthonyf is now known as Guest95842 | ||
| === Guest95842 is now known as anthonyjf | ||
| blr | morning | 20:37 |
| cjwatson | rpadovani: Have you looked at your buildbot test failures? | 22:33 |
| cjwatson | rpadovani: Some are spurious, but the failure in lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.txt is real, which is weird because I thought you said you'd run that. | 22:33 |
| wgrant | Morning. | 22:36 |
| cjwatson | rpadovani: Ah, you need to fix BugTaskNavigation.traverse_comments too | 22:37 |
| * cjwatson assumes rpadovani has gone to bed, so fixes locally and runs the bugs tests | 22:42 | |
| wgrant | cjwatson: Nothing appeared anomalous DB-wise overnight? | 22:44 |
| cjwatson | wgrant: Nothing I saw, no. | 22:45 |
| wgrant | Lovely. | 22:45 |
| wgrant | So my plan for BugBMP is that in the bzr case it will be tied exactly to BugBranch while the MP is open. | 22:48 |
| wgrant | Which is slightly messy, but preserves the existing behaviour and hopefully makes things relatively unconfusing. | 22:48 |
| cjwatson | Yeah, I was wondering how you were going to make that not weird ... | 22:48 |
| wgrant | We can probably remove the UI to link a branch directly later on, and need to change bug notifications. | 22:49 |
| wgrant | I'm very open to suggestions if you have any... | 22:49 |
| cjwatson | So you'd be able to edit BugBMP, but in the bzr case it would push it through to BugBranch? | 22:49 |
| wgrant | But I think BugBMP is the only approach that makes sense, so we have to make it usable somehow. | 22:49 |
| wgrant | Right. | 22:49 |
| wgrant | Or maybe it only resyncs when the preview diff is updated. | 22:50 |
| cjwatson | Yeah, none of my half-thoughts about how to do it on GitRef directly have made any kind of sense. | 22:50 |
| wgrant | Hmm. | 22:50 |
| cjwatson | If it only resyncs when the preview diff is updated, then editing BugBMP is difficult. | 22:50 |
| wgrant | True. | 22:50 |
| cjwatson | I don't think that makes sense. After all, today, if you link a branch to a bug then that's reflected immediately in the BMP. | 22:50 |
| cjwatson | Without updating a diff. | 22:51 |
| wgrant | Yeah. | 22:51 |
| wgrant | Well. | 22:51 |
| wgrant | No, maybe not. | 22:51 |
| wgrant | If we remove the BugBranch UI... | 22:51 |
| wgrant | Then you can only link by pushing a new rev, which will update the diff, or by clicking the link on the BMP directly, or by using the BugBranch API. | 22:51 |
| cjwatson | That would also make it impossible to see bzr branches that fix bugs unless they've had MPs proposed. | 22:52 |
| cjwatson | Which, granted, won't work in the git case. | 22:52 |
| cjwatson | (Although it does work on GitHub ...) | 22:52 |
| wgrant | Indeed. | 22:52 |
| cjwatson | We could do a BugGitRepository with very weak links. | 22:53 |
| wgrant | I don't know how GitHub decides what is relevant. | 22:53 |
| cjwatson | I'm not sure how pulling from one branch to another would work. It would have to be tied into merge detection somehow. | 22:53 |
| cjwatson | Otherwise everything is attached to everything. | 22:53 |
| wgrant | Maybe it's only the first time each rev appears. | 22:53 |
| wgrant | Right, exactly. | 22:53 |
| wgrant | The LP logic for that is very weird. | 22:53 |
| cjwatson | And there are certainly some branches with implausible numbers of bug links as a result. | 22:54 |
| wgrant | I did at one point know why trunks sometimes got bugs linked. | 22:54 |
| cjwatson | It might be worth some effort working out GH's logic. | 22:54 |
| wgrant | I can't remember now, though. | 22:54 |
| cjwatson | First time each rev appears doesn't seem a terrible start, but then what if you rename the branch | 22:55 |
| wgrant | Yup, exactly. | 23:00 |
| wgrant | Will poke GitHub. | 23:00 |
| cjwatson | Well, we can fairly cheaply work out which branches are ancestors of which other ones in a repository | 23:00 |
| cjwatson | That's near enough how our merge detection works | 23:00 |
| wgrant | Yep, but what happens if you have two trunks? | 23:00 |
| cjwatson | So it could just be any links from branches that aren't merged | 23:00 |
| cjwatson | Doesn't matter whether they're trunks or not | 23:01 |
| wgrant | What does "that aren't merged" mean, if not merged into trunk? | 23:02 |
| cjwatson | The interesting commits are the ones that are unique to a branch | 23:02 |
| cjwatson | Now, the trick is handling the case where a branch is merged into a trunk and then deleted | 23:02 |
| cjwatson | Couple of possible approaches to that | 23:03 |
| wgrant | Or where I push a branch, then push a superset of the same branch to a different name because the first name was wrong. | 23:03 |
| cjwatson | One promising one might be to have merge detection remember that everything older than the mergepoint is uninteresting for commit scans | 23:03 |
| cjwatson | Maybe | 23:03 |
| cjwatson | It's hard to tell the difference between your case and the case of the creation of a new trunk. Whatever a trunk is. | 23:05 |
| wgrant | Exactly :) | 23:05 |
| cjwatson | (Anything that relies on us knowing what a trunk is is probably wrong ...) | 23:05 |
| cjwatson | Seems like a thing worth trying out on GH, anyway. | 23:06 |
| wgrant | Yep, will do. | 23:06 |
| wgrant | cjwatson: Are you still looking at that testfix? | 23:31 |
| cjwatson | Yes | 23:32 |
| cjwatson | Nearly done, just trying to get the blasted thing to scan before pushing the fix rev | 23:32 |
| wgrant | Hah | 23:32 |
| wgrant | Thanks. | 23:32 |
| cjwatson | lp.bugs passes now though | 23:32 |
| cjwatson | wgrant: https://code.launchpad.net/~cjwatson/launchpad/testfix-unhide-comments/+merge/271043 | 23:37 |
| wgrant | Hmm. | 23:37 |
| wgrant | I'm not actually sure I like randoms being able to unhide their comments, but I guess until we have a way to fix mistakes it's awkward. | 23:38 |
| wgrant | So it'll do. | 23:38 |
| cjwatson | Yeah, it's not unproblematic. But I guess that in cases of serious abuse we'd suspend accounts anyway. | 23:39 |
| wgrant | Right, but it is open to other more subtle abuse. | 23:39 |
| cjwatson | And they can always repost, it's not like it really helps much to not let them unhide | 23:39 |
| wgrant | Making parts of the conversation disappear and then reappear when it's convenient. | 23:39 |
| wgrant | cjwatson: Did you prefer LaunchBag over self.request deliberately? | 23:40 |
| cjwatson | I didn't know that API well; I noticed that self.request was =None in Navigation's constructor, and found ILaunchBag used elsewhere | 23:41 |
| wgrant | Ah, OK, sounds reasonable. | 23:41 |
| cjwatson | Maybe comment hiding/unhiding should go in the activity log. | 23:41 |
| wgrant | No. | 23:41 |
| wgrant | Spam is a thing. | 23:41 |
| wgrant | We should fix the hiding silliness to be admin-only eventually. | 23:42 |
| wgrant | Feel like fixing the rest of that traversal check while you're there? | 23:42 |
| wgrant | It's moderately annoying for us, and it's the next line... | 23:42 |
| cjwatson | Sorry, I'm nearly asleep. What's the problem with it? | 23:43 |
| cjwatson | Oh, it doesn't allow registry does it | 23:44 |
| wgrant | Right, it should probably use Bug.userCanSetcommentVisiblity | 23:44 |
| wgrant | +i | 23:44 |
| cjwatson | So should get_visible_comments, then | 23:45 |
| wgrant | Ah, get_visible_comments doesn't allow project-privileged users, it seems. | 23:46 |
| wgrant | Which is arguably correct from a spam perspective. | 23:46 |
| wgrant | But awkwardly inconsistent. | 23:47 |
| cjwatson | So they can hide spam, but can't see that they've done it? | 23:47 |
| wgrant | Apparently. | 23:47 |
| wgrant | cjwatson: Ah, so GitHub doesn't actually track this. | 23:52 |
| wgrant | cjwatson: The PR page doesn't directly list the issues, only by linkifying them in commit messages. | 23:52 |
| wgrant | And the issue page tells you that a bug was closed by merging into the default branch, or that it was referenced in a commit (not a branch) | 23:52 |
| wgrant | Of course in Git you don't need to know the branch, just the repo, and it's always the same repo. | 23:53 |
| wgrant | Well, on GitHub it's always the same repo. | 23:53 |
| cjwatson | Ah, right, the commit link has no ref context | 23:54 |
| * wgrant tries a cross-repo reference. | 23:54 | |
| wgrant | We can't be quite so simple, as we have multiple repos. | 23:55 |
| cjwatson | wgrant: https://code.launchpad.net/~cjwatson/launchpad/testfix-unhide-comments/+merge/271043 brushed up slightly | 23:55 |
| wgrant | It seems to only show the first time it saw each commit. | 23:56 |
| wgrant | If I reference wgrant/test#5 in a commit in wgrant/test2, the issue shows that. | 23:56 |
| wgrant | But subsequently pushing that same commit to wgrant/test doesn't show anything, unless it causes the issue to be closed. | 23:57 |
| wgrant | cjwatson: Thanks, looks good. | 23:58 |
| cjwatson | Thanks, landing. | 23:59 |
| wgrant | Hmmmmmm. | 23:59 |
| wgrant | Trying to think how this should work. | 23:59 |
| wgrant | I guess it's not so bad if we don't link from the repo to the fixed bugs. | 23:59 |
| wgrant | The list on a bug will not grow large, just the list on the repo. | 23:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!