[03:53] can has review ? https://code.launchpad.net/~lifeless/python-oops-tools/amqp/+merge/79749 === elmo_ is now known as Guest35863 [04:50] lifeless: Don't want to use logging (timestamps!) rather than print? === almaisan-away is now known as al-maisan [04:51] wgrant: console debugging me [04:51] wgrant: I don't see a use case (today) for it live [04:51] eparse [04:51] Ah [04:51] Yeah, but. [04:51] OK. [04:51] s/me/meh/ [04:51] Ah! [04:52] lifeless: https://code.launchpad.net/~wgrant/launchpad/bug-735998/+merge/79905 [04:52] wgrant: btw I think the plan has changed over the last year === wgrant changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - | Critical bugtasks: 266 [04:52] wgrant: prob with the table width changes [04:53] lifeless: Probably, yes. [04:53] This timeout is reasonably new. [04:53] And mostly affects #1. [04:57] lifeless: thanks. [04:57] np, thank you [05:01] stub: Hai! Could you look over https://code.launchpad.net/~stevenk/launchpad/db-packageupload-archive-index/+merge/79903 ? [05:02] ออ [05:02] We really should have (archive, status), or perhaps (archive, distroseries, status). [05:03] As well? [05:03] Instead. [05:03] How that help the delete? [05:04] status doesn't, but it helps process-accepted. [05:05] StevenK: The immediate problem is deletion of rows? Those other indexes will be used for delete too. [05:05] stub: CASCADE deletion from Archive [05:06] We delete archives now? [05:07] We were going to make an exception in this case. 53 unused PPAs owned by open teams [05:07] We already have an index on (distroseries, status). Does that become redundant with (archive, distroseries, status). ie. when we are deleting by (distroseries, status), do we always include an archive too? [05:07] If it is one off, we can just delete the packageupload rows first, then the cascade delete from archive [05:08] There are no packageupload rows. [05:08] Or at least shouldn't be. [05:08] But there's no index to let it discover that within 2.5s. [05:08] Right [05:08] Which isn't a problem for a select [05:08] stub: Every query should specify an archive too. Except DistroSeries:+queue, which will do archive IN (1, 534). [05:09] But there shouldn't have been archiveless queries since 2007 :*( [05:09] :( [05:09] stub: Hm? [05:09] stub: We preselect the archive IDs. [05:09] Then try to delete them. [05:09] So we should drop the (distroseries status) index and add (archive, distroseries, status) which will help the immediate problem and make other stuff go faster too. [05:09] Which times out when looking for packageuploads to cascade to. [05:09] Yes. [05:09] What are the archive ids? [05:10] stub: The query is on RequestLogging in the usual place [05:10] Right, fixing the patch to drop and create an index [05:10] Will need to be two separate patches. [05:10] One will be run live with CONCURRENTLY. [05:11] Then drop the old index during downtime afterwards. === al-maisan is now known as almaisan-away [05:11] Why? [05:11] Because we can't create the index during downtime. [05:11] Too slow. [05:11] RequestLogging? [05:12] found it [05:12] https://wiki.canonical.com/InformationInfrastructure/OSA/RequestLogging/LP/SQL [05:12] -CREATE INDEX packageupload__archive__idx ON PackageUpload(archive); [05:12] +DROP INDEX packageupload__distroseries__status__idx; [05:12] Still need a patch to create the index. [05:12] Assuming the index is created beforehand [05:12] 92-1 will create [05:12] 92-2 will drop [05:12] 92-1 will be applied live with CONCURRENTLY [05:13] Yup [05:15] Changes pushing [05:25] StevenK: why are we dropping that index? [05:26] Because we're creating one on archive, distroseries, status instead [05:27] (distroseries, status) made sense until 2007. [05:27] uhm, isn't it ordered differently ? [05:27] I'm just flagging the 'btw this may break pages' aspect [05:28] If it does, the pages are buggy. We could perhaps delete in a few days after checking index usage numbers, though. [05:28] Give you once chance to guess what I think of that :) [05:28] Mmm. [05:29] I'd prefer to find the buggy pages :) [05:46] stub: Thanks for the +1, I'll lp-land it against db-devel [05:47] StevenK: Seen lifeless' comments? [05:47] Like 6 lines ago. [05:47] Yes. [05:48] wgrant: I'd rather we find the buggy pages too [05:48] Yes, but this could entirely break them. [05:48] given you guys are not on maintenance, and there is moderate risk triggering criticals... [05:49] stub: do we have index utilisation info now ? [05:49] It's likely that some queries use (distroseries, status) now. [05:49] So the stats now won't be too useful. [05:49] Because distroseries, status is reasonably selective where status is 0 or 1, which is the common case. [05:49] wgrant: the point being to get the new index up and see what moves across [05:49] lifeless: PG tracks hits on the indexes. [05:49] stub: ok cool [05:52] StevenK: I can create that index when the backups finish [05:53] lifeless: Oh, sorry, I thought you were asking if we could get stats right now. [05:53] stub: Excellent, thanks. [05:53] wgrant: So should I land this or not? [05:54] I'd land the first patch once the index is on prod, and the second once we know it's not being used significantly. [05:55] Nah... just keep them in the same branch and keep the mp open [05:56] Or that, if you don't care about having prod patched with something out-of-tree for days. [05:56] The second patch will go through staging too so get to test there for major perf regressions [05:57] Our trackrecord of catching that stuff on staging is... less than optimal. === almaisan-away is now known as al-maisan === al-maisan is now known as almaisan-away [07:10] lifeless: are you reviewing? If so, I have a very brief MP for you: https://code.launchpad.net/~jtv/launchpad/bug-878115/+merge/79919 === rvba changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: rvba* (allenap) | Critical bugtasks: 266 [07:21] jtv: Approved. [07:21] Thanks. [07:21] jtv: security.cfg changes are applied during nodowntime updates. [07:22] I thought PQM rejected any changes in database/schema for devel. [07:22] Only *-0.sql and fti.py, I believe. [07:22] And both of those restrictions are pointless now. [07:22] So I will drop the check entirely when nobody is looking.; [07:22] Nope. I ran into it a few times while fixing lint in python. [07:22] * wgrant checks. [07:23] Yeah, *-0.sql and fti.py are banned. [07:23] Everything else is permitted. [07:24] [ -z "$(bzr status -S database/schema/ | grep -e '\(-0\.sql\|/fti.py\)$')" ] [07:24] Maybe the lint I fixed was in fti.py then, but I think I touched more than one python file there. Maybe the check's been updated already. [07:25] It was fti.py, yes. [07:25] The check hasn't been updated for a few months. [07:25] It used to be stricter. [07:28] wgrant: they aren't pointless; there is a report needed before we drop them - in deployment thingy [07:28] OpenID, how many ways do I love thee? Let me count the ways: [07:29] … [07:29] Session expired. Please log in again. [07:29] … [07:29] lifeless: Except that -0 patches don't exist any more. [07:29] 404 no such page: /weird-openid-stuff-this-app-doesn't-handle [07:34] jtv: OAuth is even more awesome. [07:39] stub: Can I define a partial index in a CREATE TABLE, or must I do it afterwards? [07:40] wgrant: It needs to be done after. [07:40] :( [07:40] Thanks. [07:40] wgrant: stub: it does? [07:41] oh, depends on the definition of after [07:41] ... I mean 'same patch should be fine' [07:41] yes [07:41] stub: when would you like to catch up ? [07:41] but the sql syntax doesn't support it in the CREATE TABLE [07:41] lifeless: whenever :-) [07:41] Yeah, I meant just the syntax issue. [07:42] That was clear from your question. [07:42] However [07:42] I have one for you: [07:42] do you know how I find a particular recipe using launchpadlib? [07:42] I was wondering when you'd ask about that. [07:42] do you mean search, or obtain the object for a known recipe? [07:42] Well, here we are. :) [07:42] lifeless: a particular recipe. [07:42] What lifeless said. [07:42] Ah, you know it? [07:43] This one: https://code.staging.launchpad.net/~ubuntuone-hackers/+recipe/client-dailies [07:43] Ah, different issue. [07:43] I see. [07:44] I was hoping you were investigating the request-daily-builds failure. [07:44] However, lp.people['ubuntuone-hackers'].getRecipe(name='client-dailies') should work. [07:45] Ah, thanks! [07:46] wallyworld_: We need to continue to permit nominations. [07:46] wallyworld_: Nominations do not transcend pillar boundaries. [07:46] (approving those nominations must be permitted, too) [07:47] But adding a new pillar to a bug, or transitioning the pillar of a task when there are series tasks, must be forbidden. [07:49] Series are not intended to be a privilege boundary, so such tasks shouldn't cause any issues with further disclosure implementation. [07:50] (well, they are a weak privilege boundary: driver privileges can be granted within a single series) [07:50] But they are irrelevant for visibility. [07:58] good morning [08:00] Hello [08:05] wgrant: I'm trying to verify that bug 876594 is fixed on staging. Are you privilege to run this, by any chance? http://paste.ubuntu.com/713960/ [08:05] <_mup_> Bug #876594: rejected builds for synced packages send mail to Debian maintainer < https://launchpad.net/bugs/876594 > [08:05] I think that's the wrong bug. [08:06] But yes, I should be able to do that. [08:06] How is the wrong bug? [08:06] Oh [08:06] It just is. [08:06] Too many in progress. [08:06] Hi bigjools [08:06] The right bug is but 870130 [08:06] *bug 870130 [08:06] <_mup_> Bug #870130: shortlist error requesting recipe build < https://launchpad.net/bugs/870130 > [08:06] o/ [08:07] Morning bigjools [08:07] wgrant: any luck? [08:07] jtv: >>> recipe.requestBuild(pocket='Release', distroseries=natty, archive=recipe.daily_build_archive) [08:07] [08:08] wgrant: that sounds successful. [08:08] Suggests that the bug is indeed fixed. [08:08] Yep. [08:08] Thanks! [08:10] * jtv goes for a quick relocate [08:13] * wgrant adds bug #3 to the list to delete tasks from next week. [08:13] <_mup_> Bug #3: Custom information for each translation team < https://launchpad.net/bugs/3 > === almaisan-away is now known as al-maisan [09:02] StevenK, wgrant: w.r.t. bug 876594 I feel somewhat tempted to make get_recipients even cleverer, but I'd like to resist. [09:02] <_mup_> Bug #876594: rejected builds for synced packages send mail to Debian maintainer < https://launchpad.net/bugs/876594 > [09:03] jtv: Ignore implementation, define requirements :) [09:03] Thank you. I need some words of encouragement. [09:04] notifications got where they are today because of gradual papering-over of bugs. [09:04] Then they got a little bit better. [09:04] Well, I like to think so. [09:04] Let's see if we can work out what we actually want, and develop a consistent implementation :) [09:11] Actually, in a way, developing an inconsistent implementation may be better. [09:11] AFAICS we could just choose not to use notify() from the syncing code at all, and do something that's tailored to syncing. [09:13] It doesn't solve the existing mess, but at least it takes a chunk out of it and lets us design something that makes sense for do_copy. [09:16] * wgrant preemptively vetoes more duplicated implementations. [09:16] Unless you also schedule the removal of the old implementation. [09:16] Doesn't make it a duplicate implementation. [09:17] We're currently notifying on copy just as if it were a regular upload. [09:17] * wgrant looks disapprovingly at direct copies, delayed copies, and PackageCopyJobs. [09:18] I think we need a free rein to decide what's appropriate for copying, _not_ force "it must be the exact same code as upload notifications" onto the design a priori. [09:19] It could have different recipient determination code. [09:20] But the stuff to determine the content should be identical. [09:20] For values of "should" that tend towards "must" [09:22] Pah. That's content and I'm dealing with recipients. === al-maisan is now known as almaisan-away === almaisan-away is now known as al-maisan [11:50] bigjools: I just reported bug 878795 for those annoying failures we're getting from the archive uploader. I don't _think_ it's Critical, but not sure. [11:50] <_mup_> Bug #878795: InvalidEmailAddress in archive uploader < https://launchpad.net/bugs/878795 > [11:50] jtv: dupe :) [11:50] Oh, with what? [11:51] one that I filed earlier this week [11:51] * bigjools looks [11:51] I wish +reportedbugs would default to newest first [11:51] bigjools: find it, thanks. [11:51] bug 876348 [11:51] <_mup_> Bug #876348: Soyuz upload processing bails out with an OOPS when encountering an invalid email address < https://launchpad.net/bugs/876348 > [11:52] Yeah. Would you agree that it would make sense to treat this as an UploadError? [11:52] it;s not an oops for sure [11:52] need to reply to the uploader [11:53] the only time we need not reply is if the signature is invalid [11:53] Does UploadError imply an oops? [11:55] Oh, you mean to say the original bug description (mentioning oopses) wasn't entirely right? [11:56] bigjools: it goes wrong in parseAddress, and its docstring says it will raise UploadError if parsing of the address fails for any reason. [11:56] Will that DTRT? [12:00] jtv: the exception raising is correct, however the bit that generates oopses is crack [12:01] bigjools: the exception raising that I'm proposing, or the exception raising that already happens? [12:01] there's a catch-all at the top level in uploadprocess.py somewhere, it's not very intelligent [12:01] already happens [12:06] bigjools: I guess it's probably good though to be a bit specific about which exceptions are survivable. I just attached a branch that makes it do what the docstring says; just one possible idea. [12:08] jtv: specific is always good [12:08] * bigjools → lunch === gary-lunch is now known as gary_poster === al-maisan is now known as almaisan-away [12:58] Morning, everyone. [12:59] hi deryck [13:00] bigjools: if you agree with the idea I attached to bug 876348, please let me know and I can put it up for review. Otherwise, I'll treat it as just a suggestion. [13:00] <_mup_> Bug #876348: Soyuz upload processing bails out with an OOPS when encountering an invalid email address < https://launchpad.net/bugs/876348 > [13:33] jtv: replied [13:33] Thanks [13:33] jtv: alteratively to what I said, use reject("msg") [13:34] hi danhg! [13:34] welcome aboard! === flacoste changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | Welcome danhg | On call reviewer: rvba* (allenap) | Critical bugtasks: 266 [13:35] bigjools: I guess I made a little thinko: “this email address is invalid → there is nobody we can notify.” [13:36] jtv: wrong :) [13:36] Evidently. [13:36] jtv: actually EarlyReturn is not ideal thinking about it [13:36] we just need to add a rejection and let processing continue so it collects all the errors [13:37] I really detest the upload processor === jcsackett changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | Welcome danhg | On call reviewer: rvba* (allenap), jcsackett | Critical bugtasks: 266 [13:47] bigjools: I guess it's two problems: one, “malformed email address and policy that says to create the user” does something very different from “unknown email address and policy that says not to create the user” or “malformed email address and policy that says not to create the user.” Two, it'd be nice to have more robust handling of these error conditions. [13:48] Does that make sense? [13:49] Hey jcsackett! [13:50] morning, rvba. :-) [13:51] jtv: no [13:51] but then I am trying to do 2 things at once here [13:52] OK — it's late here anyway, so we can leave this to ferment in the hindbrain for now. [13:52] rvba: looking at your review right now, is there a test or anything that demonstrates using repr resolves the problem? === matsubara is now known as matsubara-lunch [15:11] rvba, would you be so kind as to review https://code.launchpad.net/~gmb/launchpad/weirdness-bug-867529/+merge/79972? [15:13] gmb: sure. [15:13] rvba: Thanks. === Guest35863 is now known as elmo [15:24] gmb, gary_poster: where is our lp -> leankit integration code lives? [15:25] flacoste on call. danilo ^^ [15:25] flacoste: http://launchpad.net/lp2kanban IIRC. Unless you mean, where does it run, in which case danilos would know. [15:26] gmb: weird bug, I cannot reproduce it either ... the display does not seem to use floating elements except for "See full activity log [15:27] " [15:27] rvba: I know. It's weird. I can only reproduce it in Firefox and then only occasionally. [15:27] The screenshot is indeed of a firefox window. [15:28] I /think/ (by the looks of the tabs we barely see) [15:28] * rvba hits F5 [15:29] again and again [15:30] gmb: I've seen it! [15:30] Heh [15:31] But now it's fine again .... it was simply the browser 'lagging'. [15:31] gmb: maybe your fix will force the browser to update the display more quickly. [15:32] rvba: That's what I'm hoping. It seems to be some kind of lag in Firefox's rendering - it eventually sorts itself out. [15:32] gmb: did you see it be 'broken' and stay that way? [15:32] rvba: I saw it be broken for a short while, but scrolling up and down fixes it. [15:32] well, in my experience. [15:32] gmb: ok, same thing here. [15:33] This is like trying to nail jelly to a wall. [15:34] hehe [15:34] jcsackett: hey, my official mentor is off today so maybe you will be ok to double check this tiny mp https://code.launchpad.net/~gmb/launchpad/weirdness-bug-867529/+merge/79972 ? [15:35] sure, rvba. [15:35] rvba: Merci beaucoup. [15:36] jcsackett: btw I really wanted to make it up to you because you reviewed crazy branches last week and I was rather busy with a Critical bug ... so I'm the front line this week until I EOD ;) [15:37] rvba: thanks. :-) [15:37] gmb: welcome :) [15:40] gmb, which reminded me, now you've got email about lp2kanban set-up on yellow@, so whoever wants to pick it up, go for it :) (it is running on my own server) [15:40] thanks gmb, i was interested in the code location [15:41] danilos: Okay. I'm goingt to wait for someone other than me to pick it up, since running it on my server has been historically somewhat flaky. [15:41] flacoste, if you need help in getting it going, you can still talk to me since I think I was the last person to shuffle the code around heavily :) [15:42] naba from product-strategy might be interested by it [15:42] danilos: ^^^ [15:42] so i'll point him your way if he has any hard questions ;-) [15:42] flacoste, sure, let them email me, but I hope config.ini and README should be good enough for starters as well [15:42] * cjwatson grins at the blueprint person chooser saying "Pick me" [15:44] gmb, rvba: this is the most irritating bug to try and see ever. [15:44] Yup. [15:44] * gary_poster goes for lunch [15:45] gmb: i second rvba's approval, and have nothing to add. [15:45] jcsackett: Thanks. [15:45] jcsackett: Thanks for mentoring my review ;) [15:45] you're quite welcome. :-) [15:48] flacoste: would you be able to attend https://blueprints.launchpad.net/ubuntu/+spec/foundations-p-image-build-pipeline, or suggest somebody from LP who would be useful there? It has moderate overspill into LP, although I think I've established that Ubuntu Engineering will have resources to actually do the work, so it's sort of more flagging if we're insane [15:49] (One smallish piece of self-contained work; one rather larger job.) [15:49] cjwatson: sure, i'll attend [15:49] would probably make sense for bigjools to attend virtually as well [15:50] flacoste: thanks, subscribed you [15:50] ... and bigjools [15:50] * bigjools prepares for blueprint spam === deryck is now known as deryck[lunch] === matsubara-lunch is now known as matsubara === rvba changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | Welcome danhg | On call reviewer: jcsackett | Critical bugtasks: 266 === beuno is now known as beuno-lunch === deryck[lunch] is now known as deryck [17:07] jcsackett, I appear to have been off IRC all day. DId you need me for anything? === beuno-lunch is now known as beuno [17:17] sinzui: nope, been fine, doing the review thing and chatting about mockups. [17:18] fab [17:24] Guten nacht alles [17:24] deryck, allenap: I get "SSL connection has been closed unexpectedly" on Lucid, too. [17:28] jcsackett, do you believe this SQL is safe to run in production: https://pastebin.canonical.com/54678/ [17:29] sinzui: looks fine by me. [17:29] thank you. I may just fix the production issue today if the report has less than 50 teams [17:38] abentley, dang. Hopefully allenap's fix will work for you. === jcsackett_ is now known as jcsackett|mobile === jcsackett|mobile is now known as jcsackett === jcsackett_ is now known as jcsackett === jcsackett_ is now known as jcsackett === jcsackett|afk is now known as jcsackett [19:26] deryck: cache-batches now respects the search parameters and has merged the latest stable. [19:27] abentley, nice. Thanks for the heads up/ [19:27] deryck: np [19:29] deryck: it appears we're already running a fork of storm, so maybe allenap's changes should go in. [19:29] abentley, yeah, maybe so. no since waiting when we can roll our own. :) [19:29] s/since/sense/ [19:32] deryck: wallyworld mentioned the idea of supporting infinite scrolling instead of batching. I.e. when you're scrolling, it's loading batches via ajax. [19:33] abentley, I think infinite scrolling makes sense when you have an infinite (or near infinite) list. i.e. streams like twitter or news feeds….. [19:33] deryck: yes. [19:34] abentley, I'm not sure it makes as much sense when the thing being viewed is a list or set of lists like bugs. mentally it just fits better in a numbered, ordered set of lists. [19:35] But I could certainly be wrong about this. just my gut feeling about it. [19:36] deryck: I dunno. I think the main value of batching bugs is that you don't get overwhelmed by their numbers. I find it a pain to click through to the next batch. [19:38] yeah, maybe it doesn't matter. [19:39] deryck: Anyhow, we don't have to use it for bugs, but any time we're doing batching, that's an alternative. [19:40] abentley, certainly. [19:41] * deryck reboots, brb === almaisan-away is now known as al-maisan [19:44] abentley: I think one bit of value in batch numbers is being able to say 'on page 5'... - but you could still do that with infinite scrolling [19:44] abentley: with a little care [19:44] abentley: (and its not a huge value) [19:44] lifeless: Yeah, little pale-grey "page-break" markers or something. They could have permalinks. [19:49] deryck: Is there a guide to documenting our javascript methods/functions? [19:51] abentley, at one time our style guide said the javadoc style that YUI uses. No one really follows this, though. [19:51] deryck: Okay. [19:51] abentley, I personally don't like that style, so it's hard for me to endorse. :) But I guess we should be consistent. [19:52] abentley, we probably should raise it in some forum and get consensus on it. [19:52] deryck: I'd like that. I'm not very familiar with javadoc, so if we aren't really using it, I'll just document it as I see fit for now. [19:53] abentley, that works. === al-maisan is now known as almaisan-away === jcsackett|afk is now known as jcsackett_ [20:37] jcsackett_: could you please review https://code.launchpad.net/~abentley/launchpad/update-listings/+merge/79998? [20:38] certainly. === jcsackett_ is now known as jcsackett [20:46] jcsackett: thanks. Also https://code.launchpad.net/~abentley/launchpad/cache-batches/+merge/80000 ? [20:46] i'll grab it next, abentley. :-) [20:46] jcsackett: It's a follow-on. [20:46] dig. [20:47] jcsackett: hey, look. 80000th merge proposal. [20:47] oh, nice! [20:55] thumper, rockstar: we just hit the 80000th merge proposal: https://code.launchpad.net/~abentley/launchpad/cache-batches/+merge/80000 [21:02] w00t [21:02] abentley: do you have any bzr pipeline talks? [21:03] anyone know of launchpad talks? [21:03] thumper: No, I haven't given any talks on it. [21:03] hmm... [21:03] ok [21:13] abentley: two questions on your first MP. 1) i am correct in assuming that when you call stringify on query in get_view_url, it handle the case of query being undefined (i.e. it converts undefined to an empty string?) [21:13] 2) is the use of config.io_provider in update_listing just for testing purposes? [21:14] jcsackett: I don't think the query string can be undefined, just an empty string. [21:14] abentley: ah, fair. that does make sense. :-) [21:15] jcsackett: it hasn't been an issue in local testing. [21:15] jcsackett: config.io_provider is just for testing purposes. [21:15] abentley: cool, thanks. r=me on the first MP then. [21:15] jcsackett: thanks. [21:25] abentley: and r=me on the second branch as well. [21:25] jcsackett: thanks! [21:27] you're welcome. :-) [22:09] abentley, congrats on 80000! [22:17] rockstar!! [22:17] StevenK!! [22:18] * mwhudson knows what this is about, i think [22:20] lifeless: Your r12481 tests are spuriously broken in buildbot. [22:21] lifeless: I saw it break once in ec2 a couple of days back. [22:21] I can't work out why. [22:21] I can't even find the preloading that seems to be broken... [22:30] wgrant: win! [22:30] wgrant: :( [23:17] lifeless: Can you investigate that failure as a matter of urgency? It's broken both devel and db-devel builders now. [23:17] And I can't see where the preloading happens. [23:19] its been in since feb [23:19] so this is broken by some other change [23:20] wgrant: also I'm OTP [23:20] Yes, it's clearly broken by some other change. === wallyworld_ changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | Welcome danhg | On call reviewer: wallyworld | Critical bugtasks: 266