/srv/irclogs.ubuntu.com/2010/08/12/#launchpad-reviews.txt

=== mwhudson_ is now known as mwhudson
stublifeless: So my branch had timeout as the timeout, and your branch has timeout as the goal time for the 99% calculations, and histogram_width as the timeout05:48
lifelessyeah, my first branhc, as you said, was in poor shape05:49
lifelessthe incremental changes are twofold05:49
lifelessfirstly, there is an existing bug in the report: the stddev is being mangled by the clipping of the data05:49
lifelesslikewise the variange05:49
=== Ursinha-bbl is now known as Ursinha
lifelessso I changed that, and having fixed that I then had the abilitity to do the histogram columns independently, and I figures that seeing the shape beyond the timeout would be useful05:50
stubThe logic was that if a page takes longer than the hard timeout, that is irrelevant. It doesn't render so ends up in the 'fail' column at the hard timeout, giving a nice 'kick' at the end.05:54
stubI would have labelled the last column 'timeout' if the graphing tool let me.05:55
stubI suspect the only correct answer though would be to report a timeout%, and have the stats calculated non-timeout pages.05:56
lifelessstub: well, what I've done willso have a kick05:58
lifelessand - we could do a kick at the exact timeout column if you wanted05:58
lifelessbut I wanted the stddev to be accurate, because that tells us the 99% figure05:58
lifelessstub: perhaps you could give it a spin and we can see what it looks like ?05:59
stublifeless: I don't like that we are growing a separate list for capped request times. I think we want to create this on the fly using a generator, and use numpy.fromiter instead of numpy.asarray to construct the array.06:03
lifelessstub: ok06:04
lifelessstub: is that a memory overhead concern ?06:04
stubWe have nearly 500 of these things in ram containing over 10 million 32bit objects, so yeah06:04
stubBut I pulled out the spool-to-disk version, as it seemed to be better to hit swap than open 500 temporary files. That was implemented as I thought I was crashing devpad, but it was just hardware issues.06:05
stub(monthly reports will get lots of requests, when devpad stays up long enough to actually generate one...)06:06
stubI'll give it a run as is - memory shouldn't be an issue for a daily run.06:07
lifelessI'm happy to change that for you06:09
lifelessI considered doing it on the fly but it seemed slower ;P06:09
stubIt is slower, but that is a minor part of our runtime. Parsing the logs takes time. Generating the stats is pretty much instantaneous.06:19
lifelessok06:39
lifelessso - lets see how it looks, and I'll look up fromiter06:39
stubfromiter(iterable, numpy.float32, len(self.request_times)) - we know the length so can use the third parameter06:46
lifelessand iterable is an adapter to call min ?06:47
stubhttps://devpad.canonical.com/~stub/ppr/edge/latest-daily-timeout-candidates.html is done. lpnet still generating06:48
stublifeless: I imagine it is a generator that returns the time capped to the timeout06:48
lifelessRIGHT06:49
lifelessbah06:49
lifelessRight06:49
lifelessthats what I thought too06:49
stubBwaha... LibraryFileAlias:StreamOrRedirectLibraryFileAliasView is a mess without the cap ;)06:53
stubBut I guess that is the truth.06:53
lifelessyup06:53
lifelessdo think these changes are ok then ?06:54
lifelessI've pushed up a (hopefully working) fromiter version06:56
stubThat all looks good. I've kicked off another run with the new version07:04
lifelessthanks07:05
=== Ursinha is now known as Ursinha-zzz
=== henninge_ is now known as henninge
=== noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: - || queue: [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jelmernoodles775: Could I add a branch to your queue?10:56
noodles775jelmer: please do :)10:56
jelmernoodles775: The MP is https://code.edge.launchpad.net/~jelmer/launchpad/613468-xb-ppa-db/+merge/3230710:57
=== jelmer changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: - || queue: [leonardr,jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jelmernoodles775, thanks!10:57
StevenKnoodles775: O hai, can you re-re-review https://code.edge.launchpad.net/~stevenk/launchpad/switch-ifp-to-unittests/+merge/32073 ?11:00
noodles775StevenK: indeed :)11:01
noodles775StevenK: I'm not trying to make your life hard, and hate dragging this review on, but I don't understand why test_create_builds does so much more than what the name implies... do you have time to mumble and discuss it briefly?11:33
=== noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: jelmer || queue: [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
StevenKnoodles775: Yes, doing so in a sec11:44
StevenKnoodles775: But it doesn't? It asserts the *source* for 0.1-2 exists, and then creates a build for it?11:45
StevenKnoodles775: The diff does not appear to have been updated yet, but the MP is ready for your re-re-re-review.12:15
noodles775Thanks StevenK12:15
StevenKnoodles775: No news is good news, or I'm in the queue?12:38
noodles775StevenK: looking now (had to merge as the MP hadn't updated)12:42
StevenKYes, I'm not sure what is going on there12:42
jelmernoodles775: Thanks for the review!12:50
noodles775np12:51
noodles775StevenK: looks good. I've just highlighted what seem to be a few inconsistencies in the comments, but otherwise r=me: http://pastebin.ubuntu.com/476903/12:57
=== salgado-afk is now known as salgado
wgrantjelmer: Hm, why are the extra fields JSON?13:07
wgrantThey're known to be string pairs.13:07
wgrantIt seems like an extra table would do fine, leave the table smaller, and make it possible to query them in future.13:08
jelmerwgrant: consistency with other fields in the database, where we also use JSON.13:08
wgrantAre there any, apart from on Job?13:08
StevenKnoodles775: Planning to commit http://pastebin.ubuntu.com/476907/13:08
jelmerwgrant: No, just job actually.13:09
wgrantjelmer: And it's like that so that custom job types can store different data there.13:09
wgrantI don't see why JSON should be used here, and not a separate table.13:09
jelmerwgrant: simplicity; I don't see why we would need to query that field. if we would go the way of a separate table we might want to use it for some of the other control fields as well13:17
jelmerwhich might not be a bad idea, just out of scope for me at the moment.13:17
StevenKgmb: O hai! Given noodles775's has Approved the MP at https://code.edge.launchpad.net/~stevenk/launchpad/switch-ifp-to-unittests/+merge/32073; can haz moar review?13:31
gmbStevenK, If you stop asking in lolspeak, yes. ;)13:39
gmbIn fact, I think noodles775's review is far more comprehensive than mine (hurrah for context)13:40
gmbSo I'll just mark it approved.13:40
StevenKgmb: Okay, cool13:40
=== matsubara-afk is now known as matsubara
=== Ursinha-zzz is now known as Ursinha
=== StevenK changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: jelmer || queue: [leonardr,StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
james_wjml: hi, would you be willing to do an incremental review of https://code.edge.launchpad.net/~james-w/launchpad/no-more-sampledata-2/+merge/31891 ?15:19
jmljames_w, yes. I'll look at it shortly15:19
james_wthanks15:19
=== deryck is now known as deryck[lunch]
=== Ursinha is now known as Ursinha-lunch
leonardrcan i get someone to review my really simple launchpadlib branch? i've been in the queue since yesterday16:41
bachey salgado, can you do a shipit review for me?16:41
leonardrhttps://code.edge.launchpad.net/~leonardr/launchpadlib/616055/+merge/3235216:42
salgadobac, sure!16:42
bacsalgado:  https://code.edge.launchpad.net/~bac/shipit/bug-616059/+merge/3245716:42
salgadobac, not allowed16:42
bacsalgado:  try now16:43
salgadobac, while you subscribe me to your branch, subscribe launchpad-pqm as well, or else you won't be able to land it later16:43
bacsalgado:  thanks for the tip!16:44
leonardrbac, while you wait, will you review my branch please?16:44
bacleonardr:  sure16:44
* bac looks16:44
leonardrthanks16:45
salgadobac, did you consider doing that for canonical.* instead of canonical.launchpad.*?16:47
bacsalgado:  yeah, sort of.  i thought lp was sufficient.  you think i chose wrong?16:48
salgadobac, I'd have done for canonical.* but lp.* and canonical.launchpad.* is certainly where we do most of our changes/refactorings, so it's no big deal16:49
bacsalgado:  yeah, you would've...but you're smarter than me!16:57
=== deryck[lunch] is now known as deryck
=== matsubara is now known as matsubara-lunch
salgadobac, If that was the case I'd have done this change long ago. ;)17:02
=== benji is now known as benji-lunch
=== benji-lunch is now known as benji
bacsalgado:  yeah, if you weren't such a slacker17:02
=== benji is now known as benji-lunch
salgadothat is certainly the case17:10
bacleonardr:  r=bac.17:11
salgadobac, did you get my review?  you might want to get someone from ISD to have a look as well, as they own shipit now17:11
bacsalgado:  i see it now.  did you mean to have it just be a comment?17:13
salgadono, just approved using the web UI, but it'd be good to at least let them know about the change before it lands17:13
bacsalgado:  i'll talk to achuni or someone else17:14
bacsalgado:  thanks for catching those issues17:14
baci renamed to sd_setUp b/c there is another setUp being imported in that file17:14
salgadonp.  thank you for doing this change17:15
salgadofair enough17:15
bacsd is cryptic for 'systemdocs'17:15
=== salgado is now known as salgado-lunch
leonardrbenji-lunch, it looks like we're missing a review for lazr.restfulclient/total_size_link17:44
=== Ursinha-lunch is now known as Ursinha
=== matsubara-lunch is now known as matsubara
=== benji-lunch is now known as benji
leonardrnoodles775, the branch i'd like reviewed is https://code.edge.launchpad.net/~benji/lazr.restfulclient/total_size_link/+merge/3247218:00
leonardrit's benji's branch but most of the code is mine18:01
leonardrjust fyi, when you get to me18:01
jmljames_w, done18:07
=== salgado-lunch is now known as salgado
james_wthanks jml18:25
jmlnp18:26
leonardrnoodles775, i'd also like to add this to the queue: https://code.edge.launchpad.net/~leonardr/launchpad/new-lazr.restful-and-friends/+merge/3247418:42
jelmerleonardr: I think Michael has signed off for the day.18:48
leonardrwell, damn18:48
leonardrbac, are you in the mood to do more reviews?18:48
bacleonardr:  i can do one.18:49
jelmerrockstar should be around later, or is he on leave?18:49
* bac has a full day of reviews tomorrow18:49
rockstarOh snap, forgot it was Thursday.18:49
=== rockstar changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: leonardr || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
leonardrbac: ok, rockstar will take care of these18:50
=== leonardr changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: leonardr || queue: [StevenK, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
bacthanks leonardr and rockstar18:51
rockstarleonardr, apparently there are two review to do for you.  Where's the other one?18:59
rockstar(I already did new-lazr.restful-and-friends)18:59
leonardrrockstar: ok, the other one is https://code.edge.launchpad.net/~benji/lazr.restfulclient/total_size_link/+merge/3247218:59
=== rockstar changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
bacrockstar may i add a small branch to your queue?19:05
rockstarbac, sure.19:05
rockstarbac, ftr, I'm going to clear out all the +activereviews today, so you won't have a huge queue tomorrow.19:07
bacrockstar:  that would be swell.  do leave me some, though!19:12
bacleonardr:  what is the status of this branch: https://code.edge.launchpad.net/~leonardr/launchpad/grant-permissions-oauth/+merge/2942519:30
leonardrbac: i got sidetracked by an emergency project before i could land it, and i don't want to land it on its own because it's part of another large project19:30
leonardrso, the status is "wait and see"19:31
bacleonardr:  ok19:31
=== bac changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: StevenK || queue: [bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== bryceh changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: StevenK || queue: [bac bryce] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
bacthanks for the review rockstar21:33
jelmerrockstar: I've fixed the conflicts in my branch; any chance I could add it back into your queue?22:25
rockstarjelmer, if it's in +activereviews, I'll get to it today.22:26
* rockstar is firefighting currently.22:26
jelmerrockstar: great, thanks22:26
jelmerrockstar: oh, ok22:26
thumperjelmer: are you going to propose the bzr-git update branch?22:43
jelmerthumper: yes, it should fix quite a few of the broken imports22:44
thumpercool22:44
thumperthere is someone trying the chicken import every few days22:44
jelmerthumper: yeah, he pinged me on IRC today about it.22:45
jelmerthumper: How hard it is to get the new bzr-git deployed without waiting for the next lp release?22:45
thumperjelmer: very trivial22:45
thumperjelmer: we land it, test it on staging22:45
thumperjelmer: then CP it to the production branch22:46
thumperjelmer: and get it rolled out22:46
thumperso lag time is 3 x ec2 test run + staging rollout lag + production rollout lag22:46
thumperadd in test time, and getting approval, both of which should be pretty short22:47
jelmerok22:47
=== matsubara_ is now known as matsubara-afk
jelmerthumper: https://code.edge.launchpad.net/~jelmer/launchpad/update-bzr-git/+merge/3252723:20
thumperjelmer: done23:35
jelmerthumper: bug 497428 appears to be fixed, scilab's import is working. Can I mark it fixreleased in launchpad-code?23:36
_mup_Bug #497428: Import on a large git repository is failing <code-import> <Bazaar Git Plugin:Fix Released> <Launchpad Bazaar Integration:Triaged> <https://launchpad.net/bugs/497428>23:36
thumperjelmer: yep23:36
jelmerthumper: perhaps we should increase the number of revisions imported per step with bzr-svn23:47
thumperjelmer: I'm up for that23:47
thumperjelmer: it is just a config change I think23:47
jelmerthumper: the KDE branches appear to be working now, but they're spending 15 minutes trying to figure out what 1k revisions to fetch and then 2 minutes doing the actual work.23:47
thumperjelmer: 10k seem reasonable?23:48
jelmerthumper: Yeah, definitely.23:48
jelmerthumper: Could the git batch size also be increased perhaps?23:49
thumperjelmer: yep23:49
jelmerI think it's at 10k now but we should be able to handle 30k at least23:49
thumperjelmer: sure, sounds reasonable23:49
mwhudsoni still wonder if it should be time-limited not rev-count limited23:57
jelmermwhudson: time and memory limited in that case, but yeah, that sounds reasonable.23:59

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