wgrant | wallyworld__: I've reviewed your branch | 05:04 |
---|---|---|
StevenK | "And it's all rubbish" | 05:04 |
StevenK | :-P | 05:04 |
wgrant | Not quite :) | 05:04 |
wallyworld__ | wgrant: thanks, i'll look after i get back from school pickup | 05:05 |
wallyworld__ | wgrant: i can't use rs.find().set() because that doesn't return the rowcount | 06:36 |
wgrant | Ah | 06:37 |
wallyworld__ | also, i can't include lpsprcdateuploaded < dateuploaded in the where condition, i have to use greatest() so rowcount correctly says if that row exists or not | 06:37 |
wgrant | wallyworld__: You may be better off doing an initial SELECT to work out whether you need to UPDATE or INSERT | 06:37 |
wallyworld__ | that's an extra select i'd rather avoid | 06:37 |
wallyworld__ | unless you think it won't affect things | 06:37 |
wgrant | But that turns each batch into 4 queries | 06:37 |
wgrant | Currently you're doing 2+n queries | 06:38 |
wallyworld__ | hmmm. i do i update for creator != null, 1 for maintainer != null, and a possible insert | 06:38 |
wgrant | (aha, my first test run on DF crashed with the INSERT conflict that I mentioned) | 06:39 |
wgrant | Right | 06:39 |
wallyworld__ | as well as the initial select | 06:39 |
wgrant | You do the initial select, two updates for each row, and an insert | 06:39 |
wallyworld__ | no, a possible insert | 06:39 |
wgrant | I'd suggest instead the initial select, some computation, a batch select, a batch update, and a batch insert | 06:39 |
wgrant | True | 06:39 |
wallyworld__ | hmmm. you really think that would be better? | 06:40 |
wgrant | It's going to reduce the query count by a factor of 1000 or more | 06:40 |
wallyworld__ | most times the insert wil not be required once the table is populated | 06:40 |
wgrant | Sure, but the insert is negligible | 06:41 |
wgrant | Because it's batched | 06:41 |
wgrant | Its elimination does not help performance to any measurable degree | 06:41 |
wallyworld__ | so you want to initially select from lpsprc where spr_id in (all the spr ids from the slice)? | 06:42 |
wallyworld__ | and see what archive/spn/distroseries records exist | 06:42 |
wgrant | Right | 06:42 |
wgrant | Otherwise you can't identify which rows need inserting unless you do a thousand separate UPDATEs for the batch | 06:42 |
wgrant | Which is obviously substantially more expensive than a single SELECT | 06:43 |
wallyworld__ | my thinking was most times all the updates would be required anyway | 06:43 |
wallyworld__ | once the table is populated | 06:43 |
wgrant | But then you have 1000 queries instead of 1 | 06:43 |
wgrant | Still | 06:43 |
wallyworld__ | i mean, if the updates are required, you still have 1000 queries | 06:43 |
wallyworld__ | plus the initial computation as an overhead | 06:44 |
wgrant | Not with my technique | 06:44 |
wgrant | You can do a batch update using either a few CASE expressions or a literal subselect | 06:45 |
wallyworld__ | i'd have to think a bit about that | 06:45 |
wallyworld__ | i could insert the records to be updated into a temp table. or you suggesting a sub select from the original spph table? | 06:47 |
wgrant | UPDATE lpsprc SET dateuploaded = sub.dateuploaded FROM (VALUES (some, cache, row, key, that_dateuploaded), (some, other, row, key, other_dateuploaded)) AS sub WHERE lpsprc.some = sub.some AND [...] AND lpsprc.dateuploaded < sub.dateuploaded; | 06:50 |
wallyworld__ | ok, thanks, i'll see how that works | 06:52 |
wgrant | (it's not a pattern we use anywhere else yet, because we rarely have code adequately architected to be able to benefit from a bulk update) | 06:54 |
wallyworld__ | yeah | 06:55 |
wgrant | This is a garbo job we should try to get Rightâ„¢, as it's likely to be used as a model for many more in the future. | 06:55 |
wallyworld__ | i haven't used that pattern before elsewhere either | 06:56 |
wgrant | And I don't think it's too much effort to get it Rightâ„¢, apart from getting Storm to cope with that UPDATE syntax | 06:56 |
wgrant | But it should be relatively easy to do with some of the helpers in lp.services.database.bulk | 06:56 |
wallyworld__ | yeah, no sure if thre would be native storm support for it, may need a bit of SQL("..") | 06:57 |
wgrant | There isn't native support AFAIK. You'll probably just need to define a VALUES stormexpr near your helper and then do a bit of SQL() with column definitions maybe. But the hideousness can be encapsulated in lp.services.database.bulk, as the insert/load horrors are | 07:03 |
wallyworld__ | i'll see what falls out | 07:05 |
wallyworld__ | fun fun fun | 07:05 |
wgrant | Storm has the ROW constructor nowadays, but the only use of VALUES expressions that I know of is the INSERT special case | 07:10 |
wgrant | wallyworld__: btw, I've confirmed on DF that my suggested optimisations for the getPendingUpdates query are indeed of great import. | 07:11 |
wallyworld__ | ok, ta. i'll let you know when it's ready to look at again, will be after dinner and a few errands | 07:12 |
wallyworld__ | may well need another iteration | 07:12 |
wgrant | Yeah | 07:13 |
wgrant | But this is looking very good, and I think the correctness concerns I had have been ironed out. | 07:13 |
wgrant | and with the bulk update this will be extremely fast, which is good because I was banking on the "consider more rows, but calculate and process them far more quickly" strategy. | 07:14 |
wallyworld__ | hopefully this pattern could be applicable to other similar reporting type scenarios where we do live calculations off a fairly normalised model | 07:15 |
wgrant | wallyworld__: Exactly | 07:19 |
wgrant | wallyworld__: This effort probably isn't completely worth it just to fix those pages, but it is the first of a new style of caching table that has a huge number of potential uses. | 07:19 |
wgrant | This is our first table with eventual consistency and just a few minutes of lag. | 07:21 |
wgrant | Everything else we have is either updated by triggers or a huge daily cronjob :) | 07:22 |
StevenK | We should destroy nightly.sh for being a terrible idea and making a lot of people very angry | 07:30 |
stub | In favour of? | 07:39 |
stub | smaller celery tasks on demand might be nicer | 07:40 |
wallyworld__ | afternoonly.sh | 07:40 |
stub | even if it would mean more overhead | 07:40 |
stub | manyana.sh | 07:40 |
adeuring | good morning | 09:03 |
cjwatson | wgrant: Can you think of any reason why Archive._copySources (for syncSource/syncSources) couldn't just pass unembargo=True as a direct replacement for allow_delayed_copies=True? | 10:37 |
cjwatson | It seems too eay | 10:37 |
cjwatson | *easy | 10:37 |
wgrant | cjwatson: As long as it's been fixed to not reupload then it should be fine | 10:42 |
cjwatson | Yeah, I destroyed the reuploading a while back | 10:42 |
wgrant | Yeah, I thought I recalled that | 10:43 |
cjwatson | I guess I have to destroy delayed copies in two pieces, to avoid a situation where we created a delayed copy just before a rollout and then have no way to publish it | 10:46 |
wgrant | Yeah | 10:49 |
cjwatson | And we might have to keep a tiny amount of code around for the sake of old entries in DONE queues and the like | 10:49 |
wgrant | cjwatson: Yeah, but most of the code can die | 11:05 |
adeuring | frankban: could you please review this mp: https://code.launchpad.net/~adeuring/launchpad/product-lp-limitedview-3-1/+merge/134075 ? | 11:24 |
frankban | adeuring: sure | 11:29 |
adeuring | frankban: thanks! | 11:29 |
=== frankban changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On Call Reviewer: frankban | Firefighting: - | Critical bugs: ~200 | ||
wgrant | adeuring: LimitedView delegates to View by default, so you probably don't need to check for View at all | 11:31 |
adeuring | wgrant: ah, right, let me try this | 11:32 |
wgrant | The ifs will still be a little ugly, but it's at least slightly less bad | 11:32 |
=== yofel_ is now known as yofel | ||
=== almaisan-away is now known as al-maisan | ||
=== al-maisan is now known as almaisan-away | ||
=== rick_h changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On Call Reviewer: frankban, rick_h | Firefighting: - | Critical bugs: ~200 | ||
czajkowski | rick_h: morning | 13:12 |
rick_h | morning czajkowski | 13:12 |
czajkowski | rick_h: I come with a gift :) https://bugs.launchpad.net/launchpad/+bug/1078239 | 13:13 |
_mup_ | Bug #1078239: loading team blueprints leads to timeout Person:+index <fallout> <lp-blueprints> <privacy> <private-projects> <regression> <timeout> <Launchpad itself:Triaged> < https://launchpad.net/bugs/1078239 > | 13:13 |
rick_h | czajkowski: ok, https://bugs.launchpad.net/launchpad/+bug/1075365 is in qa mode so we should hopefully have that ready for a deploy in a couple of hours | 13:14 |
_mup_ | Bug #1075365: Timeout when trying to visit previous sprint pages <fallout> <lp-blueprints> <private-projects> <qa-needstesting> <regression> <timeout> <Launchpad itself:Fix Committed by abentley> < https://launchpad.net/bugs/1075365 > | 13:14 |
rick_h | and if that doesn't fix it...well then back to the code | 13:15 |
czajkowski | rick_h: great | 13:15 |
rick_h | czajkowski: so card is added to our board and we'll make sure to follow up. | 13:16 |
jcsackett | morning all. | 14:07 |
rick_h | party | 14:09 |
abentley | morning. | 14:13 |
jml | hi | 14:30 |
=== matsubara is now known as matsubara-lunch | ||
czajkowski | sinzui: what does the tag specification mean? | 14:45 |
sinzui | The error involves the specification object | 14:46 |
rick_h | deryck: here's the test case I'm working on with the matchers https://pastebin.canonical.com/78291/ and the html content appears in line 151 of the test failure output | 14:55 |
rick_h | which is the browser.contents | 14:55 |
rick_h | I moved that <noscript> outside of the target in case the contents were what was messing up the match (#150) | 14:56 |
deryck | rick_h, ok, just one second and I'll look.... | 14:56 |
rick_h | deryck: np | 14:56 |
deryck | rick_h, looking now..... | 15:00 |
rick_h | hah https://pastebin.canonical.com/78298/ | 15:01 |
deryck | rick_h, so it does get a match but then the test fails? Is that what your "hah" paste there suggests? | 15:04 |
rick_h | deryck: no, it fails to find a match when the only thing I send (via pdb in there) is the exact string that the Tag() is setup for | 15:05 |
deryck | rick_h, ah, I see now. | 15:05 |
rick_h | I would expect that second paste to find a match since it's nothing but the matching tag? | 15:05 |
deryck | rick_h, I wonder if it's tripping over underbars, since we normally use hypens. and we've just never hit this before because of convention. | 15:06 |
rick_h | deryck: ok, will change the Tag and try it that way | 15:06 |
deryck | rick_h, I don't really see anything odd, though. Like you said on stand-up, it's pretty straight forward. | 15:07 |
rick_h | yea, wasn't expecting to lose my morning when I saw the test failure, but I must be doing something wrong :/ | 15:07 |
rick_h | deryck: ah crap, you know what. There's a second test taht failed | 15:08 |
rick_h | it only had one error in the email, but I 'fixed' that first test and a second one came out. That error message is from a later test | 15:09 |
* rick_h does a giant *DOH* | 15:09 | |
deryck | rick_h, ah, ok. np. so you need me to look at the second test too? Or you're okay now? | 15:10 |
rick_h | no, we're ok. I just missed that the failure moved on me | 15:10 |
rick_h | thanks for looking | 15:10 |
deryck | np! | 15:13 |
=== matsubara-lunch is now known as matsubara | ||
=== deryck is now known as deryck[lunch] | ||
abentley | deryck: Since access policies are calculated based on sharing policies, letting artifacts have information types forbidden by their policies means they can become inaccessible. | 16:55 |
abentley | deryck[lunch]: e.g. if the sharing policies change to forbid Proprietary, the access policies for Proprietary will be removed, and any Proprietary artifacts will become inaccessible. | 16:57 |
sinzui | frankban, rick_h, do either of you have time to review https://code.launchpad.net/~sinzui/launchpad/unicode-project-group-bug/+merge/134154 | 17:01 |
rick_h | sinzui: sure thing | 17:01 |
rick_h | sinzui: actually I've got a sprite question for you as well | 17:01 |
sinzui | okay | 17:02 |
rick_h | sinzui: I'm working on adding private images to other tables of data starting with the +upcomingwork http://uploads.mitechie.com/lp/work_items.png | 17:02 |
rick_h | I started to add another column much like is done in the branch listing | 17:02 |
rick_h | but ugh and ugh | 17:02 |
sinzui | yes | 17:02 |
rick_h | and I noticed that in some places it's done with the sprite private | 17:02 |
rick_h | but then that would replace the current sprite of the blueprint/project in the upcoming work table | 17:03 |
rick_h | are there any rules for this? | 17:03 |
rick_h | I was also trying to work out a way to build the list of classes to have both sprites, but it seems to be done adding a new method on the view to build the css based on the current context and double ugh | 17:03 |
rick_h | and not sure if we can stack sprites, haven't look that far yet | 17:03 |
sinzui | We cannot stack sprites if you mean show more than one for an item | 17:04 |
rick_h | sinzui: right ok. So then should I be working on adding another column to hold the private indicator or swap out the default sprite for a private sprite? | 17:05 |
sinzui | The badge and column approach is "canonical", and the "canonical" way does not scale and the layout is more by accident | 17:05 |
sinzui | I am happy with any solution that does not require tables | 17:05 |
rick_h | heh, well the tables are already there so hacking into there. | 17:06 |
rick_h | I suppose I can append the image to the end of the actual string of text that links to the blueprint/product | 17:06 |
rick_h | so they won't line up, but will actually look associated with the right bit of text | 17:06 |
sinzui | I like the suggestion of having an alternate sprite for private items and the "private blueprint" class can do this if we place it in the right place in our sprite css | 17:06 |
rick_h | where the screenshot shows that it's going to look joined with the wrong content | 17:06 |
sinzui | I have a zany idea | 17:07 |
rick_h | heh, I'm up for zany. Especially if it looks uncrappy :) | 17:08 |
sinzui | I think we have a large inline sprite file. We can create an image with two icons per row and adjust the left padding to ensure both show for the css selector | 17:09 |
sinzui | oh, I am wrong | 17:09 |
rick_h | we'd need a class/icon for every combo of private:item | 17:10 |
sinzui | We must have factored out the large inline sprites | 17:10 |
rick_h | ah | 17:10 |
sinzui | we could do this, and I think we can place them in block-sprites-1.css.in | 17:11 |
rick_h | ok, so the idea then is to add new sprites that combine branch, product, blueprint, bugs with private and add a new css class for those that's .sprite.XXX.private with the new double wide sprite image? | 17:16 |
rick_h | and add those css rules into block-sprites-1.css.in? | 17:16 |
sinzui | rick_h, yes | 17:23 |
sinzui | rick_h, I suppose you realise that this issue is really cause by the single-background image limitation in CSS. If we used a sprite font and css-content, we could stack all we want | 17:27 |
rick_h | sinzui: yea, which is why I was trying to see how the private was indicated in other places like the branch badge setup | 17:28 |
rick_h | sinzui: but yea, sprite would be preferred so would be good to add them and make updating the UI eaiser in the long run/lower maint. | 17:28 |
=== frankban changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On Call Reviewer: rick_h | Firefighting: - | Critical bugs: ~200 | ||
sinzui | we never get proper alignments with floats and/or wraps. only a font could provide what we want | 17:29 |
rick_h | heh, font is nice but ugh on the work to do that | 17:29 |
=== deryck[lunch] is now known as deryck | ||
deryck | abentley, can we change things such that if you change your sharing policy but have any remaining artifacts of a different info type, then we don't remove the access? | 17:59 |
deryck | abentley, or should we forbid the change until you reconcile the issues/ | 17:59 |
deryck | ? | 17:59 |
abentley | deryck: Certainly we can consider existing artifacts when changing access policies. | 18:02 |
abentley | deryck: I'm still chewing over whether we want to permit artifacts to vary from the policies. | 18:02 |
abentley | deryck: With my branch, we'll be essentially refusing to change the policy if there are any existing artifacts. | 18:03 |
abentley | deryck: But only in the limited circumstances of changing the project information type from public to proprietary. | 18:03 |
deryck | abentley, right. so we need to think on it and settle on what to do in the other cases. | 18:04 |
abentley | deryck: So I wonder whether we should make that a more general rule. | 18:04 |
deryck | abentley, right, that was my initial thought. | 18:04 |
deryck | abentley, but I don't mind if we take a day or two to think/talk through the implications of that change. | 18:04 |
abentley | deryck: Cool. | 18:10 |
rick_h | sinzui: do you know if there's somewhere we have the rest of the original sprite images? blueprint, private, produt/project aren't in the images/src dir. | 18:20 |
sinzui | yuck | 18:21 |
rick_h | I can just stack the .png would prefer to do the .svg if we have them | 18:21 |
sinzui | I don't know where those sources would be. rick_h early Lp art was contracted and we didn't always get sources | 18:21 |
rick_h | sinzui: ok cool, not a problem. Just want to make sure I'm not blind :) | 18:22 |
deryck | rick_h, hi there. can you review a branch for me? | 18:55 |
rick_h | deryck: sure thing | 18:58 |
deryck | rick_h, thanks! https://code.launchpad.net/~deryck/launchpad/top-level-bug-search-and-privacy-1069895/+merge/134164 | 18:58 |
rick_h | heh, never would have thought that tales.py would do link construction for the images/badges/sprites | 18:59 |
rick_h | deryck: r=me with a couple of questions and one comment | 19:26 |
deryck | rick_h, thanks! | 19:27 |
=== Ursinha_ is now known as Ursinha | ||
rick_h | deryck: are you up for a quick small review please since I'm OCR? https://code.launchpad.net/~rharding/launchpad/security_banner_107842/+merge/134187 | 20:11 |
deryck | rick_h, sure | 20:12 |
deryck | rick_h, r=me | 20:14 |
rick_h | deryck: ty much | 20:14 |
deryck | np! | 20:14 |
=== almaisan-away is now known as al-maisan | ||
wgrant | wallyworld__: ss_clauses.append(In( | 22:05 |
wgrant | Row(BugTaskFlat.distribution_id, | 22:05 |
wgrant | BugTaskFlat.sourcepackagename_id), | 22:05 |
wgrant | Select((SS.distributionID, SS.sourcepackagenameID), | 22:05 |
wgrant | tables=[SS]))) | 22:05 |
=== al-maisan is now known as almaisan-away | ||
wallyworld__ | wgrant: https://pastebin.canonical.com/78343/ | 23:04 |
wgrant | wallyworld__: launchpad_dev=# SELECT * FROM (VALUES (1, 2, 3), (4, 5, 6)) AS foo(bar, baz, quux); | 23:06 |
wgrant | bar | baz | quux | 23:06 |
wgrant | -----+-----+------ | 23:06 |
wgrant | 1 | 2 | 3 | 23:06 |
wgrant | 4 | 5 | 6 | 23:06 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!