wallyworld__ | it works locally without doing that | 00:00 |
---|---|---|
wgrant | 2.7's set_charset (called by set_payload) encodes it if it's a unicode. | 00:00 |
wgrant | 2.6 doesn't have that check. | 00:00 |
wallyworld__ | huh? my 2.6 does | 00:02 |
wallyworld__ | if isinstance(self._payload, unicode): | 00:02 |
wallyworld__ | self._payload = self._payload.encode(charset.output_charset) | 00:02 |
wgrant | Ahhh. | 00:02 |
wgrant | Lucid's does not. | 00:02 |
wgrant | I don't think I have 2.6 installed on oneiric... let me check. | 00:02 |
wallyworld__ | it's all thumper's fault | 00:03 |
thumper | it must be | 00:03 |
wallyworld__ | it was going to be a quick patch to help out my old kiwi mate and look what it's turned into | 00:03 |
StevenK | wallyworld__: Welcome to Soyuz. | 00:04 |
StevenK | Oh. Wait. | 00:04 |
wallyworld__ | :-) | 00:04 |
wgrant | wallyworld__: See the latest rev on http://bazaar.launchpad.net/~python-dev/python/2.6/changes?filter_file_id=43136%406015fed2-1504-0410-9fe1-9d1591cc4771%3Apython%252Ftrunk%252FLib%252Femail%252Fmessage.py | 00:05 |
* wallyworld__ looks | 00:05 | |
wgrant | http://bazaar.launchpad.net/~python-dev/python/2.6/revision/42357 | 00:05 |
wgrant | Crucially, from the old email.message docs: | 00:06 |
wgrant | 139 | 00:06 |
wgrant | 00:06 | |
wgrant | The message will be assumed to be of type :mimetype:`text/\*` encoded with | 00:06 |
wgrant | 140 | 00:06 |
wgrant | 00:06 | |
wgrant | *charset.input_charset*. | 00:06 |
wgrant | The new: | 00:06 |
wgrant | 139 | 00:06 |
wgrant | The message will be assumed to be of type :mimetype:`text/\*`, with the | 00:06 |
wgrant | 00:06 | |
wgrant | 140 | 00:06 |
wgrant | payload either in unicode or encoded with *charset.input_charset*. | 00:06 |
wallyworld__ | wgrant: well, the only thing to do then is to assume it's broken and code around it in our stuff | 00:07 |
wallyworld__ | what's the policy with upgrading python on our prod environments? i think it will just be easier to hack around it in our code? | 00:08 |
wgrant | Not hack around it. | 00:08 |
wgrant | The old Python behaviour is not buggy. | 00:08 |
wallyworld__ | you sure? "Also makes the way in which unicode gets encoded to quoted printable for other charsets more sane (it only worked by accident previously)" | 00:09 |
wallyworld__ | seems like they made some changes to fix broken/unintuitive behaviour | 00:09 |
wgrant | Right, but set_payload taking Unicode is behaviour that is new in 2.6.6 or 2.6.7 | 00:10 |
wgrant | Passing Unicode into Lucid's version violates expectations set out in the docstring. | 00:10 |
wgrant | It's certainly an unintuitive response to interface abuse, but it's still interface abuse :) | 00:10 |
wallyworld__ | which means i need to encode outside of set_payload, which is hacking around it | 00:11 |
wgrant | It's not hacking around it. | 00:11 |
wgrant | It's using set_payload properly. | 00:11 |
wgrant | set_payload, until a year ago, was intended to be given encoded data. | 00:12 |
wallyworld__ | except that it accepts a charset parameter which implies it will be doing the encoding based on that charset | 00:12 |
wgrant | The only reason unicode sometimes worked is because Python 2.x's Unicode support is terrible. | 00:12 |
wgrant | How does it imply that? | 00:12 |
wgrant | It | 00:12 |
wgrant | indicates the encoding of the message. | 00:13 |
wgrant | But the docstring is pretty clear. | 00:13 |
wgrant | It expects the message to be in text/*, encoded in the given encoding. | 00:13 |
wallyworld__ | sure. | 00:13 |
StevenK | So it wants UTF-8, not unicode | 00:13 |
wallyworld__ | perhaps i was misusing the term "hacking" | 00:13 |
wallyworld__ | i was being loose with my definition | 00:14 |
wgrant | Perhaps. | 00:14 |
wallyworld__ | i just meant i have to make changes to accommodate the old interface | 00:14 |
wgrant | Right. | 00:14 |
wallyworld__ | even though the new one supports what we want | 00:14 |
wallyworld__ | is was using hacking the the sense of "code hacking" ie code writing | 00:14 |
wallyworld__ | anyways | 00:15 |
wgrant | Anyway, easy fix, hopefully. | 00:15 |
wallyworld__ | yep | 00:15 |
wallyworld__ | so long as my system stays up | 00:15 |
* wallyworld__ wishes he had an SSD | 00:18 | |
wallyworld__ | wgrant: can you try it now? | 00:26 |
wallyworld__ | wgrant: i wasn't sure whether to use content.encode(charset) or content.encode(charset.output_charset). i used the first one | 00:28 |
wgrant | Running: | 00:50 |
wgrant | lp.code.mail.tests.test_branch.TestBranchMailerDiff.test_generateEmail_with_diff | 00:50 |
wgrant | Ran 1 tests with 0 failures and 0 errors in 0.537 seconds. | 00:50 |
wgrant | wallyworld__: ^^ | 00:50 |
wallyworld__ | wgrant: excellent! thanks for your help | 00:50 |
wgrant | wallyworld__: How does this work with non-UTF-8 files? | 00:51 |
wallyworld__ | wgrant: it works fine. there's other tests which use plain ascii diffs | 00:52 |
wgrant | ASCII diffs are also UTF-8. | 00:54 |
wallyworld__ | perhaps i misunderstood your question | 00:54 |
wgrant | What happens if I try to diff a UTF-16 file? | 00:55 |
wallyworld__ | the charset parameter to addAttachment() is optional | 00:55 |
wgrant | Yes, but BranchMailer always uses utf-8. | 00:56 |
wallyworld__ | the check is isinstance(payload, unicode) | 00:56 |
lifeless | bzr treats all files as bytestreams for diff | 00:57 |
poolie | mm kinda | 00:57 |
poolie | i'm pretty sure a utf-16 file will be reported as 'binary files differ' | 00:57 |
lifeless | right, but we don't do the diff as unicode | 00:58 |
lifeless | so diffing a slavic codepage file against the same text in utf8 will show the encoded difference, not the decoded difference. | 00:58 |
lifeless | Unless that has changed? | 00:59 |
poolie | that's correct | 00:59 |
poolie | it could usefully be changed, perhaps, but that's true at the moment | 01:00 |
lifeless | and will include slavic codepage + utf8 in the output | 01:00 |
lifeless | so the output of diff, today, doesn't have a 'safe correct' encoding. File paths we encoded to the supplied encoding, but thats the only bit of user data we have a clear content type for | 01:00 |
wgrant | That is my concern. | 01:00 |
wallyworld__ | wgrant: so the isinstance(xxx, unicode) check prevents utf-16 strings from being touched by any utf-8 stuff | 01:01 |
wgrant | Perhaps, but how does it get a unicode in the first place? | 01:02 |
wgrant | There's no unicode-safe way to handle diffs. They have to be treated as bytestreams. | 01:02 |
lifeless | erm, it stops unicode strings being processed, but the intputs are not decoded, so utf16 would still be there as utf16 (if it doesn't trip the binary check poolie mentioned) | 01:02 |
wallyworld__ | not sure. maybe thumper knows since he raised the bug | 01:02 |
wgrant | And yes, we're probably still going to be declaring UTF-16 as UTF-8, which would be Bad. | 01:02 |
lifeless | I think he was hoping it was easy; its not. | 01:02 |
wgrant | It's easy unless you've dealt with encodings before :) | 01:03 |
lifeless | personally, I'd triage it low, add a task asking for a way to get *a* defined content coding out of bzr diff, and then let the discussion happen. | 01:03 |
wallyworld__ | wgrant: why would we be declaring utf-16 as utf-8? | 01:04 |
wallyworld__ | actually, we are, but that's a bug i will fix | 01:04 |
wgrant | Fix how? | 01:05 |
wgrant | We need to investigate how we are getting decoded Unicode diffs in the first place. | 01:05 |
wgrant | Since that doesn't really make sense. | 01:05 |
wallyworld__ | wgrant: the utf-8 encoding only occurs if the payload is a unicode instance | 01:06 |
wallyworld__ | and since utf-16 is not a unicode instance, we are not declaring the payload to be utf-8 | 01:07 |
wallyworld__ | via the content type header | 01:07 |
wgrant | Yes, but there's no way that can correctly be a unicode instance. | 01:08 |
wgrant | So the special case will either never be used, or there is another bug creating "unicode" diffs. | 01:08 |
wallyworld__ | i'm not sure how the diffs get generated etc. the request was "when we attach diffs to outgoing code review and branch emails can we please utf-8 encode" | 01:10 |
wallyworld__ | so if the diffs are never unicode in the first place, that's a separate issue to solve i guess | 01:11 |
wgrant | I presume he wants them *declared* as UTF-8. | 01:11 |
wgrant | They were probably already mostly encoded in it. | 01:11 |
lifeless | the committer and paths will be utf8 | 01:11 |
lifeless | whats probably happening is that we have a TEXT column note a BYTEA column. | 01:12 |
wgrant | Diffs are in the librarian, or not stored at all. | 01:12 |
wgrant | Not in the DB. | 01:12 |
lifeless | or we're treating the text from the librarian as utf8 and decoding it, we'll be getting punning (and sporadic failures when smeone has garbage in their tree | 01:12 |
wallyworld__ | the last part of the request was "we have people with weird names committing or .po file stuff et al" | 01:12 |
wgrant | Storm will generally correctly kick you in the face if you try to do bad Unicode stuff. | 01:13 |
wallyworld__ | so yes, it's to do with the committer etc | 01:13 |
wgrant | Sounds like file content: PO headers. | 01:13 |
wallyworld__ | yes | 01:13 |
wallyworld__ | and such stuff would be in the diffs. but if the diffs never arrive at the send mail method as unicode..... | 01:14 |
poolie | lifeless, on the command line you do get a defined decoding out of bzr diff | 01:14 |
lifeless | poolie: ah, good. I though it was still a bit hairy. | 01:14 |
poolie | i'm 90% sure if you have a utf-8 locale and you diff from utf8 to cpwhatever, the + lines will have questionmarks | 01:14 |
poolie | mm | 01:15 |
poolie | i have seen the bug traffic but i'm not sure what the actual motivating case is | 01:15 |
wgrant | poolie: Hm, that's pretty unpleasant. | 01:15 |
wgrant | The diff not being a diff. | 01:15 |
lifeless | wgrant: it has the great advantage of not turning peoples terminals into mojibake | 01:15 |
wallyworld__ | poolie: you mean for bug 861979? | 01:15 |
_mup_ | Bug #861979: utf-8 encode diffs attached to outgoing email <Launchpad itself:In Progress by wallyworld> < https://launchpad.net/bugs/861979 > | 01:16 |
poolie | well, i think you can choose between "accurate but not in the right encoding" and "in the right encoding but not complete" | 01:16 |
poolie | at the api level you can | 01:16 |
wgrant | lifeless: That's true, but it has the disadvantage of being wrong. | 01:16 |
lifeless | wgrant: given there is no right answer for a unicode terminal, I find asserting its wrong to be...interesting :) | 01:16 |
poolie | lifeless, thanks, i have seen that but it's not very complete | 01:16 |
wgrant | lifeless: It is not a correct diff. | 01:16 |
poolie | if the actual thing is that they have validly encoded file names | 01:17 |
poolie | then utf-8 encoding the output diff would be a reasonable improvement | 01:17 |
poolie | it's a simpler case than having arbitrary binary transforms | 01:17 |
poolie | wgrant, the question is whether you are generating the diff to feed to patch (to apply it as a binary diff) or to look at it | 01:17 |
poolie | the two things need different results | 01:18 |
wgrant | Yes. | 01:18 |
wallyworld__ | so, given all this, am i correct in thinking lp is not given a unicode diff to attach to its emails? | 01:20 |
=== wgrant changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: wallyworld | Critical bugs: 270 - 0:[########Segmentation fault (core dumped) | ||
wgrant | lifeless: Shouldn't bug #869064 be High? It's not a released feature yet. | 01:25 |
_mup_ | Bug #869064: Batched comment loading should update the page URL to include ?comments=all <story-batched-comment-loading> <Launchpad itself:In Progress by gmb> < https://launchpad.net/bugs/869064 > | 01:25 |
wgrant | Bug #867529, too. | 01:26 |
_mup_ | Bug #867529: Dynamic loading comments load on top of page content <regression> <story-batched-comment-loading> <Launchpad itself:Triaged> < https://launchpad.net/bugs/867529 > | 01:26 |
wgrant | Or do they inherit criticality? | 01:26 |
poolie | wallyworld__, didn't you already land a fix for this? | 01:28 |
wallyworld__ | poolie: it failed ec2 due to python differences on lucid vs oneiric | 01:28 |
wallyworld__ | so it's still not landed | 01:28 |
wallyworld__ | those issues are now fixed | 01:28 |
wallyworld__ | but is there any point? | 01:28 |
wallyworld__ | if the diffs passed to lp are never unicode? | 01:29 |
poolie | 1- you need to be more precise about 'unicode' | 01:31 |
poolie | 2- it depends on how it's calling bzr (external 'bzr diff', through the api, etc) and what locale it is in | 01:31 |
wallyworld__ | poolie: unicode as in a python string u'xxxxx' ie isinstance(diff, unicode). i'm not sure what the underlying encoding details would be | 01:32 |
lifeless | wgrant: its behind a feature flag? I thought the flag was toggled ? | 01:32 |
poolie | it's called inprocess? | 01:32 |
wgrant | lifeless: I hope it's only released for betatesters... | 01:33 |
wgrant | bugs.batched_comment_loading.enabledteam:launchpad-beta-testers1on | 01:33 |
wgrant | bugs.batched_comment_loading.enabledteam:launchpad0on | 01:33 |
lifeless | fixed | 01:34 |
wallyworld__ | poolie: 2. i'm not sure how it's being called, will have to check. it's for when lp sends out codereview and branch emails. po files have committer info in the header and we want these to come through properly for international names | 01:34 |
wgrant | lifeless: However, it could possibly be priority inheritance. | 01:34 |
lifeless | wgrant: possibly; if so gmb can and (hopefully will) say 'its critical cause I'm using it to fix bug ABCDE' | 01:35 |
poolie | wallyworld__, i think if you call the right api you will get unicode diff results | 01:44 |
poolie | if there is now way to get them file a bzr bug and we'll help | 01:44 |
poolie | but i would be surprised, because it works from the command line | 01:44 |
wallyworld__ | poolie: i think the show_diff_trees api is being used | 01:44 |
poolie | hm apparently that returns a byte stream, probably utf8 encoded | 02:08 |
poolie | the way it is handled in lp seems to suggest it's expected to be a byte stream too | 02:08 |
wallyworld__ | poolie: so the change in lp is to essentially make the Content-Type header say "text/x-diff; charset="utf-8"" | 02:19 |
wallyworld__ | i think based on what you said that will be ok? | 02:19 |
poolie | i think so | 02:32 |
poolie | does it work? | 02:32 |
poolie | out for a bit | 02:41 |
nigelb | Morning! | 03:13 |
spm | o/ nigelb | 03:15 |
nigelb | ohai spm :) | 03:17 |
nigelb | wallyworld__: How's your love affair with ec2 going? :P | 03:17 |
wallyworld__ | nigelb: well, the problem is fixed. it was a python 2.6 version issue. python email encoding on lucid behaves differently to on oneiric | 03:22 |
nigelb | wallyworld__: Oh. | 03:25 |
nigelb | In a good way or bad way? | 03:25 |
wallyworld__ | nigelb: different. here a link to the diff http://bazaar.launchpad.net/~python-dev/python/2.6/revision/42357 | 03:27 |
wallyworld__ | it now accepts unicode payloads and checks that a payload is unicode before it encodes it using the supplied charset | 03:28 |
nigelb | \o/ | 03:29 |
nigelb | Nice find :) | 03:29 |
wallyworld__ | nigelb: wgrant looked at his copy of python 2.6 on lucid to discover the difference in behaviour and provided me the diff | 03:33 |
nigelb | Neat | 03:33 |
lifeless | aieieeeeeee | 04:19 |
lifeless | https://bugs.launchpad.net/launchpad/+bug/869631/comments/6 | 04:19 |
_mup_ | Bug #869631: BugTask:+editstatus timeout closing bug 353126 due to bug subscription lookups <oops> <Launchpad itself:Triaged> < https://launchpad.net/bugs/869631 > | 04:19 |
wgrant | lifeless: So the query is returning unrelated mutes? | 04:21 |
lifeless | yes | 04:21 |
lifeless | muting any bug will mute you for all notifications via duplicates | 04:21 |
wgrant | That's the production query that's insane? | 04:21 |
wgrant | Oh. | 04:21 |
wgrant | Isn't that intended? | 04:21 |
lifeless | *all* | 04:21 |
lifeless | mute bug 1. | 04:21 |
_mup_ | Bug #1: Microsoft has a majority market share <iso-testing> <ubuntu> <Clubdistro:Confirmed> <Computer Science Ubuntu:Confirmed for compscibuntu-bugs> <dylan.NET.Reflection:Invalid> <dylan.NET:Invalid> <EasyPeasy Overview:Invalid by ramvi> <GenOS:In Progress by gen-os> <GNOME Screensaver:Won't Fix> <Ichthux:Invalid by raphink> <JAK LINUX:Invalid> <LibreOffice:In Progress by bjoern-michaelsen> <Linux:New> <Linux Mint:In Progress> <The Linux OS P | 04:21 |
lifeless | then for 12134 a dupe of 456678 you won't get notified *if you would have previously* | 04:22 |
wgrant | Oh! | 04:22 |
wgrant | Unrelated duplicates. | 04:22 |
wgrant | I see. | 04:22 |
lifeless | constraining the subquery makes it a 2ms query | 04:25 |
wgrant | Heh | 04:25 |
wgrant | What if you just s/Bug,//? | 04:25 |
wgrant | That should fix it too, though it'll still be a subquery. | 04:26 |
lifeless | 3ms | 04:27 |
lifeless | all in the bug | 04:32 |
lifeless | but - wow | 04:32 |
wgrant | Yes. | 04:32 |
lifeless | its also worrying that its doing this multiple times | 04:32 |
wgrant | Most of the advanced subscriptions work is like that. | 04:32 |
wgrant | Repetitious. | 04:32 |
lifeless | all dupes are handled by one of these queries, why does it need to do it more than once | 04:32 |
=== almaisan-away is now known as al-maisan | ||
huwshimi | Have a good weekend all | 06:58 |
=== al-maisan is now known as almaisan-away | ||
=== almaisan-away is now known as al-maisan | ||
=== vila is now known as vila_at_dentist | ||
=== wallyworld__ changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - | Critical bugs: 270 - 0:[########Segmentation fault (core dumped) | ||
mrevell | Hey there | 08:01 |
=== vila_at_dentist is now known as vila | ||
lifeless | poolie: wow, silentghost, wow. | 08:58 |
lifeless | allenap: the reason its slow to kill the machine is that its a infinite loop | 08:58 |
lifeless | allenap: but failure in err() isn't the same as failure in a deferred | 08:59 |
bigjools | lifeless: yeah we figured that out just now in our call | 08:59 |
lifeless | allenap: so when it fails after an oops, thats in a deferred. It has no errback() | 08:59 |
lifeless | allenap: so that triggers a __del__ failure, and round it goes. | 09:00 |
lifeless | bigjools: cool | 09:00 |
bigjools | lifeless: Python dynamic typing sucks :) | 09:00 |
poolie | lifeless, oh in the python bug? | 09:00 |
lifeless | poolie: yeah | 09:01 |
poolie | i agree it's not the nicest part of python | 09:01 |
lifeless | bigjools: s/Python// and then yes, sure ;) | 09:01 |
poolie | also by analogy to other things, you might think that 'callable(x)' *coerces* x to be callable | 09:02 |
lifeless | bigjools: a friend of mine, erik de casto lopo, *loathes* dynamic typed languages. | 09:02 |
poolie | as for dict, int, etc | 09:02 |
bigjools | it also is the best thing since sliced bread | 09:02 |
lifeless | poolie: I believe thats one of the criticisms of callable | 09:02 |
bigjools | yeah, it requires a different mindset | 09:02 |
bigjools | was hard for me to adjust coming from C++ | 09:02 |
lifeless | bigjools: he loathes c++ too :P [like most of us, he is opinionated ;P] | 09:03 |
bigjools | :) | 09:03 |
StevenK | Erik is a good guy | 09:03 |
bigjools | what does he use, $obscure_language? | 09:03 |
StevenK | OCaml | 09:03 |
poolie | yes, ocaml | 09:03 |
StevenK | And Haskell | 09:03 |
lifeless | and C :) | 09:04 |
lifeless | http://mstation.org/erikdecl.php | 09:04 |
poolie | i was writing some python just now and thinking how much better it would be in a functional language, up to the point i needed a library | 09:04 |
lifeless | a little dated interview, before his functional phase AFAICT, but still gets him about right ;) | 09:04 |
poolie | oh i thought that was meant to convey that he liked php :) | 09:04 |
bigjools | I remember using proof by induction with functional languages. Head fuck. | 09:05 |
bigjools | php is right up there with perl | 09:05 |
bigjools | or is that down there... :) | 09:05 |
lifeless | bigjools: btw, buikld status 6 + log -> what does that mean ? | 09:06 |
bigjools | lifeless: need a bit more context than that! | 09:06 |
StevenK | Build Status 6 == BUILDING | 09:06 |
StevenK | But it has a log set | 09:06 |
StevenK | And date_finished | 09:06 |
lifeless | bigjools: there was a bug filed by spm yesterday | 09:06 |
bigjools | it's bug 717969 | 09:07 |
_mup_ | Bug #717969: storeBuildInfo is sometimes ineffective <boobytrap> <buildd-manager> <qa-untestable> <Launchpad itself:In Progress by jtv> < https://launchpad.net/bugs/717969 > | 09:07 |
lifeless | bigjools: cause nagios etc | 09:07 |
lifeless | StevenK: thanks | 09:07 |
bigjools | lifeless: dupe it to that one then | 09:07 |
bigjools | jtv is working on it | 09:07 |
lifeless | bigjools: I figured jtv would want to look at it, but I was also curious about what it means | 09:07 |
bigjools | it's a transaction issue across Deferreds | 09:07 |
StevenK | But how? storeBuildInfo() sets the log and so on, doesn't it? | 09:07 |
lifeless | bigjools: well, I think the bug is about data cleanup - or will you sort them all out at once once root cause is fixed ? | 09:07 |
bigjools | what's spm's bug? | 09:08 |
lifeless | bug 868917 | 09:08 |
bigjools | StevenK: we found a load of places where a commit() in one callback was committing more than it should | 09:08 |
_mup_ | Bug #868917: broken build alert <canonical-losa-lp> <ops> <Launchpad itself:Triaged> < https://launchpad.net/bugs/868917 > | 09:08 |
bigjools | lifeless: I say dupe it for now unless proven otherwise, 717969 is the most likely cause. /me dupes | 09:09 |
lifeless | bigjools: do we need to do the data cleanup for it ? | 09:10 |
bigjools | yes | 09:10 |
allenap | lifeless: Cheers for the explanation. I thought it was probably something like that, but it's good to know exactly what. | 09:10 |
lifeless | allenap: (thats a theory, but it fits all the facts I know about this) | 09:10 |
bigjools | lifeless: I believe the losas have a script | 09:11 |
cjwatson | bigjools: are you up for a chat about bug 572128 nowish? | 10:02 |
_mup_ | Bug #572128: Ubuntu Archive translations are missing Index metadata file <regression> <Launchpad itself:Triaged by cjwatson> <debmirror (Ubuntu):Confirmed> < https://launchpad.net/bugs/572128 > | 10:02 |
bigjools | cjwatson: sure, good timing | 10:04 |
bigjools | cjwatson: want to jump on voip/skype? | 10:04 |
=== al-maisan is now known as almaisan-away | ||
wgrant | bigjools, cjwatson: Did you get that sorted out? I guess it's pretty urgent. | 11:19 |
bigjools | yes | 11:26 |
cjwatson | bigjools: there's an XXX from cprov suggesting using a generic Release parser, and I really don't want to perpetrate even more similar horrible code - would it be OK to use debian.deb822.Release to test the contents of Release files? | 11:31 |
bigjools | cjwatson: hell yes | 11:31 |
cjwatson | then I might fix the existing code while I'm here, too | 11:32 |
bigjools | champion :) | 11:32 |
bigjools | AttributeError: type object 'ZopelessDatabaseLayer' has no attribute 'switchDbUser' | 11:33 |
bigjools | waaaaaa | 11:33 |
wgrant | bigjools: You need to use LaunchpadZopelessLayer for switchDbUser | 11:34 |
wgrant | bigjools: BUT | 11:34 |
wgrant | bigjools: As of two weeks ago that's a deprecated API. | 11:34 |
wgrant | Use lp.testing.dbuser instead. | 11:34 |
wgrant | Works in any layer. | 11:34 |
bigjools | I am not using it, my parent test class is | 11:34 |
bigjools | deep in the publisher | 11:34 |
wgrant | Use LaunchpadZopelessLayer, or fix your parent to use lp.testing.dbuser directly. | 11:34 |
wgrant | switchDbUser is now just a trivial wrapper. | 11:35 |
bigjools | going former for now, although startup time PAIN | 11:35 |
wgrant | The Zopeless layers will soon be abolished, but I need to sort out different security policies and blah. | 11:36 |
bigjools | why? | 11:36 |
wgrant | Because there's nothing special about them any more. | 11:36 |
wgrant | Basically, they run in PermissiveSecurityPolicy instead of LaunchpadSecurityPolicy, and they don't have WSGI interceptors installed. | 11:36 |
wgrant | Apart from that they are interchangeable. | 11:36 |
bigjools | so I can just have a DatabaseLayer? | 11:36 |
wgrant | You probably actually want DatabaseFunctionalLayer, but yes. | 11:37 |
wgrant | Watch out for the security policy difference, though. | 11:37 |
bigjools | I probably don't | 11:37 |
wgrant | You don't need the CA at all? | 11:37 |
bigjools | CA? | 11:37 |
wgrant | Component Architecture | 11:37 |
bigjools | yes, but I don't want the bloody librarian | 11:38 |
wgrant | DatabaseFunctionalLayer. | 11:38 |
wgrant | Unless the layer says "Launchpad" or "Librarian", there's no librarian. | 11:38 |
bigjools | good | 11:38 |
bigjools | wfm | 11:39 |
wgrant | DatabaseFunctionalLayer is basically the same as ZopelessDatabaseLayer, except that the order of the name is inexplicably different and the security policy is different. | 11:39 |
wgrant | And, until two weeks ago, stuff needed to use *Zopeless* if it wanted to change DB users. | 11:39 |
wgrant | But that was all BS historical crap. | 11:39 |
wgrant | Which I deleted. | 11:39 |
bigjools | yeah | 11:39 |
bigjools | man, storm is really frustrating sometimes | 11:40 |
wgrant | Oh? | 11:40 |
bigjools | "expected int, found Reference" | 11:41 |
wgrant | Ah. | 11:41 |
wgrant | Well, that's sort of reasonable. | 11:41 |
wgrant | What's unfortunate is that you can't do Reference == Reference | 11:41 |
bigjools | nonetheless frustrating | 11:41 |
wgrant | s/unfortunate/pointless/ | 11:41 |
=== bac changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: bac | Critical bugs: 270 - 0:[########Segmentation fault (core dumped) | ||
jml | wgrant: fwiw, ZDL came after DFL | 12:04 |
jml | wgrant: so now the only difference between "zopeless" and "functional" is default security policy, right? | 12:05 |
=== almaisan-away is now known as al-maisan | ||
danilos | jcsackett, hi, I have a feeling that bug 627631 was not really fully solved, and bugtasks for other applications have gone (like translations) in our reorg: can you confirm that suspicion I have? :) | 14:35 |
_mup_ | Bug #627631: Queries need to be updated to use usage enums, where possible <bridging-the-gap> <lp-registry> <qa-ok> <Launchpad itself:Fix Released by jcsackett> < https://launchpad.net/bugs/627631 > | 14:35 |
sinzui | danilos, many were reintroduced after the branch landed. | 14:40 |
danilos | sinzui, fwiw, I only saw one for official_rosetta with an XXX from jcsackett pointing to the bug above (other usages of official_rosetta seem to be in tests) | 14:42 |
sinzui | I added two official_malone recently. I was reading the schema. I think we need to change the schema to make it obvious that official_* is not always correct | 14:43 |
danilos | sinzui, I think we can remove the official_rosetta with that one fix if that was the ultimate goal | 14:44 |
sinzui | yes I think so to | 14:44 |
sinzui | jcsackett, pointed out my mistake and said we might be able to fix everything on in a single branch | 14:45 |
cjwatson | bigjools: https://code.launchpad.net/~cjwatson/launchpad/i18n-index/+merge/78618 for your delectation and delight | 15:12 |
bac | sinzui: do you know how the mechanism for automatically hiding the notification box works? | 15:15 |
sinzui | bac: no. I do not think I have ever seen a notification hide | 15:16 |
sinzui | bac: do you mean the click to hide that benji had worked on? | 15:17 |
bac | sinzui: or maybe it hid so fast you didn't notice! | 15:17 |
bac | sinzui: no, it is autohiding now, at least for the bug_acknowledgement_message | 15:17 |
bac | sinzui: i've searched for the JS that is doing it but haven't found it yet | 15:17 |
sinzui | I am clueless | 15:18 |
bac | sinzui: as described in bug 846163 | 15:18 |
_mup_ | Bug #846163: bug filing notice is removed before the user can read it <regression> <ubuntu-qa> <Launchpad itself:In Progress by bac> < https://launchpad.net/bugs/846163 > | 15:18 |
bac | sinzui: thanks anyway | 15:18 |
bac | i thought it would be somewhere like client.js... | 15:18 |
benji | sinzui: unfortunately I was never able to get the click-to-hide stuff just right, so I took it out | 15:18 |
bac | now it is blink-to-hide | 15:18 |
* danilos blinks | 15:19 | |
sinzui | benji, yes, I recall that. I assumed that bac was talking about a replacement implementation | 15:19 |
benji | ah | 15:19 |
abentley | flacoste: would it be terrible if CanonicalConfig.process_name was None when sys.argv is unavailable? | 15:29 |
cjwatson | bigjools: incidentally, I've run the soyuz tests now, as expected no problems there | 15:36 |
bigjools | cjwatson: excellent | 15:39 |
bigjools | I wish mine would pass | 15:40 |
=== beuno is now known as beuno-lunch | ||
=== al-maisan is now known as almaisan-away | ||
dobey | is there any way you guys can get oops logs synced faster? like on-demand? :) | 16:21 |
dobey | or on-crash as it were | 16:22 |
deryck | abentley, hey…. I have to take a long lunch today to help Wendy with something shop related. But I'll be around longer this afternoon. | 16:22 |
cjwatson | bigjools: is that merge looking plausible? sorry to rush, I just really want to get this into ec2 today if possible so that there's some chance of deploying it on Monday ... | 16:23 |
abentley | deryck: okay. I'll be taking lunch later than usual, so we may still miss :-) | 16:23 |
deryck | ah, ok :) | 16:23 |
bigjools | cjwatson: yeah, nearly done reviewing it | 16:24 |
bigjools | cjwatson: if you want to start making changes now, you need to fix the tests to make sure all the uses of open() close the file | 16:25 |
bigjools | so lots of with: :) | 16:25 |
cjwatson | hm, right | 16:25 |
cjwatson | can I get away with just changing the ones I was touching anyway? it's an endemic problem in that test file | 16:26 |
bigjools | yeah I see :/ | 16:28 |
bigjools | yeah fix those | 16:28 |
bigjools | cjwatson: also, we generally use assertEqual(expected, observed) so that the matcher debug output is correct | 16:29 |
cjwatson | aha, ok, I couldn't see docs of which way round it went | 16:29 |
cjwatson | cool | 16:29 |
bigjools | pycharm tells me :) | 16:29 |
bigjools | cjwatson: let me know when you make the changes and I'll land it | 16:32 |
cjwatson | ok, working on those now | 16:33 |
dobey | eww | 16:35 |
dobey | AttributeError: 'LaunchpadTimeoutError' object has no attribute '__traceback__' | 16:35 |
bigjools | cjwatson: I need to dash, either get someone else to ec2 land it or leave me an email | 16:37 |
=== beuno-lunch is now known as beuno | ||
=== deryck is now known as deryck[lunch] | ||
=== cody-somerville_ is now known as cody-somerville | ||
cjwatson | I've made the changes bigjools requested to https://code.launchpad.net/~cjwatson/launchpad/i18n-index/+merge/78618; could somebody send it to ec2 land for me? | 17:12 |
flacoste | abentley: no, it wouldn't be perfectly fine, it's only used to find a config file and defaults to launchpad-lazr.conf if the conf file doesn't exist | 17:43 |
flacoste | abentley: i'm curious though, what can make sys.argv not available? | 17:43 |
bac | hi flacoste, do you know of any work recently to make request.response.notifications hide themselves? i'm trying to find the JS that does it but can't locate it. | 17:48 |
flacoste | bac: you mean the code that allows you to dismiss notifications? | 17:49 |
bac | flacoste: no, the blue box shows up but disappears after 1/2 second or so | 17:49 |
bac | on its own | 17:49 |
flacoste | bac: hmm, no, i'm not aware of anything that might have introduced this | 17:49 |
cjwatson | bac: can I have a quick review of https://code.launchpad.net/~cjwatson/launchpad/precise-sync-testing/+merge/78646 ? | 17:49 |
bac | cjwatson: sure | 17:49 |
bac | flacoste: ok, maybe it is an unintended consequence of something else, like maybe the privacy notifications | 17:50 |
bac | cjwatson: approved. do you need me to land it for you? | 17:54 |
cjwatson | bac: yes please (sorry for delay, was sorting out dinner) | 18:10 |
bac | cjwatson: will do | 18:10 |
cjwatson | bac: if you could also land https://code.launchpad.net/~cjwatson/launchpad/i18n-index/+merge/78618 (already approved by Julian) that would be marvellous | 18:10 |
bac | cjwatson: man, now you're getting pushy! :) | 18:10 |
bac | cjwatson: could you please set commit messages on both merge proposals, if not done already? | 18:11 |
cjwatson | already done on i18n-index, will do so on precise-sync-testing now | 18:13 |
cjwatson | done; don't know if the latter needs [no-qa] or similar | 18:14 |
cjwatson | or if that gets added by tools | 18:14 |
bac | cjwatson: i've sent them both off to ec2. i forgot to mark the one [no-qa] but you can tag the bug after it gets deployed to qastaging | 18:40 |
bac | cjwatson: after today i'm gone for a while, so you'll have to get someone else to follow up with these branches if they don't land | 18:41 |
=== deryck[lunch] is now known as deryck | ||
cjwatson | bac: thanks! | 19:19 |
bac | np | 19:19 |
abentley | flacoste: When run under mailman, apparently. If anything in lp_sitecustomize.py imports canonical.config, mailman starts spewing errors. AIUI, this is expected behaviour for embedded interpreters: http://bugs.python.org/issue839151 | 19:40 |
flacoste | abentley: ah ok, so yes, guarding the assignment of process_name based on sys.argv is the right thing to do | 19:41 |
flacoste | everything should still work if self._process_name is None | 19:41 |
benji | I have an open question which is about changing the maintainer of a project. Any hints as to how to do that? | 20:00 |
lifeless | bac: flacoste: its the ajax notification support added by wallyworld | 20:02 |
lifeless | bac: the first ajax request completes, has no notifications in it, and so the existing ones are nuked | 20:02 |
benji | I suspect I need a registry team member. | 20:02 |
flacoste | benji: click on the maintainer and select somebody else? | 20:02 |
flacoste | benji: you are one | 20:02 |
flacoste | by virtue of ~launchpad | 20:03 |
lifeless | bac: I think I suggested in the bug, to put a X button on the notifications and have the ajax embedded notifications only ever *add* to them. | 20:03 |
sinzui | benji, click on the edit icon next to the maintainer name | 20:03 |
benji | flacoste: the maintainer is a link to the user's page and there isn't an edit icon for maintainer | 20:03 |
* sinzui looks | 20:04 | |
flacoste | benji: on https://launchpad.net/launchpad-project, do you see the edit icon on the maintainer? | 20:04 |
sinzui | benji, sorry. I was thinking team | 20:04 |
sinzui | benji, only a LOSA can change a project maintainer | 20:04 |
flacoste | or if you already the maintainer | 20:05 |
flacoste | which explains why i see it on launchpad-project | 20:05 |
bac | lifeless: do you know when that change landed? | 20:05 |
benji | flacoste: I do there but not on https://launchpad.net/fusionforge | 20:05 |
flacoste | yep, what sinzui says: ask a losa | 20:05 |
lifeless | bac: end of the recipes project | 20:05 |
benji | sinzui: awesome, thanks | 20:05 |
lifeless | bac: they were doing ajax build queueing or something like that and needed to show a warning / notification as a result of the call | 20:06 |
bac | lifeless: so that was a good while ago | 20:06 |
lifeless | yes; we have a deluge of bugs ;) | 20:06 |
bac | lifeless: i've been going through our JS code but cannot find the relevant code. | 20:07 |
lifeless | bac: wallyworld__ tried to fix it at the time, and I think he did, but something unfixed it shortly after and we didn't notice immediately | 20:07 |
lifeless | bac: huh. | 20:07 |
bac | there is a section in client.js that deals with notifications | 20:07 |
sinzui | benji, I do not know why I advised you badly the first time. I was thinking a few hours earlier that If I could temporarily change project maintainer, I could fix 777 licenses | 20:08 |
benji | sinzui: heh :) | 20:08 |
lifeless | bac: rev 12779 | 20:08 |
bac | lifeless: thanks! | 20:09 |
lifeless | bac: is the most recent thing - I remember now. wallyworld__ made it so that choosing to reset the notifications is explicit (rather than them always resetting) | 20:09 |
bac | wow, april | 20:09 |
lifeless | bac: this implies that the reset that is happening is explicit (and wrong) - it may be right other times, which means we have an unreconcilable situation and will need to find some way to cut the knot | 20:10 |
lifeless | bac: I found it by doing bzr log and looking for 'notification' until I saw one that looked relevant ;) | 20:10 |
sinzui | statement_timeout I should use when suggesting queries to in production? 2 seconds | 20:25 |
lifeless | yes | 20:30 |
lifeless | 2000 ms | 20:30 |
sinzui | thank | 20:36 |
sinzui | s | 20:37 |
lifeless | n | 20:37 |
lifeless | o probs | 20:37 |
lifeless | I'm reminded of captain kirks speech patterns | 20:37 |
sinzui | :) | 20:37 |
timrc | sinzui, Did cody-somerville point you at our automated PPA creation code yet? | 20:42 |
cody-somerville | timrc, No. | 20:42 |
timrc | :( | 20:43 |
lifeless | flacoste: bug 757298 - I've just commented - it was critical by inheritance | 20:44 |
_mup_ | Bug #757298: database bloat report ordering needs tuning <critical-analysis> <qa-ok> <Launchpad itself:Fix Released by stub> < https://launchpad.net/bugs/757298 > | 20:44 |
flacoste | lifeless: ack | 20:45 |
lifeless | flacoste: I've commented to that effect (via email so not there yet) but not changed the status | 20:46 |
lifeless | flacoste: I think it could go either way | 20:46 |
flacoste | lifeless: i'll change the status (and the category in my analysis) if I'm swayed by your argument :-) | 20:46 |
lifeless | flacoste: :) | 20:47 |
lifeless | flacoste: so the argument is that index bloat had caused timeout storms twice already by april | 20:47 |
lifeless | flacoste: and that the report helps us get on top of it earlier | 20:47 |
flacoste | that's fair | 20:48 |
sinzui | lifeless, I wanted to run this in production, but it timesout under cold cache: http://pastebin.ubuntu.com/704176/ Do you have thoughts about making this faster? Should I take the extra pain of writing a garbo job, land, released, revert? | 20:54 |
lifeless | garbo please | 20:54 |
sinzui | okay | 20:54 |
lifeless | or | 20:54 |
lifeless | and this is a little radical | 20:55 |
lifeless | could an API script do it ? | 20:55 |
lifeless | sinzui: the second half - the 77 rows - that should be totally fine on prod | 20:55 |
sinzui | well interesting proposal. I the the first part that is fine under cold cache is doable over api | 20:55 |
sinzui | I do not think the second part that is slow is doable. But I have yet to think about all the cases that those 77 packages are missing | 20:56 |
lifeless | sinzui: ah I see your times in -ops | 20:56 |
lifeless | sinzui: your methodology has a hole in it :) | 20:56 |
sinzui | yep | 20:56 |
lifeless | sinzui: try this: set the statement timeout to 15 seconds; run the *select clauses only* for both queries outside a transaction; set the statement timeout to 2000; BEGIN; do the INSERT queries; ROLLBACK; | 20:57 |
sinzui | ah, build a hot cache | 20:58 |
lifeless | we care about the duration of locks, not the overall load [well we care about both, but you see what I mean] | 20:58 |
sinzui | I do | 20:58 |
lifeless | there is some overhead you can't pre-load when inserting into the table, because indices have to be updated (which means reading them in) | 20:59 |
lifeless | so to test this a second time, use a different slave | 20:59 |
lifeless | as long as we rollback, testing something like this on a slave is fine | 20:59 |
sinzui | okay thank is very helpful to know | 21:00 |
lifeless | sinzui: another strategy is to run the exact query in a loop with rollback and low timeout until it works | 21:34 |
lifeless | we'll release any locks every (say) 1 second [the timeout] and thus won't cause timeouts | 21:35 |
flacoste | lifeless: should yuo be assigned to bug 788518? | 21:48 |
_mup_ | Bug #788518: staging services share OOPS prefix <Launchpad itself:Triaged> < https://launchpad.net/bugs/788518 > | 21:48 |
flacoste | lifeless: as your current work will fix this | 21:48 |
lifeless | flacoste: it won't really; it will just mean we get the reports. | 21:50 |
lifeless | flacoste: we will still want to identify each script properly, which means a different set of work to auto-assign reports for scripts by the script name | 21:51 |
lifeless | flacoste: which my work will enable but not itself do. | 21:51 |
flacoste | ok | 21:51 |
lifeless | flacoste: I may, later, do that | 21:51 |
flacoste | but the fact that we get the report means that the priorit drop at least | 21:51 |
lifeless | yes, I agree - once my work lands :) | 21:51 |
flacoste | lifeless: well, now it's on your radar ;-) | 21:53 |
cjwatson | hmm. lp:~cjwatson/launchpad/i18n-index failed EC2. The end of the log goes: | 21:54 |
cjwatson | successful: lp.archivepublisher.tests.test_publisher.TestPublisher.testReleaseFile | 21:54 |
cjwatson | WARNING: A test appears to be hung. There has been no output for 600 seconds. | 21:54 |
cjwatson | but I've run the next test along locally (testReleaseFileForPPA) and it succeeds - can anyone suggest a possible gotcha I might be running into? | 21:55 |
lifeless | might be a later transition, or a layer test teardown - they run after tests complete | 22:01 |
lifeless | (also layers are evil, this is one of the data points for why) | 22:01 |
wgrant | sinzui: I think we are nearly getting somewhere! | 22:50 |
wgrant | cjwatson: Could you forward me the output? | 22:52 |
wgrant | cjwatson: Is it possibly the apt-ftparchive hang in mvo1? I don't think the ec2 images have mvo2. | 22:53 |
wgrant | But I'm not sure if you've changed the test to actually exhibit the hang... | 22:54 |
cjwatson | I think the test is testReleaseFileForPPA, which would not be a-f | 22:55 |
wgrant | testReleaseFileForI18n will probably hang. | 22:55 |
wgrant | It sets include_long_descriptions = False, and then calls a-f. | 22:56 |
cjwatson | forwarded to you | 22:56 |
wgrant | Thanks. | 22:56 |
cjwatson | oh | 22:56 |
cjwatson | that would indeed be next alphabetically | 22:56 |
cjwatson | yep, in a local run it's | 22:56 |
cjwatson | successful: lp.archivepublisher.tests.test_publisher.TestPublisher.testReleaseFile | 22:56 |
cjwatson | test: lp.archivepublisher.tests.test_publisher.TestPublisher.testReleaseFileForI18n | 22:57 |
wgrant | Great. | 22:57 |
cjwatson | so this is artificial and we need to get ec2 images upgraded | 22:57 |
wgrant | I'll get the fixed version onto buildbot while we have a LOSA, and sort of buildbot images today. | 22:57 |
cjwatson | thanks; does that indicate bypassing ec2 then, or do you want to upgrade the ec2 images too? | 22:57 |
wgrant | Er. | 22:58 |
wgrant | s/sort of buildbot/sort out ec2/ | 22:58 |
cjwatson | aha | 23:03 |
wgrant | We're in a fairly good position QA-wise, so as long as I can get buildbot sorted out before thedac disappears, we should be able to deploy this at 0830 on Monday :) | 23:04 |
cjwatson | Awesome | 23:05 |
cjwatson | That will make my next week suck a bit less | 23:05 |
wgrant | That's the hope. | 23:05 |
wgrant | KeyboardInterrupt lp.archivepublisher.tests.test_publisher.TestPublisher.testReleaseFileForI18n | 23:09 |
* wgrant upgrades apt-utils to lucid-proposed | 23:09 | |
wgrant | Ran 30 tests with 0 failures and 0 errors in 27.679 seconds. | 23:11 |
wgrant | Success. | 23:11 |
=== wgrant changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - | Critical bugs: 269 - 0:[########Segmentation fault (core dumped) |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!