/srv/irclogs.ubuntu.com/2010/05/03/#launchpad-reviews.txt

thumpermwhudson: care to take a look at a change for me?02:57
thumpermwhudson: it is in resposne to BjornT's comments on the job runner oops tests02:58
thumpermwhudson: I'd feel happier if someone else took a look at the changes before I submit02:58
mwhudsonthumper: ok02:58
thumpermwhudson: I've replied to the original review with a diff, which you should get03:04
thumperRSN03:05
thumperhmm, a spike in scanner load?03:06
thumpermy branch isn't being scanned03:07
thumperheh03:07
mwhudsonit's the distro branches03:07
thumperyeah03:07
thumpermwhudson: you should see the email now03:08
mwhudsonyep03:13
thumpermwhudson: what oops related apis?03:29
mwhudsonthumper: getLastOopsReport, maybe?03:30
thumpermwhudson: it is still used03:30
mwhudsonoh ok03:30
jtvmwhudson, are you free for a tiny but urgent review?  https://code.launchpad.net/~jtv/launchpad/bug-572497/+merge/2451405:13
jtvHmm... why isn't that diff updated?05:15
mwhudsonjtv: the diff isn't updated yet, probably because the scanner is behind with all the new distro branches05:15
jtvmwhudson: all I did was make the unit tests that exercise the failing code switch to the right db user.  Maybe the diffs should be updated SJF.  :-)05:16
mwhudsonjtv: well it generally sounds fine to me05:17
mwhudson(apart from 'fiera' being a terrible username)05:17
jtvYes, but it made me use the config instead of a hard-coded name.  :)05:18
jtvmwhudson: at last, my diff has updated!05:52
mwhudsonjtv: approved05:55
jtvmwhudson: ta05:55
noodles775intellectronica: /w 408:41
noodles775hrm... sorry intellectronica, I meant to say, very simple RC MP if you've got a minute: https://code.edge.launchpad.net/~michael.nelson/launchpad/ppa-deletion-oops/+merge/2458008:42
noodles775Or anyone else who fancies a simple review ^^ :)08:43
thumpernoodles775: You'll probably find the diff takes many hours to generate as the scanner tries to catch up with 36k branch scans08:44
thumperdue to mavrick08:44
thumpernoodles775: I suggest pastebin08:44
noodles775thumper: thanks, doing so now.08:44
thumperplease pass the information on to the europeans08:44
thumperI expect the scanner to catch up sometime in the next day or two :-|08:45
thumpergood thing we've fixed this for next time08:45
=== noodles775 changed the topic of #launchpad-reviews to: **Paste your diffs for review due to scanner issue ** On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== thumper changed the topic of #launchpad-reviews to: **Paste your diffs for review due to scanning 18k mavrick branches ** On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
noodles775And here's mine: http://pastebin.ubuntu.com/426871/08:45
thumpernoodles775: I'd strip the _bug_574246 from the method name08:47
thumpernoodles775: but other than that r=me08:47
noodles775thumper: Done.. thanks :)08:47
noodles775BjornT: when you've time, a simple RC: https://code.edge.launchpad.net/~michael.nelson/launchpad/ppa-deletion-oops/+merge/2458008:51
BjornTnoodles775: approved08:54
noodles775BjornT: Ta.08:55
thumperjml: ping09:48
=== noodles775 changed the topic of #launchpad-reviews to: **Paste your diffs for review due to scanning 18k mavrick branches ** On call: - || reviewing: - || queue: [noodles] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara-afk is now known as matsubara
=== abentley changed the topic of #launchpad-reviews to: **Paste your diffs for review due to scanning 18k mavrick branches ** On call: abentley || reviewing: - || queue: [noodles] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
leonardrabentley, can you review https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/delete-entry/+merge/24421 ? diff is in mp14:52
abentleyleonardr, okay.14:52
=== abentley changed the topic of #launchpad-reviews to: **Paste your diffs for review due to scanning 18k mavrick branches ** On call: abentley || reviewing: noodles || queue: [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
abentleyleonardr, what happened with your Vary header branch?14:53
leonardrabentley: i was able to land it on thursday14:53
abentleyCool.  I'm eager to see how my lp clients perform with that changed.14:54
abentleynoodles775, your branch looks okay on its own merits, but it is part of an effort that I am concerned may duplicate or contradict our own efforts.15:01
noodles775abentley: ok, good to know... in what way?15:02
abentleynoodles775, well, I want to have a system that provides an overview of jobs that are running, have run, etc.  I termed it the "Job console".15:02
noodles775Aha (btw: we're not getting rid of the link to services.job with this work, contrary to the diagrams).15:04
abentleyThis seems to be basically the same goal as https://dev.launchpad.net/LEP/GeneralBuildHistories15:04
abentleyI tried to start an LEP for it a couple of  months ago, but jml decided we don't need that feature yet.15:05
abentleyI am glad that the link to Job hasn't been severed.15:05
noodles775abentley: I think it might be different... this is just ensuring that in the context of a PPA, we can show different types of builds (binary, recipe etc.)15:05
noodles775There may be convergence in the future though... currently what I see happening after this current work:15:06
noodles7751) getting rid of our special tables that link 'builds' to services jobs and instead just having a reference to the job directly from BuildFarmJob, and then:15:06
abentleynoodles775, the LEP discusses using the context of the builders15:06
noodles7752) eventually merging as much of BuildFarmJob off (as it duplicates what you've already got on Job).15:07
noodles775abentley: yes, sorry, and teh buildmaster builders context.15:07
abentleynoodles775, But we already have a display in the context of the builders, and I'm working right now on a home page for individual sourcepackagerecipe builds.15:08
noodles775(but (1) and (2) are just speculation on my part atm., the current focus is just to enable general builds to display in those contexts).15:08
noodles775abentley: a display that presents both binary and recipe builds batched together?15:08
abentleynoodles775, yes.  AFAICT, It Just Works.15:08
noodles775Where can I see it?15:09
abentleynoodles775, dogfood.15:09
noodles775abentley: hrm, which branch should I merge in?15:10
abentleyI don't think you need to merge anything in.  When I started creating sourcepackagerecipe builds, they just started showing up in that listing.15:11
noodles775abentley: I expected them to show up in this listing: https://edge.launchpad.net/builders15:11
noodles775but am surprised that they've shown up in the build history: https://edge.launchpad.net/builders/rothera/+history15:12
noodles775as it currently is a list of Soyuz binary package builds afaik.15:12
abentleynoodles775, it seems like the results of my earlier experiments on dogfood have been destroyed, so I can't be certain either way, but that's what I recall.15:14
noodles775abentley: the results will still be there, I just had to disable the config as per email, because it was expecting a SourcePackageRecipeNavigation class which isn't present in db-devel?15:15
noodles775(which is why I assumed a branch needed to be merged in that includes that class?)15:15
abentleynoodles775, Yes, Julian had merged in my recipe-build-index branch.15:17
noodles775(it could just be that it was renamed to SourcePackageRecipeNavigationMenu and the config on dogfood hadn't been updated?)15:17
noodles775Ah.15:17
noodles775abentley: if you look at lib/lp/soyuz/browser/builder:BuilderHistoryView and follow it, you'll end up at IBuilder.getBuildRecords() which currently returns IBinaryPackageBuildSet.getBuildsForBuilder() (ie. only soyuz binary builds), so I'm guessing what you saw was just https://edge.launchpad.net/builders displaying your recipe builds (which I'd hoped Just Worked and am glad it did :) ).15:20
abentleyOkay.15:22
abentleySo you suggested that my Job Console and your General Build History are different because the build history is filtered by PPA or Builder context.15:25
abentleyI think that the filtering isn't terribly important.15:26
abentleyI have no problem imagining filtering the Job Console per builder or per archive.15:27
abentleyThe Job Console meant to be filterable by branch, and General Build History would fall down a bit there, because it would show only Jobs that were also Builds.15:28
abentleynoodles775, so I see the Job Console as a more general idea.  I would hate for us to end up with duplicate systems: a Job Console and a "Job Console for Builds".15:30
noodles775abentley: no, I was thinking they were different because currently (in devel/db-devel), builds have nothing to do with service jobs. They are only temporarily related while they are queued.15:32
noodles775The generalised build history doesn't change that, it is just refactoring all the common info on the different build classes, so that a builder .... actually, do you mind calling? It might be easier to find out where the convergence might be?15:33
abentleynoodles775, sure.15:33
abentleynoodles775, did you mean Skype?15:35
noodles775abentley: yep, just call when ready.15:35
abentleynoodles775, I am ready, but I don't see you online.15:35
noodles775Skype lag ;)15:36
noodles775abentley: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-build-generalisation-db-changes/+merge/2252715:55
abentleynoodles775, could distro_series go on IPackageBuild instead of IBinaryPackageBuildView?16:30
noodles775abentley: it's based on the db-column, BinaryPacakgeBuild.distro_arch_series?16:33
abentleynoodles775, it's a field that SourcePackageRecipeBuild and BinaryPackageBuild will have in common.16:34
abentleynoodles775, of course, the implementation on SourcePackageRecipeBuild will not be based on distro_arch_series.16:35
noodles775Well, SPRecipeBuild has a db-column for distro_series, where as BinaryPackageBuild has a db-column for distro_arch_series.16:35
noodles775Yeah.16:35
noodles775We looked at adding distro_series to PackageBuild, but couldn't see how to do it without redundant info for a binary package build (I was chatting with Julian about it the other day).16:36
noodles775We'd thought we could instead store something on BPB that just represented "architecture", but that didn't seem right either (as it would have to be a text arch_tag, afaics).16:37
abentleynoodles775, this seems to be a similar problem to the IPackageBuildDerived ones.16:37
abentleynoodles775, basically, it's interfaces being limited by the implementation.16:38
abentleynoodles775, diff lines 342-343 need to be re-indented.16:45
noodles775abentley: Right. So do you mean that we should provide IPackageBuild.distro_series (even if it raises NotImplementedError), and just have implementations on BPB/SPRecipeBuild?16:45
abentleynoodles775, I think that would make sense.16:46
noodles775Great, will do (you'll update the MP with all this right?)16:47
* noodles775 has to run out the door.16:47
abentleynoodles775, Okay.16:49
=== fjlacoste is now known as flacoste
abentleyleonardr, what does the "lp" in lp_delete stand for?17:02
leonardrabentley: shamefully, it stands for 'launchpad'17:03
leonardror rather, 'launchpadlib'17:04
leonardrit's a naming convention used to distinguish lazr.restfulclient (formerly launchpadlib) methods from the named operations defined by the web service17:04
abentleyleonardr, I see.  If it's already established  as a convention, I'll let that be.17:05
leonardrabentley: i wish you could tell me to change it and i'd be able to change it, but it's too entrenched17:05
abentleyleonardr, r=me17:06
leonardrcool17:06
leonardri've got a wadllib branch for you in a minute17:07
=== abentley changed the topic of #launchpad-reviews to: **Paste your diffs for review due to scanning 18k mavrick branches ** On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado is now known as salgado-lunch
=== matsubara is now known as matsubara-lunch
leonardrcan i get someone to review my and james's wadllib branch?17:41
leonardrhttps://code.edge.launchpad.net/~leonardr/wadllib/fix-bind-to-anon-collection/+merge/2461317:41
leonardrgary, maybe you'll take it?17:42
gary_posterleonardr: On lunch now, and a bit swamped when I get back.  Maybe I'll get a chance to look at it later today, but if you can get someone else to do it that would probably be better for you.18:18
leonardrgary, ok18:21
leonardrbac: can you do a quick review for me?18:21
leonardror maybe barry or jml?18:24
leonardrhttps://code.edge.launchpad.net/~leonardr/wadllib/fix-bind-to-anon-collection18:24
=== salgado-lunch is now known as salgado
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-brb
sinzuiflacoste, I have an RC request. BjornT agreed it was RCable. bac reviewed the change20:02
flacostesinzui: merge proposal?20:06
sinzuiflacoste, https://code.edge.launchpad.net/~sinzui/launchpad/enable-project-suggetions/+merge/2460820:06
bacflacoste: i have an RC request too.  i believe sinzui already ran it by bjorn and sinzui has reviewed it: https://code.edge.launchpad.net/~bac/launchpad/bug-574142/+merge/2462520:07
flacostesinzui, bac: unless you already know ran it through EC2, it will have to be cherry-picked though20:08
bacflacoste: it has not been through ec220:08
sinzuibac: flacoste: I did discuss the bug with BjornT The oopses over the weekend were caused by Edwin's landing on Friday that wrongly reverted the ProductSeries branch link20:09
flacostesinzui: is your branch passing all tests?20:10
flacostenow, i mean20:10
sinzuiflacoste, yes mine does20:10
flacostesinzui: ok, land it now20:10
flacostebac: your's will have to wait tomorrow20:10
=== salgado-brb is now known as salgado
=== salgado is now known as salgado-afk
=== matsubara-afk is now known as matsubara

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