[00:02] Ah ha. lifeless's comments are a little misleading. [00:03] BugNominationView._getListingItem isn't it at all, it's BugListingBatchNavigator._getListingItem [00:05] StevenK, hey [00:05] in pursuit of splitting out the buildd tree [00:05] i'm thinking of moving the tests that rely on zopelessdblayer etc out of the buildd tree into lp [00:05] so they will rely on a buildd but not be part of it [00:06] poolie: And making lp-buildd be pulled in via sourcecode or the download-cache? [00:06] yep [00:06] https://bugs.launchpad.net/launchpad/+bug/800295 fwiw [00:06] <_mup_> Bug #800295: buildd is unnecessarily coupled to the launchpad tree through tac file and readyservice < https://launchpad.net/bugs/800295 > [00:06] welcome back, bot [00:10] Actually, it's not. It's NominatedBugListingBatchNavigator. [00:18] StevenK: Do you know if overlay distros work yet? [00:18] I think they probably function. [00:19] Enough for someone to kill archivepurpose 4 with a few hours of work. [00:19] Ha. Maybe they do. [00:20] wgrant: Should I put up a NDT for r14231? [00:20] Since I've got Julian convinced that we should kill archivepurpose 7... [00:20] StevenK: Was hoping you'd ask :) [00:20] And 4 is pretty clearly actually an overlay distro. [00:20] What's 7 again? [00:20] 7 is DEBUG [00:20] I successfully convinced him that it's a terrible idea. [00:20] And 4 is PARTNER [00:20] Yes. [00:20] They both need to die. [00:20] Yes. [00:21] And I think 4 can die pretty cheaply by being turned into https://launchpad.net/canonical-partner [00:21] And that lets us remove a tonnnnnne of awfulness. [00:21] all_distro_archives can fuck off. [00:21] Needs IS involvment to get it onto archive.c.c. [00:21] But certainly doable [00:21] We can remove all the Distribution and DistroSeries publication-related methods. [00:22] IT WILL BE BEAUTIFUL [00:22] Yes, it requires some IS involvement. [00:22] But I think it's very doable. [00:22] Although I bet it's signed with the Ubuntu archive key. [00:22] Just to make things difficult. [00:22] Oh, rofl [00:22] * StevenK starts putting together a NDT [00:24] StevenK: Have you heard about my new solution to kill off 7? [00:24] I'm not sure if I've run it past you. [00:26] Need to run it past Colin and Adam and the like too, but I think it's sensible. [00:27] wgrant: Only in vague terms, I think. [00:28] StevenK: We publish them in separate $COMPONENT/debug indices, like $COMPONENT/debian-installer, and handle the split like we do with security and ports. [00:28] Eventually when Soyuz gets archive views, we can move the split to there... but until then we might as well utilise the existing mechanism. [00:29] Doesn't that make archive.u.c explode in size? [00:30] Remember that there's already the secret magic splitter that throws ports binaries and indices elsewhere. [00:30] And keeps security on security.u.c. [00:30] We repurpose that to create a fourth archive -- ddebs.ubuntu.com [00:30] Because this is really the same thing as the ports split. [00:31] Oh, right. [00:31] We need to keep some binaries off the mirrors. [00:31] It's divided by component instead of arch, but it's really the same thing. [00:32] With the appropriate magic-mirror hacking, that sounds like a good plan. [00:32] wgrant: And ddebs.u.c already exists on macquarie [00:32] Yes. The retracers will probably need to look at both for a while. [00:33] For values of "a while" that probably number in the several years. [00:33] But details, details. [00:35] apt and apt-ftparchive seem happy with custom slashed components. [00:35] Yay [00:35] It is practical. [00:35] And the Soyuz side is dead-easy. [00:37] So, we now have feasible solutions to both partner and debug that probably take less than two days each. [00:37] One is already escalated... [00:43] wgrant: Isn't there a better method to call to get upload perms for a user than distribution.main_archive.verifyUpload(person, name, component, distroseries) ? [00:43] Don't think so. [00:44] jml cleaned it up a couple of years back. [00:44] It would be nice if it was set-based, but it's not. [00:44] Since I think the lookup of the component is grabbing SPPH, and then verifyUpload does the same thing. [00:44] It's grabbing it to check the component? [00:44] It could be cleaned up further, but it's a deep hole. [00:45] component = self.distroseries.getSourcePackage( [00:45] name).latest_published_component [00:45] if distribution.main_archive.verifyUpload( [00:45] person, name, component, self.distroseries) is None: [00:45] return True [00:45] One I plan to follow eventually, but hopefully after abolishing archivepurpose (4, 7). [00:45] StevenK: You could possibly check how branches do it.; [00:46] launchpad.Edit for IBranch needs to do the same thing as IBugNomination.canApprove. [00:49] poolie: hi. i claimed your 678090-affected-count mp and then read the comments. are you still working on it? [00:49] wgrant: Digging around IBranch isn't turning up anything [00:49] wallyworld_, not right at this moment but i will [00:50] def checkAuthenticated(self, user): [00:50] can_edit = ( [00:50] user.inTeam(self.obj.owner) or [00:50] user_has_special_branch_access(user.person) or [00:50] can_upload_linked_package(user, self.obj)) [00:50] for ssp in ssp_list: [00:50] archive = ssp.sourcepackage.get_default_archive() [00:50] if archive.canUploadSuiteSourcePackage(person_role.person, ssp): [00:50] return True [00:50] poolie: ok. just wanted to see if i needed to review now or not. thanks [00:50] do you have any comments beyond what robert and i said? [00:50] Which uses latest_published_component. [00:50] poolie: not sure. didn't get that far. i'll have a look [00:52] wgrant: Ah, where is that? [00:52] StevenK: c.l.security [00:55] poolie: so you can delete the view property 'other_users_affected_count' i think [00:55] yep probably [00:56] is my change to @cachedproperty ok? [00:56] poolie: other than that, looks like a nice change. i was going to also suggest the feature flag [00:56] poolie: i like it how you've been doing these polish 'paper cut' bugs of late [00:57] thanks [00:57] poolie: i think the cachedproperty change is ok [00:57] i've run into issues before with cached permissions but i think this case is ok [00:58] it's just cached for the duration of the page view, right? [00:58] yep. the rendering [00:59] poolie: so, a cached property is good if a property is accessed more than once during a render. is this property used more than once? [00:59] yes, the template puts it in twice [00:59] for something to do with js fallbacks [00:59] ok [00:59] which seems ugly but i didn't want to change it without fully understanding it [00:59] and just caching it seemed fine [01:00] sure [01:01] lifeless, do you have any opinion where TacTestSetup ought to live, so that it can be used by both lp and buildd tests? [01:01] it is not enormous but not trivial [01:01] In Twisted :) [01:01] it would seem like a waste to make a new package just for that [01:01] :) [01:01] perhaps it could go in testfixtures [01:02] poolie: you could create a txfixtures [01:02] avoiding packages just makes problems [01:02] as a new package, depended upon by both? [01:02] yes [01:03] txfixtures with twisted specific Fixtures [01:03] it can depend on fixtures [01:03] lp and buildd can both use txfixtures [01:03] k [01:03] aside from that i think it's all ready to be cut out [01:04] nb it doesn't actually depend on twisted [01:04] so, in a sense it could go in plain fixtures [01:04] well, you probably cannot run its owntests without twisted [01:05] wallyworld_, while you're here can you read https://code.launchpad.net/~mbp/launchpad/800295-buildd-split/+merge/81111 [01:05] * wallyworld_ reads [01:17] poolie: looks ok to me, but i think the port to listen on needs to be paramaterised [01:27] lifeless: Do you know exactly what OEM needs? Just bugs with foo and hwe-foo tasks? [01:27] lifeless: I wonder if we can just have a temporary owning context. [01:28] lifeless: There's no real need to respect multiple policies. [01:28] I don't know precisely know; nor do I understand what you mean :) [01:29] lifeless: What if we say that OEM's multi-pillar bugs favour one of the targets, and use the access policy from that target? [01:29] Rather than respecting the union of policies from each target. [01:30] I have no objection to that [01:30] seems harder to do well [01:30] but thats your problem :)O [01:32] lifeless: Also, I forget some details of yesterday's conversation. IIRC you suggested just having the enum value on Bug/BugTask, without the pillar reference. But then we have to indirect through Product/Distribution/ProductSeries/DistroSeries to work out the policy. [01:34] some of these osutils almost need their own package [01:34] for full observers [01:34] not for restricted [01:34] Sure. [01:34] which is a small set [01:34] Sure, but it makes queries far worse. [01:34] you've tested ? [01:35] I mean in terms of complexity. [01:35] Speed, I haven't tested yet. [01:35] the are a little more awkward to write, thats true. [01:35] might be a driver for having two separate tables [01:35] How? [01:35] * lifeless speculates wildly [01:35] The problem is not that there's one table. [01:36] wgrant: its a lopsided table [01:36] THe problem is that we have to indirect through four other tables to get to the one table. [01:36] wgrant: task -> policyuse -> grants [01:36] gnuoy: one in between table [01:36] bah [01:36] wgrant: ^ [01:37] lifeless: How do you get from task to policyuse unless you refer to policyuse on task? [01:37] You'd have to go through ProductSeries or DistroSeries. [01:37] task.product == policyuse.product [01:37] mmm, sure. ok - 2 tables [01:37] task -> [*series] -> policyuse [01:37] And we already have to denorm this slightly, on grant. [01:37] I'm not sure that one is avoidable. [01:38] grant knowing the asset ? [01:38] Right, an artifact-specific grant has to know the artifact, its target, and its policy. [01:38] target == user ? [01:39] 'person' ? [01:39] pillar [01:39] why does it need pillar? it can use policyuse.id [01:39] (person, artifact, policyuse) [01:39] Right, but policyuse requires target and policy. [01:39] yes [01:40] Which means we're denorming (target, policy) from bugtask onto grant. [01:40] It's indirect, but it's still denormed. [01:40] mmm [01:40] so we'd do that for reporting [01:40] ack, its a denorm [01:41] thats part of your schema anyhow though, right ? [01:41] Yes. [01:41] so no-change [01:41] The change is that you suggest I no longer denorm that on bugtask itself. [01:42] Which seems odd, when we're already denorming it elsewhere. [01:42] Further away. [01:42] well, denorms are done for a reason [01:42] whats the reason for it to be denormalised on bugtask ? [01:42] or rather [01:42] let me ask it differently [01:42] if bug is the native location for this [01:43] whats the best denorm to use on bugtask [01:43] so bug.policy = ENUM [01:43] Right, better question. [01:43] bugtask.whatshouldwehavehere. [01:44] brb, nature [01:44] I suspect policyuse. So setting Bug.access_policy calculates policyuse and throws it at BugTask and artifact. [01:45] Although then we can't optimise public bugs. [01:45] Because we'd have to look up the public policy. But that may not be a problem. [01:59] ok, lp:txfixtures exists! [02:04] lifeless: We seem to have an OOPS issue. [02:04] * 308 Exceptions [02:04] All the None/None OOPSes are missing. [02:04] * wgrant checks devpad. [02:05] django.db.utils.DatabaseError: invalid byte sequence for encoding "UTF8": 0xf8 [02:05] HINT: This error can also happen if the byte sequence does not match the encoding expected by the server, which is controlled by "client_encoding". [02:05] yay [02:05] raised with datedir: /srv/launchpad.net-logs/production/chaenomeles/2011-11-03 filename: OOPS-4e0b81ac6e642ee8099df77672c8be1a [02:05] fwiw [02:07] ['HTTP_USER_AGENT', '\xf8\x07p\x01'] [02:07] That's a new one... [02:09] M-x ^g p ^a [02:09] help! [02:09] wallyworld_, so what did you think of https://code.launchpad.net/~mbp/launchpad/800295-buildd-split/+merge/81111 [02:10] lifeless, i'm just going to copy some of the osutils functions in to txfixtures to stop this recursing too much [02:10] [11:17:10] poolie: looks ok to me, but i think the port to listen on needs to be paramaterised [02:11] some kind of global osutils [02:11] wallyworld_, where? [02:12] wgrant: win [02:12] wgrant: rye has a branch up that should help with this [02:12] lifeless: .decode('utf-8', 'replace')? [02:12] wgrant: not proposed for merging yet [02:12] poolie: _hasDaemonStarted().... return self._isPortListening('localhost', 8221) [02:12] well, the db schema is royally messed up. headers are bytes. [02:12] Not sure whether to hack code or OOPS now.. [02:13] wgrant, that particular one looks a lot like unix keyboard input not broken utf8 [02:13] Yes. [02:13] wgrant: https://bugs.launchpad.net/python-oops-tools/+bug/884265 [02:13] <_mup_> Bug #884265: req_vars header values may cause crash in dboopsloader < https://launchpad.net/bugs/884265 > [02:13] of course in general escaping might be good [02:13] poolie: but i'm not very familiar with that stuff so i could be wrong [02:13] wallyworld_, it's already hardcoded in the tac file... [02:13] the coupling is not ideal but since we're apparently happy just counting on that port for these tests i can live with it [02:14] I think someone is going around attempting an exploit [02:14] poolie: so, my ignorance - i thought hasDaemonStarted() is a generic method used for different services [02:14] hence each would be on a different port [02:14] wgrant: https://code.launchpad.net/~rye/python-oops-tools/quote-req-vars [02:14] i guess i could make a tac file in a tmpfile [02:14] lifeless: Intriguing. [02:14] wallyworld_, the version that hardcodes the value is specific to tests that always use buildd-slave-test.conf which is always 8221 [02:15] But I guess that works. [02:15] For now. [02:15] poolie: ok. thanks for clarifying [02:15] * wgrant cowboys. [02:16] np [02:16] thanks for the speedy review [02:16] all good then? [02:17] wgrant: where did you see the error ? [02:18] lifeless: Ran make update_db manually. [02:18] poolie: lazr.utils might be a place you can put osutils things [02:18] if isinstance(value, str): [02:18] Needed to add that. [02:18] Since we have floats. [02:18] Perhaps U1 does not. [02:18] ok lunch, biab [02:18] poolie: though I have not checked the dependencies it would drag in [02:18] wgrant: well, they don't have native types at all yet (rfc822 still) [02:19] i don't have a good bias towards lazr.* but perhaps it's only restfulclient [02:19] wgrant: I'm working on the IBodyProducer thing they need to migrate [02:19] lifeless: Ah, of course. [02:20] poolie: looks ok [02:22] lifeless: Thanks. [02:22] lifeless: Any thoughts on bugtask? [02:24] permit bug.accesspolicy to be NULL [02:25] I agree; trigger updates both denormed locations (grant + tasks) [02:26] Great. [02:26] wgrant: raises the question, perhaps the artifact table should denorm the policyuse, rather than the grants. [02:26] (Or does it already? [02:26] ) [02:26] It doesn't already. It could. But currently the only method to manipulate artifacts is ensure(), which is nice. [02:27] so bug.accesspolicy being null implies task.policyuse is nullable, so public case is optimisable [02:27] Yep. [02:27] I'm not sure what to do with artifact-specific permissions when the artifact becomes public. [02:27] now, when the policy changes, there are less artifact rows to change than grant rows [02:27] Indeed. [02:27] And querying through artifact for reporting should be quickish... hopefully. [02:28] which, if you're hitting artifact anyway on reports [02:28] -> I would put the policyuse on artifact for restricted observers [02:28] -> and on grant for full observers. [02:28] Right. [02:28] yes, that breaks your combined schema. I'm feeling more and more sure it should be separate. [02:29] Possibly, yes. [02:29] It shouldn't make a significant performance difference. [02:29] To have to go through artifact. [02:29] Well, I think you are anyway, aren't you ? [02:29] or were you thinking to just report 'there are N total grants' [02:30] Pretty much that. [02:30] being able to report 'there are N private objects' and 'Y security ones' might be useful itself. [02:30] In order to be fast. [02:30] Yes. [02:30] True. [02:30] so, O(artifacts) < O(restricted grants) if there are 1 restricted grant per artifact. But there may not be. [02:30] so you could denorm to both. [02:30] I had considered putting policy on artifact, but it makes reporting slightly awkward. [02:30] But should be fine. [02:30] Hard to predict. [02:31] It really depends on the project. [02:31] Security will usually have one restricted observer. [02:31] the reporter? [02:31] Artifacts in private projects probably won't have any. [02:31] Right. [02:32] ok, so plan: bug.accesspolicy nullable; task.policyuse nullable; possibly add an artifact.policyuse; possibly drop grant.policyuse [02:32] last two subject to scaling testing by you [02:32] It's going to be awkward to get something representative, but I'll do what I can. [02:33] I feel your pain [02:34] you could do what I did with bugsummary [02:34] What's that? [02:34] conversion script into temp tables [02:34] ran it repeatedly on staging until happy [02:34] FSVO "you" [02:34] we should have a reasonable corpus already, particularly if you use private branches [02:34] wgrant: s/staging/dogfood/ [02:34] Not very representative of performance, but maybe. [02:34] if its fast there... [02:34] Sometimes hundreds of times faster, sometimes infinitely slower :) [02:35] Heh [02:35] True [02:35] its not the resources that make us make things fast, its the constraints :) [02:36] Mmm, yeah, but there's so many bad queries and schema in LP that pretty much anything happening on mawson blows away the cache and kills everything. [02:36] I will turn everything off :) [02:38] lifeless: Ah, denorming like this also more easily allows us to have a master target for The OEM Conundrum. [02:39] Since the policyuse is not calculated from each task's target separately. [02:39] it seems to make that unneeded though [02:39] I suspect doing a master target may be a big fail [02:39] I wouldn't try it myself [02:40] (because what if hwe aren't allowed to see all of oem's bugs) [02:45] lifeless: Then they can have restricted observers on all the relevant artifacts, just like they do now. [02:46] true [02:46] Unless they're in the OEM bug supervisor team (which is subscribed automatically, and will be the observer in the new model), they are being subscribed manually. [02:46] How would you do it without a master task? Which target would it use to look up the policy? [02:47] would what [02:47] it == the trigger to push Bug.access_policy through to BugTask.policy_use [02:48] And to AccessPolicyArtifact.policy_use, or AccessPolicyGrant.policy_use. [02:49] wgrant: I was assuming it would run a small function (task, policy enum) on each task. [02:50] lifeless: One of those has to win, in order to be put into AccessPolicyArtifact or AccessPolicyGrant. [02:51] ah [02:51] ok, I see. Yes, go ahead. [02:51] I figure most of the cases are bugs with $foo and hwe-$foo tasks, in which case $foo should win. Not sure how we'll define that, but that can be clarified next week. [02:59] fuuuu [02:59] File "/srv/lp-oops.canonical.com/cgi-bin/lpoops/src/oopstools/oops/models.py", line 370, in _get_oops_tuple [03:00] for (start, end, db_id, statement) in statements: [03:00] ValueError: need more than 3 values to unpack [03:00] * wgrant enfixorates further. [03:00] EWTFISGOINGON [03:00] don't tell me bson is translating (foo, None) as (foo,) [03:00] that would be fuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuu [03:00] It would indeed. [03:01] I remember we ran into problems like this when I instrumented openid. [03:01] I assume that's what this is. [03:01] But I guess we'll find out in a sec. [03:01] Except we've had 2000ish openid timeouts since we moved, and they haven't been problematic. [03:02] [212, 212, 'sendmail'], [215, 215, 'sendmail'], [218, 218, 'sendmail'] [03:02] etc. [03:02] sigh. [03:03] * wgrant greps. [03:03] are they old oopses or something ? [03:03] No, this is OOPS-11a3f88d3d139e90764487de402825c4 [03:04] can we make sure we have bugs on the producer side as well as whatever you do to fix the consumption ? [03:04] The fourth item should be the detail, shouldn't it? [03:04] yes [03:04] the statement [03:04] (start, end, db_id, statement) [03:04] For this issue I'll file a producer bug, sure. [03:04] For the user-agent one it probably didn't make sense. [03:05] the missing value one needs a bug too [03:05] user agent is crap data, I'm fine with sanitising that in oops-tools [03:05] The missing value one is quite possibly the same thing. [03:06] yes [03:06] Hmm, putting such an array through bson works fine. [03:06] with None and ''. [03:06] So not quite that trivial and awful, sadly. [03:09] headddeeeeesk [03:09] ? [03:12] just at the symptoms [03:12] Oh. [03:13] fwe9ofju*(OW#YU8947238942389234 [03:13] \ [03:13] >>> bson.loads(bson.dumps({'blah': (1, 2, object())})) [03:13] {u'blah': [1, 2]} [03:13] It's unrepresentable, so just omit it :D [03:14] lifeless: reasons for hating BSON++ [03:14] wgrant: so we have unrepresentable objects in the dict ? [03:15] wgrant: I doubt json does better. [03:15] wgrant: or things we expect to stringify() ? [03:15] wgrant: anyhow, if we're doing that (or analagous) we're breaking the oops contract *anyhow* [03:16] >>> json.dumps({'blah': (1, 2, object())}) [03:16] TypeError: