| Ursinha | lifeless, hi? | 01:14 |
|---|---|---|
| * Ursinha is a sad panda | 01:14 | |
| jpds | maxb: Investigating. | 01:17 |
| maxb | cody-somerville: ^ | 01:18 |
| jpds | Found the problem. | 01:21 |
| magcius | hey jam | 01:22 |
| magcius | what do you think of pulling a github and doing: http://i.imgur.com/lhCVP.png | 01:24 |
| lifeless | Ursinha: hi? Its easter here ... | 01:50 |
| lifeless | Ursinha: but I hate the idea of you being sad :) | 01:50 |
| magcius | gah | 02:12 |
| magcius | trying to follow the instruction at https://dev.launchpad.net/Running | 02:12 |
| magcius | $ make scehma | 02:12 |
| magcius | Missing ./download-cache. | 02:12 |
| magcius | Developers: please run utilities/link-external-sourcecode. | 02:12 |
| magcius | how am I supposed to run link-external-sourcecode? | 02:13 |
| magcius | oh | 02:16 |
| magcius | I see what happened | 02:16 |
| Ursinha | lol | 03:00 |
| Ursinha | lifeless, sorry to disturb you | 03:01 |
| lifeless | Ursinha: no problem | 03:02 |
| lifeless | Ursinha: whats up ? | 03:02 |
| Ursinha | lifeless, was about a qa-tagger branch to review | 03:11 |
| Ursinha | can wait until Monday, don't worry | 03:11 |
| Ursinha | happy easter for you :) | 03:12 |
| * wgrant curses AWS. | 03:17 | |
| LPCIBot | Project windmill build #207: STILL FAILING in 1 hr 5 min: https://lpci.wedontsleep.org/job/windmill/207/ | 03:18 |
| lifeless | Ursinha: kk | 04:02 |
| jtv | hi lifeless! | 04:20 |
| jtv | lifeless: I just had another silly thought about offsetless batch navigation. | 04:21 |
| jtv | It's about presentation issues though, not performance. Let me know if you're interested. | 04:22 |
| === almaisan-away is now known as al-maisan | ||
| lifeless | jtv1: I'm pretty much always interested ;) | 05:30 |
| jtv1 | lifeless: cool! Okay, here's the thought: the style of batching I propose has some weirdness with the first page. | 05:31 |
| jtv1 | It's actually better, not worse, than our current offset-based weirdness, but more noticeable: | 05:31 |
| jtv1 | you may not get a full batch on the first page, just like you may not get one on the last page. | 05:32 |
| jtv1 | We could fill it out with items that were also on the previous page. | 05:32 |
| jtv1 | That means there's overlap, but maybe we could turn that into a virtue. | 05:33 |
| jtv1 | Maybe it'd be helpful if every batched page would show a bit of overlap with its adjacent pages, in a visually distinct style. | 05:33 |
| jtv1 | Think grey background or something. | 05:34 |
| jtv1 | So if you follow a "previous" link to a first page, and it needs to be filled out with overlapping data from the page you just visited, you'd instantly recognize it as overlap — just a bit more of it than usual. | 05:35 |
| === jtv1 is now known as jtv | ||
| jtv | In principle we could do the same with our existing batching, but there we don't even have a way to display the items that slip through the cracks between batches. | 05:36 |
| jtv | (Plus, come to think of it, we don't even have a notion of item identity across pages) | 05:37 |
| jtv | So imagine seeing a first page in a batched series: the bottom item has a grey background. | 05:37 |
| jtv | You follow the "next" link, and get to a page with a grey item from the first page, followed by regular items, and closed off by another grey item from the next page. | 05:38 |
| jtv | If you then follow "previous" and some items have disappeared off that page, you'd still get a full batch. But more items at the end would be grey, because they're from page #2 which you just saw. | 05:38 |
| stub | jtv: That is a builtin feature of Zope batching. I forget the term. | 05:38 |
| jtv | Ah! | 05:39 |
| stub | (not the greying out, but the overlap between batches) | 05:39 |
| jtv | Oh | 05:39 |
| jtv | Well it'd have to be visually distinct in some way, I think. | 05:39 |
| jtv | But it wouldn't be much use IMO if the batching has no reliable knowledge of the overlap. | 05:40 |
| stub | It might be nice, but it doesn't have to be as I've seen it work well without it - people instinctively understand the overlap. | 05:40 |
| stub | But that was with consistent overlap. | 05:40 |
| jtv | Yes, and that's not always easy to get. | 05:41 |
| jtv | And the converse, disappearing items, is much harder to understand. | 05:41 |
| jtv | Here's a problem we've had: | 05:41 |
| jtv | you're viewing a translation, filtering to show untranslated messages only. | 05:41 |
| jtv | You translate all the messages you see, and hit the button that submits and takes you to the next page. | 05:42 |
| jtv | Two things would happen: | 05:42 |
| jtv | your offset would be increased by one batch, | 05:42 |
| jtv | and the messages you just translated would drop out of the batch query's "WHERE" clause. | 05:42 |
| jtv | So you just skipped a full batch! | 05:42 |
| jtv | The fix for that is ugly and brittle. | 05:43 |
| jtv | The batching style I outlined in Dallas naturally eliminates this complication. | 05:43 |
| stub | Yup. | 05:45 |
| jtv | I've got similar problems with the translations import queue: I filter for items with a given status. I change their statuses using ajax widgets, or from forms I open in separate browser tabs. And now the "next" link has become a Mystery Prize button. | 05:45 |
| stub | Although I'm not sure why you skip a batch with the existing offset method. Because you are iterating over 'translatable items without translations' or something? | 05:45 |
| jtv | Exactly. | 05:45 |
| stub | ok | 05:46 |
| jtv | But the same thing will happen if someone else is translating concurrently. | 05:46 |
| stub | Right. So maybe you need a better set to iterate over. Or the 'translation torrent' idea that I don't think ever happened. | 05:47 |
| jtv | It gets complicated and my confidence in our compensation offsets is limited. It's fine for the scenarios it handles, but does it handle them all? | 05:47 |
| stub | 'give me a page of strings to translate' | 05:48 |
| jtv | What would constitute a better set though? I think forms that show items in a given state and let you change that state are a useful pattern. | 05:48 |
| stub | Why create a batch and step through it? Why not just a page that gives me '20 strings to translate'. | 05:48 |
| jtv | With an "I don't like these, give me some other ones" button? | 05:49 |
| jtv | Pick random ones each time? | 05:49 |
| stub | If you want to do them in some order, you could pin or reserve them using a shared temporary table or even memcache. Or just return an arbitrary set and use the laws of probability and how many remaining strings to determine the likelyhood of collision. | 05:49 |
| stub | It would be great if 10 people could sit down in a room for a day and translate some app completely. | 05:50 |
| stub | Translation sprints might happen. | 05:50 |
| jtv | Well there are good reasons to maintain ordering. | 05:51 |
| jtv | Context, basically. | 05:51 |
| jtv | I'm not sure reserving strings is a good idea, because it's hard to tell whether someone's browsing, translating, or somewhere inbetween. | 05:52 |
| stub | Unless they told you. | 05:53 |
| stub | You don't need one mega form that does everything. | 05:53 |
| jtv | Have a look at the translation teams out there. Compare them to actual contributions. | 05:53 |
| lifeless | jtv: batchnav 1.2.4 supports contraint based batching | 05:53 |
| lifeless | jtv: I don't understand the suggestion that the first batch is wonky - it isn't | 05:53 |
| jtv | In id-based batching, it is. | 05:54 |
| lifeless | I'm not sure what you mean by id based batching | 05:54 |
| jtv | The alternative to offset-based batching. | 05:54 |
| jtv | So the scheme I suggested in Dallas. | 05:55 |
| lifeless | so I describe what you described in dallas as constraint based batching | 05:55 |
| jtv | Ah ok. | 05:55 |
| lifeless | because one uses db constraints rather than offsets to find the given batch | 05:55 |
| lifeless | the 'page number' becomes entirely cosmetic | 05:55 |
| jtv | Good. | 05:55 |
| lifeless | I have a branch that switches us to batchnav1.2.4; it has some [minor] test failures | 05:56 |
| lifeless | once thats landed (and if you want to pick it up and land it be my guest - I'm on leave till tuesday with easter) | 05:56 |
| jtv | On urgent feature mission currently. :/ | 05:57 |
| lifeless | then we can write adapters for a given collection to do the constraint (for db) and nonce (for the web client to hand back to us) generation | 05:57 |
| jtv | It'd be really cool to see this. | 05:57 |
| jtv | But I maintain that the first batch is wonky: you have to show either an incomplete batch or overlap. Which accept as a matter of course with offset-based batching, but could be a bit misleading once overlap is otherwise completely eliminated. | 05:59 |
| stub | SELECT * FROM Foo WHERE id > 0 LIMIT 50; | 05:59 |
| stub | SELECT * FROM Foo WHERE id > 89 LIMIT 50; | 05:59 |
| jtv | Now delete a few rows with id ≤ 89, and then browse backwards. | 06:00 |
| stub | Delete all the rows with id < 89 and browse backwards - iterating over a mutating list always has problems. | 06:01 |
| jtv | That's the point: constraint-based batching only seems to have this one. | 06:01 |
| jtv | You have to show either an incomplete batch, or overlap with the page the user came from. | 06:02 |
| jtv | But overlap and missed items disappear completely otherwise. | 06:02 |
| stub | Browsing backwards, when something has deleted items that were in the previous batch, will show an incomplete batch. | 06:03 |
| jtv | Or overlap. | 06:03 |
| stub | Actually - it will likely show overlap. | 06:03 |
| jtv | Why are we going in circles? | 06:03 |
| stub | Yes. | 06:03 |
| jtv | Anyway, this is all old hat. | 06:03 |
| stub | I don't think anyone thinks the behaviour is a problem. | 06:04 |
| jtv | Glad to hear you haven't run into the same users I have. | 06:04 |
| stub | The existing OFFSET code has the same problem. | 06:04 |
| jtv | It has much worse problems. | 06:05 |
| jtv | What I'm saying is that given that we can eliminate those (and looking at what Robert's doing, pretty easily!) it'd be a bit misleading to leave one of them in one particular scenario by showing overlap, and it'd be a bit ugly to show an incomplete batch instead. So showing grey overlap might be a way of dealing with that. | 06:07 |
| jtv | The easy and consistent thing to do I think is show an incomplete batch. | 06:07 |
| jtv | lifeless: I'd be curious to know how batchnav deals with navigating backwards! Does it have knowledge of ordering? | 06:09 |
| stub | Sure. Just another parameter to pass. I personally am not sure what is less surprising to users. I suspect nobody actually cares, as the existing batching systems have always had this problem and nobody has bothered to fix it. | 06:09 |
| jtv | There's that. | 06:10 |
| stub | lifeless' update to batchnav lets you step through using a key, ordered. | 06:10 |
| jtv | stub: You do get situations where the top page is "latest events," and people click around a lot on the top 2 pages. But I guess partial batches look pretty natural there. | 06:11 |
| jtv | The reason I ask about browsing backwards is that AFAICS you need to reverse the query order at that point, for LIMIT, but then restore your "regular" order for display. | 06:12 |
| stub | 'latest events' generally don't get deleted until they are not-so-latest-events on latter pages. | 06:12 |
| jtv | No, but they get added. | 06:12 |
| jtv | What happens there is: you go "next" from page 1 to page 2. | 06:12 |
| jtv | You go "previous" from page 2 to page 1. | 06:13 |
| jtv | Which now shows a "previous" link that takes you to... page 0. | 06:13 |
| jtv | As I said, I guess partial batches work out to be pretty natural for that case. | 06:13 |
| stub | If a batch knows the previous id, it can step back one batch. That batch won't know the previous id, so to step back another batch will require falling back to the OFFSET code path I suspect. Unless we maintain a list of all previous batches traversed. | 06:15 |
| jtv | That'd give us the worst of both worlds. | 06:16 |
| stub | (which would scale to human sized number of batches, so might be sane. Just don't show 'previous' links if we go into hundreds of batches, as then we are dealing with a bot) | 06:16 |
| stub | Or just skip 'previous' links altogether and let users make use of that 'back' button on their browser ;) | 06:17 |
| jtv | Maintaining a list of previous batches just opens the door to more complexity and weirdness on any page when browsing backwards. | 06:17 |
| stub | There will always be weirdness browsing back over a mutated list. | 06:18 |
| stub | Doctor, it hurts when I do this... | 06:18 |
| stub | There will always be the case that all previous items have been removed after a 'previous' link has been rendered, making that link go to an empty page, the same page, or an error page. | 06:20 |
| jtv | Yes, but we can have less weirdness with less complexity. | 06:21 |
| stub | We are primarily interested in speed. If we can reduce weirdness and complexity without sacrificing query speed, great. | 06:22 |
| jtv | And yes, we can. | 06:22 |
| jtv | You can browse backwards by reversing the constraint and the sort order; it's just that then you need to put the rows inside the batch back into displayable order. | 06:23 |
| jtv | Ahhhh | 06:24 |
| jtv | Maybe they use cursors for that. | 06:24 |
| stub | That method of stepping backwards looks good. I don't see why you would end up with partial batches or overlap though, except the first batch would become partial. And I don't think that is a problem. | 06:25 |
| stub | I'm not sure what lifeless implemented in his batchnav update, but I doubt it would be much work to add your logic if he didn't. | 06:26 |
| stub | It might make indexes more complex if we are iterating by something like 'id DESC, version' (which we are on some pages I've seen) | 06:27 |
| jtv | The partial batches other than the first or last one only come in when you do what you suggested earlier: keep track of previous batches (which may have grown or shrunk) or fall back on OFFSET. | 06:27 |
| jtv | Not with OFFSET, actually, but there you get overlap again. | 06:27 |
| jtv | But anyway. | 06:28 |
| jtv | Yes, we may have to re-consider a lot of indexes... | 06:28 |
| stub | I suspect the solution is worse than the problem. I don't think the partial batches are a problem warranting the extra complexity or effort. We certainly don't want to fallback to OFFSET if we can avoid it since our nice 1s pages might suddenly spike to OOPS timeouts. | 06:28 |
| jtv | Which problem and which solution are you talking about now? | 06:29 |
| jtv | (Since we're going through so many) | 06:30 |
| stub | maintaining a list of previously seen batch starts | 06:30 |
| jtv | Oh, right, that's what I've been telling you all along. :) | 06:30 |
| jtv | However I think what you say also applies to what I said originally: | 06:30 |
| jtv | partial batches at the head aren't a problem worth solving. My idea wouldn't carry its weight. | 06:31 |
| stub | I don't see how we can allow stepping backwards an arbitrary number of batches without your approach, or without falling back to slow queries. | 06:32 |
| stub | (was that a triple negative?) | 06:32 |
| jtv | I don't know, I'm too confused. | 06:32 |
| stub | Step 1) Land lifeless' code. | 06:32 |
| stub | Step 2) Improve on it if lifeless didn't think of it already. | 06:33 |
| jtv | Well like I said, I'm curious to know how they dealt with the question of how to step backwards. What I have right now requires an ability to invert the condition. But I guess that knowledge is needed anyway, because the batcher needs to formulate constraints. | 06:34 |
| jtv | But with a cursor, I suspect there's no need to invert ordering as I thought previously. | 06:34 |
| stub | Maybe we just dropped previous links :) We certainly are not using a cursor across transactions from the webapps, because we don't have server affinity. | 06:35 |
| jtv | No, you wouldn't want to do this across transactions. But they could be used as a way to simulate "LIMIT from the end of the result backwards." | 06:35 |
| jtv | Not sure that's actually better than offset-based batching performance-wise, though. | 06:36 |
| jtv | Example: | 06:36 |
| jtv | if your regular, forwards-facing queries go "SELECT * FROM foo WHERE id > x LIMIT 10" | 06:36 |
| stub | I don't think SQL lets you do "LIMIT -100" :-) | 06:36 |
| jtv | Hmmm | 06:37 |
| jtv | Might be nice if it did. | 06:37 |
| stub | switching that to "SELECT * FROM foo WHERE id < x ORDER BY id DESC LIMIT 10" is certainly the better performing method except on the smallest batch sizes, and then it won't be noticeably slower. | 06:38 |
| jtv | But I guess you can simulate it with DECLARE cur FOR SELECT * FROM foo WHERE id < x ; MOVE ALL IN cur ; MOVE -10 IN cur ; FETCH ALL IN cur | 06:38 |
| jtv | Right. | 06:38 |
| jtv | That's going to be the more efficient way, but of course you have to put the rows back into display order afterwards. | 06:39 |
| jtv | Hence my interest in how batchnav solves this. | 06:39 |
| lifeless | jtv: stub: so , going 2 batches requires the key + offset | 06:39 |
| jtv | Both!? | 06:40 |
| lifeless | right | 06:40 |
| jtv | By the way, when you say "going 2 batches"... | 06:40 |
| jtv | Typo? | 06:40 |
| lifeless | say we're on batch N, which has as its upper row key K | 06:40 |
| lifeless | to get batch N+2 | 06:40 |
| jtv | Ah | 06:40 |
| lifeless | we select where keycols > K OFFSET batchsize LIMIT batchsize+1 | 06:41 |
| jtv | Yes, that's a harder problem — which we haven't even looked at so far. | 06:41 |
| jtv | (Not _that_ much harder of course: this is basically just a shortcut for skipping batches in a result set while it's guaranteed not to change) | 06:42 |
| lifeless | we may need to do a batchnav 1.2.5 when we start doing contraint based batches for vocabularies (which show N,N+1,N+2..N+M links at the top for some bizaare reason. | 06:42 |
| lifeless | now, going backwards is easy | 06:42 |
| jtv | Here it comes! | 06:42 |
| lifeless | say we're on batch N which has as its lower row key K | 06:42 |
| lifeless | and that the normal query is select .. order by keycols desc | 06:43 |
| lifeless | the prior batch is obtained by | 06:43 |
| lifeless | seleect where <=K order by keycols asc | 06:43 |
| lifeless | (limit batchsize +1) | 06:43 |
| lifeless | now, in the case of a concurrently edited batch, this *can* give a short batch at the start of the sequence | 06:44 |
| lifeless | so if we get a short read | 06:44 |
| lifeless | we do a second query to fix it up | 06:44 |
| lifeless | and discard any excess | 06:44 |
| jtv1 | What a time for my connection to drop. | 06:45 |
| lifeless | 17:42 < jtv> Here it comes! | 06:45 |
| lifeless | 17:42 < lifeless> say we're on batch N which has as its lower row key K | 06:45 |
| lifeless | 17:43 < lifeless> and that the normal query is select .. order by keycols desc | 06:45 |
| lifeless | 17:43 < lifeless> the prior batch is obtained by | 06:45 |
| lifeless | 17:43 < lifeless> seleect where <=K order by keycols asc | 06:45 |
| lifeless | 17:44 < lifeless> (limit batchsize +1) | 06:45 |
| lifeless | 17:44 < lifeless> now, in the case of a concurrently edited batch, this *can* give a short batch at the start of the sequence | 06:45 |
| lifeless | 17:44 < lifeless> so if we get a short read | 06:45 |
| lifeless | 17:44 < lifeless> we do a second query to fix it up | 06:45 |
| lifeless | 17:44 < lifeless> and discard any excess | 06:45 |
| lifeless | 17:45 -!- jtv1 [~jtv@58.136.24.13] has joined #launchpad-dev | 06:45 |
| lifeless | 17:45 < jtv1> What a time for my connection to drop. | 06:45 |
| jtv1 | Thanks. | 06:45 |
| lifeless | de nada | 06:45 |
| stub | jtv1: Skype for me still sucked this morning | 06:46 |
| lifeless | have a look at lazr.batchnav trunk if you're interested in the gory details | 06:46 |
| jtv1 | I'm on True right now; TOT and 3BB have been horrible lately. | 06:46 |
| stub | jtv1: Crap connection for 4 days now? | 06:46 |
| lifeless | I wrote a slice based adapter to generate the existing queries, and tested the interface reasonably comprehensively. | 06:46 |
| jtv1 | Longer. I think I heard something about plans to make the censorship infrastructure more like China's, a few months back. | 06:46 |
| jtv1 | lifeless: so far it's pretty much all what one would guess, but it leaves me with a few questions: | 06:47 |
| jtv1 | 1. The reason I asked was that I was curious about batchnavigator's knowledge of your query's ordering. Where does the natural order get restored? In SQL, in the batch navigator, in the app..? | 06:47 |
| lifeless | (oh, and the page #'s can be -totally- bogus during this :) | 06:48 |
| jtv1 | 2. Why "id <= x" and batchsize+1? | 06:48 |
| lifeless | jtv1: in the collection slice factory | 06:48 |
| lifeless | batchsize + 1 is due to an attempt to avoid calling count(*) on the collection | 06:48 |
| jtv1 | Oh, to see if there should be a "prev" link? | 06:48 |
| lifeless | and next, yes. | 06:48 |
| jtv1 | Right ho. | 06:49 |
| lifeless | its mostly defeated because we show a last link | 06:49 |
| lifeless | if we dropped 'last' and showed 'reverse order' instead, we would be better off. | 06:49 |
| lifeless | that becomes more of a win once we're not using OFFSET, so I imagine we will get to it eventually. | 06:50 |
| jtv1 | Well isn't "last" simply always a reverse batch without constraint? | 06:50 |
| lifeless | right | 06:50 |
| lifeless | but the url has the *predicted* count offset in it, because batchnav wasn't designed for mutating collections. | 06:50 |
| lifeless | Which is a bit of an oversight in a webapp :) | 06:50 |
| jtv1 | I would've expected a change in parameters there! | 06:50 |
| jtv1 | But that's the whole problem of course: most of this is pretty easy, _apart_ from the thicker interfaces. | 06:51 |
| lifeless | the concept of 'batch N' is deeply embedded. | 06:51 |
| lifeless | I wanted to solve the 'we can avoid OFFSET for expensive cases' problem in this iteration. | 06:51 |
| lifeless | we have more work to do to eliminate COUNT(*), batch #'s and so forth. | 06:51 |
| jtv1 | That'd just blow me away. | 06:51 |
| jtv1 | Still curious: when you say that the slice factory does the re-sorting of reverse batches, do you mean it generates some kind of SQL wrapper to reverse the batch, or a python sort, or what? | 06:53 |
| lifeless | for instance I think we could (say its ordered by heat) identify the 'Nth batch' (in the UI) as 'now showing bugtasks with less than 40 heat and bug id 45+' or something, and have it make sense to users | 06:53 |
| lifeless | jtv1: best if you read the code at this point | 06:53 |
| lifeless | ISliceFactory | 06:53 |
| jtv1 | \o/ | 06:53 |
| jtv1 | I have this feeling there's a fascinating rabbit hole waiting behind the question of how batching should be presented. | 06:54 |
| jtv1 | (Freudian slip: my fingers tried to turn that into "how batching should be prevented") | 06:54 |
| stub | jtv1: list.reverse() isn't expensive for a 100 item list. | 06:55 |
| jtv1 | I sometimes want "search, but don't filter the set; just take me to the right point in the batch" or "give me a little tree so I can go to 25%-or-so in one click" | 06:55 |
| jtv1 | Ah! Just reversing the list. How blindingly obvious. | 06:55 |
| jtv1 | And here I was thinking of all these complicated things. | 06:56 |
| jtv1 | lifeless: we also have some pretty weird behaviours for nonexistent batches past the end of the set (which people can actually reach in good faith because of concurrent changes, but we seem to pretend they must be playing doctor with their URLs or something) | 06:57 |
| jtv1 | With constraint-based nav, could we fix that to show the last batch? I'm sure you see where I'm going with this. | 06:58 |
| jtv1 | And then of course, make the "last" links point to the batch starting at inf. | 06:58 |
| jtv1 | No new parameter, but no count query either. | 06:59 |
| jtv1 | (At least not for the "last" link) | 06:59 |
| === jtv1 is now known as jtv-eat | ||
| jtv-eat | lifeless: on the off chance that this might solve anything ^^^ | 07:10 |
| al-maisan | jtv-eat: guten Appetit :) | 07:14 |
| lifeless | jtv-eat: uhm | 07:19 |
| lifeless | jtv-eat: so the counting aspect is the problem for last; the actual data is retrieved by selecting from the end in reverse sort order | 07:20 |
| lifeless | jtv-eat: constraint based batching will handle 'last' better always, but handling ' empty results because past end-of-collection' different would be fine; I think its orthogonal though | 07:21 |
| lifeless | I do see the link you present between that and knowing the page number, but the page number is already weakened with contraint based queries. | 07:21 |
| LPCIBot | Project windmill build #208: STILL FAILING in 1 hr 4 min: https://lpci.wedontsleep.org/job/windmill/208/ | 07:47 |
| jtv1 | lifeless: I'm not sure I expressed myself as well as I might have. I mean: AIUI you need a count query just to provide an offset for the "last" link. If past-end-of-set pages were to show the last n items (whatever the real set size is), then you no longer need to compute anything meaningful for the "last" link. Just give a very large number and the viewer will end up on the last page. | 08:31 |
| jtv1 | [where n is batch size, of course] | 08:32 |
| lifeless | jtv1: the number is cosmetic | 08:47 |
| lifeless | jtv1: page 1 of N | 08:47 |
| lifeless | jtv1: while we show it, folk will be confused by it if it goes to 10000000 when there are 120 results ;) | 08:47 |
| lifeless | jtv1: we show the number of results as well | 08:48 |
| lifeless | jtv1: until we have answers for /all/ the places the collection size is shown, we can't eliminate the count | 08:48 |
| lifeless | at the moment the count is cached on the object, so we only pay it once (but we do pay it) | 08:49 |
| lifeless | I guess I'm saying - yes, that would avoid the mechanical reason for *that* triggering of count, but its not sufficient to eliminate the count, and its also a fairly ugly ui unless we make other changes concurrently (and if we do that, we don't need to show a high N number at all) | 08:50 |
| jtv1 | lifeless: yeah I'm not considering page number _display_ at all here. Then again, maybe "showing last page of about 256" would be enough for display. | 08:55 |
| lifeless | jtv1: right, the trouble is until we've solved all the causes we haven't solved the issue ;) | 08:59 |
| jtv1 | then don't let me keep you :) | 08:59 |
| jtv1 | Seriously: great that you jumped on top of this. | 08:59 |
| jtv1 | If that expression makes any sense in English. | 08:59 |
| jtv1 | And doesn't make it sound lugubrious or salacious. | 09:00 |
| wgrant | rvba: Hi. | 09:03 |
| rvba | wgrant: Hi! | 09:03 |
| wgrant | I'm not here today, and I guess Julian probably isn't either... I think we probably need to discuss this more. ArchiveDependencies are per-archive, not per-series, so I'm not sure exactly what this UI is meant to do. | 09:04 |
| rvba | I know that the dependencies are for archives ... but since the parent/child relationship is on series, the problem you have with the UI might only be something cosmetic | 09:05 |
| wgrant | Well, what does it mean? | 09:05 |
| wgrant | What does the UI do if I have two series for one distro, one checked and one not? | 09:06 |
| rvba | actually, that was my next question :) | 09:06 |
| wgrant | We probably need a flag on DSP. | 09:06 |
| wgrant | And we need transitive archive deps, I think :( | 09:06 |
| jtv1 | wgrant: julian shouldn't be here either, no | 09:07 |
| jtv1 | oh, hi rvba! | 09:07 |
| rvba | hi jtv1 | 09:07 |
| === jtv1 is now known as jtv | ||
| lifeless | jtv: I refer you to my early perf tuesday mails where I said I would be data driven :) | 09:08 |
| rvba | wgrant: by transitive archive you mean that one archive could depend on an archive ... depending on another one? | 09:08 |
| lifeless | jtv: once we started to see slow-offset queries as a high fraction of timeouts, this became important to do :) | 09:08 |
| jtv | lifeless: just saying someone else might have decided it was too hard and we'd have been the poorer for it. | 09:09 |
| wgrant | rvba: Right. It's most obviously important for PPAs. | 09:09 |
| lifeless | jtv: thanks :) | 09:09 |
| wgrant | rvba: Since if they just have a dependency on their distribution's primary archive, they're not going to work well if that's an overlay archive. | 09:10 |
| rvba | wgrant: right | 09:10 |
| rvba | wgrant: I could look at the code but how is this supported today for PPAs? | 09:10 |
| rvba | hardcoded some way? | 09:11 |
| wgrant | rvba: We don't have transitive dependencies. But lib/lp/soyuz/adapter/archivedependencies.py has a _get_default_primary_archive_dependencies function that's used for non-primary archive if they don't have an ArchiveDependency for the primary archive. | 09:12 |
| wgrant | rvba: We want to remove that hardcoding, so PPAs will start with an ArchiveDependency on the primary archive. | 09:12 |
| wgrant | But... if the primary archive then becomes an overlay archive, we have to add new dependencies to every PPA. | 09:12 |
| wgrant | Unless ArchiveDependencies are transitive. | 09:12 |
| rvba | Understood. | 09:12 |
| rvba | wgrant: since you're not here today, I don't want to bother you too much with this, I think perhaps the best thing to do is to try to summarize the things that we (well, mostly you) think about this and send an email to the 3 of us. | 09:13 |
| rvba | I'll focus on something else today. | 09:13 |
| wgrant | rvba: That sounds like a good idea. | 09:14 |
| rvba | wgrant: but basically the UI (you should have received a mockup yesterday) is meant to be able to configure the archive dependencies for an archive. | 09:14 |
| wgrant | It's a thorny issue that we need to talk through a lot. | 09:14 |
| wgrant | JFDI is often good, but it's not going to work here :( | 09:14 |
| rvba | perhaps the stupid part is listing the *series* instead of the *archives* | 09:14 |
| wgrant | Except we do need to sometimes set it per-series. | 09:15 |
| rvba | +1 for the not JFDI on this one :) | 09:15 |
| wgrant | So, the ArchiveDependency could always exist, but DSPs are only used if they have the overlay flag set. | 09:15 |
| wgrant | Anyway, let's discuss this on Tuesday or so. | 09:19 |
| rvba | wgrant: all right. | 09:19 |
| rvba | wgrant: thanks for the chat ;). | 09:20 |
| === jtv1 is now known as jtv | ||
| magcius | ok | 10:38 |
| magcius | i'm deving launchpad | 10:38 |
| magcius | is there a way i can either add all the certificate exceptions to firefox in bulk? | 10:39 |
| magcius | or run it in not-https mode? | 10:39 |
| stub | I don't know how to add certificate exceptions in bulk, but there are only half a dozen domains. | 10:44 |
| magcius | OK | 10:49 |
| magcius | So, let's say I want to make http://bugs.launchpad.net/12345 work | 10:49 |
| magcius | and "do the right thing" | 10:49 |
| magcius | I think I've found the right place -- lib/lp/bugs/browser/malone.py | 10:50 |
| magcius | OK -- how does the subdomain system work? | 10:57 |
| magcius | Do different subdomains get different ZCMLs, and that's it? | 10:57 |
| stub | IIRC everything is actually one big tree, and there is some magic vhosting hacked in to make /bugs (or /+bugs or /malone or whatever) work for bugs.launchpad.net. But long time since I saw that code. | 11:02 |
| magcius | I'm just curious. | 11:07 |
| magcius | It looks like the vhosts don't do anything different at all. | 11:07 |
| magcius | for instance, https://blueprints.launchpad.net/ubuntu/+bug/1 works | 11:08 |
| _mup_ | Bug #1: Microsoft has a majority market share <iso-testing> <ubuntu> <Clubdistro:Confirmed> <Computer Science Ubuntu:Invalid by compscibuntu-bugs> <EasyPeasy Overview:Invalid by ramvi> <GNOME Screensaver:Won't Fix> <Ichthux:Invalid by raphink> <JAK LINUX:Invalid> <The Linux OS Project:In Progress> <metacity:In Progress> <OpenOffice:In Progress by lh-maviya> <Tabuntu:Invalid by tinarussell> <Tivion:Invalid by shakaran> <Tv-Player:New> <Ubuntu:I | 11:08 |
| stub | yes. Your jumping into some of the oldest and cruftiest code in the codebase :) | 11:09 |
| magcius | Yeah, the traversal stuff looks pretty ugly. | 11:10 |
| stub | Everything used to be off one domain, and the subdomains tacked on later. | 11:10 |
| magcius | You depend on the MRO of all the classes. | 11:10 |
| magcius | basically, all of the subclasses of Navigation. | 11:10 |
| magcius | also, what is "vostok"? | 11:10 |
| stub | One of the production servers - can't recall what its purpose is. No reason for it to be referenced in the code base. | 11:11 |
| magcius | It's a whole other codebase. | 11:12 |
| magcius | lp.vostok.publisher.VostokRoot | 11:12 |
| magcius | complete with its own root object | 11:12 |
| stub | Sounds like an internal service designed to run on a single host. | 11:12 |
| wgrant | vostok was a former production server. lp.vostok is unrelated. | 11:13 |
| magcius | OK. | 11:13 |
| magcius | what is lp.vostok then? | 11:13 |
| wgrant | It was originally to be an alternative UI for a subset of Launchpad, of which many instances would be run. | 11:14 |
| wgrant | That never eventuated, so its purpose is approximately null. | 11:14 |
| magcius | OK. | 11:14 |
| magcius | So it's not running at all. | 11:15 |
| magcius | OK. | 11:15 |
| magcius | Is there an open bug about fixing the traversal system with respect to vhosts? | 11:15 |
| wgrant | Fixing? | 11:15 |
| magcius | Yes. | 11:15 |
| wgrant | The vhosts will likely vanish soon. | 11:15 |
| magcius | Ah. | 11:15 |
| wgrant | But traversal with respect to them is not broken. | 11:15 |
| magcius | Well, each vhost doesn't get its own RootObject. | 11:16 |
| wgrant | That's correct, and intended. | 11:16 |
| wgrant | That doesn't make it broken. | 11:16 |
| magcius | Really? | 11:16 |
| magcius | Well, that means that you can't do something like I described, without some serious hackery. | 11:16 |
| magcius | as I said above, http://bugs.launchpad.net/12345 should *work*, dammit. | 11:16 |
| wgrant | The intention of the vhosts was not to do things like that. | 11:16 |
| wgrant | => they are not broken for the purpose for which they were introduced. | 11:16 |
| magcius | I don't care how they were introduced, I'd expect it to be the right behavior. | 11:17 |
| magcius | but, eh. | 11:17 |
| wgrant | They were not introduced to change the traversal hierarchy. | 11:17 |
| magcius | What was the purpose if not to isolate the services? | 11:17 |
| wgrant | So having an inflexible traversal hierarchy *is* the right behaviour. | 11:18 |
| wgrant | To provide different default views, and permit facet-specific views on some vhosts but not others. | 11:18 |
| magcius | I don't see how you can't look at "bugs.launchpad.net" and see "things related to bugs should always go here" | 11:18 |
| wgrant | Right, but that doesn't mean the traversal hierarchy needs to differ. | 11:19 |
| magcius | well, that means that http://blueprints.launchpad.net/ubuntu/+bug/1 shouldn't work | 11:19 |
| _mup_ | Bug #1: Microsoft has a majority market share <iso-testing> <ubuntu> <Clubdistro:Confirmed> <Computer Science Ubuntu:Invalid by compscibuntu-bugs> <EasyPeasy Overview:Invalid by ramvi> <GNOME Screensaver:Won't Fix> <Ichthux:Invalid by raphink> <JAK LINUX:Invalid> <The Linux OS Project:In Progress> <metacity:In Progress> <OpenOffice:In Progress by lh-maviya> <Tabuntu:Invalid by tinarussell> <Tivion:Invalid by shakaran> <Tv-Player:New> <Ubuntu:I | 11:19 |
| magcius | has the goal of faceted views been realized? | 11:19 |
| wgrant | That would be achieved by renaming Bug:+index to Bug:+bugs-index, and setting it as a view for just the bugs facet. | 11:20 |
| lifeless | we're currentlt reviewing the separate domains with an eye to removing them | 11:20 |
| wgrant | Right. | 11:20 |
| lifeless | because | 11:20 |
| lifeless | - slow (more ssl handshakes) | 11:20 |
| lifeless | - confusing | 11:20 |
| lifeless | jml keeps asking me when / how hard :) | 11:20 |
| lifeless | gnight | 11:21 |
| magcius | right -- it's stupid, ugly and confusing when you can have a large number of URLs for the same bugs | 11:21 |
| jam | magcius: honestly, I like it quite a bit, except you have to be able to combine adjacent regions so you don't overflow the text regions so much. | 11:33 |
| jam | So if your sections 1-4 were one big box | 11:33 |
| jam | then you could fit lots of text on the left | 11:33 |
| jam | without causing it to overflow in weird ways | 11:34 |
| jam | it also lets you shrink it a bit, because wrapping text on the left won't break the code lines on the right, etc. | 11:34 |
| magcius | jam, sections 1-4? | 11:55 |
| deryck | Morning, all. | 11:56 |
| === al-maisan is now known as almaisan-away | ||
| magcius | jam, http://i.imgur.com/9o0g5.png | 12:01 |
| LPCIBot | Project windmill build #209: STILL FAILING in 1 hr 4 min: https://lpci.wedontsleep.org/job/windmill/209/ | 12:02 |
| magcius | jam, I forgot to re-set the border styles... I was debugging. | 12:03 |
| jam | magcius: so I think if the contents is left justified the header should be for Revision. And I think it is using a bit too much space, but yes, I like that style | 12:27 |
| magcius | Oh, right, that. | 12:28 |
| jam | I also notice that you didn't actually allow the contents for the first revision to move into line 2 | 12:28 |
| magcius | I didn't even notice the right-aligned revision. | 12:28 |
| jam | line 1 is still wider than normal | 12:28 |
| jam | (taller) | 12:28 |
| magcius | Yeah, that's because of how the table system works. | 12:28 |
| jam | same for line 7 | 12:28 |
| magcius | I should probably use rowspan. | 12:28 |
| jam | magcius: Isn't there some sort of "span" ability? | 12:28 |
| magcius | We don't use it currently. | 12:28 |
| jam | or do you have to nest the tables to get it | 12:28 |
| magcius | Nah, there's rowspan. | 12:28 |
| jam | magcius: right, so *with* rowspan, I like it, quite a bit | 12:29 |
| jam | I don't think it is going to be particularly more costly to pull out the revision comments than it is to compute the annotation | 12:29 |
| jam | so as long as you have to explicitly ask for annotation | 12:29 |
| jam | I'm not as worried about performance of this | 12:29 |
| magcius | yeah, it's "change.comment.splitlines[0]" | 12:30 |
| magcius | The problem with rowspan is that I need to calculate the number of rows 'til the next change | 12:30 |
| magcius | Which means that I need to refactor the annotation generator. | 12:30 |
| jam | magcius: how is it being computed now? | 12:30 |
| magcius | One at a time. | 12:30 |
| magcius | In a Python generator. | 12:30 |
| jam | (given that it is already knowing how many lines before it needs to output the same info) | 12:30 |
| magcius | What do you mean? | 12:31 |
| magcius | We just set a flag "new_rev" | 12:31 |
| magcius | right now | 12:31 |
| jam | magcius: it already omits lines if they are the same as the previous entry | 12:31 |
| magcius | Right, there's a "new_rev" | 12:31 |
| jam | I don't think there is much gained by using a generator here | 12:31 |
| jam | tbh | 12:31 |
| magcius | Right. | 12:31 |
| jam | I think just returning lists would be fine, and let you do the work we actually want | 12:31 |
| magcius | Right. | 12:31 |
| jam | we already loaded *tons* of file content to do the annotation | 12:31 |
| magcius | OK | 12:47 |
| magcius | I'm confused about TAL | 12:47 |
| magcius | I have a dict of lineno => Container | 12:51 |
| magcius | and I want to access it from TAL | 12:51 |
| magcius | I thought | 12:51 |
| magcius | python:annotated[path(repeat/line/number)] | 12:51 |
| magcius | would work | 12:51 |
| magcius | WARNING:simpleTALES.Context:Exception occurred evaluating python path, exception: unsupported operand type(s) for /: 'dict' and 'unicode' | 12:52 |
| === bac changed the topic of #launchpad-dev to: We have a 9s timeout!!! (2 months early!) | https://dev.launchpad.net/ | On call reviewer: bac | https://code.launchpad.net/launchpad-project/+activereviews | ||
| danilos | bac, when we are done with the call: https://code.launchpad.net/~danilo/launchpad/bug-761257/+merge/58801 :) | 13:44 |
| bac | danilos: ok | 13:52 |
| magcius | jam, http://i.imgur.com/eHfVJ.png | 13:52 |
| jam | magcius: I really like the left border, but the width seems way too wide. | 13:53 |
| magcius | jam, I don't understand tables at all. | 13:53 |
| magcius | O | 13:53 |
| magcius | I'm not sure how to say "use the minimum size possible with the least amount of wrapping" | 13:53 |
| jcsackett | bac: i humbly request a review for https://code.launchpad.net/~jcsackett/launchpad/comment-display-rules/+merge/58760. | 13:55 |
| bac | jcsackett: ok. you're next. | 13:55 |
| jcsackett | thanks, bac. | 13:55 |
| danilos | bac, thanks | 13:56 |
| magcius | jam, I'm getting closer. | 14:21 |
| jam | magcius: I don't know it either. but even just saying "25 ex" or something might be reasonable | 14:22 |
| danilos | magcius, there is min-width, max-width CSS properties in recent browsers (and CSS2 I think) that can help you with some of the stuff, but tables are generally pretty smart | 14:23 |
| magcius | white-space: nowrap; | 14:24 |
| magcius | http://i.imgur.com/6qBAO.png | 14:24 |
| magcius | I did some other tweaks as well. | 14:24 |
| danilos | magcius, I think you'd get the best result for a table itself if you didn't have width:100% on it, then it'd take the minimum space necessary | 14:25 |
| danilos | magcius, (or equivalent of width: 100%) | 14:25 |
| magcius | But then the width wouldn't be fixed. | 14:26 |
| danilos | magcius, right | 14:26 |
| magcius | That'd be even uglier :( | 14:26 |
| danilos | magcius, how come? | 14:26 |
| magcius | The files would be all over the place/. | 14:26 |
| magcius | An empty file would be small | 14:26 |
| magcius | vs. a file with a lot of big history | 14:26 |
| danilos | magcius, oh, you have multiple tables? | 14:26 |
| magcius | Of course. One per file. | 14:27 |
| danilos | magcius, heh, sorry then, the screenshot seems to be one-table-per-web-page :) | 14:28 |
| magcius | That's what I meant. | 14:28 |
| danilos | magcius, then I don't understand what did you mean with 'the width wouldn't be fixed'? | 14:30 |
| danilos | magcius, across different web pages? | 14:30 |
| magcius | Yes. | 14:30 |
| magcius | Going between tabs and pages, the files would be inconsistent. | 14:31 |
| magcius | Or at least more inconsistent than width: 100%; | 14:31 |
| danilos | magcius, well, I'd prefer that over the too-wide first column, but it's just a personal preference :) cheers | 14:33 |
| === almaisan-away is now known as al-maisan | ||
| danilos | bac, hi, do you happen to have any questions regarding a review? | 15:00 |
| bac | danilos: i just got to watch your screencast for the bug. thanks for doing it! nice dance track. | 15:00 |
| bac | danilos: no questions yet | 15:00 |
| danilos | bac, heh :) | 15:00 |
| danilos | thanks | 15:01 |
| bac | danilos: the old method named 'unsubscribe_current_user' is misnamed now, no? | 15:13 |
| bac | you are using it to unsubscribe a team | 15:13 |
| danilos | bac, yes, there is a bunch of things like that in the code | 15:14 |
| danilos | bac, I was thinking of tackling that in the follow-up branch | 15:14 |
| bac | danilos: ok, i think that's a good idea | 15:14 |
| bac | thanks | 15:14 |
| danilos | bac, many of the methods have not even been renamed when mute functionality was introduced, so they sometimes do things related to that as well :/ | 15:15 |
| bac | danilos: also, to get around the team names clashing with other actions, perhaps we could prefix the team names with something that is not allowed and then strip it when we process the submission | 15:15 |
| danilos | bac, yeah, that's an option as well (and we could even prepend the '/~' making them actual API paths that need no processing) | 15:16 |
| bac | danilos: +1 | 15:16 |
| danilos | bac, it's still a separate bug that gary_poster said we should look into later | 15:16 |
| bac | danilos: i agree | 15:16 |
| bac | i think this branch is fine. let me add some comments | 15:16 |
| danilos | bac, it's a nice idea though! | 15:16 |
| danilos | bac, great, thanks | 15:16 |
| danilos | bac, thanks again for the review, off to land this | 15:27 |
| bac | np danilos. have a nice weekend | 15:27 |
| danilos | cheers | 15:27 |
| === al-maisan is now known as almaisan-away | ||
| === deryck is now known as deryck[lunch] | ||
| LPCIBot | Project windmill build #210: STILL FAILING in 1 hr 6 min: https://lpci.wedontsleep.org/job/windmill/210/ | 16:34 |
| sinzui | bac: do you have time to review https://code.launchpad.net/~sinzui/launchpad/bugtask-create-question-1/+merge/58826 today | 16:53 |
| bac | sinzui: of course | 17:04 |
| sinzui | thanks | 17:04 |
| bac | sinzui: are you in a rush? mind if i eat lunch first? | 17:05 |
| sinzui | thats good. I want to have lunch now too | 17:05 |
| bac | cool | 17:05 |
| === deryck[lunch] is now known as deryck | ||
| === beuno is now known as beuno-lunch | ||
| benji | bac: I've been waiting on the diff for https://code.edge.launchpad.net/~benji/launchpad/bug-768336/+merge/58830 to update before asking you to look at it, but it hasn't so... | 18:21 |
| sinzui | jcsackett: are you about to mumble? | 18:21 |
| benji | bac: the diff's up now | 18:23 |
| === beuno-lunch is now known as beuno | ||
| bac | benji: ok. i've got to do sinzui's branch first | 18:32 |
| benji | np | 18:32 |
| jcsackett | sinzui: i am available for mumble. | 18:40 |
| jcsackett | sinzui: feel free to ping me or just say hello if/when you are available again. | 18:41 |
| === almaisan-away is now known as al-maisan | ||
| bac | sinzui: done. | 18:57 |
| sinzui | thank you bax | 18:59 |
| sinzui | bac | 18:59 |
| === al-maisan is now known as almaisan-away | ||
| benji | bax: (noun, plural) one or more bac | 19:18 |
| magcius | Using default stacking branch /+branch-id/10062 at lp-81385104:///~jstpierre/loggerhead | 19:19 |
| magcius | Created new stacked branch referring to /+branch-id/10062. | 19:19 |
| magcius | yay, completely useless information! | 19:19 |
| maxb | hmm, that's not entirely optimal | 19:31 |
| maxb | on the other hand, yay for stacking that doesn't break when branches get renamed | 19:31 |
| magcius | I have no idea what that's supposed to mean. | 20:06 |
| magcius | But yeah, branches renaming is dumb too | 20:07 |
| magcius | maxb, https://bugs.launchpad.net/bzr/+bug/569361 | 20:09 |
| _mup_ | Bug #569361: Remembering an URL different from the one the user entered is surprising <Bazaar:Confirmed> < https://launchpad.net/bugs/569361 > | 20:09 |
| maxb | magcius: as it happens, that bug is actually fixed | 20:12 |
| maxb | ish, at least where launchpad is concerned | 20:13 |
| magcius | OK | 20:31 |
| magcius | What's the standard thing that implements IPerson? | 20:31 |
| magcius | I've looked all through the .zcml and .py files | 20:31 |
| magcius | I can't find a single implements(IPerson) | 20:32 |
| magcius | aha, lp.registry.model.person | 20:34 |
| magcius | should have looked harder | 20:34 |
| magcius | LocationError: (<lazr.restful.tales.WebLayerAPI object at 0xce6ee10>, 'json') | 21:21 |
| jcsackett | magcius: what's the context for that? | 21:26 |
| magcius | wow that's a terrible error message for what I did :P | 21:26 |
| magcius | I accidentally changed something on Person, and it spit out that | 21:27 |
| magcius | I'll try to get a full traceback. | 21:27 |
| jcsackett | based on the error, i'm assuming you're working with the API? | 21:27 |
| magcius | No, I'm working with the LP code. | 21:27 |
| magcius | web code | 21:28 |
| magcius | I assume something ate the error. | 21:28 |
| jcsackett | magcius: ah, dig. | 21:28 |
| magcius | http://paste.pocoo.org/show/376548/ | 21:29 |
| magcius | It's working now, I assume. | 21:30 |
| magcius | I had typo'd the "mugshot" property as "mugsoht" and got a terrible message. | 21:30 |
| jcsackett | yeah, you scared zope's traversal, i think. | 21:31 |
| magcius | I assume something ate the traversal message, and then the traversal was all fudged up. | 21:32 |
| magcius | How do you guys usually dev the server? | 21:33 |
| magcius | Do you "make run" and then ^C and run it again when you make changes? | 21:33 |
| magcius | ForbiddenAttribute | 21:33 |
| magcius | WTF? | 21:34 |
| jcsackett | magcius: yes, "make run" and then either ctrl+c or if you're running it in the background "make stop" | 21:34 |
| magcius | can I tell zope.security to shut up? | 21:35 |
| jcsackett | magcius: ForbiddenAttribute happens when you're accessing something that a) does not exist (e.g. attribute error), b) something that doesn't exist in the associated interface, or c) something you don't have permission. | 21:35 |
| magcius | jcsackett, OK, I didn't know if you had some magic to make it restart on code changes. | 21:35 |
| magcius | jcsackett, what if I don't have an interface? | 21:35 |
| jcsackett | magcius: can you show me the traceback? | 21:35 |
| magcius | http://paste.pocoo.org/show/376548/ | 21:36 |
| magcius | there's a simple diff right there | 21:36 |
| magcius | and http://paste.pocoo.org/show/376555/ | 21:36 |
| magcius | is the traceback | 21:36 |
| jcsackett | magcius: this is pulling in a lot of browser behavior into the model. | 21:37 |
| magcius | Is there somewhere else I should change it? | 21:38 |
| magcius | tales.py perhaps? | 21:38 |
| jcsackett | magcius: you might have more luck adding an attribute like "has_gravatar" to the model, and then using that to inform the views that use mugshot that they should render the gravatar stuff instead of pulling a mugshot from the librarian. | 21:39 |
| magcius | realistically, I don't want it to be an option, I'd rather that we remove the user-configured setup entirely and switch over to gravatar globally | 21:39 |
| magcius | but the flag-inistas are never going to fall for it | 21:39 |
| jcsackett | magcius: not everyone has gravatar accounts, nor should we likely require them. | 21:40 |
| magcius | there can be a link: change your gravatar by clicking here | 21:40 |
| magcius | everybody else has adapted to a gravatar | 21:41 |
| jcsackett | which requires people to sign up for a service they may not be interested in. | 21:41 |
| jcsackett | i don't think all services use gravatar, magcius. :-) | 21:41 |
| jcsackett | i agree that providing users the option of using gravatar would be cool. | 21:41 |
| magcius | jcsackett, I don't know any service that doesn't at this point. | 21:42 |
| magcius | and if you are one of the three people that hasn't signed up for gravatar, well, i doubt you're going to be inclined to upload a custom image with a widget either | 21:42 |
| magcius | so you can be content with /@@/person-mugshot | 21:43 |
| magcius | sorry, sorry, i just really hate the "A or B? Option for either!" mantra | 21:43 |
| jcsackett | magcius: i understand. frequently, i agree. | 21:44 |
| jcsackett | in this instance, i don't. :-) | 21:44 |
| magcius | i don't see what the loss is, though | 21:44 |
| magcius | is there anybody who would use a different mugshot/logo than they would use for gravatar? | 21:44 |
| magcius | jcsackett, OK, what about: gravatar by default, with the default being the librarian image? | 21:45 |
| magcius | that way it's a graceful fallback | 21:45 |
| jcsackett | magcius: quite probably. mugshots on launchpad are usually actual pictures of the user. | 21:45 |
| magcius | if you don't have a gravatar image, you fall back to the librarian | 21:45 |
| magcius | So, why does it give me a ForbiddenAttribute here? | 21:46 |
| magcius | Technically. | 21:46 |
| benji | It makes more sense to have it the other way around: we use your general purpose gravatar image unless you go to the trouble of uploading a launchpad-only image. | 21:47 |
| magcius | either way is fine with me | 21:47 |
| magcius | I'm just playing around with the Launchpad code for now | 21:47 |
| jcsackett | magcius: i think it's probably b/c the mugshot attribute is getting proxied, and there's no interface or security definitions for the gravatar objects. | 21:49 |
| jcsackett | but i'm not 100% certain of that. | 21:49 |
| magcius | How is it getting proxied? | 21:49 |
| jcsackett | mugshot is part of person, persons are enclosed in zope security voodoo. | 21:49 |
| magcius | Right, notice that I took the Storm field out and replaced it with a property. | 21:50 |
| magcius | But ouch, zope voodo. | 21:50 |
| magcius | o. | 21:50 |
| magcius | is there some decorator I can add? | 21:50 |
| magcius | Eventually, I want to hack up LP bugs so I can go from this: http://i.imgur.com/awyVF.png | 21:50 |
| magcius | to this: http://i.imgur.com/2by4O.png | 21:51 |
| magcius | (there's also a WIP here: http://i.imgur.com/Y0MiW.png) | 21:51 |
| jcsackett | magcius: responding to your question about a decorator. i don't know of one per se, but you can (temporarily) get rid of proxies by using removeSecurityProxy from zope.security.proxy | 21:57 |
| jcsackett | you might hack up tales.py where the error is raised for right now to do that, see if what your'e doing is at least on the right path, and then you can hunt down what you have to do to make zope security happy. | 21:57 |
| === bac changed the topic of #launchpad-dev to: We have a 9s timeout!!! (2 months early!) | https://dev.launchpad.net/ | On call reviewer: - | https://code.launchpad.net/launchpad-project/+activereviews | ||
| magcius | ok | 22:01 |
| magcius | I have a feeling it's something to do with the mugshot/mugshotID in interfaces/person.py | 22:02 |
| jcsackett | magcius: very possibly. | 22:03 |
| magcius | Is there someone who knows that code better? | 22:04 |
| magcius | OK, I'm still getting that ForbiddenAttribute error | 23:47 |
| magcius | http://paste.pocoo.org/show/376624/ is my latest patch | 23:48 |
| lifeless | magcius: hi | 23:51 |
| lifeless | magcius: uhm, just to let you know, I am strongly against anything like avatars causing per-person lookups inside a webapp | 23:52 |
| lifeless | magcius: we show thousands of persons at once on some pages, and the overhead is simply intolerable. | 23:52 |
| lifeless | magcius: I can't tell if you need to do that for gravatar or not, but if you do its a problem | 23:53 |
| lifeless | magcius: the second thing is that we must not disclose email addresses on anonymous views of pages (and urls with email addresses in them will disclose it) | 23:53 |
| lifeless | magcius: finally we have many many users that have selected 'do not show at all' for their email, and the same issue will apply. | 23:54 |
| magcius | lifeless, gravatar uses an md5 hash of the email | 23:54 |
| lifeless | magcius: the reason you are having trouble is you are replacing DB columns with properties. | 23:54 |
| magcius | ForbiddenAttribute: ('getURL', <lp.registry.model.person.GravatarLogo object at 0xb351fd0>) | 23:55 |
| magcius | There's nothing forbidden about that! | 23:55 |
| lifeless | well, that too | 23:55 |
| magcius | I explicitly allowed it in the ZCML! | 23:55 |
| lifeless | I can see that | 23:55 |
| magcius | http://en.gravatar.com/site/implement/hash/ | 23:56 |
| magcius | So no, there's no email leakage. | 23:56 |
| magcius | Unless you consider MD5 to be too unsafe | 23:56 |
| lifeless | its not reversible | 23:56 |
| lifeless | so that concern is addressed | 23:56 |
| lifeless | there is another | 23:56 |
| magcius | Which is? | 23:56 |
| magcius | What we're doing is replacing http://launchpadlibrarian.net with http://gravatar.com/ | 23:57 |
| lifeless | we need to make sure we generate size info so the page can render if gravatar is blocked/down/ or just slow | 23:57 |
| magcius | "size info"? | 23:57 |
| lifeless | img href width height | 23:57 |
| magcius | You request a size from gravatar. | 23:57 |
| lifeless | ok | 23:58 |
| magcius | In the case of a mugshot, it's 196x196 | 23:58 |
| magcius | So, I'd send along https://www.gravatar.com/gravatar/md5?s=196 | 23:58 |
| magcius | if we want to fall back to /@@/person-mugshot | 23:58 |
| magcius | you specify 'd' | 23:58 |
| lifeless | jcsackett's question around making this optional etc is something I think needs discussion with jml | 23:59 |
| magcius | See http://en.gravatar.com/site/implement/images/ | 23:59 |
| magcius | Sure, I'm just using this as an example of hacking LP's code. | 23:59 |
| magcius | And I'm not having much success. | 23:59 |
| magcius | I don't expect this to land tomorrow or anything. | 23:59 |
| lifeless | I'm not sure why you are getting forbidden attribute there | 23:59 |
| magcius | I'm not either. | 23:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
