/srv/irclogs.ubuntu.com/2011/10/26/#launchpad-dev.txt

StevenKwgrant: That branch is in ec2 now.00:00
wallyworld_seems defines can't use used inside tal:context00:00
wgrantwallyworld_: You're moving the confirmation message into the template?00:00
wallyworld_yup00:00
wgrant<p>Blah blah blah <tal:someproject replace="context/someproject">Some Project</tal:someproject></p>00:01
wgrantDon't use string:00:01
wallyworld_ok. other places do :-)00:01
wgrantThey're mostly wrong, and frequently holey.00:01
wgrantI've seen it mostly used to construct JS.00:01
wgrantDo you know of others that don't>?00:02
wallyworld_there's a few places in tales but i've closed my search window00:02
wgrantstring: is OK for some things sometimes. But it's very rarely OK for content/replace.00:03
wallyworld_wgrant: one place in lp form - tal:content="string:${widget/label}00:04
wgrantwallyworld_: That's slightly more acceptable, since it just wants to append a colon.00:06
wallyworld_also lots of places in <title> elements00:06
wallyworld_more than just a colon00:06
wallyworld_wgrant: so is  <p tal:content="string:${context/bar}"/> a security issue?00:07
wgrantIt's not a security issue.00:07
wallyworld_it's far less verbose than your version00:08
wgrantIt's just pointless.00:08
wgrantAnd obscure.00:08
wallyworld_it was a contrived example00:08
wgrantstring: 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
wgrantBut in your case you're constructing paragraphs with a couple of substitutions.00:08
wallyworld_compare what i just typed with00:09
wallyworld_<p>Blah blah blah <tal:someproject replace="context/someproject">Some Project</tal:someproject></p>00:09
wgrantTAL can be ugly, but putting everything in tal:content is not the solution.00:09
wallyworld_i think mine is very readable00:09
wallyworld_more so than yours00:09
wgrantstring: encourages people to do stupid things.00:10
wgrantLike use structure.00:10
wallyworld_but i'm not doing that00:10
wgrantNo.00:10
wallyworld_that's a false argument00:10
wgrantIt's not false.00:10
wgrantIt's a perfectly valid argument.l00:10
wgrantIf we permit patterns that are not hard to use insecurely, people will use them insecurely.00:11
wallyworld_my construct is readable and concise00:11
nigelbMorning!00:11
wallyworld_and not insecure00:11
wgrantIt's not insecure, but this sort of thing often leads to people putting structure in front of huge things.00:12
wgrantLike you did in your first version of the branch.00:12
wallyworld_code reviews are meant to pick up such things00:12
wgrantThey normally don't.00:12
wgrantI normally pick them up by reading diffs after they land.00:12
wallyworld_i thought structure(xxxx) is kosher?00:12
wgrantNo.00:12
wallyworld_is it deprecated then?00:12
wallyworld_and why do we have it?00:13
wgrantIts use is discouraged, in deference to templates.00:13
wallyworld_it's use for request notifications, no?00:13
wgrantBecause people have shown that they cannot use structured() safely.00:13
wgrantUntil 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 uses00:15
wallyworld_when the alternative is verbose and ugly00:15
wgrantThe alternatively is empirically less likely to be abused.00:16
wgrant-ly00:16
wgrantBecause 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 exist00:17
wgrantlifeless: s/alternatively/alternative/00:17
lifeless:>00:17
wgrantwallyworld_: So, someone wants to extend this confirmation message.00:18
wgrantThey want to emphasise "permanently", because people are deleting tasks too readily.00:18
wgrantThey 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:19
wgrantThey miss that three lines later, in the same string literal, there is a variable substitution.00:20
wgrantMixing data of different types in one literal is not a good idea.00:20
wgrantThis *does* happen.00:20
wallyworld_i'll take your word for it00:20
lifelesswallyworld_: we had a -raft- of security holes related to prior use of structured00:20
wallyworld_that points to an epic fail in our code reviews then00:21
wgrantYes.00:21
wgrantLots of things do.00:21
wgrantIt also points to an epic fail of our infrastructure.00:22
wgrantBecause it should be *very difficult* to use it insecurely.00:22
StevenKBut yay TALES00:22
wgrantI tried to fix it in March, but was foiled by inline JavaScript in HTML (not XHTML) being impossible.00:22
wgrantTALES is only barely relevant here.00:22
StevenKIt's our own code that is at fault, then?00:22
wgrantTAL00: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 it00:23
wgrantwallyworld_: Our code reviews are almost useless.00:23
wallyworld_i've found that they've offered good feedback to my mp's00:24
wgrantSometimes they do.00:24
wgrantThey often miss things.00:24
nigelb..00:24
nigelb*blink*00:24
wallyworld_one would hope that between code reviews and tests we catch most things00:25
wgrantOne would, yes.00:25
wgrantThe fulfilment of that hope is... arguable.00:25
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 way00:26
wgrantThey are responsible for doing it the right way.00:26
wgrantBut they don't.00:26
wgrantTime 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 written00:27
wallyworld_because everything would be so complicated00:27
wallyworld_because we would be guarding against every future possible issue00:28
wallyworld_that may not even arise00:28
wallyworld_and assming all lp devs are idiots00:28
wgrantJust this morning, a limited XSS was introduced through the dumps issue that you argued about a few weeks ago.00:28
wallyworld_?00:28
wgrantThe "simplejson.dumps then put it into HTML unescaped" pattern.00:29
wgrantThat has been used in several places throughout the APP.00:29
wgrant-caps00:29
wgrantYou argued when I told you in a review to avoid it.00:30
wgrantLast night an abuse was landed which introduces unescaped data.00:30
wgrantLP is complicated.00:30
wgrantPeople will copy code.00:30
wallyworld_so in that case the code could directly introduce unescaped data00:31
wallyworld_i wasn't aware that dumps could00:31
wgrantRight.00:31
wallyworld_but in the case now with the tales, it doesn;t00:31
wallyworld_it required modification to the code to do it00:31
wgrantTALES is moronically designed.00:31
wallyworld_requires00:31
wgrantIt is terribly easy to introduce unobvious holes.00:31
wallyworld_i wouldn't call adding "structure" unobvious00:32
abentleywgrant: Yes, I don't want it to be emitted every time I get data.00:33
wgrantYou'd think not.00:33
wgrantBut people throw it around carelessly.00:33
wgrantabentley: dumps is not secure in its default mode, so its direct use is strongly discouraged.00:33
wgrantIn this case, you're making our HTML invalid by injecting unescaped tags from the template.00:33
wgrantFortunately 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:34
abentleywgrant: 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:35
wgrantabentley: 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
wgrantWe fix this for most of our JSON by encoding <>&"' as \uXXXX00:36
wgrantIt's the only way to get it through unscathed.00:36
wgrantThere's a special encoder for dumps around... I forget what it's called. It's used by the IJSONRequestCache.00:37
abentleywgrant: ResourceJSONEncoder?  I thought that was just for rendering web service objects.00:37
* wgrant finds.00:38
wallyworld_i've used ResourceJSONEncoder on a dict that had both web service objects in it and other stuff00:38
wallyworld_so perhaps it does both things00:39
abentleywallyworld_: Oh, it certainly works fine for plain types.  I just didn't know it had any advantage for plain types.00:39
wgranteggs/lazr.restful-0.19.4-py2.6.egg/lazr/restful/_resource.py:class ResourceJSONEncoder(simplejson.encoder.JSONEncoderForHTML):00:39
wgrantYou can use ResourceJSONEncoder, or JSONEncoderForHTML directly.00:40
wallyworld_i wasn't sure whether it did, but was guessing that perhaps it did so00:40
abentleywgrant: great, thanks.00:40
wgrantabentley: 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
wgrantSo probably best to fix before we turn the flag on widely.00:41
abentleywgrant: Yeah, I can do that.00:41
wgrantBut I still don't understand why you can't conditionally inject it into the IJSONRequestCache. Is it because that is sometimes reloaded?00:42
wgrantPerhaps we need a separate static version.00:42
abentleywgrant: I don't want to get it when I access the cache via ++model++.00:42
wgrantRight.00:42
=== Ursinha is now known as Ursinha-afk
lifelessanyone around that knows yuixhr stuff ?02:15
pooliewgrant: so should an unexpected error from dkim be a warning (and oops) or just info?02:26
wgrantpoolie: Warning, I think.02:26
lifelesspoolie: if you want an engineer to look at it without users complaining, an oops.02:28
lifelesspoolie: otherwise info02:28
lifelesspoolie: thats what warnings (and oopses) mean to us02:28
poolieyeah02:29
pooliewe oughta fix pydkim to catch this internally02:29
pooliemy concern is just whether that actually counts as critical02:29
wgrantlifeless: What's the yuixhr issue?02:29
lifelessunexplained errror from the fixture tets with 'Unexpected error: Unknown error'02:30
lifelessreproducable in https://code.launchpad.net/~lifeless/launchpad/useoops with bin/test -vvt test_yuixhr_fixture02:31
pooliewgrant: ok then https://code.launchpad.net/~mbp/launchpad/881237-dkim-error/+merge/8041302:36
wgrantpoolie: Approved, thanks.02:38
wgrantlifeless: blink02:38
wgrant  Running:02:38
wgrant lp/testing/tests/test_yuixhr_fixtureSegmentation fault02:38
lifelesswgrant: \o/02:38
pooliewoo, record low latency02:39
pooliefor me02:39
wgrantpoolie: I can rescind my approval if you're uncomfortable with such speed.02:39
wgrantlifeless: Does it in devel too :(02:39
poolieit's ok, there's still a good chance it will stall before deployment02:39
lifelesswgrant: oh, phew.02:39
pooliebut if we're lucky it will be under 24h from a critical being filed to being fixed02:39
lifelessI'll toss this at ec2 and see if its happy or not02:40
wgrantpoolie: Unless stuff goes wrong we should be able to deploy it before lunch tomorrow :)02:40
lifelesswgrant: devel for me passed, so I thought it was my branch02:40
wgrantlifeless: It probably is your branch, but it's difficult to say because it's so damn unreliable.02:41
lifelessec2-win02: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
lifelessalsy yay boto for not wrapping errors and instead spewing xml02:41
wgrantHello launchpadlib...02:42
nigelbI was about say boto is WIN, but wgrant wins.02:51
wgrantwallyworld_: Ah, good.02:54
* nigelb looks around for a bug to hack on for today's holiday02:54
wallyworld_huwshimi: there seems to be split opinion as to whether to hide/disable links which the user cannot use02:56
nigelbwgrant: 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
huwshimiwallyworld_: Can these links *ever* be used on a private bug?02:58
wallyworld_huwshimi: yes, but they are to be disabled when the bug has bugtasks targetted at certain pillar types02:59
huwshimiwallyworld_: 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 deleted03:00
wallyworld_or retargetted03:00
wgrantnigelb: I'm afraid it probably requires OOPS and DB access to diagnose :(03:01
nigelbwgrant: :(03:01
nigelbMoving on then.03:01
lifelessnigelb: jsoops :_03:01
nigelbAHA!03:01
StevenKwgrant: So, paragraph on +activate-ppa, or missing link?03:01
nigelblifeless: Thanks for reminding. jsoops it is today :)03:02
huwshimiwallyworld_: Is the state that makes the options unavailable transitory?03:03
wallyworld_huwshimi: not really03:03
wallyworld_huwshimi: if a bug has a bug task for a project, it doesn't make sense to add additional bug tasks for a distro03:04
wallyworld_huwshimi: it's a new constraint due to the disclosure work03:04
wallyworld_huwshimi: so instead the user would create a new bug affecting the distro/source package and link the bug to the other one03:05
wallyworld_when bug linking is done03:05
mwhudsonhttps://lp-oops.canonical.com/oops.py/?oopsid=2107EE48 is really strange03:08
mwhudsonah unless the form has changed since that oops03:09
huwshimiwallyworld_: 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 login03:09
wallyworld_huwshimi: i agree that they should be hidden. i was just making sure given that some folks dislike hidden links :-)03:10
wallyworld_thanks03:10
lifelesswallyworld_: some folk will dislike everything we do03:10
StevenKmwhudson: Why would the form change since the 8th of October?03:10
lifeless:)03:10
mwhudsonStevenK: the work to use the code import infrastructure with rcs_type == bzr instead of mirrored branches is pretty recent03:11
huwshimiwallyworld_: 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
StevenKmwhudson: Except we don't have a code team anymore ...03:11
wgrantcode team == jelmer03:12
mwhudsonStevenK: err what?03:12
lifelessThe 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
mwhudsonhm03:12
StevenKlifeless: Saw that this morning, annoying.03:12
wgrantlifeless: I get that a couple of times a day.03:12
mwhudsontimestamp: Thu 2011-10-06 17:33:40 +000003:12
mwhudson^ this was the change hitting devel that disabled the ability to create mirrored branches03:13
mwhudsonso it could well not have been deployed until the 8th or 9th03: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 concise03:13
StevenKIndeed03:13
StevenKAnd it hasn't OOPS'd since the 8th03:13
wallyworld_huwshimi: and add to a already very busy page03:14
huwshimiwallyworld_: Right, but it needs to be clear why things might be different on that page (i.e. missing links)03:14
wgrant"Private bugs cannot affect multiple project."03:15
wgrant+s03:15
wallyworld_that is concise but my fear is that people won't appreciate what is means03:16
wallyworld_but it's better than nothing03:16
wallyworld_i'll add it to the template for display if a link is to be hidden03:17
poolieStevenK: can you give me an opinion on https://code.launchpad.net/~mbp/launchpad/612171-diff-generation-spam/+merge/8028803:27
StevenKI'm not sure why we do care about pending writes on a branch03:36
StevenKBut in general, yes, we should just retry the job.03:37
StevenKMPJs make me very sad in many ways.03:38
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.png03:41
wgrantwallyworld_: You're going to s/distribution/package/, right?03:42
wallyworld_wgrant: i was going to but thought it may warrant some more feedback03:42
wallyworld_but if you are sure....03:43
wgrantI'm only mostly sure.03:44
huwshimiwallyworld_: I think so, it might be nice in a slight grey. Let me see if there's an appropriate class03:44
wgrantIt could be considered separate.03:44
wallyworld_wgrant: that's what i was thinking03:45
huwshimiwallyworld_: Is that a <p>?03:45
wallyworld_huwshimi: no, a div03:45
huwshimiwallyworld_: No problems03:45
wallyworld_huwshimi: also this is the css i used https://pastebin.canonical.com/54929/03:46
wallyworld_ah bollocks. css is a bit screwed03:47
huwshimiwallyworld_: Do you want to add color: #666; to that class?03:48
wallyworld_huwshimi: will do. i think the css is not quite right though. perhaps is should be div.private03:49
huwshimiwallyworld_: Ah ok, well whatever class it is for that message03:49
wallyworld_yes03:50
wallyworld_it works as is but is horrible. fixing03:50
wallyworld_on second thoughts, i think it's ok. i got myself confused03:51
lifelesswgrant: useoops diff is up to 2756 lines - but AFAIK there is the one mysterious failure left.03:53
lifelesswgrant: its smaller than it sounds: +413, -87503:54
lifelesswgrant: are you willing to review this ?03:54
wgrantlifeless: This moves everything to AMQP?03:56
lifelesswgrant: for tests, they are now all using self.oops or (doctests) CaptureOops03:57
lifelesswgrant: tests generating external oopses now always get them from amqp (by calling sync() before inspecting self.oopses)03:57
lifelesswgrant: production won't have rabbit configured yet so won't send over amqp03:58
lifelesswgrant: but it will be ready to when we get a live rabbit03:58
lifelesswgrant: I intend to get the oops-tools listener deployed before landing this (e.g. later this week)03:58
wgrantAh.03:58
wgrantGood.03:58
lifelessalternatively, when we make rabbit live we can disable the oops exchange which will disable it03:59
wgrantlifeless: Note that txlongpoll is currently crap and exposes the entire vhost to the web.03:59
wgrantSo it's not safe for anything else to use rabbit yet.03:59
lifelesswgrant: different vhosts ftw03:59
wgrant(this regressed post-Dublin while I wasn't watching)03:59
wgrantAlso, I guess this introduces the controversial BSON change.04:00
lifelessanyhow04:00
lifelessif I click you in the ui for a review and move to needs-review, is that cool.04:01
wgrantSure.04:01
lifelessalt - you can click on claim-it04:01
wgrant'tis claimed.04:01
lifelesswgrant: https://code.launchpad.net/~lifeless/launchpad/useoops/+merge/7951604:01
lifelessah, you found it, cool04:01
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
wgrantlifeless: CaptureOops can't be used as a context manager?04:02
lifelesswgrant: why not ?04:03
wgrantWell, you use setUp/cleanUp over small chunks of code in eg. xx-request-expired.txt04:03
huwshimiwallyworld_: Great. Do you mind quickly showing me a screenshot?04:03
lifelessdoctests04:03
* lifeless hates em04:03
wallyworld_huwshimi: yep, just a sec04:03
huwshimiwallyworld_: Thanks, I know it's a hassle04:03
wallyworld_huwshimi: it's no problem at all. i'd rather get it right up front04:04
wgrantlifeless: This looks generally good, but it'll be a while before I'm done with it :)04:04
lifelessof course04:05
lifelessno rush04:05
wallyworld_huwshimi: http://people.canonical.com/~ianb/missing-link-text1.png04:05
lifelesswgrant: 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:05
huwshimiwallyworld_: Yep that looks great. I think it looks more like a notification now and less like content04:06
huwshimiwallyworld_: I'll go deal with the mp04:06
wallyworld_huwshimi: yes, agreed04:06
wallyworld_thanks04:06
huwshimiwallyworld_: No problems. Approved.04:10
* wallyworld_ fires up ec204:10
StevenKHm, my branch should be almost done04:11
StevenKAlthough ec2 seems to have grown out to 4h30 again04:11
StevenKAnd there is no windmill to disable to gain 30 minutes :-(04:12
wallyworld_StevenK: and there's nothing to blame when it goes wrong wither04:17
wallyworld_either04:17
jtvwgrant: 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:18
StevenKwgrant: http://people.canonical.com/~stevenk/PPA-change.png04:21
wgrantStevenK: Perhaps.04:23
StevenKwgrant: You like it, you don't?04:23
StevenKSprinkle 'or' to taste04:24
wgrantI don't, but I'm not sure what would be much better.04:24
wgrantjtv: Haven't you been in that code lately?04:28
jtvwgrant: it does seem somewhat related, but not that code, no.04:28
wgrant"that code" == "Soyuz notification code"04:29
jtvwgrant: this bug sounded more like it concerned _an input to_ the notification code to me.  Is that impression wrong?04:34
wgrantjtv: I forget what you've been working on, but it's very related.04:34
wgrantAh, notification recipients, right.04:34
wgrantYes, somewhere between very and extremely related.04:34
wgrantjtv: Do you want spoilers?04:37
jtvwgrant: please be clear with me today.04:39
wgrantSo, lp.soyuz.adapter.notification will include Changed-By if it can find an email address.04:39
wgrantIt finds the email address using SPR.creator.preferredemail04:40
wgrantBecause gina is shit, SPR.creator for this SPR is actually the Maintainer field from the source package.04:40
wgrantLooking in the DB, this is python-apps-team04:40
wgrantWhich no longer has the relevant email address, but must have when the SPR was created some time ago.04:41
wgrantIt no longer has any email address at all.04:41
wgrantSo there is no preferredemail.04:41
wgrantSo Changed-By doesn't show up in the email.04:41
wgrantThat explains the magical disappearing field.04:41
wgrantAnd the rest is a matter for whoever is designing derivative distros.04:42
StevenK"LOL"04:42
jtvI was secretly hoping that this might also do something for the bug I was working on, but apparently not.04:42
wgrantIt's very very relevant to the bug you're working on.04:43
wgrantIt will be sorted out in the mid-late stages of the derivative distros project, after StevenK's refactoring of the notification code.04:43
wgrantBecause the notification stuff needs a big rework before we can seriously consider even going into beta.04:44
StevenKwgrant: So I can't use create_initialized_view for Person:+index :-(04:49
wgrantStevenK: Not currently, no. But you probably want to investigate why not and fix it.04:49
wgrantIt would make lots of tests much nicer.04:50
wgrantI would dig, but I have a lot piled up at the moment :/04:50
jtvthanks wgrant for looking into this.  Will have to process later.04:55
=== jtv is now known as jtv-eat
=== almaisan-away is now known as al-maisan
=== al-maisan is now known as almaisan-away
wgrantlifeless: What was the difference between OopsHandler and OopsLoggingHandler?06:22
lifelesswgrant: Do you really care ? ;)06:32
lifelesswgrant: OH was warning and up and uses the global error utility and didn't honour record.exc_info06:32
lifelesswgrant: OLH was error and up, used a supplied error utility and only oopsed if record.exc_inf was non-None06:33
lifelesswgrant: OLH was unused.06:33
lifelessI filed a bug, bah, haven't linked it06:33
lifelessI want a search dialog on link-a-bug06:34
lifelesslinked now06:34
wgrantThanks.06:34
lifelessOH will be moving out of tree soon anyhow06:34
wgrantGood, good.06:35
lifelessto python-oops06:35
wgrantlifeless: 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
wgrantOOPSes don't seem to be ending up in /var/tmp/lperr.06:38
lifelesswgrant: interesting :)06:39
wgrantAh, looks like non-existent exchanges aren't an immediate error...06:39
lifelesswgrant: first oops geerated should trigger a failed send, no ?06:39
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
wgrantI'm not sure if amqplib handles return messages like that at all.06:40
lifelessamqplib looks for a reply on verbs it sends that generate acks, except when you tell it not to06:41
lifelessthat one sounds particularly evil though06:41
lifelesspoolie: qq.com being broken is just lovely-ironic06:51
pooliemm?06:52
poolieit's some kind of chinese yahoo-like thing, right?06:53
poolieoops, i've got to go06:54
lifelesspoolie: qq in online games -> whinging or complaining06:55
spmaiui, '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.06:59
wgrantWC2, IIRC.07:00
spmI was curious one day. and looked it up. :-)07:00
spmcould be.07:00
rvbaThanks for the review StevenK!07:12
=== jtv-eat is now known as jtv
lifelesswgrant: thanks for the review07:44
lifelesswgrant:07:44
lifelessCould you please explain how this relates to OopsLoggingHandler?07:44
lifelesswgrant: what do you mean by that07:44
wgrantlifeless: Argh, you explained that here.07:45
wgrantNevermind :)07:45
lifelesswgrant: ditto the oopshandler query I presume07:45
wgrantlifeless: Indeed.07:45
lifelesswgrant: also I know about the routing key; I want to permit us to change config in future without needing to patch the tree07:45
lifelesswgrant: the thing with doctests is that the start and end have narrative in between them07:47
lifelesswgrant: so using as a context manager is nontrivial07:47
wgrantAh, indeed.07:47
lifelesstwo statements is due to line length07:48
wgrant= (07:48
wgrant   blah)?07:48
lifelessless readable07:48
wgrantMore obvious and less namespace-polluting.07:49
wgrantBut OK.07:49
lifelessThis import is probably in the wrong place.07:49
lifeless> +import oops_amqp07:49
lifelessI don't understand07:49
wgrantIt refers to the line above.07:49
lifelessthe next line is import testtools07:49
wgrantlp_customize, IIRC.07:49
wgrantWhich is an in-tree import, so belongs in the last import section.07:49
lifelessah, unchanged but yes wrong07:49
lifelessWhy not use self.oopses[-1]? If you want to handle the case where07:51
lifelessthere'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 loop07:51
lifeless-1 would get the last07:51
lifelessI'm not trying to enforce one and only one07:51
lifelessI caught some places where we double-oops07:52
lifelessbut it wasn't the focus of the branch, and I'm trying to contain scope07:52
adeuringgood morning08:01
=== almaisan-away is now known as al-maisan
lifelesswgrant: the runner logs adequately08:20
lifelesswgrant: it was logging *twice*, in two layers of calls08:21
lifelesswgrant: can you expand on  '> +    errorUtility._ignored_exceptions = set()08:21
lifelessCan you feel my piercing glare of disapproval?08:21
lifeless'08:21
lifelesswgrant: the emails get = line extended but the doctest is looking at the wire representation08:21
allenapStevenK: 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:23
wgrantlifeless: Setting private attributes in production code upsets me.08:24
StevenKallenap: I read it previously, but I don't feel confident enough in pgbouncer to review it.08:24
=== StevenK changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | Welcome danhg | On call reviewer: - | Critical bugtasks: 266
allenapStevenK: Okay, thanks anyway :)08:24
allenapAnyone else in a reviewy mood?08:24
lifelesswgrant: it was previously a subclass setting a private attribute; swings and roundabouts08:25
lifelesswgrant: I can make it public easily enough. Shrug.08:25
wgrantlifeless: This is true.08:25
lifelesswgrant: I'm inclined to ignore it, its private to the extent of don't fiddle without reason08:25
wgrantlifeless: k08:26
lifelessbut python explicitly has this consenting adults approach08:26
lifelesswgrant: I think you misunderstand notify_publisher08:26
wgrantYes, so I do.08:27
wgrantI misread it.08:27
wgrantBut now I don't understand how OOPSes aren't added to self.oopses twice: once over AMQP, once through local event.08:27
lifelessthey are if you sync without reason08:28
lifelessbut sync() isn't free so its not done on __getitem__ in oopses08:28
wgrantOr if you sync after generating both in-process and out-of-process OOPSes...08:28
lifelesswgrant: which no tests do08:29
wgrantSure, I gathered that.08:29
wgrantBut then it's not so much syncing as pulling everything from AMQP.08:29
wgrantIncluding stuff that may already be there.08:29
lifelesseverything since the fixture was created08:29
wgrantThis should be in some docstring somewhere, I think.08:29
lifelessYour last statement suggests a misunderstanding08:30
lifelesstests can only get their own oopses (assuming no rogue processes)08:30
wgrantSure.08:30
wgrantSo, I generate an OOPS in process, and also one in a subprocess.08:30
lifelessand each oops from amqp is delivered only once08:30
wgrantI call .sync()08:31
wgrantlen(self.oopses) == 308:31
lifelessyes08:31
wgrantThat is unobvious.08:31
lifelessI can doc that08:31
wgrantNeeds documentation.08:31
lifelessok, I'll act on the review later this week.08:34
wgrantThanks.08:34
lifelessthere will be some more test fixes and I'll ask for incremental review on that08:34
bigjoolswgrant: so arch-all domination changes didn't break the world, it seems08:41
wgrantbigjools: Not noticeable so, at least.08:42
wgrantbigjools: Speaking of which, we should reobsolete all the obsoleted series.08:42
bigjoolssome 30 seconds slower per suite at first08:42
bigjoolswgrant: for your deathrow fix?08:43
wgrantbigjools: Right.08:43
bigjoolswgrant: sure, I'll approve if you request08:44
wgrantNot quite sure how to request.08:44
* wgrant tries the easy way.08:44
cjwatsonDoes 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:10
cjwatsonAlso I could have sworn I saw that adios build start and then flip back to needs-building.10:11
cjwatsonbigjools: Do you expect the slowness to reduce later, BTW?10:13
cjwatsonIt's added about ten minutes to the distro publisher10:14
bigjoolscjwatson: it will not get better10:14
cjwatsonActually maybe that's an exaggeration, but I notice it stretching out to :45 now10:15
cjwatsonmaybe more like high thirties normally; before, it usually finished around :3210:16
bigjoolsit depends on how many outstanding packages are waiting to be superseded10:17
bigjoolssince that number has now increased10:17
bigjoolsit re-considers them every time10:17
bigjoolswe also have to do a 2-pass domination, which has added 30 seconds per suite10:18
* bigjools OTP10:18
cjwatsonIt 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
cjwatsonBut maybe I can argue it's a good idea anyway.10:21
bigjoolsthere are probably many optimisations10:23
cjwatsonYeah, I was just naively hoping for a big bang :)10:24
cjwatsonhttps://launchpad.net/ubuntu/precise/+builds?build_text=&build_state=pending&arch_tag=amd64 -> 7 results, build queues empty.  Definitely something broken here10:24
bigjoolsI'll look in a bit10:30
cjwatsonthanks10:30
wgrant10 minutes!?10:37
wgrantYou said it wasn't going to be much slower :(10:37
allenapjtv: 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:49
jtvallenap: can be with you in a few minutes.10:50
allenapjtv: Thank you.10:50
cjwatsonThe 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
cjwatsonHmm, the buildlogs just say "Waiting for slave process to be terminated" though11:04
cjwatsonAnd there https://launchpad.net/ubuntu/+source/cdargs/1.35-8/+build/2870409 flipped back to needs-building.  It's dead Jim.11:05
cjwatsonSame on i386.11:07
jtvallenap: got some questions up on the MP.11:27
allenapjtv: Thanks, just saw those.11:30
jtvCode notes to follow.11:30
danhgIs flacoste here? Am I spelling it right?11:59
wgrantThat's right.11:59
danhgthanks wgrant11:59
wgrantHe's probably not here quite yet.11:59
danhgfair enough11:59
wgrantHe was also unwell yesterday, I believe.11:59
danhgIs jsackett the right irc name for Jon S?11:59
wgrantjcsackett11:59
danhgaha11:59
danhgthanks!11:59
danhgjcsackett - are you around?12:00
wgrantHe's not here yet.12:00
danhgOK12:00
bigjoolsheh12:00
wgrantflacoste is in the channel, but idle.12:00
danhgI'm just too eager today eh? :-D12:00
wgrantjcsackett is not here at all.12:00
bigjoolscan I help danhg?12:00
danhghey bigjools, na, it's all fine. Just waiting to send Jon something, thanks tho12:01
bigjoolsok12:01
rvbadanhg: jcsackett should be here in ~2 hours.12:01
danhgthanks rvba - i'll email him, it's all good12:02
=== 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
allenapIs there anyone around who can give me upload rights on http://pypi.python.org/pypi/pgbouncer?14:55
flacosteallenap: only lifeless and stub can do that15:07
flacostebest to send an email to lifeless15:07
flacosteas stub is flooded15:07
allenapflacoste: Ta.15:15
=== beuno is now known as beuno-lunch
danhgjcsackett: Hey16:36
=== beuno-lunch is now known as beuno
lifelessallenap: flacoste: fixing; we should really have teams in pypi18:00
lifelessallenap: whats your pypi account ?18:02
flacosteyep, we should18:04
lifelessflacoste: dunno if you've seen it, my useoops branch is now 2.7K LoD (+450, -850 LoChange)18:25
lifelessflacoste: mega-branchzilla-incoming-this-week :)18:25
lifelessha!18:38
lifelesshttp://limpesuatela.tumblr.com/18:38
flacostelol18:40
allenaplifeless: Oops, it's gavinpanella.19:03
lifelesson TL call now will add you post that19:03
allenaplifeless: Thanks.19:03
lifelessallenap: added19:33
mwhudson[rollback=14192] back out r14191, <- something looks wrong here <wink>20:11
lifelesslol20:12
mwhudsoni think 14192 is probably correct...20:12
abentleylifeless: I have to go, but could you point out *where* I'm violating the principle that a memo is opaque?21:05
lifelessabentley: int(memo)21:08
lifelessline 87 of the diff21:08
lifelessabentley: storm range factory memos are (IIRC) uuencoded blobs21:08
lifelessabentley: I may have the encoding wrong, but they aren't ints.21:09
abentleylifeless: that's a test helper.21:10
lifelessabentley: ah!. I have a dentist appt, but I will re-review with that in mind afterwards21:11
lifelessabentley: hopefully I just read things badly and its all copacetic21:11
bigjoolsmwhudson, lifeless: mea culpa21:13
lifelessabentley: looking briefly I suspect it will be21:13
mwhudsonbigjools: np, it just made me laugh21:13
bigjoolsI blame bzr for making me think about two versions when backing out!21:14
sinzuiwallyworld, I had to visiting sound setting and toggle mute the other day22:03
sinzuiwallyworld_, The feature is still listed on the board to do: http://launchpad.leankitkanban.com/Boards/View/1272055322:13
pooliehi all, thanks for the review abentley22:18
StevenKsinzui, wallyworld_, jcsackett: http://people.canonical.com/~stevenk/PPA-change.png22:27
wgrantmwhudson: 14192 is the number I gave him at 1:30am, so it's more likely to be wrong, but let me check.23:05
mwhudsonwgrant: i checked23:05
wgrantAh, good.23:05
wgrantpoolie: There may still be hope, then.23:05
wgrantAha, buildbot finished a couple of minutes ago.23:07
pooliewgrant: 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
pooliewbn23:08
wgrantYes.23:08
poolieno, wait, that one is older23:09
wgrantThe general Exception catcher that I reviewed yesterday, anyway.23:10
pooliehttps://code.launchpad.net/~mbp/launchpad/881237-dkim-error/+merge/8041323:10
pooliefailed ec2 overnight because of some spurious timing related error :/23:10
wgrantAh23:10
wgrantFail, then.23:10
pooliereminds me i need to resubmit and file a bug23:10
poolie File "/var/launchpad/test/lib/canonical/launchpad/utilities/ftests/test_gpghandler.py", line 173, in testHomeDirectoryJob23:13
poolie   self.assertTrue(now <= floor(os.path.getmtime(fname)))23:13
wgrantpoolie: Ah, that lovely one.23:15
pooliei haven't looked at it yet but i guess it's asserting something takes less than a second23:15
wgrantNot quite. It's checking that the mtime of some files is current.23:16
wgrantAfter they were set back 12 hours or so and a job run to bring them up to date.23:16
pooliehttps://bugs.launchpad.net/launchpad/+bug/88232423:17
_mup_Bug #882324: timing-dependent spurious test failure in testHomeDirectoryJob <Launchpad itself:Triaged> < https://launchpad.net/bugs/882324 >23:17
lifelesspoolie: why do you think its spurious ?23:28
poolieoh, do you think my code broke it?23:29
pooliemaybe23:29
pooliewe'll see if it fails a second time23:29
poolieanything comparing timestamps in a test looks pretty suspicious to me23:30
pooliebut of course some times they're valid23:30
* poolie looks23:30
wgrantThis one isn't valid.23:30
poolieit obviously is23:31
poolieit is essentially counting on all completing before the clock ticks23:31
poolies//obviously is dangerous23:31
wgrantYep.23:31
pooliehm.23:32
poolieactually i think it must be something more subtle than that23:36
poolie- floating point rounding?23:37
poolie- setting the directory mtime didn't do what we expected?23:38
poolie-23:40
poolie- different precision between time() and getmtime() causing different rounding behaviour?23:40
lifelesspoolie: well the now() is captured before the touching23:42
lifelesspoolie: and they are both floored23:42
pooliemm23:42
poolieso how can it fail?23:42
lifelessI don't see any time sensitivity thing there, though floating point comparisons really do need a precision for safety23:43
poolie- clock nomonotonicity on the test machine23:43
lifelesspoolie: as you say floating point precision may be the cause23:43
poolie*non23:43
poolieperhaps we should remove the 'floor' call on the rhs of the <=23:43
poolieit does not seem to help anything and it may break something23:43
pooliei might set up to run it repeatedly here and see if it ever fails23:44
lifelessthats a good idea23:44
poolieoh, could there be a symlink in that directory?23:53
pooliei think you can't utime() them and that could account for it sometimes being old23:53

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