/srv/irclogs.ubuntu.com/2012/11/13/#launchpad-dev.txt

wgrantwallyworld__: I've reviewed your branch05:04
StevenK"And it's all rubbish"05:04
StevenK:-P05:04
wgrantNot quite :)05:04
wallyworld__wgrant: thanks, i'll look after i get back from school pickup05:05
wallyworld__wgrant: i can't use rs.find().set() because that doesn't return the rowcount06:36
wgrantAh06: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 not06:37
wgrantwallyworld__: You may be better off doing an initial SELECT to work out whether you need to UPDATE or INSERT06:37
wallyworld__that's an extra select i'd rather avoid06:37
wallyworld__unless you think it won't affect things06:37
wgrantBut that turns each batch into 4 queries06:37
wgrantCurrently you're doing 2+n queries06:38
wallyworld__hmmm. i do i update for creator != null, 1 for maintainer != null, and a possible insert06:38
wgrant(aha, my first test run on DF crashed with the INSERT conflict that I mentioned)06:39
wgrantRight06:39
wallyworld__as well as the initial select06:39
wgrantYou do the initial select, two updates for each row, and an insert06:39
wallyworld__no, a possible insert06:39
wgrantI'd suggest instead the initial select, some computation, a batch select, a batch update, and a batch insert06:39
wgrantTrue06:39
wallyworld__hmmm. you really think that would be better?06:40
wgrantIt's going to reduce the query count by a factor of 1000 or more06:40
wallyworld__most times the insert wil not be required once the table is populated06:40
wgrantSure, but the insert is negligible06:41
wgrantBecause it's batched06:41
wgrantIts elimination does not help performance to any measurable degree06: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 exist06:42
wgrantRight06:42
wgrantOtherwise you can't identify which rows need inserting unless you do a thousand separate UPDATEs for the batch06:42
wgrantWhich is obviously substantially more expensive than a single SELECT06:43
wallyworld__my thinking was most times all the updates would be required anyway06:43
wallyworld__once the table is populated06:43
wgrantBut then you have 1000 queries instead of 106:43
wgrantStill06:43
wallyworld__i mean, if the updates are required, you still have 1000 queries06:43
wallyworld__plus the initial computation as an overhead06:44
wgrantNot with my technique06:44
wgrantYou can do a batch update using either a few CASE expressions or a literal subselect06:45
wallyworld__i'd have to think a bit about that06: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
wgrantUPDATE 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 works06: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__yeah06:55
wgrantThis 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 either06:56
wgrantAnd I don't think it's too much effort to get it Rightâ„¢, apart from getting Storm to cope with that UPDATE syntax06:56
wgrantBut it should be relatively easy to do with some of the helpers in lp.services.database.bulk06:56
wallyworld__yeah, no sure if thre would be native storm support for it, may need a bit of SQL("..")06:57
wgrantThere 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 are07:03
wallyworld__i'll see what falls out07:05
wallyworld__fun fun fun07:05
wgrantStorm has the ROW constructor nowadays, but the only use of VALUES expressions that I know of is the INSERT special case07:10
wgrantwallyworld__: 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 errands07:12
wallyworld__may well need another iteration07:12
wgrantYeah07:13
wgrantBut this is looking very good, and I think the correctness concerns I had have been ironed out.07:13
wgrantand 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 model07:15
wgrantwallyworld__: Exactly07:19
wgrantwallyworld__: 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
wgrantThis is our first table with eventual consistency and just a few minutes of lag.07:21
wgrantEverything else we have is either updated by triggers or a huge daily cronjob :)07:22
StevenKWe should destroy nightly.sh for being a terrible idea and making a lot of people very angry07:30
stubIn favour of?07:39
stubsmaller celery tasks on demand might be nicer07:40
wallyworld__afternoonly.sh07:40
stubeven if it would mean more overhead07:40
stubmanyana.sh07:40
adeuringgood morning09:03
cjwatsonwgrant: 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
cjwatsonIt seems too eay10:37
cjwatson*easy10:37
wgrantcjwatson: As long as it's been fixed to not reupload then it should be fine10:42
cjwatsonYeah, I destroyed the reuploading a while back10:42
wgrantYeah, I thought I recalled that10:43
cjwatsonI 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 it10:46
wgrantYeah10:49
cjwatsonAnd we might have to keep a tiny amount of code around for the sake of old entries in DONE queues and the like10:49
wgrantcjwatson: Yeah, but most of the code can die11:05
adeuringfrankban: could you please review this mp: https://code.launchpad.net/~adeuring/launchpad/product-lp-limitedview-3-1/+merge/134075 ?11:24
frankbanadeuring: sure11:29
adeuringfrankban: thanks!11:29
=== frankban changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On Call Reviewer: frankban | Firefighting: - | Critical bugs: ~200
wgrantadeuring: LimitedView delegates to View by default, so you probably don't need to check for View at all11:31
adeuringwgrant: ah, right, let me try this11:32
wgrantThe ifs will still be a little ugly, but it's at least slightly less bad11: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
czajkowskirick_h: morning13:12
rick_hmorning czajkowski13:12
czajkowskirick_h: I come with a gift :) https://bugs.launchpad.net/launchpad/+bug/107823913: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_hczajkowski: 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 hours13: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_hand if that doesn't fix it...well then back to the code13:15
czajkowskirick_h: great13:15
rick_hczajkowski: so card is added to our board and we'll make sure to follow up.13:16
jcsackettmorning all.14:07
rick_hparty14:09
abentleymorning.14:13
jmlhi14:30
=== matsubara is now known as matsubara-lunch
czajkowskisinzui: what does the tag specification mean?14:45
sinzuiThe error involves the specification object14:46
rick_hderyck: 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 output14:55
rick_hwhich is the browser.contents14:55
rick_hI moved that <noscript> outside of the target in case the contents were what was messing up the match (#150)14:56
deryckrick_h, ok, just one second and I'll look....14:56
rick_hderyck: np14:56
deryckrick_h, looking now.....15:00
rick_hhah https://pastebin.canonical.com/78298/15:01
deryckrick_h, so it does get a match but then the test fails?  Is that what your "hah" paste there suggests?15:04
rick_hderyck: 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 for15:05
deryckrick_h, ah, I see now.15:05
rick_hI would expect that second paste to find a match since it's nothing but the matching tag?15:05
deryckrick_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_hderyck: ok, will change the Tag and try it that way15:06
deryckrick_h, I don't really see anything odd, though. Like you said on stand-up, it's pretty straight forward.15:07
rick_hyea, wasn't expecting to lose my morning when I saw the test failure, but I must be doing something wrong :/15:07
rick_hderyck: ah crap, you know what. There's a second test taht failed15:08
rick_hit 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 test15:09
* rick_h does a giant *DOH*15:09
deryckrick_h, ah, ok.  np.  so you need me to look at the second test too?  Or you're okay now?15:10
rick_hno, we're ok. I just missed that the failure moved on me15:10
rick_hthanks for looking15:10
derycknp!15:13
=== matsubara-lunch is now known as matsubara
=== deryck is now known as deryck[lunch]
abentleyderyck: 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
abentleyderyck[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
sinzuifrankban, rick_h, do either of you have time to review https://code.launchpad.net/~sinzui/launchpad/unicode-project-group-bug/+merge/13415417:01
rick_hsinzui: sure thing17:01
rick_hsinzui: actually I've got a sprite question for you as well17:01
sinzuiokay17:02
rick_hsinzui: I'm working on adding private images to other tables of data starting with the +upcomingwork http://uploads.mitechie.com/lp/work_items.png17:02
rick_hI started to add another column much like is done in the branch listing17:02
rick_hbut ugh and ugh17:02
sinzuiyes17:02
rick_hand I noticed that in some places it's done with the sprite private17:02
rick_hbut then that would replace the current sprite of the blueprint/project in the upcoming work table17:03
rick_hare there any rules for this?17:03
rick_hI 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 ugh17:03
rick_hand not sure if we can stack sprites, haven't look that far yet17:03
sinzuiWe cannot stack sprites if you mean show more than one for an item17:04
rick_hsinzui: 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
sinzuiThe badge and column approach is "canonical", and the "canonical" way does not scale and the layout is more by accident17:05
sinzuiI am happy with any solution that does not require tables17:05
rick_hheh, well the tables are already there so hacking into there.17:06
rick_hI suppose I can append the image to the end of the actual string of text that links to the blueprint/product17:06
rick_hso they won't line up, but will actually look associated with the right bit of text17:06
sinzuiI 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 css17:06
rick_hwhere the screenshot shows that it's going to look joined with the wrong content17:06
sinzuiI have a zany idea17:07
rick_hheh, I'm up for zany. Especially if it looks uncrappy :)17:08
sinzuiI 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 selector17:09
sinzuioh, I am wrong17:09
rick_hwe'd need a class/icon for every combo of private:item17:10
sinzuiWe must have factored out the large inline sprites17:10
rick_hah17:10
sinzuiwe could do this, and I think we can place them in block-sprites-1.css.in17:11
rick_hok, 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_hand add those css rules into block-sprites-1.css.in?17:16
sinzuirick_h, yes17:23
sinzuirick_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 want17:27
rick_hsinzui: yea, which is why I was trying to see how the private was indicated in other places like the branch badge setup17:28
rick_hsinzui: 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
sinzuiwe never get proper alignments with floats and/or wraps. only a font could provide what we want17:29
rick_hheh, font is nice but ugh on the work to do that17:29
=== deryck[lunch] is now known as deryck
deryckabentley, 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
deryckabentley, or should we forbid the change until you reconcile the issues/17:59
deryck?17:59
abentleyderyck: Certainly we can consider existing artifacts when changing access policies.18:02
abentleyderyck: I'm still chewing over whether we want to permit artifacts to vary from the policies.18:02
abentleyderyck: With my branch, we'll be essentially refusing to change the policy if there are any existing artifacts.18:03
abentleyderyck: But only in the limited circumstances of changing the project information type from public to proprietary.18:03
deryckabentley, right. so we need to think on it and settle on what to do in the other cases.18:04
abentleyderyck: So I wonder whether we should make that a more general rule.18:04
deryckabentley, right, that was my initial thought.18:04
deryckabentley, but I don't mind if we take a day or two to think/talk through the implications of that change.18:04
abentleyderyck: Cool.18:10
rick_hsinzui: 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
sinzuiyuck18:21
rick_hI can just stack the .png would prefer to do the .svg if we have them18:21
sinzuiI don't know where those sources would be. rick_h early Lp art was contracted and we didn't always get sources18:21
rick_hsinzui: ok cool, not a problem. Just want to make sure I'm not blind :)18:22
deryckrick_h, hi there.  can you review a branch for me?18:55
rick_hderyck: sure thing18:58
deryckrick_h, thanks! https://code.launchpad.net/~deryck/launchpad/top-level-bug-search-and-privacy-1069895/+merge/13416418:58
rick_hheh, never would have thought that tales.py would do link construction for the images/badges/sprites18:59
rick_hderyck: r=me with a couple of questions and one comment19:26
deryckrick_h, thanks!19:27
=== Ursinha_ is now known as Ursinha
rick_hderyck: are you up for a quick small review please since I'm OCR? https://code.launchpad.net/~rharding/launchpad/security_banner_107842/+merge/13418720:11
deryckrick_h, sure20:12
deryckrick_h, r=me20:14
rick_hderyck: ty much20:14
derycknp!20:14
=== almaisan-away is now known as al-maisan
wgrantwallyworld__:             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
wgrantwallyworld__: launchpad_dev=# SELECT * FROM (VALUES (1, 2, 3), (4, 5, 6)) AS foo(bar, baz, quux);23:06
wgrant bar | baz | quux23:06
wgrant-----+-----+------23:06
wgrant   1 |   2 |    323:06
wgrant   4 |   5 |    623:06

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!