/srv/irclogs.ubuntu.com/2012/12/10/#launchpad-dev.txt

=== wgrant changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On-call reviewer: - | Firefighting: - | Critical bugs: ~160
StevenKHmmmm00:42
StevenKNot really happy with this query00:43
StevenKAnd it seems BulkUpdate does not love list types00:43
wgrantIndeed, probably not00:44
StevenKI don't like the NULLs, either00:45
wgrant?00:45
StevenKwgrant: http://pastebin.ubuntu.com/1422314/00:46
wgrantRight, those will want to be removed.00:46
wgrant(potentially by coalescing the potentially null array with an empty one)00:47
StevenKHmmmm, can I just use COALESCE for that?00:48
wgrantYes00:49
StevenKHmmm, it will only return the first, though00:51
wgrantHm?00:51
StevenK"The COALESCE function returns the first of its arguments that is not null."00:52
StevenKIf I feed it an array of {NULL,0.9,1:0.9} ...00:52
* StevenK plays00:52
wgrantHmmmm?00:53
wgrantYou'd either remove NULL from the resultant array, or coalesce a null array00:53
wgrantNot coalesce an array that contains null00:53
StevenKSo my query as it stands is okay?00:54
wgrantPerhaps something like https://pastebin.canonical.com/80039/00:56
* StevenK searches for his phone for 2FA01:01
StevenK[(1, '{0.9}')]01:07
StevenKLooks like success to me01:07
StevenKExcept for that quoting01:07
wgrantIt's probably because it's a debversion[], not a text[], so psycog2 doesn't know what to do with it01:08
StevenKTypeError: Expected unicode, found <type 'str'>: '{'01:12
StevenKThat's the error I'm getting from the BulkUpdate01:12
StevenKBut searchable_versions = List(type=Unicode()) because type=StringCol() didn't look right01:13
wgrantBulkUpdate may not cope with lists01:13
wgrantYou'll need to check01:14
StevenKYeah, I'm about to dig in and see what UPDATE statement it is creating, but I wasn't sure if my List() defintion was sane01:14
StevenKHmmm01:17
StevenKPerhaps it's my fault, since it's barfing over my values list01:17
StevenK[[<storm.variables.IntVariable object at 0xa39b050>, <storm.variables.ListVariable object at 0x88cc738>]]01:23
StevenKWell, that's very handy, storm. Thanks.01:23
bigjoolsthere's something oddly satisfying about typing "dput -fu"01:25
lifelessbigjools: you need to get out more :)01:25
wgrantpoppy deserves it01:25
StevenKwgrant: I think you might be right.01:26
bigjoolslifeless: said without a trace of irony :)01:26
StevenK(Pdb) p values[0][1]._value01:26
StevenK'{0.9}'01:26
StevenKMe thinks a ListVariable wants something more list-like01:26
bigjoolstrevormosey must be really famous01:26
wgrantbigjools: Heh, yes01:26
wgrantThere's a bug for that01:27
* bigjools blames the dput ppa script01:27
bigjoolsputting in that ~ by default was a huge mistake01:27
StevenKThat always trips me up too01:28
wgrantbigjools: Well01:31
wgrantbigjools: It probably comes from the ppa.launchpad.net HTTP layout01:31
wgrantThat was the first place to drop the tilde01:31
* StevenK doesn't like the thought of results[1][1:-1].split(',')01:31
wgrantStevenK: No01:31
wgrantStevenK: Perhaps try casting the array to a text[]01:31
wgrantSee if psycopg2 interprets it then01:32
wgrantIf you manually parse the array I will strangle you :)01:32
StevenKwgrant: You'll have to get in line behind me01:32
StevenKI will strangle myself first01:32
bigjoolswgrant: that was also stupid :(01:32
wgrantbigjools: Certainly01:32
bigjoolsmy fault, sadly01:33
wgrantReally?01:33
wgrantIt was very early in your time...01:33
bigjoolsI had the opportunity to fix it when we added multi-ppas01:33
wgrantTrue01:33
StevenKwgrant: ARRAY[]::text[] == bang01:33
wgrantStevenK: Bang?01:33
StevenKIf that's what you mean01:33
wgrantThat seems like casting the empty array01:33
bigjoolsyeah it was early in my time01:34
wgrantWhich will fail because you're trying to append it to a debversion[]01:34
wgrantCast the entire result array01:34
wgrantMulti-PPAs would indeed have been the best time to add them01:35
wgrantAlthough I thought that feature was somewhat influenced by the initial implementation that was removed before PPAs were released.01:35
wgrantIf I recall correctly from my bug...01:35
StevenKHmmmm01:36
StevenKhttp://pastebin.ubuntu.com/1422367/01:37
StevenKHoly fuck, it works01:37
StevenKPerhaps I shouldn't be so surprised01:37
wgrantOh, so my BulkUpdate hacks just worked with a list when the type was correct?01:38
StevenKwgrant: So it seems01:38
wgrantI'm disappointed01:38
StevenKI put the entire block in ()::text[]  which I thought was dubious at best, but worked01:38
wgrantWell, we're stuffing them into a text[] column in the end, and we don't want to perform version comparisons on them01:39
wgrantSo it's the right thing to do01:39
StevenKwgrant: Right, but I've not had to do manual casting in postgres before, so I was guessing01:39
wgrantAh01:40
* StevenK ponders commiting evil on DF01:40
=== jtv1 is now known as jtv
=== jtv2 is now known as jtv
StevenKwgrant: Didn't we have a PQM check that you couldn't modify anything outside database/ in a DB branch? Or did that die?03:07
wgrantStevenK: Sometimes you need to modify tests and similar03:35
wgrantWe rely on people thinking about deployment03:35
=== almaisan-away is now known as al-maisan
=== al-maisan is now known as almaisan-away
wgrantStevenK: https://code.launchpad.net/~wgrant/launchpad/improved-html_escape/+merge/13890204:12
StevenK52-56 can be done without my eyes bleeding, surely?04:13
wgrantNot really.04:13
wgrantHow?04:13
StevenKLooping over replacements?04:14
wgrantMmm, it'll be slightly shorter.04:15
StevenKwgrant: What about invalid unicode for line 51?04:17
wgrant(or longer)04:17
wgrantStevenK: It's hideous but I didn't change it04:17
StevenKSo you didn't04:17
wgrantWe assume that all messages are a unicode or an ASCII bytestring.04:17
StevenKBecause WCPGW, right?04:18
wgrantNo04:18
wgrantBecause what else can you do?04:18
StevenKWell04:18
StevenKBe sane04:18
wgrantIt's a reasonable assumption, and it will crash if it's not valid04:18
wgrantWhich is all that it's reasonable to do04:18
StevenK% bzr grep 'import cgi' | wc -l04:19
StevenK3004:19
StevenKI guess they're next on the chopping block?04:20
wgrantIndeed04:20
wgrantBut it's a big diff04:20
wgrantSo it's a separate branch04:20
wgranttwo weeks ago I moved this stuff into its own module04:20
wgrantThis branch makes it usable generally04:20
wgrantThe next two branches eliminate the other four escaping mechanisms04:20
StevenKwgrant: r=me04:21
wgrantThanks04:21
StevenKwgrant: Adding "I will be destroying all other escaping mechanisms in terms of LoC gain in a future branch" would have been nice, but meh04:22
StevenKDear mawson, where does it hurt?04:59
wgrantWell05:02
wgrantthat's a few more test failures than I expected05:02
StevenKOn buildbot?05:02
wgrantYeah05:02
wgrant110 or so05:02
wgrantMostly due to apostrophes05:02
StevenKOh yes, aren't you a naughty boy05:02
StevenKwgrant: You're working on the 111 bad tests?05:21
wgrantYes05:22
StevenKwgrant: How do you feel about reviewing my +298,-60 garbo branch? :-)05:22
wgrantI may end up reverting, but I suspect I'll be able to fix the tests before anyone else wants to land stuff05:22
wgrantMaybe :)05:22
StevenKIt depends on 40-1 being deployed before it can land, but I can work on making it wgrant-approved first05:22
StevenK2012-12-10 05:28:29 DEBUG2  [PopulatePackageUploadSearchableVersions] Iteration 591 (size 2155.0): 5.698 seconds05:28
StevenKDaaaaaaaaaaaamn05:28
wgrantRight, no trigram indices and the rows should be much smaller :)05:31
wgrantTrigram indices are awful05:31
wgrantSo they should only be used where necessary05:31
StevenKI am incredibly curious just how long it will take to create the index on DF when the columns are fully populated05:32
wgrantYeah05:32
StevenKwgrant: They're awful for performance? But in this case good for searching performance?05:32
wgrantYou'll probably want a GIN index on the searchable_versions05:32
wgrantStevenK: They perform awfully in all respects05:33
wgrantThey're just less awful than a seq scan when doing a substring match05:33
wgrantThat's the only respect in which they have any value05:33
StevenKOh. So they're totally awful, but they're best we have for the problem.05:34
wgrantRight05:34
wgrantAnd it's not feasible to get much better05:34
wgrantIt's a hard problem05:34
wgrantSo it should be avoided05:34
wgrantBy not supporting substring matches unless it's unavoidable05:34
StevenKwgrant: I don't have branches for the indicies yet, only a diff that gives me the TGRM for searchable_names05:38
wgrantRight05:39
StevenKwgrant: Hmmm, since searchable_versions is TEXT[], can I ask for rows where that column has more than one value?05:43
wgrantStevenK: Why do you want them?05:43
StevenKwgrant: Curosities sake, not for code05:43
StevenKWas wondering if there was a short cut05:44
wgranta GIN index won't do that for you. You'd have to index array_length(searchable_versions) directly to do that efficiently.05:44
StevenKwgrant: You misunderstand. I only want to check rows on DF manually, not in the code at all05:45
wgrantAh05:45
StevenKHowever WHERE array_length(searchable_versions) > 1 will probably work then05:45
wgrantWHERE array_length(searchable_versions) > 1?05:45
wgrantYeah05:45
StevenKERROR:  function array_length(text[]) does not exist05:46
StevenK... but it's an array, you still DBMS?05:46
wgrantOh05:46
StevenKs/still/silly/05:46
wgrantarray_length(searchable_versions, 1)05:47
wgrantNeeds a dimension05:47
wgrant(and it's 1-based, because who needssanity)05:47
StevenKDatabase developers are all insane, anyway05:48
wgrantblarfgghergherger05:56
wgrantmanual escaping ftl05:56
wgrantsome of these errors have been being double-escaped for years05:56
wgrantApplause06:19
StevenKHm?06:20
wgrantWell06:20
wgrantLaunchpadValidationError is used by lots of fields06:20
wgrantIt HTML-escapes all errors pushed through it, so you can push through markup if you need to06:20
wgrantBut the fields are also used by eg. XML-RPC and email interfaces06:21
wgrantWhere HTML-escaping is not right06:21
wgrantThis has historically gone largely unnoticed, as only <, >, & were escaped globally06:21
wgrantAnd they're relatively rare characters06:21
wgrants/And/As/06:21
wgrantFor now I'm going to take the easy way out and change the error message to not contain an apostrophe...06:22
wgrant:)06:22
StevenKNaughty06:24
wgrantI try.06:24
nigelbTMI.06:28
StevenKwgrant: https://code.launchpad.net/~stevenk/launchpad/populate-searchables-for-pu/+merge/13890606:41
wgrant17 to go...06:52
StevenKFailures?06:53
wgrantYes06:53
wgrantOoh06:54
wgrantA real bug, finally06:54
wgrant1920 lines of diff later and all but three should be fixed...07:02
StevenKHaha07:06
StevenKThat's going to be fun to review07:06
wgrantStevenK: Do you perhaps want to split the garbo job into a separate branch?07:08
wgrantSo it can be more easily reverted later07:08
wgrantAnd to make it easier to review07:08
wgrant(also, I'd say "addSearchableName" rather than "appendSearchableNames")07:09
wgrantaddBuild is hideous; pls rewrite07:11
wgrantappendSearchable* will want to check that the given string isn't already in there07:11
wgrantIt's probably best to parse the string into a set, then add the new names and reserialise07:12
StevenKwgrant: I can split out the two garbo jobs if you wish, but they're very well contained07:14
wgrantThis is true07:14
wgrantThey could also use a comment or two :)07:14
StevenKThey have two each, I though?07:14
StevenKWell, the tests do. :-)07:14
wgrantDoesn't seem to07:14
wgrantIt's just;07:14
wgrantMASSIVE BLOB OF THE MOST HIDEOUS SQL KNOWN TO MAN07:14
wgrantMASSIVE BLOB OF STORM MOLESTATION07:15
wgrantfin07:15
wgrant:)07:15
wgrantAlthough I guess they'll be gone soon07:15
StevenKs/BLOB/TEMPORARY BLOB/07:15
StevenKwgrant: I'm quite proud of the two SQL statements we cooked up.07:15
wgrantSpeaking of which07:15
StevenKThey are HIDEOUS and eat babies, but they work and are performant07:15
wgrantWhy not do them both at once?07:15
wgrantTHey're going to be rewriting the same rows07:16
wgrantMight as well do it in one hit07:16
StevenKwgrant: Because I went to change the SQL statement in Populate...SearchableNames and it nearly bit my hand off07:16
wgrantYou should just be able to keep the two expressions separate, but in the same findspec.07:17
StevenKMmmmm07:17
StevenKRight, so your complaints are: addBuild is terrible, and I should fix it, appendSearchable* should be renamed and parse the existing values if any to a set and then add the new bits, the garbo jobs should be put on a diet, IE, from two to one, and the new one could use some comments?07:20
wgrantRight07:21
StevenKAnything else?07:21
wgrantThe comments are going to be useful for people wanting to write similar garbo jobs in future07:21
wgrantIt otherwise looks sensible07:21
StevenKExcept the comments will die shortly07:21
wgrantThough the bit that sets searchable_* from the PCJ could do with a comment saying the rest will be added when the PU. is added07:21
wgrantStevenK: Sure, but they'll be in history07:21
StevenKPerhaps I should put the comments in the caching job?07:22
StevenKSince that is sticking around07:22
wgrantPerhaps so07:22
StevenKwgrant: The rest will be added when the PU is? EPARSE07:22
wgrantStevenK: The PU[SBC]07:23
StevenKI don't think we call any of the three add methods on a PCJ'd PU07:23
StevenKThe binaries from any builds will get a new PU07:24
wgrantIs that method only used for PCJs?07:24
StevenKYeah07:24
wgrantIndeed07:25
wgrantThat's fine, then07:25
nigelbOoh.07:35
nigelbI finally upgraded.07:35
nigelbThis means I can setup LP in an LXC.07:35
StevenKHaha07:36
StevenKI guess "Warning: Permanetly added the RSA host key for IP address '<IP of taotie>' to the list of known hosts." means my laptop hasn't hit up LP for a long while07:38
cjwatsonwgrant,StevenK: we don't have a use case for substring matching on versions in the upload queue, but we do have a use case for exact matching.  Are you preserving that facility, or are we going to need to perpetrate a workaround?08:49
wgrantcjwatson: Exact matching is easy and cheap and is being preserved08:49
cjwatsonExcellent08:50
wgrantI will strongly resist any attempt to preserve substring matching :)08:50
cjwatson(That use case being "queue accept -e packagename/version")08:50
wgrantyep08:50
cjwatsonWhich reminds me, I should flip the -e default in the queue client, maybe after all this lands08:50
StevenKAfter I'm done, source='foo', version='bar', exact_match=False will do a substring match for foo, but an exact match for bar08:50
cjwatsonThat's fine for our needs, yes08:51
cjwatsonAnd I think what we were expecting to happen anyway08:51
StevenKIt doesn't right now08:51
wgrantexact_match=True should be the default everywhere08:51
wgrantIf your client does not already, pls fix :)08:51
cjwatsonI know, it only doesn't because:08:51
cjwatson(a) it was a translation of the LP script which was insane08:52
wgrantHeh08:52
wgrantYeah08:52
wgrantLP has some bad history there08:52
wgranteg. getPublishedSources, IIRC08:52
cjwatson(b) exact matching actually turns out to be quite inconvenient without SPN matching on build uploads08:52
StevenKcjwatson: I'll be fixing b08:52
wgrantAh, indeed08:52
cjwatsonExactly, so I should be able to flip the default after this lands08:53
StevenK'This'08:53
StevenKIt's a mess of seven branches :-)08:53
cjwatsonI scoff at your attempt to impose reality on my grammatical choices08:53
StevenKHah08:54
czajkowskialoha09:05
=== Nigel_ is now known as G
=== almaisan-away is now known as al-maisan
=== benji___ is now known as benji
=== al-maisan is now known as almaisan-away
=== BradCrittenden is now known as bac
=== benji changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On-call reviewer: benji | Firefighting: - | Critical bugs: ~160
benjiI am going to assume that bcsaller is not in this morning and that we can clear up any remaining issues with his review of https://codereview.appspot.com/6909046/ later and land my branch.14:01
benjiand I am going to talk about it in the wrong channel, is that cool with everyone?14:01
rick_h_benji: that's cool and I approve14:07
benjirick_h_: good, now I have an excuse for my crosstalk14:07
=== slank_away is now known as slank
gmb`benji, Morning. Is there any reason for me to not mark BjornT's merge proposal for lp2kanban as approved so Tarmac can land it: https://code.launchpad.net/~bjornt/lp2kanban/bugs-to-cards/+merge/13590914:43
gmb`?14:43
benjigmb`: not that I know if.  I believe it is good to go.14:43
gmb`Awesome.14:44
=== gmb` is now known as gmb
* gmb kills the imposter14:44
nigelbOh dear.14:45
gary_postergmb, as a poster I find that concerning.  on another note, are you helping BjornT land his lp2kanban branch?  If not I can try to do so14:48
gmbgary_poster, I thought lp2kanban was all Tarmac goodness these days.14:49
gmbIf not, then I'm happy to take care of it.14:50
gary_postergmb, dunno, forgot.  I was going to try to add BjornT to whatever group he needed to be in in order to land the code...14:50
gary_posterlooking to see what that would be...14:51
gmbgary_poster, ~launchpad.14:51
gary_posteroh :-/14:51
gmb(Well, that's who owns the trunk branch)14:51
gary_posterright14:51
gmbSo I'll land it now.14:51
gary_posterok thanks gmb14:53
gmbWow, rocketfuel-get takes a while... Remind me, why did we write all this code/15:05
gmb?15:05
sinzuigmb, that reminds me...15:08
sinzuigmb, gary_poster is there a rule to remove tarballs from lp-sourcecode deps? I went to remove a few last week, then panicked when I realised other projects (not ~launchpad) use it15:09
gmbsinzui, Really? Which projects?15:10
sinzuioops tools15:10
sinzuimaybe others15:10
gmbUrgh.15:10
gary_postersinzui, I know of no rule.  The proper solution has always been to not use a branch and instead have tarballs hosted somewhere internally and never delete anything.15:10
gary_posteryeah I think other projects too15:10
gary_posterRobert wanted them to be shared15:11
gary_posterThis is an unintended downside, I think15:11
=== almaisan-away is now known as al-maisan
gmbbac, I've added a section for Green Squad to the lp2kanban config; can you update it on the Canonistack instance please?15:35
bacgmb: sure, in an hour or two.  on standup now15:36
gmbbac, Sure, that's fine. Thanks.15:36
bacgmb: actually the config should be added to lp-tarmac-configs project not lp2kanban15:37
bacgmb: the one in lp2kanban is an example only15:37
gmbbac, lp-tarmac-configs is the one I've updated.15:38
gmbSorry, didn't make that clear.15:38
bacgmb: just for fun, you could login, poweroff, and then bin/startmeup.  would be nice for someone else to exercise that path.15:39
gmbbac, I would if I could remember to what I should ibe logging in.15:40
=== bdmurray_ is now known as bdmurray
=== matsubara is now known as matsubara-lunch
=== deryck is now known as deryck[lunch]
=== matsubara-lunch is now known as matsubara
=== deryck[lunch] is now known as deryck
=== Ursinha is now known as Ursinha-afk
=== Ursinha-afk is now known as Ursinha
=== BradCrittenden is now known as bac
jcsackettbenji: you have time to review https://code.launchpad.net/~jcsackett/launchpad/no-private-releases/+merge/139092 ?20:52
sinzuibenji, do you have time to review https://code.launchpad.net/~sinzui/launchpad/person-owned-teams/+merge/13909120:52
rick_h_jcsackett: wins!20:53
* jcsackett laughs20:53
benjijcsackett and sinzui: sure, but first you invoked the rarely used simultanious review smackdown20:54
sinzuiI am wearing glasses20:54
sinzuiYou cannot hit a four-eyed developer even if he is complete an utter bastard20:55
jcsackettsinzui: i am also four eyed. i think that nixes it. :-P20:55
=== mwhudson_ is now known as mwhudson
jcsacketthowever, i like sinzui. if i must hit him to be reviewed first, i'll wait to be second. :-P20:55
rick_h_ok, then sinzui must hit you to be reviewed first20:56
* jcsackett laughs20:56
rick_h_there must be smack in the smack down20:56
sinzuihow?20:56
sinzuiI shall invoke the one-hand clapping smack20:56
jcsacketttell you what. sinzui can drive down hear to smack me. in the meantime, benji can review my branch. :-P20:57
jcsacketts/hear/here/20:57
* sinzui claps20:57
benjiheh20:57
* sinzui has pulled a muscel and may have broken a nail20:58
benjijcsackett and sinzui: both of your branches are reviewed.  I won't tell you which I did first.21:05
sinzuithank you benji21:05
benjimy pleasure21:05
jcsackettthanks, benji.21:06
benjiany time21:06
lifelessOOPS-3de41986b624ef0f542b92653ce1cfe8 <- ?21:12
sinzuilifeless, timout inserting a bug. looks like a row lock21:14
lifeless\o/21:14
lifelessbugs 1088650 and 1088651 appear either private or missing, to me21:14
lifelessI tried again and got ...221:15
sinzuilifeless, I can see the first, not the second, and I don't have power to confirm if the private-is-404 rule21:17
lifelessinteresting, cool.21:17
lifelessI am suffering cat-in-lapsy21:18
lifelessit is very distracting21:18
wgrantderyck: Hi22:05
=== benji changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On-call reviewer: - | Firefighting: - | Critical bugs: ~160
deryckhi wgrant22:31
wgrantderyck: I see that a couple of your DB changes have model changes as well.22:32
wgrantThat's forbidden because we can't confirm it's safe (we don't deploy DB and app changes at the same time)22:32
wgrantIn particular, one of your branches makes a column not null at the same time as it sets a default in the model.22:32
deryckwgrant, I don't think my revs do.  but maybe abel's does.  I didn't look at his honestly, sorry.22:32
wgrantAh22:32
lifeless'royal your' :P22:33
wgrantWell, yes :)22:33
deryckwgrant, ah, no that is mine.  I checked with stub and thought it was cool.22:33
deryckwgrant, we already protect against in the model, and I did some query checks to make sure we were safe.22:33
wgrantzAh22:35
wgrantIt's actually safe, because there's a default in the DB22:35
wgrantBut we forbid model changes in the same branch to avoid any confusion22:35
wgrantAnd ensure that the devel code and db-devel DB can coexist22:35
wgrantRather than having to reason about it manually22:35
deryckwgrant, ok, sorry about that. I assume it's okay to deploy this time, so long as I don't do it again? :)22:36
wgrantIndeed.22:36
deryckok, thanks.  sorry again.22:36
wgrantabel's branch is also OK to deploy, but it's unnecessarily mixing DB and app changes, so it has to be manually verified.22:36

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