| wallyworld_ | wgrant: some already filed a bug http://youtrack.jetbrains.net/issue/PY-2741 | 00:14 |
|---|---|---|
| wallyworld_ | someone | 00:14 |
| wallyworld_ | actually, it seems like it pertains to code completion but it will be related | 00:20 |
| lifeless | wallyworld_: this is the whole 'our packaging can only support one version at a time' issue. | 00:31 |
| wallyworld_ | lifeless: thanks. btw, a gentle reminder about https://code.launchpad.net/~xaav/loggerhead/export-tarball/+merge/66408 :-) | 00:33 |
| lifeless | thanks ;P | 00:33 |
| wallyworld_ | an adaptor question - i think we should be able to specify a class as being an adaptor for more than one interface, wither via @adaptor(foo, bar) or in zcml for="foo bar"? it appears not since it doesn't work. and so i have to have one zcml adaptor element per interface, which the same adapter class/factory specfified. is there something i'm missing? | 00:37 |
| wgrant | wallyworld_: I'm not sure... but it's probably not an adapter for more than one interface. | 00:39 |
| wgrant | Those interfaces probably share a subinterface. | 00:40 |
| wallyworld_ | wgrant: in this case it is the same adapter implementation for more than one interface that i would like to specify. they do share a subinterface (in fact > 1) and the adaptor makes use of those >1 subinterfaces. you'd think it would be possible | 00:41 |
| wallyworld_ | in this case, i want to say "this specified class adapts both IDistribution and IProduct" | 00:42 |
| wallyworld_ | i can do it but need to have multiple adapter declarations with just a different for="xxx" which seems dumb | 00:43 |
| wallyworld_ | and i can't user @adapter() so am forced to use zcml | 00:43 |
| wgrant | StevenK: QA! | 00:46 |
| StevenK | wgrant: Yeah yeah :-P | 00:47 |
| StevenK | lifeless: Can haz bazaar-experts team deleted? | 01:01 |
| lifeless | StevenK: we've deployed everywhere since? | 01:01 |
| lifeless | StevenK: including crannies like soyuz ? | 01:01 |
| StevenK | I know cocobanana has been deployed to since, and one of germanium's trees is in NDT. Not sure about the other tree | 01:02 |
| wgrant | poppy doesn't use bazaar-experts, so meh. | 01:10 |
| StevenK | I'd be a little concerned if it did ... | 01:11 |
| nigelb | Good morning | 01:48 |
| wgrant | lifeless: Do you think the timeouts are regressions, or bloat? | 02:05 |
| wgrant | We went from 300 to 1100 in a week. | 02:05 |
| StevenK | Wheee | 02:06 |
| wgrant | I suspect the subscription changes. | 02:06 |
| wgrant | But that doesn't explain Person:EntryResource:searchTasks | 02:06 |
| wgrant | I don't think. | 02:06 |
| lifeless | wgrant: the soft counts last week would have been large if it was creeping bloatism | 02:08 |
| StevenK | lifeless: So, can haz bazaar-experts deleted, or do you want to wait until every single tree doesn't mention the celebrity? | 02:10 |
| lifeless | I'd like to wait | 02:10 |
| lifeless | easy to do, reduces risk, doesn't slow anything down. | 02:10 |
| StevenK | lifeless: Until when? :-) | 02:10 |
| StevenK | If you say the heat death of the universe, I'm booking a flight to NZ :-P | 02:11 |
| lifeless | no live trees with the celeb in them | 02:11 |
| StevenK | The only services I can think of are germanium's poppy and probably forster | 02:12 |
| StevenK | Oh, and the librarian? | 02:12 |
| lifeless | likely, yes. | 02:17 |
| lifeless | wallyworld_: hi | 02:35 |
| wallyworld_ | hello | 02:35 |
| lifeless | wallyworld_: your affiliation work; it looks like its going to do lots of eager loading. | 02:35 |
| wallyworld_ | i put in tests for the query count (for bugtask) - it's only 4 quesies soince stuff is already loaded | 02:36 |
| lifeless | per person | 02:36 |
| wallyworld_ | yes, that is true | 02:37 |
| lifeless | s/eager loading/lazy loading/ | 02:37 |
| wallyworld_ | so i probably need to fix that | 02:37 |
| lifeless | wallyworld_: anytime you end up writing adapters, and object orientated code that does data access, its probably going to involve late evaluation/lazy loading problems. | 02:38 |
| lifeless | wallyworld_: the -only- case I know where such code is fine is when it happens /once/ on a page/http request as part of establishing the context (rather than potentially in inner loops etc) | 02:38 |
| wallyworld_ | i'm starting to see that. i still haven't got it hard wired in my brain that zope object navigation = queries | 02:39 |
| lifeless | also inTeam() for instance. | 02:39 |
| lifeless | userA.inTeam(teamT) | 02:39 |
| wallyworld_ | i'm used to popo/pojo type domain models | 02:39 |
| lifeless | userB.inTeam(teamT) | 02:39 |
| lifeless | will do 2 queries, even if userA, userB and teamT are all already loaded. | 02:40 |
| wallyworld_ | to access the join table? | 02:40 |
| lifeless | yup | 02:40 |
| wallyworld_ | ah, well it's behind a feature flag at least. i'll do a branch to optimise. that's for pointing it out | 02:40 |
| wallyworld_ | except i think we turn on the feature flag recently, must check | 02:42 |
| wgrant | picker_enhancements is on. | 02:42 |
| wallyworld_ | lifeless: the issue with the implementation is that the call to the affiliation adapter is done inside getPickerEntry(), which itself is an adapter method which is done per picker row | 02:47 |
| wallyworld_ | so there's potentially a bit of work to do to change all that | 02:48 |
| wallyworld_ | still, must be done | 02:49 |
| wallyworld_ | not quite so straightforward, since each item in a batch might not use the same picker entry adaptor | 02:56 |
| lifeless | right | 02:56 |
| lifeless | It's common to run into friction like this | 02:57 |
| wallyworld_ | yeah, so now picker entry adapters and HugeVocabularyJSONView are getting a bit of rework | 03:05 |
| wallyworld_ | lifeless: the current affiliation adaptor implmentation may not be so bad. the inTeam checks will be cached and the 4 queries i mentioned before are not per person i don't believe - eg once the pillar.bug_supervisor is fetched, it is not required to hit the db again for each person to check | 03:49 |
| wallyworld_ | actually, the inteam caching assertion is bogus | 03:51 |
| lifeless | the current caching implementation is (team, member) specific | 03:52 |
| lifeless | what you need is an interface for populating many such cache tuples at once | 03:52 |
| wallyworld_ | there's lots of other places where we do inTeam(xxx) or inTeam(yyy) eg canBeUnsubscribedBy() | 03:53 |
| lifeless | and to call that with all the teams, for all the rows, and all the row person-or-team objects at once. | 03:53 |
| wallyworld_ | that is a solution but it severely breaks encapulation of the adapter implementation | 03:54 |
| lifeless | yes | 03:55 |
| lifeless | such encapasulation is harmful in our environment TBH | 03:55 |
| wallyworld_ | it may be easier to load the team members inside the adapter implementatiin and cache those | 03:55 |
| wallyworld_ | encapsulation is preserved and cachiing can still occur | 03:56 |
| lifeless | is the adapter per row ? | 03:56 |
| wallyworld_ | the adapter is per row but the teams are static for a given adaper instance | 03:56 |
| lifeless | to solve the problem you need to have set based interfaces. | 03:57 |
| lifeless | anything that is object based or row based will have late evaluation symptoms | 03:57 |
| wallyworld_ | or load the team members the adapter knows it needs up front | 03:57 |
| lifeless | no | 03:57 |
| lifeless | that won't work because the adapter is per row | 03:58 |
| wallyworld_ | it will work - each row is checking for emebrship in fixed set of teams, known in advance | 03:58 |
| lifeless | uhm | 03:58 |
| wallyworld_ | so if those team members are loaded and cached, no extra db access is needed | 03:58 |
| wgrant | Bug #822500 | 03:59 |
| _mup_ | Bug #822500: select <Launchpad itself:New> < https://launchpad.net/bugs/822500 > | 03:59 |
| lifeless | wallyworld_: This doesn't fit with my understanding of the code. | 03:59 |
| StevenK | Nice title | 03:59 |
| lifeless | wallyworld_: calling personA.inTeam(teamT) will not seed the cache for calling personB.inTeam(teamT) | 04:00 |
| StevenK | In fact, I found something odd last night -- https://code.launchpad.net/~aaacavazos/+recipe/launchpad-daily-1 | 04:00 |
| wallyworld_ | lifeless: yes, that's why i am saying we need to load and cache these relationships | 04:00 |
| wallyworld_ | do it once up front for the known, fixed teams | 04:01 |
| lifeless | wallyworld_: which requires knowing all the personX's and all the teamY's, doesn't it ? | 04:01 |
| StevenK | And there are seven recipes for stable ... | 04:01 |
| wallyworld_ | lifeless: a person (row) is affiliated with the context (eg bug task) if they are a member of (eg) the security contacts team or the bug supervisors team | 04:01 |
| wallyworld_ | so we can load and cache the team membership of those 2 teams | 04:02 |
| wallyworld_ | then as each row is checked, no extra db acces sis needed | 04:02 |
| wallyworld_ | i guess that doesn't account for indirect membership | 04:02 |
| wallyworld_ | but it could | 04:03 |
| lifeless | wallyworld_: this seems both more work, and more code, than accumulating the set of (person, team) pairs we need an doing one call to check them all. | 04:03 |
| wallyworld_ | but it preserves encapulation and the inheritance model used but the adapter implenmentation | 04:04 |
| lifeless | by more work I mean more runtime work. | 04:04 |
| lifeless | You have to be sure what you're encapsulating; if you're encapsulting the wrong things its a pessimisation. | 04:05 |
| wallyworld_ | hmm. not sure. maybe. | 04:05 |
| wallyworld_ | what's being encapulated is the method to find out a person's affiliation with a given context | 04:06 |
| lifeless | yes, (I've read the patches) | 04:06 |
| lifeless | this is *exactly* the sort of thing I've had to rip out to fix performance issues up and down the LP stack. bugs. soyuz. questions. blueprints. | 04:06 |
| wallyworld_ | and as dicussed, it hooks into the existing workflow of one call per picker row | 04:06 |
| lifeless | productseries has its own driver | 04:07 |
| lifeless | a product with 15 series will have 15 series teams to check. | 04:07 |
| wallyworld_ | i don't think that sort of check is done currently | 04:07 |
| lifeless | (if you honour them all) | 04:07 |
| wallyworld_ | if the context is a product, then series doesn't come into it | 04:08 |
| lifeless | anyhow, I can see this isn't making sense to you | 04:08 |
| lifeless | so we can drop it and be evidence based. | 04:08 |
| wallyworld_ | it is, i'm just not quite agreeing with the suggested approach (yet) | 04:08 |
| lifeless | All I can say, is that everything I've seen over the last year predisposes me to consider *any* per-single-object interface flawed by design. | 04:09 |
| wallyworld_ | sure, but there's a balance between that and adhering to good oo design principals | 04:09 |
| lifeless | not for code involved in batches/iteration/queries | 04:10 |
| wallyworld_ | yes, i can see the merit in that statement | 04:11 |
| lifeless | (oh and for loops, which is implicit in iteration) | 04:11 |
| lifeless | OO design hasn't scaled to non-main-memory datasets IMO | 04:11 |
| wallyworld_ | there's two ways of solving it - use set based interfaces or provide a caching implementation that prevents single calls from hitting the db | 04:12 |
| lifeless | I agree there are two approaches, but the latter isn't one of the two - it doesn't work. | 04:12 |
| wallyworld_ | ie for a given use case, load the domain model known to be required to satisfy the request | 04:13 |
| lifeless | (the second approach I'd layout is the object query language approach) | 04:13 |
| wallyworld_ | i've used that in combination with my second approach above | 04:14 |
| lifeless | there are two disjoint problems with caching implementations | 04:14 |
| lifeless | if you avoid one you get the other | 04:14 |
| lifeless | the first problem is bog standard late evaluation | 04:15 |
| lifeless | even if you cache each single call, when you iterate you make a new call for each person, and thats a cache miss | 04:15 |
| wallyworld_ | but if the domain model that is being reference for each call is the same... | 04:16 |
| lifeless | the answer to that is (broadly) readahead: when you get two similar lookups your cache can decide to load everything related | 04:16 |
| lifeless | wallyworld_: its not in your case, but we'll come back to that :) | 04:16 |
| lifeless | the problem with readahead is that it blows out in size | 04:16 |
| lifeless | e.g. readahead in the inTeam case would be to select person from teamparticipation where team=self.id, in inTeam | 04:17 |
| wallyworld_ | readahead is suboptimal - i normally don't like tyo use it except ib very narrow, specific circumstances | 04:17 |
| lifeless | rather than looking up single members. | 04:17 |
| lifeless | for LP, we have some very big teams: | 04:17 |
| lifeless | select count(*) from teamparticipation group by team order by count(*) desc limit 20; | 04:17 |
| lifeless | count | 04:17 |
| lifeless | ------- | 04:17 |
| lifeless | 18490 | 04:18 |
| lifeless | (dropping to 855 at the 20th) | 04:18 |
| lifeless | but still, those big teams, in a naive implementation, will be a bit of a headache. | 04:18 |
| lifeless | (for instance, loading 18K rows from disk, if we had a cold cache situation, is ~ 40 seconds on its own) | 04:19 |
| wallyworld_ | what's that large team called? | 04:19 |
| lifeless | that one is locoteams | 04:20 |
| lifeless | 18490 | 2968414 | locoteams | 04:20 |
| lifeless | 11026 | 2979678 | locoteams-approved | 04:20 |
| lifeless | 3842 | 2388044 | launchpad-users | 04:20 |
| lifeless | 2936 | 1121852 | ubuntu-us | 04:20 |
| lifeless | wallyworld_: but I only brought that up to illustrate the solution-is-worse-than-the-problem nature of using on demand caching strategies in this sort of problem | 04:22 |
| wallyworld_ | given the size of the largest teams, i can see the issue. | 04:23 |
| wallyworld_ | although i do wonder whether such team would ever be a product owner or bug supervisor etc | 04:24 |
| wallyworld_ | i had the feeling such attributes would be individuals or small teams | 04:24 |
| lifeless | for loco team issue tracking, sure ;) | 04:24 |
| wallyworld_ | fair enough. one of my underlying assumptions was that the team sizes involved in affiliation would be smallish | 04:25 |
| wallyworld_ | thus allowing other interfaces than a set based one | 04:26 |
| lifeless | ubuntu-bugs has 416 people in it | 04:26 |
| lifeless | openstack has 1300 | 04:26 |
| lifeless | ubuntu-marketing 1400 | 04:27 |
| wallyworld_ | that's not too bad - caching 1300 id tuples | 04:27 |
| wallyworld_ | but anyway, that seems like suboptimal approach now | 04:28 |
| wallyworld_ | it also means that the adaptor based approach needs to be rethought i suspect | 04:29 |
| wallyworld_ | actually, maybe not | 04:29 |
| lifeless | if you expose the things that need looking up | 04:29 |
| wallyworld_ | just the getAffiliation interface | 04:30 |
| lifeless | a single call can seed the caches of the various teams with the results for all the queries that may be made | 04:30 |
| lifeless | this is a set based interface | 04:30 |
| lifeless | set based doesn't mean 'no abstraction' :) | 04:30 |
| wgrant | I think we probably need to do away with adapters at some point. | 04:30 |
| wgrant | It is always tempting to use them because they are nice and we use them already, but they make it awkward to setify. | 04:31 |
| wallyworld_ | i think i just need to change getAffiliationBadge(self, person) to getAffiliationBadges(self, people) | 04:31 |
| wgrant | Oh, if the API is like that, that's not too bad. | 04:31 |
| wallyworld_ | or something like that, plus all the upstream changes for hooking it into the vocab | 04:31 |
| wgrant | As long as there aren't many affiliation contexts. | 04:31 |
| wallyworld_ | well, atm, each batch of picker results is for the same context, so there's only one | 04:32 |
| wallyworld_ | lifeless: i think the TeamParticipation table is good for indirect memberships too, right? | 04:33 |
| wgrant | Yes. | 04:33 |
| wgrant | TeamParticipation includes indirect memberships. | 04:34 |
| wallyworld_ | right. makes it a bit easier. | 04:34 |
| lifeless | wallyworld_: yes, I would encourage any implementation to start with refactoring inTeam() to have a single multi-lookup method it goes through. | 04:34 |
| wallyworld_ | lifeless: agreed. inTeam will likely not be used now | 04:34 |
| lifeless | wallyworld_: cool; I suggest a single method so that we don't have duplication | 04:38 |
| wallyworld_ | lifeless: depending on how many usages of inTeam there are, I may optimise the affiliation stuff separately | 04:39 |
| wallyworld_ | i want to get that done asap without the baggage associated with an inTeam refactoring | 04:39 |
| wallyworld_ | across the entire product | 04:40 |
| lifeless | wallyworld_: you shouldn't need to touch other callers of inTeam | 04:40 |
| lifeless | they are either fine, or separate problems. | 04:40 |
| lifeless | I'm just talking 'extract Method' | 04:40 |
| wgrant | And leave an inTeam wrapper around the new method, for compatibility. | 04:41 |
| wallyworld_ | lifeless: oh, i see what you are saying i think. as if i would cut'n'paste code everywhere :-/ | 04:41 |
| lifeless | wgrant: compat/convenience | 04:41 |
| lifeless | wgrant: all the searchTasks timeouts are on one person | 04:45 |
| wgrant | Aha. | 04:45 |
| wgrant | I didn't look that far. | 04:45 |
| lifeless | could well be patching | 04:45 |
| lifeless | 276 https://api.launchpad.net/1.0/%7Eubuntu-security-sponsors (Person:EntryResource:searchTasks) | 04:45 |
| lifeless | OOPS-2045A18, OOPS-2045A33, OOPS-2045A34, OOPS-2045A39, OOPS-2045A52 | 04:45 |
| lifeless | 276 SELECT BugTask.assignee, BugTask.bug, BugTask.bugwatch, BugTask.date_assigned, BugTask.date_close ... = FALSE)) AS BugTask JOIN Bug ON BugTask.bug = Bug.id ORDER BY BugTask.id LIMIT $INT OFFSET $INT: | 04:45 |
| lifeless | I suspect its the high offset issue | 04:46 |
| wgrant | So adeuring will save the world. | 04:46 |
| === danilo_ is now known as danilos | ||
| === henninge changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: henninge | Critical bugs: 247 - 0:[#######=]:256 | ||
| bigjools | morning | 08:00 |
| adeuring | good morning | 08:22 |
| mrevell | Hi | 08:24 |
| bigjools | wgrant or StevenK, care to talk about bug 820941 for a moment? | 09:47 |
| _mup_ | Bug #820941: Distro uploads always email XXXX_derivatives@packages.qa.debian.org <derivation> <Launchpad itself:Triaged> < https://launchpad.net/bugs/820941 > | 09:47 |
| wgrant | bigjools: New field on distribution to provide a template? | 09:47 |
| bigjools | yeah that's my first reaction too | 09:48 |
| bigjools | well, since it's not going to change it can be fixed, we just need to decide whether a distro is going to email or not | 09:48 |
| wgrant | Nobody is ever going to want to email anybody except Debian? | 09:49 |
| bigjools | there's currently zero requirement to do that | 09:50 |
| bigjools | and I don't want to fix nonexistent bugs | 09:50 |
| wgrant | We can almost freely remove a hard-coded Debian-specific string. | 09:50 |
| bigjools | it should move to config | 09:50 |
| wgrant | Why? | 09:51 |
| wgrant | It's distribution-specific. | 09:52 |
| bigjools | no, it's Debian specific | 09:52 |
| wgrant | And not everybody wants to send stuff to Debian. | 09:52 |
| bigjools | there is no requirement to do this for any other LP-hosted distro | 09:52 |
| wgrant | They may want to send it elsewhere. | 09:52 |
| bigjools | "may" | 09:52 |
| wgrant | We need either add a bool or a string field. | 09:52 |
| bigjools | but they don't, right now. I am not interested in fixing nonexistent bugs | 09:53 |
| wgrant | If we add a bool, we get to keep the hardcodedness for another 5 years. | 09:53 |
| wgrant | If we add a string field instead, we get an extra line of code and remove the hardcoded string. | 09:53 |
| wgrant | The overhead of making it customisable is probably somewhere around two minutes. | 09:54 |
| bigjools | loving your optimism | 09:54 |
| bigjools | you're usually pessimistic ;)( | 09:54 |
| bigjools | where's lifeless's wiki page on the DB change process? | 09:56 |
| wgrant | At the moment, don't. | 09:57 |
| lifeless | the usual place | 09:57 |
| wgrant | There's https://dev.launchpad.net/PolicyAndProcess/DatabaseSchemaChangesProcess | 09:57 |
| wgrant | But it will probably be at least two weeks before we can do it. | 09:57 |
| bigjools | lifeless: that's not helpful :) | 09:57 |
| lifeless | bigjools: :P I was looking fo rit | 09:57 |
| bigjools | PolicyAndProcess isn't linked from the front page | 09:58 |
| bigjools | lifeless: are we blocked on DB changes until it's implemeted? | 09:59 |
| lifeless | bigjools: fastdowntime? not blocked no. *If* we reach the end of the monthly cycle without being ready to use it, we can manually schedule a 90 minute legacy downtime. | 10:02 |
| bigjools | lifeless: ok thanks | 10:02 |
| jml | lifeless: so that announcement about something being the last 90 minute downtime was optimistic? | 10:12 |
| lifeless | jml: slightly | 10:14 |
| lifeless | jml: its certainly the last as-regular-schedule | 10:14 |
| lifeless | jml: I would consider us having to do another as a big deal. | 10:15 |
| bigjools | stub: where do you keep your list of db patch numbers? I need one and I can't remember where it lives. :( | 10:15 |
| lifeless | bigjools: its linked on that wiki page | 10:15 |
| jml | lifeless: ok. fair enough. I sort of thought from skimming it that fast db rollouts were a done thing. | 10:15 |
| stub | lp:~launchpad/+junk/dbpatches | 10:15 |
| jml | but that was a reckless assumption. | 10:16 |
| lifeless | jml: we're in pretty good shape | 10:16 |
| bigjools | stub: ta | 10:16 |
| lifeless | jml: code is written, staging is updating using it | 10:16 |
| jml | lifeless: that's pretty good. | 10:21 |
| wgrant | We really have no idea how long it will take on production, though :( | 10:23 |
| lifeless | well | 10:26 |
| lifeless | the first one will be nerve racking | 10:26 |
| lifeless | and will change nothing. | 10:26 |
| wgrant | Yes. | 10:26 |
| lifeless | Stuart is investigating slony 1.2 as a possible major improvement | 10:26 |
| wgrant | Well. | 10:26 |
| wgrant | We *hope* it will change nothing. | 10:26 |
| lifeless | wgrant: -har-, yes. | 10:27 |
| wgrant | It's slony1 v2, not slony 1.2 :) | 10:27 |
| lifeless | blah :P | 10:27 |
| wgrant | (the product is named slony-i or slony1) | 10:27 |
| * lifeless dons the not-a-details-job hat | 10:27 | |
| wgrant | Heh | 10:28 |
| StevenK | bigjools: I agree with William, make it a string | 10:36 |
| stub | My attention span: |---| | 10:36 |
| stub | Packaging learning curve: |-------------------------------------------------------------| | 10:36 |
| wgrant | stub: What are you packaging? | 10:36 |
| wgrant | Not Slony-I 2? | 10:37 |
| stub | Will need a 2.0.7 package, but basic packaging has been on my todo list for 6 years now... | 10:37 |
| wgrant | That's easy. | 10:37 |
| stub | :-p | 10:37 |
| wgrant | Assuming none of the patches need changing, apt-get source slony1-2; cd slony1-2-2.0.6; uupdate /path/to/new/tarball, dch -e and edit the changelog as fit, debuild -S -sa, dput ppa:what/ever ../slony1-2*_source.changes | 10:39 |
| stub | Its arcane. Hunger is winning out over this introductory documentation atm. | 10:39 |
| bigjools | StevenK: I am doing that. I just dislike only having one solution discussed. | 10:42 |
| StevenK | bigjools: So, you can either kill it, or make it generic. | 10:43 |
| jtv | henninge: could I beg for a review? https://code.launchpad.net/~jtv/launchpad/bug-820796/+merge/70701 | 10:43 |
| StevenK | Killing it requires a discussion with distro and Debian | 10:43 |
| StevenK | Making it generic with a string is a my vote. | 10:43 |
| henninge | jtv: sure, you may beg ;) | 10:43 |
| jtv | Good, that's step 1. | 10:43 |
| jtv | "henninge, please, _please_ review my branch for me!" | 10:44 |
| jtv | That's step 2. | 10:44 |
| henninge | jtv: I will do that. | 10:44 |
| jtv | Thanks! That's step 3. | 10:44 |
| henninge | jtv: welcome | 10:44 |
| henninge | baby steps? ;-) | 10:44 |
| StevenK | bigjools: There. Two solutions discussed. :-P | 10:45 |
| henninge | why is this diff taking so long ... Is it long? | 10:45 |
| jtv | henninge: nope—looks like the differ broke. :( | 10:45 |
| jtv | It's a very simple branch. | 10:46 |
| henninge | cool, I'll pull | 10:46 |
| henninge | branch, I mean | 10:46 |
| henninge | whatever | 10:46 |
| bigjools | StevenK: you've been taking lessons in diplomacy from your wife | 10:46 |
| jtv | cjwatson, are you back? | 10:48 |
| StevenK | He is back today | 10:49 |
| adeuring | lifeless: fancy a review of a StormRangeFactory MP? https://code.launchpad.net/~adeuring/launchpad/bug-739052-4/+merge/70717 | 10:56 |
| danilos | mrevell, hi | 11:04 |
| wgrant | stub: Any luck? | 11:09 |
| lifeless | adeuring: conflicts | 11:09 |
| stub | eh? | 11:09 |
| lifeless | adeuring: | 11:09 |
| lifeless | Diff against target:1338 lines (+879/-25) 21 files modified (has conflicts) | 11:09 |
| lifeless | Text conflict in lib/canonical/launchpad/webapp/batching.py | 11:09 |
| wgrant | stub: With the evils of packaging. | 11:09 |
| lifeless | etc | 11:09 |
| bigjools | stub: db MP winging its way to you | 11:10 |
| adeuring | lifeless: gahhh | 11:10 |
| stub | wgrant: your cheat sheet + common sense + google got me as far as a debsign failure. Docs bored me. Still hungry. | 11:10 |
| wgrant | stub: Heh. Probably just your name + email in debian/changelog not matching your OpenPGP identity exactly. | 11:11 |
| wgrant | stub: You can use debsign's -k option to specify a different key ID. | 11:11 |
| stub | Yer - need a (Work) in there | 11:12 |
| adeuring | lifeless: devel merged; new revision pushed | 11:12 |
| stub | wgrant: But that would involve me knowing what I'm doing and where to edit that :) | 11:12 |
| wgrant | stub: Should be reasonably obvious in debian/changelog. | 11:12 |
| wgrant | Although this is Debian-obvious, not classical obvious. | 11:12 |
| stub | wgrant: I meant getting different options passed to debsign | 11:13 |
| wgrant | stub: Should just be able to give it to debuild. | 11:13 |
| stub | wgrant: debbuild done now. Now to learn about ppas and uploading | 11:13 |
| wgrant | http://help.launchpad.net/Packaging/PPA/Uploading | 11:14 |
| stub | wgrant: At what point to I get to specify 'lucid and above'? | 11:17 |
| wgrant | stub: You have to upload to one, and copy to the rest later. | 11:17 |
| wgrant | Normally one would upload to the earliest, as that is what it will build for. | 11:17 |
| wgrant | Then the binaries and source will be copied to maverick/natty/oneiric. | 11:17 |
| wgrant | AFAICT this should build fine on Lucid, but I guess we're about to find out. | 11:18 |
| wgrant | stub: The upload series is specified on the top line in debian/changelog, in case you hadn't seen that. | 11:18 |
| wgrant | Defaults to 'unstable'. | 11:19 |
| wgrant | Which obviously won't work too well for Ubuntu. | 11:19 |
| stub | ta | 11:19 |
| stub | wgrant: version number 2.0.7-0~ppa1~lucid1 or 2.0.7-0~ppa1 ? | 11:20 |
| wgrant | stub: That's one of the great unanswered questions of the universe. | 11:20 |
| wgrant | stub: I'd probably use the latter. | 11:20 |
| wgrant | I actually tend to use -0ppa1 | 11:21 |
| stub | what I have atm. docs are trying to confuse me :) | 11:21 |
| stub | Ahh... what I had earlier before something told me to put in a twiddly | 11:21 |
| wgrant | stub: It depends. | 11:23 |
| wgrant | We often backport stuff, in which case we'll normally use $(original version)~ppa1 | 11:23 |
| wgrant | But -0ppa1 < -0ubuntu1 < -1, so it's cleaner and still works fine. | 11:23 |
| stub | its in there now... lets see what lp does with it while I get fud | 11:24 |
| wgrant | And building already. | 11:26 |
| lifeless | jml: can we unsubscribe testtools-dev from the bug tracker ? | 11:27 |
| lifeless | https://bugs.launchpad.net/testtools/+subscriptions | 11:28 |
| jml | lifeless: you just want to be on the mailing list? | 11:28 |
| lifeless | jml: I think having the mailing list get a copy of every bug change makes it useless for discussion | 11:29 |
| jml | lifeless: oh huh. I'm not getting those mails. | 11:29 |
| lifeless | jml: see the page I linked; you may need to look under 'other subscriptions' or something | 11:30 |
| jml | lifeless: I'm not getting those mails. | 11:30 |
| lifeless | jml: thats because they have the same message id | 11:30 |
| lifeless | jml: and you are also subscribed directly | 11:31 |
| jml | ah ok. | 11:31 |
| lifeless | jml: and you are using a MUA that dedupes on message id. | 11:31 |
| lifeless | https://lists.launchpad.net/testtools-dev/ | 11:31 |
| lifeless | jml: its also getting merge proposals | 11:32 |
| lifeless | jml: have you, by chance, combined a discussion-group and a permission-granting-team ? | 11:32 |
| jml | sigh | 11:32 |
| jml | no. | 11:32 |
| jml | I uncombined it. | 11:32 |
| jtv | henninge: any questions about the review so far? I'm hoping to scoot off soon. | 11:33 |
| henninge | jtv: no, sorry, did not get that far yet. | 11:35 |
| jtv | henninge: in the meantime, I replaced the core of the Ubuntu distribution infrastructure and it performed its first-ever production run. I think I need to peddle out the tension on my bicycle soon. :-) | 11:36 |
| henninge | jtv: you mean, you pushed a new revisionß | 11:36 |
| henninge | jtv: but go ahead and peddle | 11:36 |
| henninge | s/ß/?/ | 11:37 |
| jtv | henninge: sort of — Tom rolled out the necessary cron change for me. And then we all sat around worrying until the script ran. | 11:37 |
| henninge | ah that | 11:38 |
| lifeless | adeuring: reviewed; couple of tiny things | 11:38 |
| adeuring | lifeless: thanks! | 11:38 |
| adeuring | lifeless: >>> def f(a,b): | 11:40 |
| adeuring | ... return a+b | 11:40 |
| adeuring | ... | 11:40 |
| adeuring | >>> reduce (f, 1, 2, 3) | 11:40 |
| adeuring | Traceback (most recent call last): | 11:40 |
| adeuring | File "<stdin>", line 1, in <module> | 11:40 |
| adeuring | TypeError: reduce expected at most 3 arguments, got 4 | 11:40 |
| adeuring | so, reduce(And, clauses) is right, I think | 11:41 |
| lifeless | adeuring: huh? that seems unrelated ;) | 11:41 |
| adeuring | lifeless: argh, sorry, got your comment wrong | 11:41 |
| lifeless | And is a CompoundExpr | 11:42 |
| lifeless | def __init__(self, *exprs): | 11:42 |
| lifeless | self.exprs = exprs | 11:42 |
| adeuring | right | 11:42 |
| henninge | jtv: no questions, r=me | 11:44 |
| jtv | thanks henninge! That's step… 4? | 11:44 |
| henninge | jtv: wasn't that 5 already? | 11:45 |
| jtv | Could be, could be. | 11:45 |
| wgrant | stub: So, looks like slony1-2 all went well. | 12:13 |
| === benji changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: henninge, benji | Critical bugs: 247 - 0:[#######=]:256 | ||
| stub | Need to copy it into natty so I can test with it | 12:13 |
| wgrant | You should see a handy https://launchpad.net/~stub/+archive/launchpad/+copy-packages | 12:14 |
| wgrant | Make sure to select to copy with binaries, and to the right archive. | 12:14 |
| stub | Not rebuild? | 12:14 |
| stub | Guess better if I test the actual lucid binaries | 12:15 |
| stub | What are p-series and q-series? | 12:16 |
| wgrant | stub: Ubuntu 11.10 and 12.04 LTS, which should not be shown on +copy-packages. | 12:17 |
| wgrant | But we don't really have a way to model that properly yet. | 12:17 |
| wgrant | Er, 12.04 LTS and 12.10. | 12:18 |
| wgrant | I am 6 months behind, apparently. | 12:18 |
| === danilo_ is now known as danilos | ||
| bigjools | wgrant: should be very easy to filter in the vocab - check for len(supported archs ) == 0 | 12:21 |
| wgrant | bigjools: Yeah, but what if it's partially initialised and oh god everything comes back to this doesn't it. | 12:22 |
| bigjools | argh | 12:26 |
| bigjools | wgrant: we're going to have to suck that bullet and just fix this | 12:27 |
| wgrant | Probably. | 12:27 |
| StevenK | But we have a field in DSP for this purpose? | 12:27 |
| wgrant | DSP is not relevant here; not everything has a parent. | 12:27 |
| bigjools | needs to be a bool on the series I expect | 12:28 |
| StevenK | Which we did have ... | 12:28 |
| StevenK | Which hooked into DSP, and was a horrid hack, IIRC | 12:30 |
| StevenK | benji: You haz QA to do! | 12:31 |
| StevenK | gary_poster: QA for you as well :-P | 12:31 |
| benji | thanks, StevenK; I'll look now | 12:31 |
| gary_poster | StevenK, looking thanks | 12:31 |
| jtv | Who will review some security.cfg extensions? benji, I've already bothered your co-OCR today so I'm angling for you: https://code.launchpad.net/~jtv/launchpad/bug-822640/+merge/70730 | 12:48 |
| benji | jtv: sure, I'll take a look now | 12:48 |
| jtv | thanks | 12:49 |
| benji | jtv: done, looks fine | 12:50 |
| jtv | Great, thanks! | 12:50 |
| deryck | Morning, all. | 13:13 |
| abentley | deryck: welcome back. | 13:14 |
| deryck | abentley: thanks! | 13:14 |
| danilos | abentley, hi, I was wondering about bug 820511, where it would be most useful to have the job ID logged? I was thinking of doing it in the existing debug statements in the runner, but I wonder if that'd be enough for the circumstances we were in? | 13:33 |
| _mup_ | Bug #820511: Job scripts don't report job ids <jobs> <Launchpad itself:In Progress by danilo> < https://launchpad.net/bugs/820511 > | 13:33 |
| abentley | danilos: on the phone | 13:33 |
| abentley | danilos: The normal logging should be fine. I think it should be "info", so it still shows when the script is run non-verbosely. | 13:44 |
| danilos | abentley, right, that's what I was thinking as well | 13:45 |
| danilos | abentley, thanks | 13:45 |
| abentley | danilos: cool. | 13:45 |
| bigjools | jml: is it wrong that when I saw the sperm whale stranded on the Australian coast earlier, I thought "Beached as!" | 13:57 |
| jml | bigjools: not at all :) | 13:57 |
| jml | although I do believe that was a kiwi whale in the yt video | 13:57 |
| bigjools | it may have crossed the Tasman | 13:58 |
| jml | heh | 13:58 |
| henninge | benji: Hi | 15:11 |
| henninge | benji: are you currently reviewing something? | 15:12 |
| benji | hi, henninge | 15:12 |
| benji | henninge: not at the moment | 15:12 |
| henninge | benji: ok, I'll take jelmer's big one, then. | 15:12 |
| jcsackett | sinzui: can you mumble? | 15:13 |
| sinzui | yes | 15:13 |
| jcsackett | can you hear me, sinzui? | 15:16 |
| henninge | jelmer: Hi! | 15:32 |
| bigjools | hi henninge or benji, can I bother one of you for a review please? | 15:35 |
| benji | bigjools: I can after I finish this one. | 15:35 |
| bigjools | benji: thank you | 15:35 |
| bigjools | https://code.launchpad.net/~julian-edwards/launchpad/code-bug-820941/+merge/70753 | 15:35 |
| henninge | benji: which one are you doing? | 15:38 |
| benji | henninge: gary asked me to do https://code.launchpad.net/~gary/launchpad/bug821531/+merge/70738 | 15:38 |
| jelmer | henninge: hi! | 15:38 |
| henninge | benji: ah, ok | 15:38 |
| henninge | jelmer: Hi! | 15:39 |
| henninge | jelmer: I just wanted to ask if your safe-branch-open branch is strictly refactoring. | 15:39 |
| henninge | jelmer: I assumed so, just wanted to double-check. | 15:39 |
| jelmer | henninge: yes, it should not change any of the existing behaviour | 15:40 |
| henninge | jelmer: cool, I already approved it. | 15:40 |
| jelmer | henninge: Great, thanks for the review :) | 15:40 |
| === henninge changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: benji | Critical bugs: 247 - 0:[#######=]:256 | ||
| henninge | jelmer: I'll do your other one before I finish. | 15:41 |
| jelmer | henninge: Cool | 15:44 |
| henninge | jelmer: Line 120 of the diff: I think you wanted to use the "port" parameter here. | 15:51 |
| benji | bigjools: I'm done with the review. I had one small question. | 15:51 |
| bigjools | ok | 15:52 |
| bigjools | benji: In the pre-requisite schema branch the SQL updates Ubunbu address. Nothing else needs one right now. | 15:53 |
| bigjools | Ubuntu, even! | 15:53 |
| benji | bigjools: cool, I figred it was something like that | 15:53 |
| bigjools | benji: thanks for reviewing | 15:53 |
| benji | my pleasure | 15:54 |
| jelmer | henninge, that's a good point. or potentially, we could get rid of the port parameter altogether | 15:55 |
| henninge | or that | 15:55 |
| gary_poster | bac, someone wants is to "_REALLY_ DELETE ACCOUNT" (https://answers.launchpad.net/launchpad/+question/167045). "please delete my account permanently. i still can sign in even when i use the deactivate procedure recommended." Are they out of luck? | 16:04 |
| gary_poster | I'm going to say "no" and point them to https://answers.launchpad.net/launchpad/+faq/968 | 16:10 |
| sinzui | gary_poster, SSO reenables the user profile. Deactivate works, just do not sign in again to make it active | 16:10 |
| === Ursinha` is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Guest6606 | ||
| === Guest6606 is now known as Ursula | ||
| gary_poster | thanks sinzui, yes, that's where I was going. While I have your attention, someone wants to convert a person to a team (~lintian-maint fwiw). Am I right that they cannot do this, and that they should create a new team and then fiddle with the names if absolutely necessary? | 16:13 |
| === Ursula is now known as Ursinha | ||
| * gary_poster going to lunch; will be back later | 16:20 | |
| === deryck is now known as deryck[lunch] | ||
| sinzui | gary_poster, a person can be converted (claimed?) to a team in one case, but I cannot remember how that is done. | 16:34 |
| sinzui | gary_poster, see lp/registry/stories/team/xx-team-claim.txt | 16:36 |
| gary_poster | thank you sinzui | 16:47 |
| === beuno is now known as beuno-lunch | ||
| benji | allenap: I think there's a prblem with the Python-dict-in-JS branch, my thoughts are in the MP: https://code.launchpad.net/~allenap/launchpad/javascript-utils-2/+merge/70760 | 16:53 |
| gary_poster | abentley, would you mind if I assigned you a code import question? https://answers.launchpad.net/launchpad/+question/167251 Alternatively, I'm happy to formulate a reply if you give me the gist, or do other necessary grunt work. | 17:18 |
| abentley | gary_poster: looking | 17:18 |
| gary_poster | (that is, I'm happy to do other necessary grunt work if directed) | 17:18 |
| gary_poster | thank you | 17:18 |
| abentley | gary_poster: I'll take it. I looks like the answer is "this is a bug in bzr-hg". | 17:22 |
| gary_poster | ok thanks abentley | 17:22 |
| === deryck[lunch] is now known as deryck | ||
| === beuno-lunch is now known as beuno | ||
| === jacob_ is now known as j-johan-edwards | ||
| j-johan-edwards | Hello, is there any way to get a dump for soyuz packaging records in the Ubuntu archives? | 17:48 |
| j-johan-edwards | The sample data is pretty sparse... | 17:48 |
| j-johan-edwards | Hey, sorry to repeat (won't again) but is there any way to get a dump for soyuz packaging records in the Ubuntu archive? | 18:20 |
| abentley | benji: could you please review https://code.launchpad.net/~abentley/launchpad/ubuntu_merges/+merge/70772 ? | 18:35 |
| benji | abentley: sure | 18:35 |
| benji | I appreciate the courtesy space after the URL. ;) | 18:36 |
| benji | abentley: done | 18:42 |
| lifeless | morning y'all | 18:57 |
| lifeless | sinzui: hi; re bug 298152 | 19:10 |
| _mup_ | Bug #298152: privacy portlet is not visible enough for indicating private objects <disclosure> <lp-web> <oem-services> <privacy> <qa-ok> <ui> <Launchpad itself:Fix Released by jcsackett> < https://launchpad.net/bugs/298152 > | 19:10 |
| sinzui | yes | 19:10 |
| * jcsackett perks up. | 19:10 | |
| lifeless | sinzui: 2 questions;. A) there was some user feedback that the new ribbon is less noticable than the old border thing, particularly when someone has been switching tabs and is no longer at the top of page. | 19:11 |
| lifeless | and B) have we enabled the feature flag to show it to everyone ? | 19:11 |
| sinzui | lifeless, the privacy ribbon cannot be scrolled off. | 19:11 |
| lifeless | sinzui: it used to have a close button | 19:12 |
| jcsackett | lifeless: sinzui and i were going to add it to beta-testers. i forgot to ping a losa on that point. | 19:12 |
| sinzui | I asked jcsackett to enable the ribbon for all beta testers. We will blog about it to encourage feedback | 19:12 |
| jcsackett | i'll correct that now. | 19:12 |
| lifeless | sinzui: so I guess the scenario is 'close, scroll, switch windows, come back and forget'. | 19:12 |
| sinzui | lifeless, I question the hide button because it is not obvious how to unhide it | 19:13 |
| jcsackett | sinzui, lifeless: i'm plus one on removing the hide part of it. | 19:13 |
| lifeless | sinzui: anyhow, my question (A) is, has that user reaction been taken into account? Perhaps the bug should stay open, or a new follow on be opened, if not. | 19:13 |
| thedac | jcsackett: did you need something done now? | 19:13 |
| sinzui | lifeless, https://bugs.launchpad.net/launchpad/+bug/789566 < scroll and confirm it is fixed at the top of your screen | 19:14 |
| jcsackett | thedac: yeah, i'm looking up a feature flag rule i need. :-) | 19:14 |
| thedac | ok | 19:14 |
| sinzui | lifeless, the ribbon is fixed using css2. if your browser does not support it, please update to a browser released in the last 10 years | 19:14 |
| lifeless | sinzui: I don't actually have an opinion about it myself, just noticed the feedback in https://bugs.launchpad.net/launchpad/+bug/298152/comments/35 and no apparent response | 19:14 |
| _mup_ | Bug #298152: privacy portlet is not visible enough for indicating private objects <disclosure> <lp-web> <oem-services> <privacy> <qa-ok> <ui> <Launchpad itself:Fix Released by jcsackett> < https://launchpad.net/bugs/298152 > | 19:14 |
| lifeless | sinzui: yes, its fixed when not hidden. | 19:15 |
| lifeless | sinzui: I have no concerns about the implementation :) | 19:15 |
| sinzui | sorry 'fixed' is a css position property. I did not mean this bug is fixed when the user can see the ribbon | 19:16 |
| lifeless | sinzui: I meant 'pinned to the top of the visible area' when I said fixed. | 19:16 |
| sinzui | lifeless, We had positive reaction, but NO reaction from non Canonical staff so I asked that we broaden the test | 19:17 |
| lifeless | sinzui: that was because only canonical staff could see it :P | 19:17 |
| sinzui | indeed that is why I asked | 19:17 |
| lifeless | sinzui: sounds like its all under control... cool | 19:17 |
| lifeless | thanks | 19:17 |
| sinzui | lifeless, I wanted to remove the hide action, but that would be me second guessing that users do not want to shoot themselves in the foot | 19:18 |
| lifeless | sinzui: could we tell how many people hide the ribbon somehow? | 19:19 |
| sinzui | no I cannot, nor did I agree with the reason for that addition | 19:20 |
| jcsackett | sinzui, lifeless: maybe we should just create a mockup with hide and without hide on some page and throw it to user testing? | 19:20 |
| sinzui | I think there was confusion between the desire to show state and the need for a universal notification presentation that did not suck | 19:20 |
| sinzui | jcsackett, I am hoping for someone to be reasonable a realise that the cost of a leak could be millions of dollars and individuals should not be given an opportunity to be the source of the leak | 19:22 |
| jcsackett | sinzui: fair point. | 19:22 |
| sinzui | jcsackett, as we discussed last week. The privacy portlet is not on subordinate pages. So the hide code that changes that portlet will not work | 19:25 |
| jcsackett | sinzui: right, the hide code doesn't do the highlight portlet thing if there is no portlet. | 19:25 |
| jcsackett | the original code already had a toggle for hiding without highlighting; based on our conversation i thought we elected to not remove "hide" and simply go with "only try to highlight the portlet if it exists". | 19:27 |
| sinzui | jcsackett, mpt reported a bug where the ribbon separates the state from when you change the state. The privacy portlet is largely an artefact of the 2/3x designs. Moving the control to change privacy to the ribbon would 1 Remove the need for the hide button's effect. | 19:27 |
| abentley | lifeless: AIUI, Launchpad URLs should never contain whitespace or non-ascii characters in the path portion of the URL. Bug 819841 and bug 819859 would be solved if such URLs automatically 404ed. Do you think it makes sense to apply that at the URL parsing level? | 19:27 |
| _mup_ | Bug #819841: OOPs rendering page with non-ascii URL <oops> <Launchpad itself:Triaged> < https://launchpad.net/bugs/819841 > | 19:27 |
| lifeless | sinzui: what stops you removing the hide button ? | 19:27 |
| _mup_ | Bug #819859: launchpad urls permit spurious whitespace <oops> <Launchpad itself:Triaged> < https://launchpad.net/bugs/819859 > | 19:27 |
| lifeless | abentley: do these urls currently work sometimes, or not at all ? | 19:28 |
| jcsackett | sinzui: interesting. so we get rid of the privacy portlet (or at least the edit part of it) and move that into the ribbon, simultaneously killing the "hide" button? | 19:28 |
| sinzui | understanding of why it was added. As I wrote above. I think it was a misunderstanding. Privacy is not a notification, it should not be dismissable | 19:28 |
| lifeless | abentley: and by parsing level, do you mean in the incoming url routing area, or in our 'regex to figure out what is a url in a page' ? | 19:28 |
| abentley | lifeless: urls with ascii whitespace work. urls with non-ascii whitespate oops. | 19:28 |
| abentley | s/whitespate/whitespace/ | 19:28 |
| abentley | lifeless: I mean in the incoming url routing area. | 19:29 |
| sinzui | lifeless, yet one requirement may have been that the notice cannot take up vertical space that is needed on small screens | 19:29 |
| lifeless | abentley: ok. You probably don't have this at hand, but do we emit such urls-with-ascii-whitespace from our regex linkifier code? E.g. would we break some in-site links from user content doing this? | 19:29 |
| lifeless | sinzui: you could have a button that moves it to the left hand side ? | 19:29 |
| sinzui | That was my original plan that was scraped by Huw's testing/design/work | 19:30 |
| lifeless | sinzui: FWIW I accept your assertion that being able to hide it seems liable to increase the risk of accidental disclosure | 19:30 |
| lifeless | abentley: I think 404ing on urls that cannot possibly resolve to urls we would generate makes a great deal of sense. | 19:31 |
| abentley | lifeless: I can test whether we linkify them. | 19:31 |
| lifeless | abentley: and even if we do break some things we linkify today, thats ok - we can fix the linkifier | 19:31 |
| lifeless | abentley: I would ask, if we expect nontrivial fallout, that we do both at once; if the fallout is going to be minor, 2 landings, or even 1 with the second maybe-or-maybe-not happening would be fine with me | 19:32 |
| sinzui | lifeless, the ribbon is officially in customer acceptance as of Wednesday. I expect to get feedback about the design and bugs about what is or is not displayed private | 19:32 |
| abentley | lifeless: urls with ascii whitespace can and do resolve today. So you would forbid only the non-ascii ones? | 19:32 |
| lifeless | abentley: I would be ok with either all or only non-ascii as you choose; my thinking is that if we don't emit it, there is no reason to accept it, and room for interesting bugs if we do. | 19:33 |
| lifeless | abentley: s/don't emit it/don't emit it or explicitly expect it [e.g. search form submission]/ | 19:34 |
| abentley | lifeless: Right. And with with search form submission I believe we use the query, not the path. | 19:34 |
| lifeless | abentley: yes | 19:35 |
| lifeless | abentley: (you said 'urls' rather than 'url paths' before, and I was trying to avoid being pedantic while still giving useful accurate answers) | 19:35 |
| abentley | lifeless: I started with "path portion of the url". Sorry if I was unclear. | 19:36 |
| lifeless | abentley: ah, I failed to parse! | 19:36 |
| lifeless | abentley: so, we're synced :) | 19:36 |
| abentley | lifeless: We do linkify URLs with whitespace: https://bugs.qastaging.launchpad.net/bzrtools/+bug/760435 | 19:37 |
| _mup_ | Bug #760435: failUnless & co cause PendingDeprecationWarning <easy> <selftest> <verification-done> <Bazaar:Fix Released by mbp> <Bazaar 2.3:Fix Released by vila> <bzr-pipeline:Fix Committed> <BzrTools:Fix Committed> <bzr (Ubuntu):Fix Released> <bzr (Ubuntu Natty):Fix Released by jelmer> < https://launchpad.net/bugs/760435 > | 19:37 |
| lifeless | abentley: that particular case I think is fine, cause its already hex escaped | 19:38 |
| lifeless | I meant something like <https://foo bar/> | 19:38 |
| abentley | lifeless: I meant something like %20 or %A0 | 19:38 |
| lifeless | abentley: in the web request yes; I was worried about someone finding a way to cause that from what looks like whitespace | 19:39 |
| lifeless | abentley: e.g. the example I've put into that same qastaging bug | 19:39 |
| sinzui | I am having network difficulties. Maybe no one will see this. I am restarting. | 19:41 |
| abentley | lifeless: So to give examples, I think https://bugs.qastaging.launchpad.net/bzrtools/+bug/760435 should work, https://bugs.qastaging.launchpad.net/bzrtools/+bug/760435%20 and https://bugs.qastaging.launchpad.net/bzrtools/+bug/760435%C2%A0 should 404. | 19:42 |
| _mup_ | Bug #760435: failUnless & co cause PendingDeprecationWarning <easy> <selftest> <verification-done> <Bazaar:Fix Released by mbp> <Bazaar 2.3:Fix Released by vila> <bzr-pipeline:Fix Committed> <BzrTools:Fix Committed> <bzr (Ubuntu):Fix Released> <bzr (Ubuntu Natty):Fix Released by jelmer> < https://launchpad.net/bugs/760435 > | 19:42 |
| _mup_ | Bug #760435: failUnless & co cause PendingDeprecationWarning <easy> <selftest> <verification-done> <Bazaar:Fix Released by mbp> <Bazaar 2.3:Fix Released by vila> <bzr-pipeline:Fix Committed> <BzrTools:Fix Committed> <bzr (Ubuntu):Fix Released> <bzr (Ubuntu Natty):Fix Released by jelmer> < https://launchpad.net/bugs/760435 > | 19:42 |
| _mup_ | Bug #760435: failUnless & co cause PendingDeprecationWarning <easy> <selftest> <verification-done> <Bazaar:Fix Released by mbp> <Bazaar 2.3:Fix Released by vila> <bzr-pipeline:Fix Committed> <BzrTools:Fix Committed> <bzr (Ubuntu):Fix Released> <bzr (Ubuntu Natty):Fix Released by jelmer> < https://launchpad.net/bugs/760435 > | 19:42 |
| abentley | lifeless: Right now, the second is equivalent to the first, and the third oopses. | 19:43 |
| jcsackett | ? | 19:44 |
| * jcsackett fails at keyboards again. | 19:45 | |
| lifeless | abentley: I agree | 19:45 |
| abentley | lifeless: excellent. | 19:45 |
| lifeless | abentley: I don't think the linkifier needs touching | 19:46 |
| abentley | lifeless: cool. | 19:46 |
| lifeless | abentley: based on this discussion + experiment | 19:46 |
| lifeless | if someone hex encodes whitespace into a url in a page, we should linkify that | 19:46 |
| abentley | lifeless: do you know where we parse/validate incoming urls? | 19:46 |
| lifeless | abentley: A reasonable place to start would be lib/canonical/launchpad/webapp/publisher.py | 19:46 |
| abentley | lifeless: Yes, I'll have a look there. | 19:47 |
| lifeless | abentley: I don't know precisely | 19:47 |
| thedac | jcsackett: to confirm feature flag change is on production, correct? | 19:55 |
| abentley | gary_poster: do you happen to know where would be a good place to add extra validation rules for incoming URLs (i.e. when a browser does a GET or POST on us)? | 19:55 |
| jcsackett | thedac: yes. | 19:55 |
| thedac | jcsackett: and do you have a one liner log message I can put in? | 19:56 |
| gary_poster | abentley, so, at the very beginning of processing a URL? | 19:56 |
| abentley | gary_poster: Yes. I want to verify that the "path" portion does not contain any whitespace or non-ascii characters. | 19:57 |
| abentley | gary_poster: And by "whitespace", I mean %20 as well as " ". | 19:57 |
| gary_poster | abentley, and I assume the goal is to not produce an OOPS. It would relatively easy to do this if we didn't mind an OOPS with a IStartRequestEvent handler... I wonder if we can somehow short-circuit anything there. alternatively...maybe we have overridden enough of the publisher that you can do it in there. Lemme dig for a sec abentley | 19:59 |
| abentley | gary_poster: okay, thanks. | 20:00 |
| gary_poster | abentley, so, I suggest that you work after the request has been made, probably in beforeTraverse. Take a look at eggs/zope.publisher-3.12.0-py2.6.egg/zope/publisher/publish.py and in lib/canonical/launchpad/webapp/publication.py . | 20:13 |
| gary_poster | In publish.py, you'll see a publish function. That's the basics of how Zope publishes a request. It uses a publication object, which we have customized in the second file I listed. As you can see in publish, if you raise an error in beforeTraverse, then the publication's handleException will be called. | 20:13 |
| gary_poster | You'll want to raise an exception that has a view registered for it that gives the user the information they will want about the error. | 20:13 |
| gary_poster | There are examples of times we do that in the LP code now; see zope.app.publication.browser.BrowserPublication.handleException (we override it but the meat you want is elsewhere) for how that works if you want the details. | 20:13 |
| gary_poster | You might want to advertise this to devs after you are done; I wonder if there are any other "quit now" shortcut rules we might want to implement this way. | 20:13 |
| abentley | gary_poster: Okay, thanks for the pointers. | 20:14 |
| gary_poster | welcome | 20:16 |
| gary_poster | lifeless, thanks for review! I agree and normally self-review those, but I'm asking benji to make sure everything works for him in Gnome. It's kind of a double-check for Oneiric | 20:47 |
| lifeless | ah cool | 20:49 |
| lifeless | I didn't realise | 20:49 |
| deryck | abentley: hey, was talking and didn't realize I was on mute. ;) | 20:49 |
| abentley | deryck: no worries. I didn't mean to be in Mumble anyhow :-) | 20:50 |
| deryck | abentley: heh. Was coming back to my normal home from another call :) | 20:50 |
| === benji changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - | Critical bugs: 247 - 0:[#######=]:256 | ||
| === lifeless changed the topic of #launchpad-dev to: Performance Tuesday | https://dev.launchpad.net/ | On call reviewer: - | Critical bugs: 247 - 0:[#######=]:256 | ||
| lifeless | bah, missed gary | 22:01 |
| lifeless | benji: still here, perchance ? | 22:01 |
| lifeless | abentley: btw - def publishTraverse(self, request, name): | 22:08 |
| lifeless | in publisher.py | 22:08 |
| lifeless | seems to do the ascii only enforcement, so is close to what you were looking at | 22:09 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!