[00:00] <StevenK> wgrant: That branch is in ec2 now.
[00:00] <wallyworld_> seems defines can't use used inside tal:context
[00:00] <wgrant> wallyworld_: You're moving the confirmation message into the template?
[00:00] <wallyworld_> yup
[00:01] <wgrant> <p>Blah blah blah <tal:someproject replace="context/someproject">Some Project</tal:someproject></p>
[00:01] <wgrant> Don't use string:
[00:01] <wallyworld_> ok. other places do :-)
[00:01] <wgrant> They're mostly wrong, and frequently holey.
[00:01] <wgrant> I've seen it mostly used to construct JS.
[00:02] <wgrant> Do you know of others that don't>?
[00:02] <wallyworld_> there's a few places in tales but i've closed my search window
[00:03] <wgrant> string: is OK for some things sometimes. But it's very rarely OK for content/replace.
[00:04] <wallyworld_> wgrant: one place in lp form - tal:content="string:${widget/label}
[00:06] <wgrant> wallyworld_: That's slightly more acceptable, since it just wants to append a colon.
[00:06] <wallyworld_> also lots of places in <title> elements
[00:06] <wallyworld_> more than just a colon
[00:07] <wallyworld_> wgrant: so is  <p tal:content="string:${context/bar}"/> a security issue?
[00:07] <wgrant> It's not a security issue.
[00:08] <wallyworld_> it's far less verbose than your version
[00:08] <wgrant> It's just pointless.
[00:08] <wgrant> And obscure.
[00:08] <wallyworld_> it was a contrived example
[00:08] <wgrant> string: is useful when you're applying minimal formatting around a some variables.
[00:08] <wallyworld_>  <p tal:content="Blah blah blah string:${context/bar} blah blah blah"/>
[00:08] <wgrant> But in your case you're constructing paragraphs with a couple of substitutions.
[00:09] <wallyworld_> compare what i just typed with
[00:09] <wallyworld_> <p>Blah blah blah <tal:someproject replace="context/someproject">Some Project</tal:someproject></p>
[00:09] <wgrant> TAL can be ugly, but putting everything in tal:content is not the solution.
[00:09] <wallyworld_> i think mine is very readable
[00:09] <wallyworld_> more so than yours
[00:10] <wgrant> string: encourages people to do stupid things.
[00:10] <wgrant> Like use structure.
[00:10] <wallyworld_> but i'm not doing that
[00:10] <wgrant> No.
[00:10] <wallyworld_> that's a false argument
[00:10] <wgrant> It's not false.
[00:10] <wgrant> It's a perfectly valid argument.l
[00:11] <wgrant> If we permit patterns that are not hard to use insecurely, people will use them insecurely.
[00:11] <wallyworld_> my construct is readable and concise
[00:11] <nigelb> Morning!
[00:11] <wallyworld_> and not insecure
[00:12] <wgrant> It's not insecure, but this sort of thing often leads to people putting structure in front of huge things.
[00:12] <wgrant> Like you did in your first version of the branch.
[00:12] <wallyworld_> code reviews are meant to pick up such things
[00:12] <wgrant> They normally don't.
[00:12] <wgrant> I normally pick them up by reading diffs after they land.
[00:12] <wallyworld_> i thought structure(xxxx) is kosher?
[00:12] <wgrant> No.
[00:12] <wallyworld_> is it deprecated then?
[00:13] <wallyworld_> and why do we have it?
[00:13] <wgrant> Its use is discouraged, in deference to templates.
[00:13] <wallyworld_> it's use for request notifications, no?
[00:13] <wgrant> Because people have shown that they cannot use structured() safely.
[00:15] <wgrant> Until we have a sane templating system, which is blocked on HTML being fucking terrible and IE not supporting XHTML, we have to discourage bad practices, because we cannot make things secure by default.
[00:15] <wallyworld_> just about any programming construct can be misused, that's not a valid reason to reject valid uses
[00:15] <wallyworld_> when the alternative is verbose and ugly
[00:16] <wgrant> The alternatively is empirically less likely to be abused.
[00:16] <wgrant> -ly
[00:17] <wgrant> Because it keeps content of different security levels labelled appropriately.
[00:17] <lifeless> 'The alternatively is empirically less like to be abused.' ?
[00:17] <wallyworld_> i think you're creating an issue where one doesn't exist
[00:17] <wgrant> lifeless: s/alternatively/alternative/
[00:17] <lifeless> :>
[00:18] <wgrant> wallyworld_: So, someone wants to extend this confirmation message.
[00:18] <wgrant> They want to emphasise "permanently", because people are deleting tasks too readily.
[00:19] <wgrant> They add markup to the template, but find it's being escaped. That region of the data is clearly literal, so they add the "structure" keyword.
[00:20] <wgrant> They miss that three lines later, in the same string literal, there is a variable substitution.
[00:20] <wgrant> Mixing data of different types in one literal is not a good idea.
[00:20] <wgrant> This *does* happen.
[00:20] <wallyworld_> i'll take your word for it
[00:20] <lifeless> wallyworld_: we had a -raft- of security holes related to prior use of structured
[00:21] <wallyworld_> that points to an epic fail in our code reviews then
[00:21] <wgrant> Yes.
[00:21] <wgrant> Lots of things do.
[00:22] <wgrant> It also points to an epic fail of our infrastructure.
[00:22] <wgrant> Because it should be *very difficult* to use it insecurely.
[00:22] <StevenK> But yay TALES
[00:22] <wgrant> I tried to fix it in March, but was foiled by inline JavaScript in HTML (not XHTML) being impossible.
[00:22] <wgrant> TALES is only barely relevant here.
[00:22] <StevenK> It's our own code that is at fault, then?
[00:23] <wgrant> TAL
[00:23] <wallyworld_> i see your point but i'm sad that i'm being forced to make something simple more complex just because someone *might* extend it insecurely in the future and the code review *might* not catch it
[00:23] <wgrant> wallyworld_: Our code reviews are almost useless.
[00:24] <wallyworld_> i've found that they've offered good feedback to my mp's
[00:24] <wgrant> Sometimes they do.
[00:24] <wgrant> They often miss things.
[00:24] <nigelb> ..
[00:24] <nigelb> *blink*
[00:25] <wallyworld_> one would hope that between code reviews and tests we catch most things
[00:25] <wgrant> One would, yes.
[00:25] <wgrant> The fulfilment of that hope is... arguable.
[00:26] <wallyworld_> i still believe that something should be done in the simplest way that makes sense now (YAGNI comes to mind) and anyone extending it is responsible for doing so the right way
[00:26] <wgrant> They are responsible for doing it the right way.
[00:26] <wgrant> But they don't.
[00:27] <wgrant> Time and time again, people introduce holes through this sort of thing.
[00:27] <wallyworld_> if that argument were to be taken a step or two further, no code would ever get written
[00:27] <wallyworld_> because everything would be so complicated
[00:28] <wallyworld_> because we would be guarding against every future possible issue
[00:28] <wallyworld_> that may not even arise
[00:28] <wallyworld_> and assming all lp devs are idiots
[00:28] <wgrant> Just this morning, a limited XSS was introduced through the dumps issue that you argued about a few weeks ago.
[00:28] <wallyworld_> ?
[00:29] <wgrant> The "simplejson.dumps then put it into HTML unescaped" pattern.
[00:29] <wgrant> That has been used in several places throughout the APP.
[00:29] <wgrant> -caps
[00:30] <wgrant> You argued when I told you in a review to avoid it.
[00:30] <wgrant> Last night an abuse was landed which introduces unescaped data.
[00:30] <wgrant> LP is complicated.
[00:30] <wgrant> People will copy code.
[00:31] <wallyworld_> so in that case the code could directly introduce unescaped data
[00:31] <wallyworld_> i wasn't aware that dumps could
[00:31] <wgrant> Right.
[00:31] <wallyworld_> but in the case now with the tales, it doesn;t
[00:31] <wallyworld_> it required modification to the code to do it
[00:31] <wgrant> TALES is moronically designed.
[00:31] <wallyworld_> requires
[00:31] <wgrant> It is terribly easy to introduce unobvious holes.
[00:32] <wallyworld_> i wouldn't call adding "structure" unobvious
[00:33] <abentley> wgrant: Yes, I don't want it to be emitted every time I get data.
[00:33] <wgrant> You'd think not.
[00:33] <wgrant> But people throw it around carelessly.
[00:33] <wgrant> abentley: dumps is not secure in its default mode, so its direct use is strongly discouraged.
[00:33] <wgrant> In this case, you're making our HTML invalid by injecting unescaped tags from the template.
[00:34] <wgrant> Fortunately the template is not user-controlled, so it will merely break parsers rather than being a terrible security hole.
[00:34] <wgrant> (breaking correct parsers is what let us know about a security hole in February)
[00:35] <abentley> wgrant: Yes, I didn't think this particular use was a security issue.  I'll look into the unescaped tags issue.  Do you have an example handy?
[00:36] <wgrant> abentley: HTML is... special. Tags inside a <script> are sometimes parsed as tags, but you also can't escape them using &gt; etc. as you would in normal XML content.
[00:36] <wgrant> We fix this for most of our JSON by encoding <>&"' as \uXXXX
[00:36] <wgrant> It's the only way to get it through unscathed.
[00:37] <wgrant> There's a special encoder for dumps around... I forget what it's called. It's used by the IJSONRequestCache.
[00:37] <abentley> wgrant: ResourceJSONEncoder?  I thought that was just for rendering web service objects.
[00:38]  * wgrant finds.
[00:38] <wallyworld_> i've used ResourceJSONEncoder on a dict that had both web service objects in it and other stuff
[00:39] <wallyworld_> so perhaps it does both things
[00:39] <abentley> wallyworld_: Oh, it certainly works fine for plain types.  I just didn't know it had any advantage for plain types.
[00:39] <wgrant> eggs/lazr.restful-0.19.4-py2.6.egg/lazr/restful/_resource.py:class ResourceJSONEncoder(simplejson.encoder.JSONEncoderForHTML):
[00:40] <wgrant> You can use ResourceJSONEncoder, or JSONEncoderForHTML directly.
[00:40] <wallyworld_> i wasn't sure whether it did, but was guessing that perhaps it did so
[00:40] <abentley> wgrant: great, thanks.
[00:41] <wgrant> abentley: I'm not sure if BugTarget:+bugs has the same strict parsers running over it, but if this leaked onto BugTak:+index it would certainly break things.
[00:41] <wgrant> So probably best to fix before we turn the flag on widely.
[00:41] <abentley> wgrant: Yeah, I can do that.
[00:42] <wgrant> But I still don't understand why you can't conditionally inject it into the IJSONRequestCache. Is it because that is sometimes reloaded?
[00:42] <wgrant> Perhaps we need a separate static version.
[00:42] <abentley> wgrant: I don't want to get it when I access the cache via ++model++.
[00:42] <wgrant> Right.
=== Ursinha is now known as Ursinha-afk
[02:15] <lifeless> anyone around that knows yuixhr stuff ?
[02:26] <poolie> wgrant: so should an unexpected error from dkim be a warning (and oops) or just info?
[02:26] <wgrant> poolie: Warning, I think.
[02:28] <lifeless> poolie: if you want an engineer to look at it without users complaining, an oops.
[02:28] <lifeless> poolie: otherwise info
[02:28] <lifeless> poolie: thats what warnings (and oopses) mean to us
[02:29] <poolie> yeah
[02:29] <poolie> we oughta fix pydkim to catch this internally
[02:29] <poolie> my concern is just whether that actually counts as critical
[02:29] <wgrant> lifeless: What's the yuixhr issue?
[02:30] <lifeless> unexplained errror from the fixture tets with 'Unexpected error: Unknown error'
[02:31] <lifeless> reproducable in https://code.launchpad.net/~lifeless/launchpad/useoops with bin/test -vvt test_yuixhr_fixture
[02:36] <poolie> wgrant: ok then https://code.launchpad.net/~mbp/launchpad/881237-dkim-error/+merge/80413
[02:38] <wgrant> poolie: Approved, thanks.
[02:38] <wgrant> lifeless: blink
[02:38] <wgrant>   Running:
[02:38] <wgrant>  lp/testing/tests/test_yuixhr_fixtureSegmentation fault
[02:38] <lifeless> wgrant: \o/
[02:39] <poolie> woo, record low latency
[02:39] <poolie> for me
[02:39] <wgrant> poolie: I can rescind my approval if you're uncomfortable with such speed.
[02:39] <wgrant> lifeless: Does it in devel too :(
[02:39] <poolie> it's ok, there's still a good chance it will stall before deployment
[02:39] <lifeless> wgrant: oh, phew.
[02:39] <poolie> but if we're lucky it will be under 24h from a critical being filed to being fixed
[02:40] <lifeless> I'll toss this at ec2 and see if its happy or not
[02:40] <wgrant> poolie: Unless stuff goes wrong we should be able to deploy it before lunch tomorrow :)
[02:40] <lifeless> wgrant: devel for me passed, so I thought it was my branch
[02:41] <wgrant> lifeless: It probably is your branch, but it's difficult to say because it's so damn unreliable.
[02:41] <lifeless> ec2-win
[02:41] <lifeless> <Response><Errors><Error><Code>InvalidInstanceID.NotFound</Code><Message>The instance ID 'i-3bbc4e58' does not exist</Message></Error></Errors><RequestID>a71551e1-6488-407c-b4ae-8288ad4c8762</RequestID></Response>
[02:41] <lifeless> alsy yay boto for not wrapping errors and instead spewing xml
[02:42] <wgrant> Hello launchpadlib...
[02:51] <nigelb> I was about say boto is WIN, but wgrant wins.
[02:54] <wgrant> wallyworld_: Ah, good.
[02:54]  * nigelb looks around for a bug to hack on for today's holiday
[02:56] <wallyworld_> huwshimi: there seems to be split opinion as to whether to hide/disable links which the user cannot use
[02:58] <nigelb> wgrant: hey can haz more details on bug 583392?
[02:58] <_mup_> Bug #583392: IntegrityError raised setting a branch for a project series. <branches> <easy> <lp-code> <oops> <series> <Launchpad itself:Triaged> < https://launchpad.net/bugs/583392 >
[02:58] <huwshimi> wallyworld_: Can these links *ever* be used on a private bug?
[02:59] <wallyworld_> huwshimi: yes, but they are to be disabled when the bug has bugtasks targetted at certain pillar types
[03:00] <huwshimi> wallyworld_: And in what circumstances can the links become re-enabled?
[03:00] <wallyworld_> so sometimes they are valid but the user may not be able to change it (depending on their permission)
[03:00] <wallyworld_> huwshimi: existing bug tasks need to be deleted
[03:00] <wallyworld_> or retargetted
[03:01] <wgrant> nigelb: I'm afraid it probably requires OOPS and DB access to diagnose :(
[03:01] <nigelb> wgrant: :(
[03:01] <nigelb> Moving on then.
[03:01] <lifeless> nigelb: jsoops :_
[03:01] <nigelb> AHA!
[03:01] <StevenK> wgrant: So, paragraph on +activate-ppa, or missing link?
[03:02] <nigelb> lifeless: Thanks for reminding. jsoops it is today :)
[03:03] <huwshimi> wallyworld_: Is the state that makes the options unavailable transitory?
[03:03] <wallyworld_> huwshimi: not really
[03:04] <wallyworld_> huwshimi: if a bug has a bug task for a project, it doesn't make sense to add additional bug tasks for a distro
[03:04] <wallyworld_> huwshimi: it's a new constraint due to the disclosure work
[03:05] <wallyworld_> huwshimi: so instead the user would create a new bug affecting the distro/source package and link the bug to the other one
[03:05] <wallyworld_> when bug linking is done
[03:08] <mwhudson> https://lp-oops.canonical.com/oops.py/?oopsid=2107EE48 is really strange
[03:09] <mwhudson> ah unless the form has changed since that oops
[03:09] <huwshimi> wallyworld_: OK, so really we're in a situation where the bug is in a state where the control isn't temporarily unavailable and there's no direct way to enable the control, so this seems like a situation to hide the links. If it's ambiguous as to why they are hidden it may require an in-place explanation.
[03:09]  * StevenK sighs at lp-oops forgetting the query string after login
[03:10] <wallyworld_> huwshimi: i agree that they should be hidden. i was just making sure given that some folks dislike hidden links :-)
[03:10] <wallyworld_> thanks
[03:10] <lifeless> wallyworld_: some folk will dislike everything we do
[03:10] <StevenK> mwhudson: Why would the form change since the 8th of October?
[03:10] <lifeless> :)
[03:11] <mwhudson> StevenK: the work to use the code import infrastructure with rcs_type == bzr instead of mirrored branches is pretty recent
[03:11] <huwshimi> wallyworld_: right, but people don't like hidden links when they are hidden inappropriately or without explanation.
[03:11] <wallyworld_> lifeless: yes, but one of those is a lp dev, so i was just being 100% sure :-)
[03:11] <StevenK> mwhudson: Except we don't have a code team anymore ...
[03:12] <wgrant> code team == jelmer
[03:12] <mwhudson> StevenK: err what?
[03:12] <lifeless> The connection to login.ubuntu.com was interrupted while the page was loading.
[03:12]  * StevenK rattles wgrant for an answer to his question.
[03:12] <mwhudson> hm
[03:12] <StevenK> lifeless: Saw that this morning, annoying.
[03:12] <wgrant> lifeless: I get that a couple of times a day.
[03:12] <mwhudson> timestamp: Thu 2011-10-06 17:33:40 +0000
[03:13] <mwhudson> ^ this was the change hitting devel that disabled the ability to create mirrored branches
[03:13] <mwhudson> so it could well not have been deployed until the 8th or 9th
[03:13] <wallyworld_> huwshimi: i'm not sure if any explanation is required on the page. the wording might turn out to be quite difficult to get concise
[03:13] <StevenK> Indeed
[03:13] <StevenK> And it hasn't OOPS'd since the 8th
[03:14] <wallyworld_> huwshimi: and add to a already very busy page
[03:14] <huwshimi> wallyworld_: Right, but it needs to be clear why things might be different on that page (i.e. missing links)
[03:15] <wgrant> "Private bugs cannot affect multiple project."
[03:15] <wgrant> +s
[03:16] <wallyworld_> that is concise but my fear is that people won't appreciate what is means
[03:16] <wallyworld_> but it's better than nothing
[03:17] <wallyworld_> i'll add it to the template for display if a link is to be hidden
[03:27] <poolie> StevenK: can you give me an opinion on https://code.launchpad.net/~mbp/launchpad/612171-diff-generation-spam/+merge/80288
[03:36] <StevenK> I'm not sure why we do care about pending writes on a branch
[03:37] <StevenK> But in general, yes, we should just retry the job.
[03:38] <StevenK> MPJs make me very sad in many ways.
[03:41] <wallyworld_> huwshimi: this ok? in this case, one link is missing. but sometimes also only the "Target to Series" link is visible http://people.canonical.com/~ianb/missing-link-text.png
[03:42] <wgrant> wallyworld_: You're going to s/distribution/package/, right?
[03:42] <wallyworld_> wgrant: i was going to but thought it may warrant some more feedback
[03:43] <wallyworld_> but if you are sure....
[03:44] <wgrant> I'm only mostly sure.
[03:44] <huwshimi> wallyworld_: I think so, it might be nice in a slight grey. Let me see if there's an appropriate class
[03:44] <wgrant> It could be considered separate.
[03:45] <wallyworld_> wgrant: that's what i was thinking
[03:45] <huwshimi> wallyworld_: Is that a <p>?
[03:45] <wallyworld_> huwshimi: no, a div
[03:45] <huwshimi> wallyworld_: No problems
[03:46] <wallyworld_> huwshimi: also this is the css i used https://pastebin.canonical.com/54929/
[03:47] <wallyworld_> ah bollocks. css is a bit screwed
[03:48] <huwshimi> wallyworld_: Do you want to add color: #666; to that class?
[03:49] <wallyworld_> huwshimi: will do. i think the css is not quite right though. perhaps is should be div.private
[03:49] <huwshimi> wallyworld_: Ah ok, well whatever class it is for that message
[03:50] <wallyworld_> yes
[03:50] <wallyworld_> it works as is but is horrible. fixing
[03:51] <wallyworld_> on second thoughts, i think it's ok. i got myself confused
[03:53] <lifeless> wgrant: useoops diff is up to 2756 lines - but AFAIK there is the one mysterious failure left.
[03:54] <lifeless> wgrant: its smaller than it sounds: +413, -875
[03:54] <lifeless> wgrant: are you willing to review this ?
[03:56] <wgrant> lifeless: This moves everything to AMQP?
[03:57] <lifeless> wgrant: for tests, they are now all using self.oops or (doctests) CaptureOops
[03:57] <lifeless> wgrant: tests generating external oopses now always get them from amqp (by calling sync() before inspecting self.oopses)
[03:58] <lifeless> wgrant: production won't have rabbit configured yet so won't send over amqp
[03:58] <lifeless> wgrant: but it will be ready to when we get a live rabbit
[03:58] <lifeless> wgrant: I intend to get the oops-tools listener deployed before landing this (e.g. later this week)
[03:58] <wgrant> Ah.
[03:58] <wgrant> Good.
[03:59] <lifeless> alternatively, when we make rabbit live we can disable the oops exchange which will disable it
[03:59] <wgrant> lifeless: Note that txlongpoll is currently crap and exposes the entire vhost to the web.
[03:59] <wgrant> So it's not safe for anything else to use rabbit yet.
[03:59] <lifeless> wgrant: different vhosts ftw
[03:59] <wgrant> (this regressed post-Dublin while I wasn't watching)
[04:00] <wgrant> Also, I guess this introduces the controversial BSON change.
[04:00] <lifeless> anyhow
[04:01] <lifeless> if I click you in the ui for a review and move to needs-review, is that cool.
[04:01] <wgrant> Sure.
[04:01] <lifeless> alt - you can click on claim-it
[04:01] <wgrant> 'tis claimed.
[04:01] <lifeless> wgrant: https://code.launchpad.net/~lifeless/launchpad/useoops/+merge/79516
[04:01] <lifeless> ah, you found it, cool
[04:02] <wallyworld_> huwshimi: i reused the formHelp class (color = #777 plus a tiny bit of spacing). looks nice. for completeness, can you Approve the mp now? :-)
[04:02] <wgrant> lifeless: CaptureOops can't be used as a context manager?
[04:03] <lifeless> wgrant: why not ?
[04:03] <wgrant> Well, you use setUp/cleanUp over small chunks of code in eg. xx-request-expired.txt
[04:03] <huwshimi> wallyworld_: Great. Do you mind quickly showing me a screenshot?
[04:03] <lifeless> doctests
[04:03]  * lifeless hates em
[04:03] <wallyworld_> huwshimi: yep, just a sec
[04:03] <huwshimi> wallyworld_: Thanks, I know it's a hassle
[04:04] <wallyworld_> huwshimi: it's no problem at all. i'd rather get it right up front
[04:04] <wgrant> lifeless: This looks generally good, but it'll be a while before I'm done with it :)
[04:05] <lifeless> of course
[04:05] <lifeless> no rush
[04:05] <wallyworld_> huwshimi: http://people.canonical.com/~ianb/missing-link-text1.png
[04:05] <lifeless> wgrant: a particular concern to look out for is a getLastOops (direct or indirect) looking for no-change replaced with self.oopses w/out a sync call.
[04:06] <huwshimi> wallyworld_: Yep that looks great. I think it looks more like a notification now and less like content
[04:06] <huwshimi> wallyworld_: I'll go deal with the mp
[04:06] <wallyworld_> huwshimi: yes, agreed
[04:06] <wallyworld_> thanks
[04:10] <huwshimi> wallyworld_: No problems. Approved.
[04:10]  * wallyworld_ fires up ec2
[04:11] <StevenK> Hm, my branch should be almost done
[04:11] <StevenK> Although ec2 seems to have grown out to 4h30 again
[04:12] <StevenK> And there is no windmill to disable to gain 30 minutes :-(
[04:17] <wallyworld_> StevenK: and there's nothing to blame when it goes wrong wither
[04:17] <wallyworld_> either
[04:18] <jtv> wgrant: any chance you could triage bug 881476 for me?  I'm out of my depth.
[04:18] <_mup_> Bug #881476: Spurious Changed-By fields in native-sync -changes email. <derivation> <Launchpad itself:New> < https://launchpad.net/bugs/881476 >
[04:21] <StevenK> wgrant: http://people.canonical.com/~stevenk/PPA-change.png
[04:23] <wgrant> StevenK: Perhaps.
[04:23] <StevenK> wgrant: You like it, you don't?
[04:24] <StevenK> Sprinkle 'or' to taste
[04:24] <wgrant> I don't, but I'm not sure what would be much better.
[04:28] <wgrant> jtv: Haven't you been in that code lately?
[04:28] <jtv> wgrant: it does seem somewhat related, but not that code, no.
[04:29] <wgrant> "that code" == "Soyuz notification code"
[04:34] <jtv> wgrant: this bug sounded more like it concerned _an input to_ the notification code to me.  Is that impression wrong?
[04:34] <wgrant> jtv: I forget what you've been working on, but it's very related.
[04:34] <wgrant> Ah, notification recipients, right.
[04:34] <wgrant> Yes, somewhere between very and extremely related.
[04:37] <wgrant> jtv: Do you want spoilers?
[04:39] <jtv> wgrant: please be clear with me today.
[04:39] <wgrant> So, lp.soyuz.adapter.notification will include Changed-By if it can find an email address.
[04:40] <wgrant> It finds the email address using SPR.creator.preferredemail
[04:40] <wgrant> Because gina is shit, SPR.creator for this SPR is actually the Maintainer field from the source package.
[04:40] <wgrant> Looking in the DB, this is python-apps-team
[04:41] <wgrant> Which no longer has the relevant email address, but must have when the SPR was created some time ago.
[04:41] <wgrant> It no longer has any email address at all.
[04:41] <wgrant> So there is no preferredemail.
[04:41] <wgrant> So Changed-By doesn't show up in the email.
[04:41] <wgrant> That explains the magical disappearing field.
[04:42] <wgrant> And the rest is a matter for whoever is designing derivative distros.
[04:42] <StevenK> "LOL"
[04:42] <jtv> I was secretly hoping that this might also do something for the bug I was working on, but apparently not.
[04:43] <wgrant> It's very very relevant to the bug you're working on.
[04:43] <wgrant> It will be sorted out in the mid-late stages of the derivative distros project, after StevenK's refactoring of the notification code.
[04:44] <wgrant> Because the notification stuff needs a big rework before we can seriously consider even going into beta.
[04:49] <StevenK> wgrant: So I can't use create_initialized_view for Person:+index :-(
[04:49] <wgrant> StevenK: Not currently, no. But you probably want to investigate why not and fix it.
[04:50] <wgrant> It would make lots of tests much nicer.
[04:50] <wgrant> I would dig, but I have a lot piled up at the moment :/
[04:55] <jtv> thanks wgrant for looking into this.  Will have to process later.
=== jtv is now known as jtv-eat
=== almaisan-away is now known as al-maisan
=== al-maisan is now known as almaisan-away
[06:22] <wgrant> lifeless: What was the difference between OopsHandler and OopsLoggingHandler?
[06:32] <lifeless> wgrant: Do you really care ? ;)
[06:32] <lifeless> wgrant: OH was warning and up and uses the global error utility and didn't honour record.exc_info
[06:33] <lifeless> wgrant: OLH was error and up, used a supplied error utility and only oopsed if record.exc_inf was non-None
[06:33] <lifeless> wgrant: OLH was unused.
[06:33] <lifeless> I filed a bug, bah, haven't linked it
[06:34] <lifeless> I want a search dialog on link-a-bug
[06:34] <lifeless> linked now
[06:34] <wgrant> Thanks.
[06:34] <lifeless> OH will be moving out of tree soon anyhow
[06:35] <wgrant> Good, good.
[06:35] <lifeless> to python-oops
[06:38] <wgrant> lifeless: I have a dev appserver running on a fresh rabbit instance, that's never had amqp2disk run to set up the exchange and queue.
[06:38] <wgrant> OOPSes don't seem to be ending up in /var/tmp/lperr.
[06:39] <lifeless> wgrant: interesting :)
[06:39] <wgrant> Ah, looks like non-existent exchanges aren't an immediate error...
[06:39] <lifeless> wgrant: first oops geerated should trigger a failed send, no ?
[06:40] <wgrant> "The exception refers to the previous exchange, but that's as good as it gets with AMQP. When you send a Basic.Publish, there is no confirmation of whether it succeeded or not. If there was an error, you get a Channel.Close message with the reason, but this can happen after you already sent another Basic.Publish message."
[06:40] <wgrant> I'm not sure if amqplib handles return messages like that at all.
[06:41] <lifeless> amqplib looks for a reply on verbs it sends that generate acks, except when you tell it not to
[06:41] <lifeless> that one sounds particularly evil though
[06:51] <lifeless> poolie: qq.com being broken is just lovely-ironic
[06:52] <poolie> mm?
[06:53] <poolie> it's some kind of chinese yahoo-like thing, right?
[06:54] <poolie> oops, i've got to go
[06:55] <lifeless> poolie: qq in online games -> whinging or complaining
[06:59] <spm> aiui, 'qq' used to be the key combo to 'exit game' in one of the RTS's (command & conquer??? doesn't matter). so in a player vs player match, if your getting upset/beaten, you'd "qq" and exit. In the vernacular it's come to mean "cry baby goes home" more or less.
[07:00] <wgrant> WC2, IIRC.
[07:00] <spm> I was curious one day. and looked it up. :-)
[07:00] <spm> could be.
[07:12] <rvba> Thanks for the review StevenK!
=== jtv-eat is now known as jtv
[07:44] <lifeless> wgrant: thanks for the review
[07:44] <lifeless> wgrant:
[07:44] <lifeless> Could you please explain how this relates to OopsLoggingHandler?
[07:44] <lifeless> wgrant: what do you mean by that
[07:45] <wgrant> lifeless: Argh, you explained that here.
[07:45] <wgrant> Nevermind :)
[07:45] <lifeless> wgrant: ditto the oopshandler query I presume
[07:45] <wgrant> lifeless: Indeed.
[07:45] <lifeless> wgrant: also I know about the routing key; I want to permit us to change config in future without needing to patch the tree
[07:47] <lifeless> wgrant: the thing with doctests is that the start and end have narrative in between them
[07:47] <lifeless> wgrant: so using as a context manager is nontrivial
[07:47] <wgrant> Ah, indeed.
[07:48] <lifeless> two statements is due to line length
[07:48] <wgrant> = (
[07:48] <wgrant>    blah)?
[07:48] <lifeless> less readable
[07:49] <wgrant> More obvious and less namespace-polluting.
[07:49] <wgrant> But OK.
[07:49] <lifeless> This import is probably in the wrong place.
[07:49] <lifeless> > +import oops_amqp
[07:49] <lifeless> I don't understand
[07:49] <wgrant> It refers to the line above.
[07:49] <lifeless> the next line is import testtools
[07:49] <wgrant> lp_customize, IIRC.
[07:49] <wgrant> Which is an in-tree import, so belongs in the last import section.
[07:49] <lifeless> ah, unchanged but yes wrong
[07:51] <lifeless> Why not use self.oopses[-1]? If you want to handle the case where
[07:51] <lifeless> there's more than one new OOPS, you'll need to compare the index and length.
[07:51] <lifeless> -> because I want to capture the first oops generated in each loop
[07:51] <lifeless> -1 would get the last
[07:51] <lifeless> I'm not trying to enforce one and only one
[07:52] <lifeless> I caught some places where we double-oops
[07:52] <lifeless> but it wasn't the focus of the branch, and I'm trying to contain scope
[08:01] <adeuring> good morning
=== almaisan-away is now known as al-maisan
[08:20] <lifeless> wgrant: the runner logs adequately
[08:21] <lifeless> wgrant: it was logging *twice*, in two layers of calls
[08:21] <lifeless> wgrant: can you expand on  '> +    errorUtility._ignored_exceptions = set()
[08:21] <lifeless> Can you feel my piercing glare of disapproval?
[08:21] <lifeless> '
[08:21] <lifeless> wgrant: the emails get = line extended but the doctest is looking at the wire representation
[08:23] <allenap> StevenK: Up for a shortish review? https://code.launchpad.net/~allenap/python-pgbouncer/reliable-shutdown/+merge/80382. Unfortunately I have to go out nowish so I can't be around to field questions, so push back if that's a problem.
[08:24] <wgrant> lifeless: Setting private attributes in production code upsets me.
[08:24] <StevenK> allenap: I read it previously, but I don't feel confident enough in pgbouncer to review it.
=== StevenK changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | Welcome danhg | On call reviewer: - | Critical bugtasks: 266
[08:24] <allenap> StevenK: Okay, thanks anyway :)
[08:24] <allenap> Anyone else in a reviewy mood?
[08:25] <lifeless> wgrant: it was previously a subclass setting a private attribute; swings and roundabouts
[08:25] <lifeless> wgrant: I can make it public easily enough. Shrug.
[08:25] <wgrant> lifeless: This is true.
[08:25] <lifeless> wgrant: I'm inclined to ignore it, its private to the extent of don't fiddle without reason
[08:26] <wgrant> lifeless: k
[08:26] <lifeless> but python explicitly has this consenting adults approach
[08:26] <lifeless> wgrant: I think you misunderstand notify_publisher
[08:27] <wgrant> Yes, so I do.
[08:27] <wgrant> I misread it.
[08:27] <wgrant> But now I don't understand how OOPSes aren't added to self.oopses twice: once over AMQP, once through local event.
[08:28] <lifeless> they are if you sync without reason
[08:28] <lifeless> but sync() isn't free so its not done on __getitem__ in oopses
[08:28] <wgrant> Or if you sync after generating both in-process and out-of-process OOPSes...
[08:29] <lifeless> wgrant: which no tests do
[08:29] <wgrant> Sure, I gathered that.
[08:29] <wgrant> But then it's not so much syncing as pulling everything from AMQP.
[08:29] <wgrant> Including stuff that may already be there.
[08:29] <lifeless> everything since the fixture was created
[08:29] <wgrant> This should be in some docstring somewhere, I think.
[08:30] <lifeless> Your last statement suggests a misunderstanding
[08:30] <lifeless> tests can only get their own oopses (assuming no rogue processes)
[08:30] <wgrant> Sure.
[08:30] <wgrant> So, I generate an OOPS in process, and also one in a subprocess.
[08:30] <lifeless> and each oops from amqp is delivered only once
[08:31] <wgrant> I call .sync()
[08:31] <wgrant> len(self.oopses) == 3
[08:31] <lifeless> yes
[08:31] <wgrant> That is unobvious.
[08:31] <lifeless> I can doc that
[08:31] <wgrant> Needs documentation.
[08:34] <lifeless> ok, I'll act on the review later this week.
[08:34] <wgrant> Thanks.
[08:34] <lifeless> there will be some more test fixes and I'll ask for incremental review on that
[08:41] <bigjools> wgrant: so arch-all domination changes didn't break the world, it seems
[08:42] <wgrant> bigjools: Not noticeable so, at least.
[08:42] <wgrant> bigjools: Speaking of which, we should reobsolete all the obsoleted series.
[08:42] <bigjools> some 30 seconds slower per suite at first
[08:43] <bigjools> wgrant: for your deathrow fix?
[08:43] <wgrant> bigjools: Right.
[08:44] <bigjools> wgrant: sure, I'll approve if you request
[08:44] <wgrant> Not quite sure how to request.
[08:44]  * wgrant tries the easy way.
[10:10] <cjwatson> Does anyone know why (a) https://launchpad.net/ubuntu/+source/adios/1.3-7/+build/2855760 is "Needs building" and (b) https://launchpad.net/builders shows the amd64 queue empty and all distro amd64 builders idle?
[10:11] <cjwatson> Also I could have sworn I saw that adios build start and then flip back to needs-building.
[10:13] <cjwatson> bigjools: Do you expect the slowness to reduce later, BTW?
[10:14] <cjwatson> It's added about ten minutes to the distro publisher
[10:14] <bigjools> cjwatson: it will not get better
[10:15] <cjwatson> Actually maybe that's an exaggeration, but I notice it stretching out to :45 now
[10:16] <cjwatson> maybe more like high thirties normally; before, it usually finished around :32
[10:17] <bigjools> it depends on how many outstanding packages are waiting to be superseded
[10:17] <bigjools> since that number has now increased
[10:17] <bigjools> it re-considers them every time
[10:18] <bigjools> we also have to do a 2-pass domination, which has added 30 seconds per suite
[10:18]  * bigjools OTP
[10:21] <cjwatson> It does unfortunately mean that the value to platform in speeding up cron.germinate is diluted, since it no longer looks as if that will be enough to get us to 30-minute cycles.
[10:21] <cjwatson> But maybe I can argue it's a good idea anyway.
[10:23] <bigjools> there are probably many optimisations
[10:24] <cjwatson> Yeah, I was just naively hoping for a big bang :)
[10:24] <cjwatson> https://launchpad.net/ubuntu/precise/+builds?build_text=&build_state=pending&arch_tag=amd64 -> 7 results, build queues empty.  Definitely something broken here
[10:30] <bigjools> I'll look in a bit
[10:30] <cjwatson> thanks
[10:37] <wgrant> 10 minutes!?
[10:37] <wgrant> You said it wasn't going to be much slower :(
[10:49] <allenap> jtv: Are you in the mood for a review? Indeed, are you still "at work"? If you are, on both counts, would you mind looking at https://code.launchpad.net/~allenap/python-pgbouncer/reliable-shutdown/+merge/80382?
[10:50] <jtv> allenap: can be with you in a few minutes.
[10:50] <allenap> jtv: Thank you.
[11:04] <cjwatson> The builders are doing something now; it's possible my recent batch of retries kicked something into action.  I'll see if they manage to flush completely.
[11:04] <cjwatson> Hmm, the buildlogs just say "Waiting for slave process to be terminated" though
[11:05] <cjwatson> And there https://launchpad.net/ubuntu/+source/cdargs/1.35-8/+build/2870409 flipped back to needs-building.  It's dead Jim.
[11:07] <cjwatson> Same on i386.
[11:27] <jtv> allenap: got some questions up on the MP.
[11:30] <allenap> jtv: Thanks, just saw those.
[11:30] <jtv> Code notes to follow.
[11:59] <danhg> Is flacoste here? Am I spelling it right?
[11:59] <wgrant> That's right.
[11:59] <danhg> thanks wgrant
[11:59] <wgrant> He's probably not here quite yet.
[11:59] <danhg> fair enough
[11:59] <wgrant> He was also unwell yesterday, I believe.
[11:59] <danhg> Is jsackett the right irc name for Jon S?
[11:59] <wgrant> jcsackett
[11:59] <danhg> aha
[11:59] <danhg> thanks!
[12:00] <danhg> jcsackett - are you around?
[12:00] <wgrant> He's not here yet.
[12:00] <danhg> OK
[12:00] <bigjools> heh
[12:00] <wgrant> flacoste is in the channel, but idle.
[12:00] <danhg> I'm just too eager today eh? :-D
[12:00] <wgrant> jcsackett is not here at all.
[12:00] <bigjools> can I help danhg?
[12:01] <danhg> hey bigjools, na, it's all fine. Just waiting to send Jon something, thanks tho
[12:01] <bigjools> ok
[12:01] <rvba> danhg: jcsackett should be here in ~2 hours.
[12:02] <danhg> thanks rvba - i'll email him, it's all good
=== jtv is now known as jtv-eat
=== Ursinha-afk is now known as Ursinha
=== abentley changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | Welcome danhg | On call reviewer: abentley | Critical bugtasks: 266
=== jtv-eat is now known as jtv
=== al-maisan is now known as almaisan-away
[14:55] <allenap> Is there anyone around who can give me upload rights on http://pypi.python.org/pypi/pgbouncer?
[15:07] <flacoste> allenap: only lifeless and stub can do that
[15:07] <flacoste> best to send an email to lifeless
[15:07] <flacoste> as stub is flooded
[15:15] <allenap> flacoste: Ta.
=== beuno is now known as beuno-lunch
[16:36] <danhg> jcsackett: Hey
=== beuno-lunch is now known as beuno
[18:00] <lifeless> allenap: flacoste: fixing; we should really have teams in pypi
[18:02] <lifeless> allenap: whats your pypi account ?
[18:04] <flacoste> yep, we should
[18:25] <lifeless> flacoste: dunno if you've seen it, my useoops branch is now 2.7K LoD (+450, -850 LoChange)
[18:25] <lifeless> flacoste: mega-branchzilla-incoming-this-week :)
[18:38] <lifeless> ha!
[18:38] <lifeless> http://limpesuatela.tumblr.com/
[18:40] <flacoste> lol
[19:03] <allenap> lifeless: Oops, it's gavinpanella.
[19:03] <lifeless> on TL call now will add you post that
[19:03] <allenap> lifeless: Thanks.
[19:33] <lifeless> allenap: added
[20:11] <mwhudson> [rollback=14192] back out r14191, <- something looks wrong here <wink>
[20:12] <lifeless> lol
[20:12] <mwhudson> i think 14192 is probably correct...
[21:05] <abentley> lifeless: I have to go, but could you point out *where* I'm violating the principle that a memo is opaque?
[21:08] <lifeless> abentley: int(memo)
[21:08] <lifeless> line 87 of the diff
[21:08] <lifeless> abentley: storm range factory memos are (IIRC) uuencoded blobs
[21:09] <lifeless> abentley: I may have the encoding wrong, but they aren't ints.
[21:10] <abentley> lifeless: that's a test helper.
[21:11] <lifeless> abentley: ah!. I have a dentist appt, but I will re-review with that in mind afterwards
[21:11] <lifeless> abentley: hopefully I just read things badly and its all copacetic
[21:13] <bigjools> mwhudson, lifeless: mea culpa
[21:13] <lifeless> abentley: looking briefly I suspect it will be
[21:13] <mwhudson> bigjools: np, it just made me laugh
[21:14] <bigjools> I blame bzr for making me think about two versions when backing out!
[22:03] <sinzui> wallyworld, I had to visiting sound setting and toggle mute the other day
[22:13] <sinzui> wallyworld_, The feature is still listed on the board to do: http://launchpad.leankitkanban.com/Boards/View/12720553
[22:18] <poolie> hi all, thanks for the review abentley
[22:27] <StevenK> sinzui, wallyworld_, jcsackett: http://people.canonical.com/~stevenk/PPA-change.png
[23:05] <wgrant> mwhudson: 14192 is the number I gave him at 1:30am, so it's more likely to be wrong, but let me check.
[23:05] <mwhudson> wgrant: i checked
[23:05] <wgrant> Ah, good.
[23:05] <wgrant> poolie: There may still be hope, then.
[23:07] <wgrant> Aha, buildbot finished a couple of minutes ago.
[23:08] <poolie> wgrant: oh for the fix to bug 878140 to get out in <24h?
[23:08] <_mup_> Bug #878140: process-mail.py failed to resolve dns. Raised NXDOMAIN <dkim> <oops> <qa-untestable> <Launchpad itself:Fix Committed by mbp> < https://launchpad.net/bugs/878140 >
[23:08] <poolie> wbn
[23:08] <wgrant> Yes.
[23:09] <poolie> no, wait, that one is older
[23:10] <wgrant> The general Exception catcher that I reviewed yesterday, anyway.
[23:10] <poolie> https://code.launchpad.net/~mbp/launchpad/881237-dkim-error/+merge/80413
[23:10] <poolie> failed ec2 overnight because of some spurious timing related error :/
[23:10] <wgrant> Ah
[23:10] <wgrant> Fail, then.
[23:10] <poolie> reminds me i need to resubmit and file a bug
[23:13] <poolie>  File "/var/launchpad/test/lib/canonical/launchpad/utilities/ftests/test_gpghandler.py", line 173, in testHomeDirectoryJob
[23:13] <poolie>    self.assertTrue(now <= floor(os.path.getmtime(fname)))
[23:15] <wgrant> poolie: Ah, that lovely one.
[23:15] <poolie> i haven't looked at it yet but i guess it's asserting something takes less than a second
[23:16] <wgrant> Not quite. It's checking that the mtime of some files is current.
[23:16] <wgrant> After they were set back 12 hours or so and a job run to bring them up to date.
[23:17] <poolie> https://bugs.launchpad.net/launchpad/+bug/882324
[23:17] <_mup_> Bug #882324: timing-dependent spurious test failure in testHomeDirectoryJob <Launchpad itself:Triaged> < https://launchpad.net/bugs/882324 >
[23:28] <lifeless> poolie: why do you think its spurious ?
[23:29] <poolie> oh, do you think my code broke it?
[23:29] <poolie> maybe
[23:29] <poolie> we'll see if it fails a second time
[23:30] <poolie> anything comparing timestamps in a test looks pretty suspicious to me
[23:30] <poolie> but of course some times they're valid
[23:30]  * poolie looks
[23:30] <wgrant> This one isn't valid.
[23:31] <poolie> it obviously is
[23:31] <poolie> it is essentially counting on all completing before the clock ticks
[23:31] <poolie> s//obviously is dangerous
[23:31] <wgrant> Yep.
[23:32] <poolie> hm.
[23:36] <poolie> actually i think it must be something more subtle than that
[23:37] <poolie> - floating point rounding?
[23:38] <poolie> - setting the directory mtime didn't do what we expected?
[23:40] <poolie> -
[23:40] <poolie> - different precision between time() and getmtime() causing different rounding behaviour?
[23:42] <lifeless> poolie: well the now() is captured before the touching
[23:42] <lifeless> poolie: and they are both floored
[23:42] <poolie> mm
[23:42] <poolie> so how can it fail?
[23:43] <lifeless> I don't see any time sensitivity thing there, though floating point comparisons really do need a precision for safety
[23:43] <poolie> - clock nomonotonicity on the test machine
[23:43] <lifeless> poolie: as you say floating point precision may be the cause
[23:43] <poolie> *non
[23:43] <poolie> perhaps we should remove the 'floor' call on the rhs of the <=
[23:43] <poolie> it does not seem to help anything and it may break something
[23:44] <poolie> i might set up to run it repeatedly here and see if it ever fails
[23:44] <lifeless> thats a good idea
[23:53] <poolie> oh, could there be a symlink in that directory?
[23:53] <poolie> i think you can't utime() them and that could account for it sometimes being old