wallyworldno, not :)00:00
lifelesswallyworld: one common idiom is an 'integration' branch holding things merged from other people that is merged to trunk00:00
lifelessanother is a 'bits' branch where you do little trivial things00:00
lifelesssee e.g. stubs pending-db-patches branch00:00
wallyworldyes, i totally ignored those00:00
wallyworldin my thnking00:00
lifelesswallyworld: its ok, really. LP has -massive- use cases00:00
wallyworldbad mistake. mad at myself for making it00:00
lifelessand our object model being so heavy warps our thinking.00:01
lifelessI do it all the time, and have to back up and do another run at the problem.00:01
wallyworldthat's true actually, about the object model00:01
wallyworldespecially for newer people like me00:01
lifelessits a map, with lots of here-b-dragons, and not the territory.00:02
lifelesswallyworld: here's another case - one I'm looking at at the moment00:02
wallyworldit's also "decay" in the codebase00:02
lifelesswallyworld: https://bugs.qastaging.launchpad.net/ubuntu/+source/afflib/+bug/230350/+index00:02
_mup_Bug #230350: Missing Debian Maintainer field <Ubuntu:Invalid> <afflib (Ubuntu):Fix Released by saivann> <alac-decoder (Ubuntu):Fix Released by warp10> <axiom (Ubuntu):Invalid> <beneath-a-steel-sky (Ubuntu):Fix Released> <bibletime-i18n (Ubuntu):Fix Released by txwikinger> <binkd (Ubuntu):Fix Released> <bzr-builddeb (Ubuntu):Won't Fix> <capisuite (Ubuntu):Invalid> <chinput (Ubuntu):Fix Released by warp10> <chmsee (Ubuntu):Fix Released> <ciso (U00:02
lifelessthat has 160 bug tasks.00:03
* wallyworld looks00:03
lifelesswallyworld: this is just an example of where users go waaaaay outside the expected bounds us mere developers created.00:03
* wallyworld tried to look but got a timeout00:03
lifelessthus I'm looking at it :>00:04
* lifeless goes back to bug-27951300:05
StevenKJenkins, I'm disappointed.00:06
wgrantlifeless: Thanks.00:06
StevenKI was expecting a "trigger another build when this one is started."00:06
lifelessStevenK: I think the advanced triggers plugin can do that00:07
StevenKlifeless: I can't see that on the plugins page00:09
lifelessStevenK: #jenkins I think00:11
lifelessa quick google doesn't show me anything00:11
lifelessit would be easy to do a plugin for it00:11
lifelessbut I'm reasonably sure there is one00:11
StevenKI can't see one either, but I can work around it00:13
LPCIBotProject db-devel build #407: STILL FAILING in 5 hr 36 min: https://hudson.wedontsleep.org/job/db-devel/407/00:14
StevenKwgrant: So, what do I do to this to make it only run windmill tests?00:14
wgrantStevenK: How do you call the test suite at the moment?00:15
StevenKmake TESTOPTS="--subunit" check | subunit2junitxml -f -o test-results.xml00:15
wgrantAdd '--layer=WindmillLayer' to TESTOPTS00:15
StevenKmake TESTOPTS="--subunit --layer=WindmillLayer" check | subunit2junitxml -f -o test-results.xml00:16
StevenKScheduled a new build of it00:17
wgrantThey only take ~35 minutes on ec2.00:17
wgrantStevenK: Is there a bug for the checkwatches.txt thing?00:19
StevenKNot that I can recall00:19
StevenKIt's only 500 lines, it should be shot.00:19
wgrantI need to rewrite most of it to get rid of more OOPSes.00:21
wgrantSo I might.00:21
wgrantBut checkwatches :(00:21
wgrantAh, good.00:22
thumperforgot to add a particular tag to that landing00:30
wgrantthumper: Which?00:32
thumperwgrant: the mantis reland00:32
thumperI did an ec2 land00:33
wgrantIt seems to have been rejected anyway.00:33
wgrantWhat was missing? It doesn't want to be --rollback.00:33
wgrantIt looks fine to me.00:33
thumperoh, ok00:33
wgrantI already rolled it back last night.00:33
wgrantSo we are clear QA-wise.00:33
* thumper needs lunch00:34
=== Ursinha is now known as Ursinha-afk
wgrantFinished: SUCCESS01:28
wgrantStevenK: windmill worked.01:29
StevenKIt took an hour, though. :-(01:30
StevenKI should be able to cut about 30 minutes off Jenkins build times by working on EBS01:30
StevenKBut I have no idea where to start01:30
lifelessis bug 181368 fixed?01:31
wgrantCertainly not released.01:32
wgrantBut maybe fixed.01:32
lifelesswgrant: I thought the publisher was in nodwontime01:35
wgrantlifeless: No, because of poppy.01:35
wgrantNo separate tree yet.01:35
thumperhttps://code.launchpad.net/~thumper/lazr.enum/case-insensitive-getTokenByTerm/+merge/51843 anyone?01:47
thumperwhy does jenkins now show recent success for db-devel?01:52
StevenKIt doesn't?01:54
thumperlast success 6 days ago01:56
thumperthat isn't recent to me01:56
StevenKOh. s/now/not/ ?01:57
* thumper is on school run01:57
StevenKBecause checkwatches.txt has been failing pretty much every test run01:57
StevenKBut not on ec2 or buildbot :-(01:58
StevenKNeither wgrant or I have any clue02:16
wgrantthumper: I adjusted a couple of things relating to that test.02:18
wgrantthumper: So I'm going to look at it this afternoon.02:18
wgrantAnd either fix or delete it.02:18
wallyworldlifeless: you mentioned before that the lp templates mostly show bug tasks and not bugs..... however i think we want to stick with just displaying bugs for revisions listing on the branch index page02:19
wallyworlda bug can have many bug tasks and we really just want the one bug item which the user can click on don't we? there's already a lot of info displayed02:20
lifelesswallyworld: bugtask:+index already picks a single task02:20
lifelesswallyworld: see the method I pointed you at ;)02:20
wallyworldthe bug summarises the issue the mp is solving02:20
wallyworldif you mean the getLinkedBugTasks(),  i think we need an anlogous getLinkedBugs() method02:21
wallyworldwe don't always want the entire list of bug tasks, sometimes just the bugs02:21
wgrantWe have too many h2s.02:22
wgrantBut it is tempting to keep them because they just look so nice.02:22
wallyworldlifeless:  i think the current extended revision listing looks ok as is, but we just want to be able to hide private bugs properly02:23
lifelesswallyworld: you're not making any sense.02:23
lifelesswallyworld: get linked bug tasks already chooses *precisely one task*02:24
lifelesswallyworld: the difference you are arguing for is precisely empty.02:24
wallyworldbut a bug can have many associated bug tasks, no?02:24
lifelessyes, but thats unrelated to the discussion02:24
lifelessbecause getLinkedBugtasks never returns more than one task per found bug02:24
wallyworldjust saw that now02:25
lifelesswallyworld: you *must* have a task to show importance, status, tags, context. Bugs on there own are approximately never interesting.02:25
wallyworldwhy is that?02:25
wallyworldwhy only return just the one task?02:26
lifelesswallyworld: because thats what bugtask:+index spent 500 queries figuring out.02:26
lifelesswallyworld: I was merely fixing the performance not the definition.02:26
wallyworldok. fair enough. where in lp then can the user see all bug tasks associated with a branch?02:27
wallyworldand why is the method called getLinkedBugTasks if it only returns one?02:27
lifelessbecause it returns one task per bug - it returns many tasks02:28
lifelessjust not many tasks per linked bug02:28
wallyworldof course, reading comprehension problem :-(02:29
lifelessthere is nowhere in LP that you can see all the bug tasks associated with a branch. The model links to bug, but linking to bug isn't useful for the rest of the system.02:29
lifelesswallyworld: series branches only show bugtask for bugs which have an open bugtask; regular branches show a bugtask for all their bugs.02:30
lifelesswallyworld: whether this is right or wrong is a separate discussion IMO : I want to just highlight a few key things:02:30
lifeless - everything about a bug except visibility, description, comments,attachments - is on bugtask02:31
lifeless - our /convention/ throughout the system is to not show bugs, only bugtasks.02:32
wallyworldand also on bug are the associated branches from memory02:32
lifelesswallyworld: bugs, questions, specs and other htings are indeed related by bu.02:32
lifeless - we show bugtasks because we show status and importance. And we badge things with 'has brnach', 'has spec' etc etc.02:33
wallyworldok. there's currently functionality which constructs mp emails using bugs not bugtasks. i guess this needs changing too?02:33
lifelesswallyworld: case by case: we're talking web UI here.02:33
lifelessthe web UI is different to email because:02:34
lifeless - its live: folk expect data to be presented rather than clicking through02:34
lifeless - email they expect it to be stale - to be given deltas rather than seeing it as it is right now02:34
wallyworldi ask because i think (from memory) the email stuff uses branch.linked_bugs and that needs to be removed02:35
wallyworldbecause that also exposes private bugs to email recipients02:36
wallyworldso it possibly needs to replaced with something using getLinkedBugTasks02:37
wgrantThe codebrowse OOPSes have stopped!02:37
StevenKSurely not!02:37
wgrantMaybe it's just hung again :P02:38
lifelesswallyworld: right, I'd use getLinkedBugTasks02:39
lifelesswallyworld: you could use task.bug02:39
lifelesswallyworld: or you could show bug status as well in the email02:40
wallyworldlifeless: yes, i'm doing just that elsewhere atm02:40
wallyworldusing task.bug i mean02:40
wallyworldhave to look at performance though. hopefully any alternatives are no worse that using branch.linked_bugs02:41
wallyworldwgrant: saw the email about windmill. me sad. a windmill test of mine actually caught a legitimate bug so we do need those tests02:42
StevenKwallyworld: Which are being run on Jenkins02:43
wgrantwallyworld: Sure, we need to turn them back on at some point.02:43
wgrantBut for now they are out of the critical path, but Jenkins will tell us if things go bad.02:43
wallyworldStevenK: yes but not as part of ec2 land hence we can now land bad code02:43
wgrantwallyworld: It was turned off for most of last year anyway.02:44
wgrantThe number of issues that it catches is miniscule.02:44
wallyworldsure, we just got lucky02:44
wgrantWe would be better served by finding them later and being able to fix them more quickly.02:44
wgrantWhich is what this gives us.02:44
wgrantBecause branches will no longer be delayed by 10 hours due to Windmill being shit.02:44
wallyworldwell if we use more and more XHR then miniscule won't be necessarily true02:45
wgrantThis is a short-term thing.02:45
lifelessqatagger is lying02:45
wgrantEventually we will convince deryck to fix it.02:45
wallyworldwgrant: just wanted to hear that it is short term :-)02:45
wgrantlifeless: Oh?02:45
lifelesslook at it now02:45
lifelessthen file a bug :)02:45
wgrantYou mean how it shows it as deployable?02:45
wgrantIt has always done that.02:46
lifelessI mean the 2nd line02:46
lifelessearlier today it was showing 1248602:46
lifelesswhich was correct02:46
* wallyworld needs food02:46
lifelessnow its showing 1248802:46
lifelessrather than 'nothing can be deployed'02:46
wgrantlifeless: That was probably before the rollback was in place.02:46
lifelesswgrant: no02:50
lifelesswgrant: it wasn't02:50
lifelessthe /rev/ can be ok without it being show deployable at the top02:50
lifelesswgrant: this is a previously unobserved corner case02:51
lifelessthe rule fo rdeployable is 'all the revs before deployable and any rollbacks for broken revs included in the deploy02:51
lifelesswgrant: https://bugs.launchpad.net/qa-tagger/+bug/72755202:55
_mup_Bug #727552: when first rev is bad, with a later rollback, qatagger incorrectly reports it deployable atthe top <qa-tagger:Triaged> < https://launchpad.net/bugs/727552 >02:55
wgrantlifeless: Thanks.02:55
lifelessits probably an off by one02:55
* thumper hangs head02:57
LPCIBotProject devel build #492: FAILURE in 6 hr 8 min: https://hudson.wedontsleep.org/job/devel/492/02:57
wgrantWTF @ Hudson02:58
wgrantBut checkwatches.txt passed again...02:59
wallyworldwhat's the sop for deprecating something in the web services interface?03:00
lifelesswe don't03:00
thumpergetUtility(ILaunchBag).user.preferredemail.email inside the email handler03:00
lifelessif its in beta, leave it there.03:00
wallyworldeven if it's a security/privacy flaw?03:00
lifelessif its in 1.0 leave it there.03:00
lifelessif its devel, just delete it from devel.03:00
wgrantwallyworld: What is?03:01
thumperthere isn't a user in the launchbag for process mail03:01
lifelesswallyworld: change its behaviour03:01
wallyworldbecause we export branch.linked_bugs03:01
wgrantwallyworld: lazr.restful checks launchpad.View on each object in a collection before it returns it.03:01
lifelesswallyworld: just change it to only return visible bug. But I fixed this already I thought.03:01
thumperit is handled by lazr.restful03:01
thumperwe don't need to mess with it03:01
wallyworldcurrently it's still defined as an SQLRelatedJoin03:01
lifelesswallyworld: do you *think* its broken, or have you *shown* it03:02
wallyworldok. didn't realise lazr.restful did some filtering03:02
lifelesswallyworld: it will be doing sucky late evaluation at the moment03:02
lifelesswallyworld: and the collection size will be wrong.03:02
poolielifeless, i guess "7% headroom" comes from 13/1403:02
lifelesswallyworld: so its improvable, but it won't actually be disclosing03:02
lifelesspoolie: 1/14, yes.03:02
lifelesspoolie: I realised the flaw right after I hit send.03:03
pooliebut i don't really see how that corresponds to requests/day03:03
poolieoh ok03:03
poolienice mail otherwise though03:03
lifelesspoolie: we'll free up 1 second for every request that is > 13 seconds today.03:04
lifelesspoolie: which is a smaller number. A thousand or so03:04
* wallyworld still thinks we should be able to mark stuff as deprecated if required whether it be a security hole or implementation issue03:04
lifelesswallyworld: we can in extremis, but it needs to be last-resort03:04
wallyworldpython, java etc all do it :-)03:05
lifelesswallyworld: this is why I don't want *any* new verbs / attributes or types in the 1.0 web service.03:05
poolieso you'll save ~701s of cpu time per day?03:05
poolieoh, a bit more than that if you also kill those in the >13s range03:05
lifelesswallyworld: we made a promise to not break 1.0 for the folk using it in shipped Ubuntu versions.03:05
poolieover 32 cores?03:05
lifelesspoolie: the 701 is the >14 requests.03:05
poolieright, got that03:06
poolieso say 1000 requests, and you'll cut off one second each03:06
lifelesspoolie: currently over 18 python processes.03:06
wallyworldso we could mark something as deprecated and then support it until the lts runs out03:06
lifelesswallyworld: there is no deprecation marker.03:06
lifelesswallyworld: we'd just remove it from devel.03:06
poolieso it's less than .001% more headroom03:06
wallyworldyes, and i'm suggesting we need such a marker :-)03:06
lifelesswallyworld: I think it would be counterproductive.03:07
pooliestill worth doing03:07
lifelesswallyworld: 1.0 users cannot use the replacement.03:07
lifelesswallyworld: devel users don't need the old method anymore.03:07
wallyworldbut in the general case, marking things as deprecated is a fairly common paradigm03:08
lifelessthis isn't the general case03:08
lifelessits a webservice with fairly specific constraints03:08
lifelessone of which is that things work smoothly and seamlessly with the 1.0 version of the service.03:09
lifelessSeparately, I don't want use carrying the debt of things we want to remove *in devel* until we delete every prior edition.03:09
lifelesswe have enough debt.03:09
wallyworldsure, but we should still be able to work towards a 1.1 version, no? ie we need a story around how we evolve the interface03:10
lifelesswallyworld: we have one03:10
lifelesswallyworld: whats the problem you are trying to solve?03:10
wallyworldtalking generalities. the problem of being able to marked certain things as "do not use, will disappear in a future version". give people time to migrate away03:11
lifelesswallyworld: but it doesn't for us.03:11
wallyworldthis is more a pub discussion :-)03:12
lifelesswallyworld: the timescale impedance is immense.03:12
lifelesswallyworld: 5 years vs daily change.03:12
lifelesswallyworld: and new things *are not accessible* until you step over the version, at which point - unless we carry that *5 year* burden for *10 years* - the old stuff is flat gone.03:12
wallyworldyes, the timeframes are large03:13
wallyworldit would be interesting to know which apis were being actively used. i don't suppose we gather those sorts of metrics03:14
lifelesswe have logs that can tell us reasonably well03:14
wallyworldmoot point for now. i was more just thinking out loud. i have to stop doing that03:15
lifelesswallyworld: its fine - I do that too03:15
lifelesswallyworld: but expect to be engaged on such utterances ;)03:15
lifelesswallyworld: happens to me all the time - surely you've seen me go 'I have no real answer to hand' :)03:16
wallyworldyes :-)03:18
lifeless\o/ I now can start coding today.03:19
lifelessthis is why I'm so unproductive, no cycles to write code03:19
wallyworlddoesn't help you get dragged into various discussions all the time03:20
jtvYou know your jet lag is bad when you catch yourself thinking that you want database sharting.03:24
thumperit seems to me that there should never be a user in the ILaunchBag during process-mail03:30
lifelesswhy not ?03:32
lifelesswhen the sender of the mail is identified we should setup a participation and security policy for them.03:32
lifelessI filed a bug about this.03:33
StevenKRargh, and then devel #492 fails with _lock_actions garbage03:37
wgrantlifeless: https://code.launchpad.net/~wgrant/launchpad/unbreak-incoming-mail/+merge/5185003:46
wgrantSlow scan and diff generation are slow.03:46
=== StevenK changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | firefighting: - | On call reviewer: StevenK | https://code.launchpad.net/launchpad-project/+activereviews
lifelessis lib/lp/soyuz/tests/../stories/soyuz/xx-distributionsourcepackagerelease-pages.txt of any use03:55
lifelessor is it redundant with unit tests?03:56
StevenKlifeless: That doctest makes me sad04:02
wgrantStevenK: It's a doctest.04:02
lifelesscan I just delete soyuuz ?04:15
wgrantBut delete checkwatches first.04:15
StevenKlifeless: Patches welcome04:15
lifelesswhy is default distroseries different to current04:16
wgrantDefault distroseries?04:16
lifeless    - http://launchpad.dev/ubuntutest/+source/testing-dspr/1.004:16
lifeless    + http://launchpad.dev/ubuntutest/hoary-test/+source/testing-dspr/1.004:16
lifelessin that test04:16
lifelessafter I tell getPubSource where to publish - the ubuntutest currentseries04:17
wgrantThat's a DSPR wersus a DSSPR...04:17
wgrantWow. versus.04:17
lifeless    >>> anon_browser.open(04:17
lifeless    ...     'http://launchpad.dev/ubuntutest/+source/testing-dspr')04:17
lifeless    >>> anon_browser.getLink('1.0').click()04:18
lifelessare the lead in lines04:18
lifeless    >>> print anon_browser.url04:18
wgrantgetLink('1.0') is a little ambiguous.04:20
lifelesswgrant: I'd really like to just delete the test :)04:21
lifelesswgrant: so04:31
lifelessyou think there are multiple 1.0 links ?04:31
wgrantlifeless: Quite possibly.04:32
wgrantCan't check details right now though, sorry.04:32
lifelesswgrant: argh04:57
lifelesswgrant: I *think* I may have found a reason for distro.getCurrentSourceReleases.04:58
lifelesswgrant: perhaps not a good reason04:58
lifelesswgrant: /ubuntu/+source/packagedeletedinseries-104:58
wgrantlifeless: Latest upload?04:59
lifelesswhat about dropping 'latest upload' as uninteresting05:07
wgrantComponent, Maintainer and Architectures are potentially useful.05:10
wgrantUrgency and Latest upload are not.05:10
lifelessmaintainer etc all come from 'current'05:15
lifelessso, does this need to handle packages never published in 'currentseries' ?05:15
* lifeless thinks05:15
wgrantIt would be nice. But it's not really critical.05:16
lifelessI'm going to shelve the branch05:16
lifelessits taking up too much time05:16
wgrantI say use the current series' publications, otherwise say it's not published.05:16
lifelessmakes sense to me too05:17
lifelessbut the perf win for this is no where near big enough to justify the time investment05:17
lifelesshttps://code.launchpad.net/~lifeless/launchpad/bug-279513/+merge/51063 - abandoned05:19
lifelessanyone know - does storm sniff tables in SQL(...) ?05:37
lifelesswhats a good test class for Archive:getPublishedSources?05:39
lifelesswgrant: ^05:48
lifelesswgrant: also, soyuz permissions - are they lists of attributes, or just-add-to-interface-and-its-done ?05:49
wgrantlifeless: You may need to create a new class in test_archive.05:50
wgrantlifeless: Most of Archive is tested in archive.txt.05:50
wgrantlifeless: As for permissions, it depends on the class.05:50
wgrantYou'll have to check configure.zcml.05:50
lifelesswgrant: thanks05:59
stublifeless: No, SQL is essentially a string. Storm doesn't know how to parse SQL - just generate it.06:01
lifelessstub: thanks06:01
lifelessI'll use a using clause06:01
lifelesswhats the easiest way to find what a Archive:EntryResource:getPublishedSources?ws.op=getPublishedSources looks like given that its timing out06:03
wgrantYou want to know the data from a specific URL that is timing out?06:04
wgrant(also, QA party)06:04
lifelesswgrant: yes06:05
lifelesswgrant: we're qa'd ?06:05
wgrantNo, it's time for a QA party to QA all the stuff that is now on qas.06:06
lifelesswgrant: ah06:06
lifelessI'll do mine straight up06:06
lifeless!!! frell06:07
wgrantStill broken?06:07
lifelessjust getting a traceback06:07
lifeless('Could not adapt', <lp.code.model.seriessourcepackagebranch.SeriesSourcePackageBranch object at 0xaefff50>, <InterfaceClass lp.code.interfaces.linkedbranch.ICanHasLinkedBranch>)06:07
wgrantI suggest rolling both back.06:07
wgrant... and adding tests.06:08
wgrantMe, or you?06:08
lifelessyou, if you would; its 7pm here and I'm about to get snatched for dinner06:08
lifelessI landed this by accident if you recall :(06:09
wgrantBut it passed buildbot06:10
lifelessso ec2 would not have helper.06:10
lifelessI'm just saying its cursed.06:10
wgrantIn PQM.06:14
wgrantOK, how do I delete launchpadlib's credentials?06:23
wgrantI am trying to log in as one of my alternate SSO accounts on qas.06:23
wgrantBut I can't work out how to remove my system authorization.06:24
wgrantHm, maybe my system launchpadlib still uses GNOME Keyring.06:24
wgrantAh, there.06:25
lifelessso, we get 800 hits a day on /branches06:26
lifeless> 800 - 800 soft timeouts06:27
lifelessso probably 0.02% of traffic or so, if we wanted to let it die for a day.06:27
lifelessjust a thought06:27
wgrantIt should be through buildbot in 7 hoursish.06:28
wgrantAnd there is no QA required after what is there now.06:29
wgrantSo, in the likely event that nobody requests one overnight, we can do it in the morning.06:29
wgrantwallyworld: I like your fix for the AJAX build request.06:31
wallyworldwgrant: great. it seems to work better now06:32
wgrantAssuming that this revert works, we are QA'd up.06:32
LPCIBotProject windmill build #2: FAILURE in 1 hr 9 min: https://hudson.wedontsleep.org/job/windmill/2/06:35
wgrantMy point exactly.06:35
wgrantwallyworld: Could you check that test? It passes locally, but this probably indicates that it has a race.06:37
lifelessTacException: Error running ['/usr/bin/python2.6', '-Wignore::DeprecationWarning', '/home/robertc/launchpad/lp-branches/working/bin/twistd', '-o', '-y', '/home/robertc/launchpad/lp-branches/working/daemons/librarian.tac', '--pidfile', '/tmp/tmpZckiy9/librarian.pid', '--logfile', '/tmp/tmpZckiy9/librarian.log']: unclean stdout/err: No handlers could be found for logger "QueueProcessorThread"06:37
wgrantOh, it's setting daily_build, not requesting a build.06:37
wgrantSo not your fault :P06:37
LPCIBotProject db-devel build #408: STILL FAILING in 2 min 18 sec: https://hudson.wedontsleep.org/job/db-devel/408/06:37
wgrantbzr: ERROR: http://bazaar.launchpad.net/~launchpad-pqm/launchpad/db-devel/.bzr/repository/packs/09fb66d3dfb9df78e2bb1ca7b6c7aac9.pack is redirected to https://launchpad.net06:38
wgrantNot again :/06:38
lifelessand poolie is gone06:38
wallyworldwgrant:  i don't think that test is one of mine. i would have to look to be sure06:38
wgrantlifeless: It doesn't appear to have been during a push.06:38
lifelesssomething solidly else then06:39
lifelessthe redirect is berk06:39
wgrantPQM has been checking my branch for 25 minutes.06:39
lifelessspeak of the man06:39
wgrantShouldn't push for another couple.06:39
wgrantIt always used to redirect on 404, if branch-rewrite failed.06:39
wgrantNot sure if it still does.06:39
wgrantIt doesn't.06:40
wgrantpoolie: https://hudson.wedontsleep.org/job/db-devel/408/06:40
wgrantAnother pack redirect.06:40
wgrantBut PQM wasn't pushing at the time.06:40
stublifeless: Just a guess, but shouldn't that command line be bin/py or python2.6 -S for all the buildout deps and environment config to be pulled in?06:40
lifelessstub: bin/twistd is a similar alias06:41
pooliehi wgrant06:41
lifelessstub: that wraps /usr/bin/twistd with the local python etc etc06:41
stubyup. ic.06:41
lifelessthe fail is the 'no handlers' message06:41
stublifeless: We silence some loggers in lib/lp_sitecustomize.py if that is appropriate here.06:42
pooliewgrant, well, the big thing is to just work out what the redirect message is, and who is sending it06:42
pooliebut i'll reopen the bug06:42
lifelessstub: I haven't looked06:43
wgrantpoolie: I don't think devpad has codehosting HTTP logs :(06:44
lifelessstub: it may be a message we want - won't know until we dig06:45
stubwtf do I feel like I have a hangover? Maybe I should have gone out last night and at least had a reason?06:45
lifelesswgrant: for crowberry ?06:45
lifelessstub: cause you slept in :)06:45
wgrantlifeless: Yes.06:45
stublifeless: Getting better. Awake at noon today. Might even make our call tomorrow :-P06:45
LPCIBotProject db-devel build #409: STILL FAILING in 0.47 sec: https://hudson.wedontsleep.org/job/db-devel/409/06:45
poolieare there logs at all for this?06:45
wgrantEr, still?06:45
wgrantAh, looks like the slave needs a good killing.06:46
wgrantpoolie: I presume there are logs from Apache...06:47
pooliei discovered that's not a reliable kind of presumption06:48
wgrantI know we don't keep haproxy logs.06:48
jamHi poolie, funny to see you online when I'm waking up :)06:48
wgrantMorning jam.06:49
wgrantjam: You'll be pleased to know that loggerhead has stopped OOPSing.06:49
jamwgrant: what was up ?06:49
wgrantThanks for fixing that.06:49
jamOr that is the 16k oops / day thing?06:49
wgrantjam: The 16000 daily socket errors.06:49
pooliecongratulations on waking up06:50
jamthere are more fixes in loggerhead trunk, to handle various bits of that correctly all the way through the stack06:50
poolieat what seems like a reasonable time in that tz06:50
poolieas it happens i'm just answering your mail06:50
jampoolie: Still getting my head around the area, and setting up K with the new schools, etc. So I'll probably only be around a half-day today06:50
wgrantjam: Although when QAing it I found bug #72698506:50
_mup_Bug #726985: codebrowse OOPSes with GeneratorExit when connection closed early <Launchpad itself:Triaged> < https://launchpad.net/bugs/726985 >06:50
jambut yeah, jet lag doesn't seem to be too bad for me this time06:50
jammy son isn't quite sure about the new TZ, though.06:50
stublifeless: So we only gain using pgbouncer for appserver connections. At the moment, at any point in time we have 160 connections to the master db. With pgbouncer, that will usually be sitting at 40-50 (about half the main connections are active at any point in time, only < 10 of the session connections are active at any point in time - they run in autocommit and normally are touched only at the start of an appserver request).06:51
jamwgrant: seems pretty easy to fix up. I'm curious who is killing the generator, or how it is falling out of the stack early, etc.06:52
jamI think there is already a StopIteration check, probably fine to add GeneratorExit06:52
wgrantjam: Yeah, I think so.06:52
wgrantIt's not OOPSing on prod, at least.06:52
stubhmm... can use it too for librarian, codehosting and friends too. I think everything but the cronjobs.06:52
jamI think the issue is just that *any* exception is getting logged06:52
jameven the ones that don't mean much.06:52
jamlike GeneratorExit means we exited early06:52
jambut is likely to not actually matter06:53
jambtw, poolie, welcome back from your bike ride, sounds like you had a good time06:54
lifelessstub: we only have 18*4 = 72 appserver threads06:54
lifelessstub: so 1/2 active would be a reduction of 32 on that 160 ?06:55
stublpnet1-8,lpnet11-14 each using 4 threads. lpnet15 using 32.06:56
deryckMorning, all.06:56
stubI'm not even counting edge actually - guess I should be here.06:57
wgrantMorning deryck.06:57
lifelessstub: 15 is getting reconfigured06:57
stubLooks like lpnet15 is a bogus config06:57
lifelessstub: yes, along with 9 and 10 that are not currently running06:57
wgrantderyck: I have some questions about writing Windmill tests properly, if you have a moment.06:58
lifelesswe have 13 lpnet servers, 5 edge06:58
lifelessfor 18*4 once lpnet15 is fixed06:58
deryckwgrant, I think the sprint I'm in will start shortly.  But ask away and I'll respond as I can.06:58
wgrantderyck: Ah, didn't realise you were at one.06:58
lifelessstub: will be going up to 80 with singlre threaded (but we'll actually have two threads per appserver to permit opstats queries06:58
deryckwgrant, yes, in South Africa.  Ensemble sprint.06:58
stublifeless: So If the 50% active connections remains, it could be a win. If you are correct about GIL contention and stuff the activity will grow. I need to decide if it is enough of a win anyway to bother.06:58
deryckwgrant, this is why I haven't looked at Windmill this week yet. ;)06:59
lifelessstub: I'd be inclined to move ahead with it, if only because we can more easily kick everyone off06:59
lifelessstub: though you were saying we had cross transaction temp tables?06:59
wgrantderyck: Looking at lib/lp/code/windmill/tests/test_recipe_index.py, test_inline_recipe_daily_build just failed on our Windmill-specific Jenkins job.07:00
wgrantderyck: It is a race of some sort.07:00
wgrantderyck: The lack of a wait between the getUserClient and click looks very suspicious to me.07:00
wgrantShould there be one there?07:00
LPCIBotProject db-devel build #410: STILL FAILING in 0.49 sec: https://hudson.wedontsleep.org/job/db-devel/410/07:00
wgrantI don't really know how much waiting a page load does.07:00
* deryck looks07:00
wgrantStevenK: Could you please kill/clean the db-devel Jenkins slave?07:01
deryckwgrant, what line in the test?  I don't see a getUserClient call.  do you mean the getClientFor stuff?07:02
wgrantderyck: https://hudson.wedontsleep.org/job/windmill/lastCompletedBuild/testReport/ has the failure.07:02
stublifeless: Yes. A pgbouncer instance runs in a single pooling mode. Session or Transaction interest us. Transaction would save us idle database connections, but doesn't support cross transaction resources. Session doesn't change our utilization, so we would be trading some extra connection control for crappier authentication and extra complexity.07:02
wgrantderyck: Er, getClientFor, yes.07:02
StevenKwgrant: i-3EDE07B407:02
wgrantderyck: It loads the page them immediately tries to click on a JS widget.07:02
wgrantderyck: Which sounds prone to races.07:02
lifelessstub: So I think what I care about is perf and reliability07:03
lifelessstub: if we won't gain measurable perf, and will code reliability, lets defer it.07:03
wgrantStevenK: Yes.07:03
lifelessstub: we can work on getting rid of those tables and cursors later.07:03
deryckwgrant, there's a waits.forPageLoad in the getClientFor method before it returns.  So I think it's fine to click immediately after the method.07:03
stubI'm looking at pgpool-ii to see if it gives us better options.07:03
wgrantderyck: :(07:03
lifelessstub: cool07:04
lifelessstub: go with what makes the most sense to you07:04
wgrantderyck: So that waits for all the JS to finish?07:04
stubI don't think we want to drop temporary tables and cursors entirely - they are the only sane way of doing some bulk operations. The rosetta scripts make use of them extensively.07:04
deryckwgrant, sorry, sprint starting.  will look again when I can.07:05
wgrantderyck: Thanks, no rush.07:05
wgrantIt's not blocking anything any more.07:05
deryckwgrant, but yes, the js should be loaded and done by that method return.07:05
lifelessstub: they aren't crash proof though07:06
lifelessstub: I think if we make a crash proof implementation for those scripts, we wouldn't need cursors / cross trnasaction temp tables.07:06
StevenKwgrant: Done. Why?07:07
poolielifeless, re bug heat, perhaps it would be useful to try to gauge how many users actually find it useful?07:07
wgrantStevenK: It failed to check out the branch due to an odd pack redirect.07:07
pooliei think it could be potentially useful but it never actually seems useful07:07
lifelesspoolie: the distro do use it07:08
poolieand like it?07:08
pooliethat's good to know07:08
lifelesspoolie: I think we should finish and polish it before assessing whether its good or not.07:08
lifelesslike many things its been done just-enough, rather than made excellent07:08
pooliethat doesn't seem like a very general algorithm07:08
pooliebut, i think it's worth working out whether the problems are essential or just bugs07:09
stublifeless: The pattern is to prepare the data into a holding area and then perform bulk operations in multiple transactions with small batches.07:09
lifelessstub: yup, thats what I thought it was.07:09
lifelessstub: the crash proofness I refer to is the assumption [or fact] that identifying the work is expensive.07:10
stub I think you are proposing the holding area be a real table rather than a temporary one, which is doable but a little more effort (db patch to create the table, extra code to clean out the holding area when necessary)07:10
lifelessstub: for instance, yes. alternatively make identifying the remaining work cheap.07:10
stublifeless: Even when identifying the remaining work is cheap, it is still more expensive than doing it upfront. Garbo and the librarian-gc have made use of pretty much every possible technique.07:12
lifelessdo storm StringColumns support a .like() call for queries?07:13
lifelesse.g. foo.like('%bnar%')07:13
stubI think they do, or there is a LIKE function.07:13
lifelessstub: I think it depends on the operation whether upfront checking is needed, is all I'm saying07:14
lifelessstub: I recognise the need for variety.07:14
stubyes. For our current use cases, I think we could minimize their use if we wanted to, but I don't think it would be sane to eliminate them altogether.07:14
lifelessstub: if we *were* to eliminate them, we could:07:15
lifeless - parallelise more easily07:15
lifeless - kill the tasks during db deploys more easily07:15
lifeless - use pgbouncer more easily07:15
lifelessits just a thought07:15
stubstorm.expr has a Like binary operator. Don't know about foo.like() syntax.07:16
=== almaisan-away is now known as al-maisan
LPCIBotProject devel build #493: STILL FAILING in 4 hr 43 min: https://hudson.wedontsleep.org/job/devel/493/07:41
wgrantThat's not good.07:42
lifelessdoes IntColumn.__eq__(Enum) work ?07:43
wgrantI don't know, but it would certainly make you a bad person.07:44
lifelesswgrant: migrating Archive.getPublishedSources to storm so I can DRS it07:49
pooliewho has permission to mark a branch as an official source package branch?07:52
lifelessthe tb07:52
pooliebecause they're the owner of the distro series it's going into?07:55
wgrantlifeless: I think you just need ~ubuntu-branches membership.08:04
lifelessBecause they are in the special group created by jmls initial work08:04
wgrantCelebrities, yay.08:04
lifelessand that group is meant to only have the TB in it now08:05
lifelessbecause it effectively controls upload rights08:05
lifelesswe should indeed move that to a right granted by owning the distro08:05
poolieso allowing them to only push the button if they also own the branch would fix this?08:05
wgrantWe also need to delete bazaar-experts.08:05
poolieit would avoid them shooting themselves in the foot by accidentally trusting someone else's branch08:06
lifelesspoolie: uh08:07
lifelesspoolie: I am missing a lot of context here I presume08:07
lifelesswhat button, what problem, what accidental trust ?08:07
pooliei sent mail08:07
pooliebug 51670908:07
_mup_Bug #516709: revisit official package branch permissions <launchpad> <lp-code> <Launchpad itself:Triaged> <Ubuntu Distributed Development:Invalid> < https://launchpad.net/bugs/516709 >08:07
wgrantpoolie: Owner == Maintainer08:08
poolienot urgent08:08
wgrantAnd that is shown on Distribution:+index.08:08
pooliethere's still a bug as to how you're supposed to know owner==maintainer08:09
wgrantPillars don't have the "owner" term exposed in the UI. It's called "Maintainer" instead (and "Registered by" is the owner for both of those objects, but that is a bug)08:09
wgrantWhere have you seen a reference to a distribution owner?08:10
wgrantOr a project owner?08:10
poolierobert's comment in the thread about that bug, and the api08:10
lifelesspoolie: its not clear what you are trying to optimise for in that discussion08:12
poolie> Lets aim for 'excellent' not 'least work' (while still aiming for 'done soon').08:13
pooliethat's kind of vague08:13
pooliewho could disagree?L08:13
pooliebut it's not very helpful in working out what is actually simplest08:13
lifelesssimplest to implement? use? describe?08:13
lifelessI'm essentially getting confused08:14
lifelessI'm not sure if you're trying to redesign the distro branches feature [which isn't fully implemented, and the design calls for the permissions to override - thats why there is a bug open for that]08:15
lifelessor something else08:15
pooliei'm trying to clarify what if anything needs to be done for https://bugs.launchpad.net/udd/+bug/51670908:16
_mup_Bug #516709: revisit official package branch permissions <launchpad> <lp-code> <Launchpad itself:Triaged> <Ubuntu Distributed Development:Invalid> < https://launchpad.net/bugs/516709 >08:16
lifelesspoolie: we need to ensure the identity statement that only uploaders to the distro can upload to the distro08:16
pooliethere are various possibilities to get there08:17
lifelesswhat are your constraints08:17
lifelesswhat are your requirements, and the requirements of your stakeholders?08:17
pooliei think the only hard requirement is, just as you said, that only package uploaders should be able to write to these branches08:18
poolieafaict that is in fact already met08:18
lifelesswell, its not.08:18
pooliethen there are two soft requirements08:18
poolie1- it should not be too easy to make mistakes that open up access-08:18
lifelessbecause the permissions | together rather than excluding.08:18
poolie2- it should not complicate lp's access control user model any more than it is at present08:18
pooliewhich permissions?08:19
lifelesspoolie: I'm going to put on my paranoid hat and say that 1 is not a soft requirement.08:19
lifelesspoolie: its a *hard* requirement that it be effectively impossible to open up access.08:19
lifelesspoolie: this has been well established over the years of UDS discussions.08:19
pooliewhat i mean is it is not a black and white easy08:19
pooliehow is 'too easy'?08:19
lifelessif its possible to have something look like its distro uploader only, it must be distro uploader only.08:20
pooliewell, that's really my second point, that the user model should be understandable08:20
lifelessI'd rather for 2) we say 'we want it to be easy to understand and use'08:20
poolies//black and white issue08:21
pooliesure, i agree with that too, i'm just trying to be more specific08:21
lifelesspoolie: so we have a trivial to code tolerable solution mooted in the bug08:23
lifelesspoolie: I assert that something needs to be done because we don't satisfy the identity requirement today.08:23
poolieit has the drawback that we will then have branches owned by say ~mbp that can't be written by ~mbp08:23
pooliei think this does poorly on transparency and understandability08:24
lifelessif you want to incrementally improve things, and aim for shortest path to tolerable source uploads from bzr08:24
lifelessthen I think you should accept that we already have 'branches own by ~mbp that ~ubuntu-devel can write to'08:24
lifelessthis neither much worse, nor much better than the status quo.08:24
pooliei didn't know we could already have such branches08:25
pooliebut this is on the path to bzr source uploads08:25
pooliebecause having them be adequately secure is important08:25
lifelesspoolie: if we bless, for instance, ~mbp/bzr/packaging as lp:ubuntu/bzr08:25
lifelessthen ubuntu-devel get write access to ~mbp/bzr/packaging08:26
poolieright, and i keep it08:26
pooliethat's what i ithkn 516709 is about08:26
poolieis blessing existing branches actually useful?08:26
lifelessI can't tell if we're violently agreeing or what08:26
lifelesspoolie: unless you rework the feature from groundup, its necessary.08:26
lifelessI'd love to see it reworked.08:27
poolieit seems to me, from this thread, that we should get away from blessing existing branches08:27
pooliei mean, why would it require redoing the feature?08:27
lifelessbecause there is *no* lp:ubuntu/bzr branch - at all, and no space for it.08:27
lifelesslp:ubuntu/bzr is *defined* as being an alias.08:27
poolieso, given the thread so far, it seemed like it would be cleanest to make it an alias to a branch owned by, say, ~techboard or something similar08:29
lifelessthat will still require lp changes08:29
lifelessand will need care to accomodate linaro etc08:29
lifelesswho have a different governance structure08:29
lifeless[but the distro permissions wouldn't need reworking, if we go with 'distro permissions are all that matter for blessed branches']08:29
pooliegood point about linaro08:30
pooliehowever, i don't think that making the branches owned by the series owner should be a problem08:31
wgrant(I'm pretty sure that source package branches need a complete rethink for when we want to support multiple distributions. More complete than you suggest.)08:31
lifelesswgrant: Possibly.08:31
pooliethat just effectively formalizes that the only  way to them is through the distro permissions08:31
lifelesspoolie: this sets off all sorts of alarm bells08:31
LPCIBotYippie, build fixed!08:31
LPCIBotProject windmill build #3: FIXED in 1 hr 9 min: https://hudson.wedontsleep.org/job/windmill/3/08:31
lifelesspoolie: and I really don't want to spend a lot of time painting the bikeshed08:32
poolieand it's late08:32
lifelesspoolie: the requirement we were given was 'match the distro permissions'.08:32
lifelessI can see two routes to that:08:32
lifeless - genuine *distro* branches that are not aliases.08:32
lifeless - *completely* overriding the permissions when blessed.08:32
pooliethose both have some problems08:33
lifelessI've seen no proposals that meet the requirement other than those two. The former is a moderate amount of work. The latter is going to be confusing if the feature is misused but easy to do and secure.08:33
pooliethe first is apparently a fair amount of work; the second complicates the already hellacious security model08:33
lifelessthe incremental complication is near-zero.08:34
lifelessBecause the elephant is inside the room already.08:34
pooliei'm confused because i think i'm proposing exactly what you proposed on 18/208:34
lifelessits fine to argue we shouldn't arbitarily bless branches. But thats orthogonal to meeting the requirement.08:34
poolie> I'd make *that* owner for these branches the owner08:34
poolieof the distro series the branch is for, not a celebrity.08:34
lifelesspoolie: you've elided the context08:35
lifeless'We probably want an owner in the same sense that a team has an owner:08:35
lifelesssomeone that has administrative privilege over the thing but no direct08:35
lifelessaccess to the content of the thing'08:35
poolieanyhow, it's late08:36
lifelessthat is, people that are members of distro.maintainer can say 'this is the branch' or 'thats the branch' but cannot write to the content of the branch unless they separately have upload permissions08:37
lifelessthis is part of doing genuine distro branches08:37
pooliei'm trying to work out a path to unblock the problems identified in <https://bugs.launchpad.net/udd/+bug/516709/comments/8> with the existing branch08:37
_mup_Bug #516709: revisit official package branch permissions <launchpad> <lp-code> <Launchpad itself:Triaged> <Ubuntu Distributed Development:Invalid> < https://launchpad.net/bugs/516709 >08:37
pooliei don't want to bake into the user model the idea that branches owned by random users can be blessed08:37
lifelesspoolie: its already there08:37
lifelessI appreciate the motivation, but *its already there*08:38
lifelesspoolie: conflating 'fix this previous design shortcut' and 'meet the stakeholder requirements' will push your critical path way back.08:38
poolieok, good night08:39
pooliei'll think about it08:39
lifelesspoolie: I'm in town in the weekend08:39
lifelesswhich reminds me, I need to mail folk and say 'hi, meetup?'08:40
wgrantjam: Around?08:41
poolielifeless, so essentially you're disagreeing with jml's assertion in #8 that the usability consequences need to be fixed at the same time?08:43
poolieyou may be write that will be faster08:43
lifelessits a bit like your thing on reviews08:43
lifelessthat asking for more work at the same time often just stalls things.08:44
lifelessdesigning a better solution than we collectively came up with the first time around is possible.08:44
lifelessButs its going to be more work than finishing our initial design.08:45
pooliei wonder what people do own official branches at the moment and how they use them08:46
pooliedoes anyone rely on having separate access through that, or does anyone have it but not need it08:46
poolieit may not actually cause a problem08:46
poolielifeless, this feels like the kind of thing where lp puts in a poor user model, and then spends more time explaining and supporting it than it would have taken to just finish it properly in the first place08:48
lifelesspoolie: quite possibly. its a 4 year old design08:48
lifelesspoolie: the design was optimised for delivery not explainability or usability.08:49
lifelesslike many things at the time.08:49
poolieok, perhaps you're right we should split them, then separately get away from blessing existing branches08:51
poolieis that in fact what you're saying?08:52
lifelessI'm not trying to defend blessing at all08:53
lifelessits fugly08:53
adeuringgood morning08:55
poolieok, i see08:56
pooliewell, really good night then08:56
bigjoolsmorning all09:07
wgrantMorning bigjools.09:07
wgrantP-a-s makes me cry.09:07
maxbit does that to most people :-)09:08
bigjoolswgrant: I'm sure a boy of you calibre can fix it09:11
wgrantlifeless: How do I push a config overlay onto the FS?09:27
lifelesswgrant: see layers.py09:27
bigjoolswgrant: so, sparc builders09:35
wgrantbigjools: We worked out the problem.09:35
bigjoolswgrant: yeah, I'm interested to hear details, steve said it was a corrupt bq record?09:36
wgrantbigjools: https://wiki.canonical.com/IncidentReports/2011-03-02-LP-build-with-wrong-status09:36
wgrantNo BQ.09:36
wgrantBut the BFJ was BUILDING, when it should have been FAILEDTOBUILD.09:36
wgrantVery similar to Bug #71796909:37
_mup_Bug #717969: storeBuildInfo is sometimes ineffective <soyuz-build> <Launchpad itself:Triaged> < https://launchpad.net/bugs/717969 >09:37
wgrantExcept the opposite.09:37
StevenKOh, yes, sorry, a BFJ record09:37
bigjoolswgrant: the report doesn't say *why* the build candidates were not being considered09:38
wgrantbigjools: The 80% rule.09:38
wgrantDue to the 80% PPA rule, this phantom build prevented further sparc builds in that PPA from being dispatched.09:38
bigjoolswhich PPA?09:38
wgrantIt's in the description!09:38
bigjoolsok I need to teach you how to write incident reports09:39
wgrantthis was all written well afterwards, so it was sort of not very well done.09:39
bigjoolsthe other bug is that nonvirtual PPAs should not be doing the 80% check09:40
bigjoolsso the sBI bug is serious09:40
wgrantI have seen it only three times. But yes, it is rather serious.09:41
bigjoolsI concur that there's a missing commit09:41
wgrantNFI where, though.09:41
bigjoolsbut what's worse is that we have a problem with transaction boundaries09:41
bigjoolsin that the boundary is wrong, if we can get in an unrecoverable situation09:41
bigjoolsthe b-m should have seen the build and fixed it09:41
bigjoolsthe first step is a minimal test cast to re-create it09:42
wgrantBut the first step of that is working out what has gone wrong.09:43
bigjoolsno, it's not09:43
bigjoolsit's working out a test case that re-creates it :)09:44
wgrantHow do you propose to write a test case when we don't know how to reproduce it?09:44
wgrantWell, yes.09:44
bigjools"what has gone wrong" is not the same as "how to reproduce it09:44
bigjoolsbut we're on the same wavelength, so no worries09:44
bigjoolswgrant: need to talk to you at some point about folding the SoyuzTestPublisher into the LaunchpadObjectFactory09:55
bigjoolsthe stuff the LOF does is completely wrong in some cases09:56
wgrantbigjools: james_w had a branch to do that at one point.09:57
wgrantAnd I've wanted to get that landed for a long time.09:57
wgrantI occasionally improve STP when writing tests... but some of it is just so wrong that it needs a big rethink.09:57
bigjoolsI want to deprecate STP09:57
bigjoolsbut the LOF needs fixing, it creates bad data09:57
wgrantStill, not data that is as bad as the sampledata :)09:58
danilosstub, hi, I'd like to create a (constraint) trigger that stops a row from being removed if it's the last one for a foreign key10:07
danilosstub, I have https://pastebin.canonical.com/44140/ so far: I am using a subselect to be able to add a LIMIT 1, and COUNT() to ensure I get a value in the variable ("IF FOUND" didn't work for me)10:08
wgranthenninge: Hi.10:08
henningeHi wgrant!10:08
stuba foreign key references a single row...10:08
danilosstub, also, one thing I am worried about is race conditions10:08
wgranthenninge: Did you land that sharing information thing just now?10:08
danilosstub, right, but I might have multiple rows that reference that same single row in the other table10:08
henningewgrant: yes10:08
danilosstub, basically 1 StructuralSubscription -> n BugSubscriptionFilters10:08
henningewgrant: PQM was *very* quick.10:08
wgranthenninge: How long did it take from when you submitted it?10:09
wgranthenninge: It should have been less than a minute.10:09
danilosstub, BugSubscriptionFilter.structuralsubscription is what references StructuralSubscription10:09
henningewgrant: seemed like it10:09
wgranthenninge: Great, thanks.10:09
stubdanilos: So normally, we would just allow the removal and have a garbo job clear out the lost rows.10:09
henningewgrant: did you do that? Great work!10:09
henningethanks a lot10:09
danilosstub, oh, I want to stop removal of the final row, to avoid race conditions10:09
danilosstub, iow, I stop the removal in python, but it's potentially easy to cause a race condition and still remove it10:10
stubAnd you want to trigger an OOPS instead?10:11
wgranthenninge: Yeah, previously we ran buildout and then bin/test to run a test that ran one line of shell... now we just do that directly in PQM.10:11
wgrantIt's a little bit faster.10:11
henningejust a tiny bit ... ;)10:11
danilosstub, will that happen if delete removes 0 rows?10:11
danilosstub, (and even so, I'd prefer that)10:12
danilosstub, or, I could allow removal and then insert another row, would that be better?10:12
wgrantI don't understand why you want to prevent removal.10:12
daniloswgrant, so python code can assume that there is always at least one BugSubscriptionFilter for every StructuralSubscription (that's what we have already agreed on as the model)10:13
stubdanilos: The DELETE trigger can allow or block the deletion. I don't think it can make it fail silently or remove a different row. That would be lying.10:13
wgrantdanilos: Right, but that doesn't really explain why :)10:13
daniloswgrant, well, to clean up the semantics, allow easier linking etc (i.e. use just bugsubscriptionfilter for linking instead of having to have complex logic for choosing structuralsubscription of bugsubscriptionfilter)10:15
daniloswgrant, today, you can have "no filtering" in two different ways: SS with no BSFs, or SS with BSF with no filters10:15
daniloswgrant, we don't like the duality10:15
wgrantAh, right, forgot that you wanted to link them to notifications.10:16
stubI don't see why a structuralsubscription must have >= 1 bugsubscriptionfilter's myself.10:16
danilosstub, right, the trigger I have so far seems to give me the right behaviour, I am just wondering if that's the optimal or good way to do it?10:17
danilosstub, see above, though we can argue if they are strong enough reasons10:17
stubdanilos: It doesn't work if two transactions delete the last two referencing rows in separate transactions at the same time.10:18
stubBoth sessions think there is another reference, both succeed, both rows are removed.10:18
danilosstub, that's what I was wondering; I see that one can have a CONSTRAINT TRIGGER to happen at the end of transaction, which would work better, but that one can only be an AFTER trigger, meaning it can't stop deletion (naturally, since everything else in the transaction might be messed up)10:19
danilosstub, also, would it be possible to explicitely lock rows that I care about?10:19
stubYour just shuffling the race condition around10:19
stubYes, SELECT FOR UPDATE will do that.10:19
stubBut this is a complex way of proceeding. Ideally we find a non-complex way of solving the problem.10:19
danilosstub, agreed re shuffling race condition around, but having more safeguards in place reduces the chances of it happening, and ideally, I'd be avoiding it completely10:20
danilosstub, any suggestions on a non-complex way? (not that I see this as overly complex)10:20
stubWe just keep outerjoining with the table and not fighting the model SQL gives us.10:21
danilosstub, that's something I'd have to (re)discuss with my team, since we are generally relying on this already in a few places; if it's not too many places, perhaps we can go back10:22
danilosstub, one of the reasons we are going this way is to allow easier merging of structuralsubscription and bugsubscriptionfilter tables in the future10:25
wgrantbigjools: Do you want to review https://code.launchpad.net/~wgrant/launchpad/generalise-add-missing-builds/+merge/51873, or should I grab someone else?10:26
stubfwiw, there is nothing in the data model preventing a bug from having no linked bugtasks, but that hasn't been a problem.10:26
wgrantbigjools: Also, is there an existing wiki page on adding new archs, or should I create one?10:27
bigjoolswgrant: I can do it in a bit10:27
wgrantI searched around earlier, but found nothing :/10:27
wgrantWhich I guess is unsurprising, given the questionable results of the last two new ones :)10:28
bigjoolswgrant: feel free to start one10:28
danilosstub, right, so your suggestion is to just ignore the potential race condition? (in our particular case, it is very unlikely, but if we don't put any safeguards in, a malicious person will be able to do it, especially if we allow API access)10:34
stubdanilos: So to do this. In Python, you do 'SELECT * FROM BugSubscriptionFilter FOR UPDATE', then check there is at least one other link. If there is, proceed with the deletion.10:34
stubIf not, render an error message10:35
stubIf two transactions try that at the same time, one will block until the other commits, then a serialization exception is raised and Zope retries the request10:35
stubdanilos: Doing it in the Python level means you catch the case with a nice error10:36
danilosstub, right, the reason I wanted this as the trigger is because I ran into a place where someone was not using the appropriate Python API, but instead removed a row directly using store.find(...).remove()10:36
danilosstub, true, agreed10:36
stubdanilos: The trigger would need to duplicate this logic, but it will cause an OOPS.10:36
danilosstub, right, but if we guard the proper API in Python, this would indicate a bug, which is exactly what we want :) though, I can't make it work in the trigger10:37
stubdanilos: I'd ignore the race condition - do you really care if there is a structuralsubscription with no bugsubscription? Malicious isn't an issue - you can only shoot yourself in the food.10:38
danilosstub, do you know how to "discard results of a SELECT using PERFORM"?10:38
danilosstub, right, but I hate to see food flying around :)10:38
danilosstub, and you are definitely right, we can cause problems for ourselves in many more different ways, so I'll just guard against it in the python code10:38
danilosstub, thanks10:38
stubdanilos: Yes, but the fallout from all the extra complexity you are adding is likely worse than the problem.10:39
stubPERFORM is a pl/pgSQL statement so only useful in a stored procedure. Syntax similar to SELECT but it throws the results away.10:40
danilosstub, I assume that last comment is re trigger implementation?10:40
stubIt depends on how contrived your race condition is.10:41
stubIf you have a form """( ) bugsubscription a (x) bugsubscription b""", it is reasonable to assume someone clicks 'delete', thinks 'oh shit', changes the check box and clicks submit again. So catching this in Python is reasonable.10:43
stubIf you need to contrive an example with two browser windows opened to the correct page and the user submitting both at once, you might not want to bother.10:43
danilosstub, atm it's the latter, but the UI is not settled yet10:43
stubI certainly wouldn't bother catching the case of the form submission being processed at the same time as some background script running that does the same removal (esp. as I doubt we would have a background script performing that removal)10:44
danilosstub, I can imagine the UI becoming what you described first, and it seems it's not hard to guard against it with SELECT FOR UPDATE10:45
danilosstub, ok, and just for fun, the trigger works with PERFORM FOR UPDATE, so something I learned today :)10:47
stubRight. Should be able to simplify it too. Once you have locked all the rows referencing the structural subscription, you only care if there is a result from SELECT TRUE FROM BugSubscriptionFilter WHERE structuralsubscription=OLD.structuralsubscription AND id <> OLD.id'10:49
stubI think you can lie and silently not do the deletion, but I think it is better to raise an exception here.10:50
danilosstub, ah, SELECT TRUE trick is a very nice one11:00
danilosstub, is it worth doing a LIMIT on that query as well? (just for my education, it seems it is :)11:00
stubdanilos: That wouldn't lock all the rows then11:00
stubOh... the check if the result exists. Yes.11:01
danilosstub, ah, so I would put the FOR UPDATE in this query as well?11:01
stubThought you meant on the FOR UPDATE, which should not have a LIMIT :)11:01
danilosstub, right :)11:01
bigjoolswgrant: DistroSeries:+queue just hove into view on the oops reports11:27
bigjoolsI hate that page11:27
wgrantbigjools: Yes. :(11:27
bigjoolsI remember when it used to have 3000+ queries on it.  I was quite chuffed when I got it to ~60011:27
bigjoolsthat looks big now :/11:27
wgrantA lot of it is the same problem as +copy-packages: createMissingBuilds is shit.11:28
bigjoolsthere's also a massive problem when there are many packages with custom files11:28
bigjoolsI only optimised binary and source uploads, custom packages are still bad11:28
wgrantIt's fairly easy to optimise now that we're allowed to do caching on the model.11:29
wgrantGETs, that is.11:29
wgrantPOSTs are still hard.11:29
bigjoolswe can redirect11:29
bigjoolsI think it does that already11:29
wgrantWe already do. Most of the work is skipped on POST.11:29
Davieybigjools, I /just/ noticed bug #610491 is fixed... but I can't get it to work... getPublishedSources(package_signer=IPerson)... i would have thought would return a packageset?12:19
_mup_Bug #610491: [API] Please expose getPublishedSources(package_creator,package_signer) <api> <lp-soyuz> <Launchpad itself:Triaged> < https://launchpad.net/bugs/610491 >12:19
bigjoolsDaviey: why do you think it was fixed?12:20
bigjoolsgetPublishedSources returns publications, not packagesets BTW12:21
Davieybigjools, ah yeah... that is what i want12:22
bigjoolswe can add the filtering as per the bug, but it's low priority12:23
danilosStevenK, hi, are you still awake? :)12:23
bigjoolsI'm sure everyone wants a fast, non-oopsing Launchpad first :)12:23
=== danilos changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | firefighting: - | On call reviewer: danilos | https://code.launchpad.net/launchpad-project/+activereviews
Davieybigjools, i thought it was fixed because i confused it with bug #37270412:26
_mup_Bug #372704: expose Signed-by and Changed-by via API <api> <lp-soyuz> <Launchpad itself:Fix Released by julian-edwards> < https://launchpad.net/bugs/372704 >12:26
jmlMy laptop is busted.12:35
StevenKdanilos: Hai?12:36
danilosStevenK, you were listed as OCR, I've removed you thinking you might be done with that12:36
StevenKdanilos: It's 1130pm, I so am :-)12:37
danilosStevenK, ok, cool, enjoy the night then :)12:37
bigjoolsjml: that sucks12:39
wgrantjam: Hi.12:41
jamhi wgrant12:42
wgrantjam: We have another chance to roll out the forking service next week. Do you think we should try?12:42
jamwgrant: I would like to land one more patch, but I think I can make it by next week12:43
jambut poolie approved it, pending switching to signal.alarm()12:43
wgrantjam: Right, I've seen that sitting around.12:43
wgrantI think I would prefer an explicit handler.12:43
=== henninge is now known as henninge-lunch
jamwell, it seems approved from the conversation, I'll certainly put it up again for review12:43
jamwgrant: something to handle SIGALRM?12:44
wgrantBut I guess it doesn't really matter.12:44
jamwgrant: it makes the testing easier12:45
jamsince otherwise I have to worry about it when testing that specific code12:45
jamI'll think about it12:45
jamI can certainly just install a handler for the test itself12:45
jamwgrant: any idea why ^C in the middle of the test cancels the current test with OK, rather than generating a traceback and failure where it is blocked?12:49
jamI think this changed with testtools12:49
jammaybe jml ^^ is better to ask (though prob not in this timezone :)12:49
jmljam: it's complex. depends on the test.12:50
jamjml: I've seen it on just about all test runs I've tried12:50
jamwgrant: do we have a way to tell that a process, which is not a direct child, has exited?12:58
wgrantjam: "We" meaning victims of POSIX? I don't think so.12:59
jmlpoll ps :P13:00
jamjml: yeah, that's what I thought was required. which... I'm not doing. But thanks for confirming13:00
jmljam: I guess you could do something like upstart13:01
* danilos -> food13:02
jamwgrant: https://code.launchpad.net/~jameinel/launchpad/lp-serve-child-hangup/+merge/5189313:10
jamif you want to give it a look ovre13:10
jamthe only other thing I can think of, is wanting to have the full TIMESTAMP and PID in log files, rather than mutter's default of time-since-started13:10
jambut that doesn't seem like a blocker for rollout13:10
wgrantjam: Thanks. I'll have a look tomorrow.13:12
=== Ursinha-afk is now known as Ursinha
leonardrhey, benji, i'd like to talk about the way in which exceptions in launchpad are turned into http responses in lazr.restful13:34
leonardrthe more i look at this, the more it looks like two systems that do the same thing13:35
leonardrfor instance, you filed bug 63171113:35
_mup_Bug #631711: Ability to specify HTTP result code when using lazr.restful.error.expose <lazr.restful:Triaged> < https://launchpad.net/bugs/631711 >13:35
leonardrbut we have that--it's the webservice_error() decorator13:35
leonardrbut the webservice_error() decorator on its own doesn't work13:35
leonardryou have to call expose() on the exception object as well13:36
leonardrand i can't change webservice_error() to call expose() on the exception class, because expose() is for objects13:36
leonardrso you have to change it in two places--call webservice_error() on the class and expose() on the object13:36
leonardrthis has been bothering people for a while and i think i'll do something about it13:36
leonardrbut, i'd like to check in with you since it looks like you are responsible for the other system13:37
benjileonardr: I'm on a call at the moment.  I'll be off in about 5 minutes.13:38
LPCIBotYippie, build fixed!13:43
LPCIBotProject devel build #494: FIXED in 5 hr 11 min: https://hudson.wedontsleep.org/job/devel/494/13:43
benjigary_poster: thanks, we'll need it; I'm almost certain we're upside down, but there's always the option of a short sale13:46
LPCIBotProject db-devel build #411: STILL FAILING in 6 hr 4 min: https://hudson.wedontsleep.org/job/db-devel/411/13:46
benjigary_poster: thanks for the pointer, it's hard to find a good agent13:46
gary_posterbenji, ack, and following you around on the channels is exciting ;-)13:46
benjiok, I really need to do something about that13:47
* benji starts a local chapter of Channel Hoppers Anonymous13:47
benjileonardr: the root difference is that one is about exposing exception classes and the other is about exposing exception instances (especially instances of built-in exceptions)13:48
leonardrthe problem is that on its own, exposing an exception class does absolutely nothing13:49
leonardryou also have to expose the instance13:49
benjioh, you do?  I thought that if the framework caught an instance of an exposed exception class then it would handle it specially13:49
leonardrno, it doesn't13:50
leonardri don't know if it used to and doesn't anymore, or what13:50
leonardrideally there would be one method that you could call within a class definition, or on a class, or on an instance13:50
benjitwo courses of action suggest themselves: make exposing an exceptoin class actuall do something or do away with the class-based mechanism alltogether13:51
leonardrthe class-based mechanism is very useful because you can take care of the whole thing in one place13:53
leonardrthe reason you filed that bug is because expose() only works for exception classes that give a 400 error code13:55
leonardrso... let me try to come up with a solution13:55
abentleyhenninge: standup?14:02
henningeabentley, adeuring: Can we do skype?14:02
abentleyhenninge: sure.14:02
* adeuring needs to start a machine where skype is installed. 2 minutes, please14:03
leonardrbenji: argh, we actually have _three_ systems, only one of which works14:29
benjileonardr: yow14:30
leonardrbut two of the systems are alternate spellings14:30
=== abentley changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | firefighting: - | On call reviewer: danilos, abentley | https://code.launchpad.net/launchpad-project/+activereviews
leonardrbenji: do you know of any reason why it shouldn't be allowable to call expose() on an exception __class__ (or do the equivalent--basically, force a class to implement a "error handled specially by the web service" interface14:42
benjileonardr: that seems reasonable; I'd make it a class decorator (as well as a normal function) so you could use it either way14:44
bigjoolssinzui_: will your connection stay up long enough to answer a question? :)15:00
sinzui_doubtful. unity cannot decide to show menu in the menu bar or the window. My windows are shaking with anger15:01
bigjoolssinzui_: well let me try. rvba is having problems using create_initialized_view in a test with a principal of None15:02
sinzui_he is rendering content?15:02
bigjoolshttps://pastebin.canonical.com/44154/ causes https://pastebin.canonical.com/44155/15:02
rvbasinzui_: calling render on the view triggers a LocationError: 'name_link'15:03
rvbasinzui_: (Hi by the way ;-))15:03
sinzui_He needs to pas the principal because menus links and login often needs to get the user form the interaction15:03
bigjoolswhat about an anonymous user?15:04
sinzui_Most Lp pages will not render with an anonymous user. The interaction is not setup right with anon15:05
rvbasinzui_: ok15:05
bigjoolsdidn't know that15:05
sinzui_he can pass the arg like15:05
bigjoolsI guess I was lucky to find a page that does when I've done this15:05
sinzui_principal=distro.owner to hack the case15:05
rvbasinzui_: login(someuser) won't do?15:05
sinzui_the login() will still work for permission15:05
bigjoolsrvba: login() is for the zope interaction15:06
sinzui_rvba, We prefer login_person() so that we have the person to pass as the principal15:06
rvbasinzui_: got it15:07
rvbabigjools: I though every security check was "zope interaction"15:08
bigjoolsrvba: yes, logging in sets up the user for the interaction15:08
rvbabigjools: what is the part that is *not* zope interaction?15:09
leonardrbenji: see if this makes sense to you: http://pastebin.ubuntu.com/574501/15:09
* benji looks15:09
bigjoolsrvba: anything that's not using database objects, views and utilities generally15:09
bigjoolsso not much :)15:10
rvbabigjools: ok, good to know15:10
rvbasinzui_: thanks for the clarification15:11
benjileonardr: that sounds like it accurately describes the status quo15:13
leonardrok, now to figure out a better system15:13
leonardri'm kind of thinking of a system in which we do a named operation based on __lazr_webservice_error15:13
leonardrer, a named adapter lookup15:14
leonardrsince the way we describe this class of exception is by saying what response code we want to send instead of 50015:14
leonardrso if you could do @error_status(400) on an exception class, or expose(exception, 400) on an instance15:14
leonardrlazr.restful would do an named lookup adapting to IWebServiceErrorView, '400', and you would get a ClientErrorView15:15
=== danilos changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | firefighting: - | On call reviewer: abentley | https://code.launchpad.net/launchpad-project/+activereviews
danilosabentley, hi, do you mind taking a look at https://code.launchpad.net/~danilo/launchpad/devel-bug-720826-delete-race/+merge/51890?15:57
abentleydanilos: sure.  OTP.15:58
danilosabentley, great, thanks, I'll be around for discussion as well15:58
leonardrbenji: what do you think of this? http://pastebin.ubuntu.com/574528/16:14
leonardr(skip to "a better system")16:14
abentleydanilos: back.16:15
danilosabentley, generally, the branch is quite simple; few things I have on the top of my head: perhaps I should split the delete_final test into a test-per-attribute, though I am not sure about that; second, I am not testing the race condition handling16:15
=== salgado is now known as salgado-lunch
danilos"I have on the top of my head" is an interesting way to put it, though :)16:16
abentleydanilos: I haven't seen "FOR UPDATE" before.  Is that why you didn't use Storm expressions?16:18
danilosabentley, yeah16:18
danilosabentley, that's how one locks rows in Postgres, I just learned that today from stub :)16:18
abentleydanilos: cool way of verifying that an object is deleted.  I've been grabbing the ID and then querying for it.16:19
danilosabentley, this should handle race conditions where someone tries to delete it from a different place, the check succeeds in both, and then you try the delete; this way, one of them gets locked out until the check and delete are done16:20
abentleydanilos: I meant using self.assertIs(None, Store.of(bug_subscription_filter))16:21
danilosabentley, ah, right :)16:21
abentleydanilos: what is getDescriptionFor?16:22
danilosabentley, oh, it returns you things like Choice(blahblah) object from the interface; I've looked it up using help(Interface)16:23
danilosabentley, perhaps I could say it returns an attribute "descriptor"16:24
abentleydanilos: Okay, cool.16:24
abentleydanilos: if I were writing it, I'd probably look up the default on a separate line, just to avoid clutter.  But that's a matter of personal taste.16:26
abentleydanilos: r=me.16:26
danilosabentley, a thought did occur to me as well, so since it's not just me, I'll do it :)16:27
danilosabentley, and thanks!16:30
abentleydanilos: np.16:30
=== al-maisan is now known as almaisan-away
=== matsubara is now known as matsubara-lunch
=== salgado-lunch is now known as salgado
=== beuno is now known as beuno-lunch
lifelessUrsinha: matsubara-lunch: whats the landing process for qatagger ?17:29
Ursinhalifeless, it's all explained on the README file (I believe)17:30
Ursinhalifeless, wait, let me find the wiki page17:30
lifelessUrsinha: its not :)17:31
lifelessUrsinha: I have this branch diogo approved, I would like to land it17:31
Ursinhalifeless, first what I do is to merge it on trunk17:31
Ursinhaafter that, a make dist17:31
UrsinhaI pick the package and copy over to devpad, untar it, make install17:32
Ursinhaand create a qa link for that folder17:32
Ursinhachange the cron entry and watch17:32
Ursinhaah, I copy the files last-revno-* from current to qa folder17:32
leonardrbenji: got some time for more brainstorming?17:32
Ursinhaand create a link for logs and current-milestone17:33
UrsinhaI'm sure that's explained somewhere, I guess a wiki page I wrote17:33
Ursinhajust a moment17:33
Ursinhalifeless, did you manage to get access to lpqateam?17:33
Ursinhaon devpad, that is17:33
lifelessUrsinha: haven't moved that forward17:33
lifelessUrsinha: thats *deploy*17:33
lifelessUrsinha: how do I get my change into trunk ?17:34
Ursinhafor me that's land :)17:34
lifelessUrsinha: I like it :)17:34
Ursinhalifeless, if that's approved, merge into devel branch and push17:34
Ursinhathen you have to deploy17:34
Ursinhathat's what I described17:34
lifelessUrsinha: ah, so I can't run the test suite at the moment17:34
lifelesssome buildout error17:34
lifelessperhaps you could check I haven't broken a test and push it to devel if its ok ?17:35
Ursinhaah, I know17:35
UrsinhaI guess that's trying to find a launchpadlib version that isn't there anymore, just a moment17:35
lifelessError: Couldn't find a distribution for 'launchpadlib==1.6.3'.17:40
Ursinhalifeless, it seems mars removed his pypi folder that contains the dependencies, I'll add one to ~ursula17:40
Ursinhayou can fix the find-links to point to ~ursula, I'll fix that17:40
lifelessUrsinha: can you merge this one into trunk - I need to go chase two in-progress timeout bugs17:41
Ursinhalifeless, of course :)17:42
lifelessUrsinha: I appreciate you fixing the process blockers immediately - thats great stuff.17:42
benjileonardr: sure, give me about 10 minutes17:42
lifelessUrsinha: thanks17:42
Ursinhalifeless, np17:42
lifelessbigjools: are you around for much longer ?17:46
bigjoolslifeless: 10 mins17:46
lifelessI'm having trouble converting archive.getPublishedSources to storm17:46
bigjoolsho ho :)17:46
lifelessI want to do that to change the prejoins to set based eager loaders17:47
bigjoolswhy are you doing that?17:47
_mup_Bug #727560: Archive:EntryResource:getPublishedSources <timeout> <Launchpad itself:Triaged> < https://launchpad.net/bugs/727560 >17:47
lifeless15 second query atm17:47
lifelessyou could say that17:47
lifelessbigjools: are there unit tests for this, or just 'archive.txt' ?17:50
bigjoolslifeless: just the doctest :/17:51
bigjoolsfeel free to rip it out17:51
lifelessmigrate to unit tests ?17:51
bigjoolshowever, that method is used *everywhere* - loads of other code and tests use it17:51
lifelessthat would make it a lot easier to debug17:51
bigjoolsyes, migrate if you want17:51
bigjoolsarchive.txt is way too big17:52
lifelessI'll have a look see17:52
bigjoolslifeless: did you want any more help?17:55
=== matsubara-lunch is now known as matsubara
sinzui_abentley, do you have time to review https://code.launchpad.net/~sinzui/launchpad/sane-definition-status-0/+merge/5193217:58
bigjoolstoo late if you do, I'm off :)17:58
bigjoolsgood night all17:58
leonardrbenji: i think i've reduced my remaining problems do "why doesn't this component lookup succeed?" want to take a look?18:15
benjileonardr: sure18:15
leonardrok, pushing18:15
=== beuno-lunch is now known as beuno
leonardrif you run bin/test there's a breakpoint right at the bit where everything goes south18:16
leonardrlook at the SystemErrorView registration in configure.zcml. that's the one that's failing18:17
leonardrand look in _resource.py, the except clause in ReadWriteResource.__call__18:17
leonardrthat's where the lookup happens18:17
abentleysinzui_: happy to.18:19
leonardri've manually verified IException.providedBy(e), IWebServiceLayer.providedBy(self.request), and Interface.providedBy(IWebServiceErrorView)18:20
leonardrhm, it succeeds if i copy simple.py and remove the thing i'm trying to adapt to18:22
leonardrbut i get the wrong status code18:22
leonardri get SystemErrorView18:22
leonardrrather than WebServiceExceptionView18:23
benjilooking at it now18:25
leonardri think i got it, actually. i'm just not sure why i had to start adapting to Interface (presumably the default) instead of IWebServiceErrorView18:26
leonardrlet me get it working and then you can critique the working version18:27
leonardrbenji: pushed working version18:35
leonardrbasically, WebServiceExceptionView is now the default view for all exceptions unless another view is registered18:35
leonardrand if the exception happens to have been annotated (by any mechanism), WebServiceExceptionView will defer to the annotation when it comes to the status of the response18:35
leonardrand the exception handler does a lookup for any exception, not just for a client error18:37
abentleysinzui_: Why are you addressing this issue by providing a new enum?  Why not just check that the value is in a set of acceptable values?18:39
benjileonardr: looks good very good to me18:43
leonardrbenji: ok, maybe you'll like this other change, which takes it one step further...18:44
leonardrbenji: check the new version. it *always* turns an exception into a view, and gets rid of IWebServiceError altogether18:49
benjileonardr: wouldn't that mean that an "accidental" exception would result in a 400?  It seems a 500 would be more appropriate.18:54
leonardrbenji: an accidental exception would turn into an un-annotated WebServiceExceptionView18:54
leonardrand its status would be the default, 50018:54
benjileonardr: ok, sounds like a plan; simpler code and it behaves the way we want18:55
leonardrall right, i'll fix the test failures and submit a formal mp18:56
leonardrbenji: this does mean that the publisher will not be handling any errors. is that ok?19:04
lifelessleonardr: will we still get OOPSes on unannotated exceptions?19:05
benjileonardr: hmm, does our custom publisher do retries on DB availability errors (like conflicts)?  If so, it won't get the error to know that the error happened.19:06
leonardrlifeless: i don't know. lazr.restful will set X-Lazr-OopsId if request.oopsid is present, but i imagine there's more to "getting an oops" than that19:07
sinzui_maybe I should install unity-2d. The constant shaking windows is making me ill.19:07
lifelessleonardr: I mean the server side mechanism where raising is called()19:07
benjileonardr: lifeless makes a good point; we may want to only catch the errors that were marked as "not exceptional exceptions" instead of intercepting potentially hair-on-fire exceptions (MemoryError comes to mind)19:08
lifelessbenji: you credit me with too much... that is also an excellent point19:09
leonardrbenji: what if we raised exceptions if their error code was 500, for whatever reason? either because no other code was specified or because the error was explicitly marked as a server error?19:09
lifelessbenji: my concern was more prosaic - I want our feedback loop for fixing crasher bugs to remain intack.19:09
lifelessleonardr: benji: may I ask what problem you're trying to solve ?19:10
lifelesss/trying to solve/solving/19:10
benjileonardr: I'm not sure I understood that part ("what if we raised exceptions if their error code was 500...")19:11
leonardrlifeless: we have 2.5 systems for telling lazr.restful to treat a given exception/exception class as a certain http response code, and only one of them works19:11
leonardrthat's done; this is ancillary cleanup work, in which the question is "which exceptions can lazr.restful handle on its own, and which should it delegate to the publisher?"19:11
leonardrbenji: so, earlier we effectively re-raised exceptions if their status code was not 40019:12
lifelessoh. Have you considered 'if lazr.restful does not explicitly know, its not its business' ?19:12
benjileonardr: I favor reverting that last bit (r181)19:12
leonardrlifeless: yes. the question is what it means to 'explicitly know' this19:13
lifelessleonardr: that would seem to be dependent on the system you've chosen for telling lazr.restful how to determine result codes for exceptions19:13
leonardrbenji: if i revert the last bit, then exceptions will be re-raised unless they have been annotated with a return value. i don't think that's necessary or sufficient19:14
benjileonardr: that's not the way I read the code.  It seems to me that we reraised if the exception wasn't marked as "an exception that the client caused".19:14
leonardran exception that the client caused -> 4xx response code19:14
leonardrwe can make that explicit19:14
leonardrlet me do some code and tests and show you waht i mean19:15
benjiI think we're talking about this bit, right? http://pastebin.ubuntu.com/574616/19:15
leonardrbenji: yeah. the comment is wrong19:16
benjileonardr: the comment before or after the change is wrong?19:16
leonardrif an exception is annotated as 301 or 503 (or 200 for some reason)19:17
leonardrlazr.restful would have handled it19:17
leonardreven though its decoration does not indicate that it's the client's fault19:17
benjiah, ok19:17
leonardri think it makes sense to only handle exceptions where the response code is 4xx19:17
leonardrand let the publisher handle the rest19:17
leonardrthat way, if you explicitly decorate an exception as 5xx, it will become an oops19:18
benjithe exceptions-as-control-flow thing was throwing me19:18
lifelessleonardr: +119:19
benjithe issue there would be that (if I'm understaning all the mechanisms correctly) that if you decorate an exception as a 50X but the code in question re-raises the exception, the publisher's error view may well retorn something other than 50X (say it may have 500 hard-coded).  Does that jive with your understanding?19:20
leonardrbenji: yes, that might happen19:21
leonardri think that failure mode is minor compared the the failure modes that might happen if lazr.restful handled all the errors19:21
benjiIt seems to me that since we're not crying out for more controll over 500s, we should designate this decoration mechanism as for status codes from 400 and down, that way we don't have to get our fingers in the "a *real* error happened reporting story" while also greatly improving the "client did something wrong" story.19:24
leonardri think just handing 4xx is good enough--that's effectively what we were doing before (except we only handled 400)19:25
leonardrand the publisher may have some way of handling exceptions that are marked as redirects, or something weird like that19:25
leonardrok, i think we have a winner19:37
LPCIBotProject db-devel build #412: STILL FAILING in 5 hr 17 min: https://hudson.wedontsleep.org/job/db-devel/412/19:45
lifelessoh yay, tz boobytraps in archive.getPublishedSources19:45
leonardrbenji: https://code.launchpad.net/~leonardr/lazr.restful/consistent-error-exposing/+merge/5194619:51
leonardrpunt to abentley if you don't want to do this right now19:51
benjileonardr: I'd love to do the review, but I have some pressing work I really need to put some time into19:53
leonardrabentley, can you take a look?19:54
LPCIBotProject windmill build #5: FAILURE in 1 hr 13 min: https://hudson.wedontsleep.org/job/windmill/5/19:55
=== Ursinha is now known as Ursinha-food
thumpermatsubara: ping20:41
matsubarahi thumper20:42
thumpermatsubara: sorry for not getting back to you earlier20:42
thumpermatsubara: you were asking about some recipe testing20:42
thumpermatsubara: what do you need from me?20:42
matsubarathumper, can we have a quick call?20:43
thumpermatsubara: sure... skype or mumble?20:43
matsubarathumper, let's try mumble20:43
thumpermatsubara: ok20:44
matsubaraI'm on the product channel20:44
matsubarathumper, https://dev.launchpad.net/QA/ExploratoryTesting20:45
leonardrbenji, i'm not sure why we have both webservice_error and @error_status--they do the same thing with slightly different syntax. do you have any ideas? should we standardize?20:59
benjileonardr: nope, no idea; I'm pretty sure both existed when I started looking at them21:01
* leonardr will leave it alone for now21:03
thumperleonardr: mumble?21:04
leonardrthumper, yes!21:04
=== wallyworld___ is now known as wallyworld
leonardrthumper: https://code.launchpad.net/~leonardr/lazr.restful/consistent-error-exposing/+merge/5194621:26
thumperleonardr: ok, I'm left a little confused with your merge proposal21:39
thumperleonardr: I think I was fine up until the last test21:40
thumperwhich left me questioning21:40
leonardrthumper: an alternative is to prohibit the caller from marking an exception with a response code not in the 4xx or 5xx series21:40
thumperleonardr: in test_decorated_exception_instance_becomes_error_view when the resource is called, you get a result21:41
thumperleonardr: but in the last test, when you call the resource it raises21:41
leonardrthumper: because of line 27 in the diff21:42
leonardrif the response code is 4xx we handle the response (since the error was the client's fault)21:42
leonardrin all other cases we let the publisher handle it21:42
thumperalthough the 200 case seems a bit of an edge case21:42
thumperI guess it is there for completeness21:43
leonardrthumper: yeah, we could also prohibit it, like i said21:43
leonardrbut if the user wants to do that, we don't really know what they're trying to do, so better let the publisher handle it21:43
leonardri'll land but not do a release, as integrating into launchpad may require other changes21:44
=== Ursinha-food is now known as Ursinha
lifelessmatsubara: please triage new bugs :) - 72805921:56
matsubaralifeless, will do21:57
matsubaraI just filed them21:57
pooliei just got Error 324 (net::ERR_EMPTY_RESPONSE): Unknown error. from https://bugs.launchpad.net/~canonical-bazaar/+assignedbugs21:59
pooliegood morning btw21:59
lifelesswallyworld: BrowsesWithQueryLimits is your best option for that test22:01
wallyworldlifeless: yeah. i'll need to modify it to suit.22:07
wallyworldcurrently it takes a context object and i need to give it a url22:08
wallyworldi'm keen for the branch to land to improve +daily-builds performance even more22:09
lifelesswallyworld: I would give it an object and a view selector22:12
lifelessdefaulting to +index22:12
lifelesswallyworld: then you can use the Root object and +daily-builds22:12
wallyworldok. haven't really thought too much about it yet. have to context switch back. sounds good what you're suggesting22:13
lifelesswallyworld: or you could make a parallel one (perhaps sharing code) that takes a url22:14
lifelesswallyworld: we need to talk separately about your bug task search patch22:14
wallyworldok. i'll take another look and see if i can decorate the rs outside the search implementation22:15
lifelesswallyworld: can you tell me what you need to do ?22:15
wallyworldbasically the current search functionality only returns bugtask and i need to also access tables from the query eg BugBranch22:15
lifeless[this is why I don't like itsy bitsy landings without context - they are harder to review]22:15
lifelesswallyworld: /why/22:15
wallyworldso i can, for each bugtask, create a data structure that allows be to lookup a bugtask based on branch id, so i can find all info i need with one query22:16
wallyworldotherwise i would have to do one query per branch22:17
lifelesswallyworld: this is the privacy thing you're fixing ?22:18
lifelesswallyworld: I can give you a one liner to do what you need a different way22:18
wallyworldand when using the search method and passing in linked_branches=any(), the BugBranch table is in the query but i don't get to access it currently22:18
lifelessvisible_bugs = getUtility(IBugTaskSet).searchBugIds(BugTaskSearchParams(user=self.user, bug=any(*(bug.id for bug in bugs_you_have_today))22:20
lifelessthis is one extra query, but should be fairly lean, and it will be constant.22:21
lifelessthe problem with passing a result decorator into the query is that you actually want a wider tuple out, which such a decorator won't give you22:22
wallyworldactually mine does. it works :-)22:23
wallyworldand doesn't cause any extra sql22:23
wallyworldbut i'll take a look at your suggestion22:24
lifelesspastebin it ?22:24
wallyworldlifeless: https://pastebin.canonical.com/44193/22:25
wallyworldlifeless: with your suggestion, at first look, it still seems like i won't have access to the info i need with one query, which is bugtasks for each branch22:29
=== matsubara is now known as matsubara-afk
wallyworldi'll look in more detail in case i'm missing something22:30
lifelessuhm, I'm worried about the interface basically22:30
lifelessthere is a compiler in there that is fiendish22:30
lifelesswhat you have will actually cause more queries22:31
lifelessif you have *visible* private bugs22:31
lifelessbecause their visibilty won't be cached22:31
wallyworldhmm. ok22:31
lifelessif you tested with *invisible* private bugs, it will seem fine.22:31
lifelessbecause you're fixing half the problem22:32
wallyworldthe main problem is that the search() assumes you only want bugtasks and not other stuff from the query22:32
wgrantsinzui: No critical issues with your CSS changes?22:32
lifelesswallyworld: thats what its designed for22:32
sinzuino critical22:32
wgrantOK, we shall deploy then.22:33
wallyworldhmmm. don't like the design then :-)22:33
lifelesswallyworld: there are some options - eager load (e.g. into a cache property) the branches; ask for related things in the result22:33
lifelesswallyworld: Here is another problem with your fix22:33
lifelesswallyworld: you'll get multiple rows back for the same bugtask.22:33
lifelesswallyworld: (if branch A and B are both linked to bug C, then you'll get a row for (C, A) and for (C, B))22:35
wallyworldso long as the related branch in each row is correct that won't matter22:35
wallyworldi building a dict of branch->related bug tasks22:35
lifelesswallyworld: why won't it matter? It will still cost cpu: cache refreshing of duplicate rows in storm.22:36
wallyworldbut the rows for (C,A) and (C,B) are valid and required22:36
lifelesswallyworld: I would do a separate single query on bugbranch for (BugBranch.bugID.is_in(bug_ids), BugBranch.branchID.is_in(source_branch_ids))22:36
wallyworldsounds reasonable. get the bug ids from the search() and then another query to load the required info22:37
wallyworldif visible private bugs were cached, my way would require less queries :-P22:38
wallyworldthanks for the input. i'll get back to it after food and after i resubmit the +daily-builds perf improvement stuff22:40
wgrantLooking at the stable merge failure.22:41
wgrantsinzui: Oh, yesterday I also killed Windmill and made PQM fast.22:45
huwshimiwgrant: oh just that, I'm surprised you remembered at all!22:45
sinzuiwgrant: you made 5 minute PQM (a merge in less than 5 minutes) actually work as promised!22:45
sinzuiI think we gave up on that 18 months ago22:46
wgrantSadly prod config landings are still slow.22:46
wgrantBut they are rare enough.22:47
pooliehi all22:47
wgrantOh no :(22:47
wgrantNow PQM is going to spam us more often with stable->db-devel conflicts :(22:47
jcsackettwgrant: the 5 min for main launchpad stuff is amazing. i think we can all bare the configs being longer. :-)22:48
wgrantA test went missing when r12399 was automatically merged into db-devel.22:55
wgrantlifeless: Compare the stable r12399 and db-devel r10214 diffs, lib/lp/registry/tests/test_person.py22:56
lifelesswgrant: if it was already in db-devel22:57
lifelessthen it wouldn't show in the diff22:57
wgrantIt's not in db-devel now, and doesn't seem to have been removed since.22:57
lifelessok, thats more worrying then22:58
wgrantBut let's see the tree at that rev...22:58
wgrantNo, not there in 10214 either.22:58
lifelessok, thats whack22:59
lifelessif you merge the two revs locally, does bzr do that?22:59
wgrantWaiting for that to happen.22:59
wgrantWarning: criss-cross merge encountered.  See bzr help criss-cross.22:59
wgrantAnd it's missing.22:59
wgrantSo the criss-cross kills it without a conflict.23:00
wgrantThis is sort of really bad.23:00
lifelesswe can teach pqm to abort on criss cross23:00
wgrantI'll revive the test now and look through the stable->db-devel diff to check that nothing else is missing.23:01
spivlifeless: not just whack, wiggida wiggida wiggida wack!23:01
lifelesshi daddy :)23:01
spivlifeless: well it's a criss-cross problem :P23:02
jcsackett...gonna make you jump, jump.23:02
lifelessspiv: oh, I missed the ref.23:03
lifelessspiv: 5:45 starts on thursday ;)23:03
wgrantI think I may be too young for this.23:03
jcsackettwgrant: http://en.wikipedia.org/wiki/Kris_Kross23:05
wgrantAh, yes, 1992.23:05
spivwgrant: http://www.youtube.com/watch?v=SmO4xdnf6Gk, especially around the 40 second mark23:06
huwshimiwgrant: That was the year you were born, right?23:07
wgranthuwshimi: '91.23:07
huwshimiwgrant: oh, my apologies :D23:07
wgrantI am at least a few months older than this... abomination.23:08
lifelessno excuses then23:08
huwshimiwgrant: That would only make you a little younger than they where when the song came out23:09
lifelessI'm not an apple afficiondo23:12
lifelessbut this is sweet hardware: http://www.apple.com/ipad/features/23:12
wgrantYes, even I have to admit that :(23:13
* huwshimi is wondering if Ubuntu will work on his released-last-week MacBook Pro23:14
* huwshimi is about to find out23:15
lifelesshow is %foo% meant to be escaped for storm23:20
lifeless  File "/home/robertc/launchpad/lp-sourcedeps/eggs/storm-0.18-py2.6-linux-x86_64.egg/storm/database.py", line 366, in _check_disconnect23:20
lifeless    return function(*args, **kwargs)23:20
lifeless  File "/home/robertc/launchpad/lp-branches/working/lib/lp/testing/pgsql.py", line 115, in execute23:20
lifeless    return self.real_cursor.execute(*args, **kwargs)23:20
lifeless("SELECT COUNT(*) FROM SourcePackageName, SourcePackagePublishingHistory, SourcePackageRelease WHERE SourcePackagePublishingHistory.archive = %s AND SourcePackagePublishingHistory.sourcepackagerelease = SourcePackageRelease.id AND SourcePackageRelease.sourcepackagename = SourcePackageName.id AND (SourcePackageName.name LIKE '%' || 'cd' || '%')", (9,))23:20
lifelessIndexError: tuple index out of range23:20
=== salgado is now known as salgado-afk
lifelessargh, I'm blind23:28
lifelesssqlobject vs storm difference on % escaping. Baby saviour wept.23:28
wgrantAre you blind for not seeing it, or blind because you saw it?23:29
lifelessblind having fixed it23:30
lifeless" '%%%%' || %s '%%%%'" % quote_like(name)23:30
lifelessdamned if I know why the change was needed23:31
LPCIBotYippie, build fixed!23:31
LPCIBotProject windmill build #6: FIXED in 49 min: https://hudson.wedontsleep.org/job/windmill/6/23:31
lifelessoh fail23:36
lifelessjulian used bool(resultset) all over with getPublishedSources23:36
lifelesswgrant: pop quiz23:38
lifelessbinary_copies)                                                                                       |            clauses.append(23:38
lifelesslib/lp/soyuz/scripts/packagecopier.py line 56423:38
lifelesswould .one() or .first() be appropriate?23:38
wgrantlifeless: Which line?23:39
wgrant        source_copy = source_in_destination[0]23:39
wgrantone will cras23:39
lifelessis a failover server?23:40
lifeless\o/ test passes23:40
wgrantYay, appservers updated.23:45
lifelesshah, and a trivial query reduction in distroseriesdifference.py23:46
lifeless*sql is not python*23:46
lifelesssecond hunk23:46
lifelessalso in findSourcePackageRelease23:51
lifelesssame bug23:51
lifelessand getSourceAncestry23:52
lifelessalso know as LBYL is slow23:53
lifeless-        if pubs:23:56
lifeless+        try:23:56
lifeless             return pubs[0]23:56
lifeless-        else:23:56
lifeless+        except IndexError:23:56
lifelessis cleaner23:56
wallyworldabentley: you still there?23:59

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