| === wgrant changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: sinzui || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| stub | jtv: Can you sanity check https://code.edge.launchpad.net/~james-w/launchpad/archive-job-db/+merge/28437 with a quick db review? | 07:19 |
|---|---|---|
| jtv | hi stub! Sure, just taking care of something hang on | 07:19 |
| jtv | stub: looking now | 07:29 |
| jtv | stub: why disable and immediately re-enable triggers? | 07:31 |
| stub | That is just fluff from the sample data build script. There is no point to doing it, but pg_dump seems to do it anyway. | 07:32 |
| stub | Actually, it might ensure that the triggers are enabled after the restore, which might be a desirable feature. | 07:33 |
| stub | But it is still noise to us | 07:33 |
| jtv | Oh, you're saying enabling them twice is an error but disabling them twice isn't, so you disable first? | 07:33 |
| jtv | It's unfortunate that we end up with tables for "jobs of various sorts that happen to have a lot to do with a branch" and now one for "jobs that do stuff with an archive" | 07:35 |
| jtv | Oh, call coming up | 07:35 |
| stub | I have no idea. Its an artifact from pg_dump anyway and the sample data generation script isn't smart enough to strip the noise. | 07:38 |
| stub | What do you find unfortunate? | 07:39 |
| stub | I was sort of hoping the separate queues would make things easier but maybe it needs rethinking | 07:40 |
| stub | I worry that having FooJob and Job mean we get neither the benefits of separate queues nor the benefits of a shared queue. | 07:45 |
| jtv | stub: sorry for being unresponsive—that call is currently in progress | 07:45 |
| stub | There are things we could be doing with the central Job table, like centralised scheduling, but we are not doing that. | 07:46 |
| stub | No worries. | 07:46 |
| jtv | stub: off call. noodles775: g'day | 07:56 |
| noodles775 | Hi jtv | 07:56 |
| jtv | stub: what I find unfortunate is what looks like a growing taxonomy of classes based on what they happen to contain. I know it's more or less inevitable sometimes, so I'm not saying it's wrong—just a bit concerned over the trend. | 07:59 |
| stub | Well... its a database, not an object hierarchy. Or are you thinking of the code that uses these tables for storage? | 08:02 |
| jtv | stub: Bit of both. IIRC we get to have a scheduler per table, and then use the class model to figure out what to do for each type of job. | 08:03 |
| jtv | So a very mixed model, with class tagging etc. | 08:03 |
| jtv | Might be best to have a single scheduler that just outer-joins BranchJob and ArchiveJob and then delegates the "what does processing this job entail, anyway" part to the Python type system in some reasonably uniform way. Maybe that's what's being done here; I don't know that yet. | 08:06 |
| wgrant | We're also about to stuff buildfarm jobs into Job. | 08:06 |
| jtv | wgrant: your timing is impeccable | 08:07 |
| wgrant | jtv: Oh? | 08:10 |
| jtv | wgrant: I just mean it's great that you're bringing in this fact just now. Could help avoid some future stumbling-in-the-dark. :-) | 08:10 |
| wgrant | Ah. | 08:12 |
| jtv | wgrant: though come to think of it, the same scheduler should probably ignore those, right? | 08:13 |
| wgrant | Well, 'about to' may be an exaggeration. It's designed, though I'm not sure if it's scheduled, and I'm hoping that someone gets sufficiently irritated by the insane queue structure that we have now. | 08:13 |
| wgrant | Right, this sort of thing would have to be ignored by any standard scheduler. | 08:14 |
| wgrant | Since it's handled by the buildmaster. | 08:14 |
| jtv | Which has its own very extensive scheduling logic. | 08:14 |
| wgrant | Um. | 08:15 |
| wgrant | Not... exactly. | 08:15 |
| jtv | No? | 08:15 |
| noodles775 | Which could happily die a quick death IMO. | 08:15 |
| noodles775 | (It's overcomplicated for what it does). | 08:15 |
| jtv | I was thinking of the stuff with architectures, availability, private archives etc. | 08:15 |
| wgrant | Mm, I guess. | 08:15 |
| jtv | Compared to BranchJob scheduling, which is simple FCFS | 08:16 |
| jtv | AFAIK | 08:16 |
| jtv | So I guess that the BuildFarmJob would simply attach to Job in the same way but live outside the BranchJob/ArchiveJob scheduling world. | 08:16 |
| noodles775 | jtv: for example, couldn't private archives just be a *separate* queue if we used a messaging system/other scheduler? | 08:16 |
| wgrant | I think we need to move SPRBs and TTBJs over to the New World, rip out all the old stuff, and think about how we can kill even more given the new simplified design. | 08:17 |
| wgrant | Because at the moment it doesn't really work. | 08:17 |
| noodles775 | +100 | 08:17 |
| wgrant | And is very overcomplicated for the whole not-working thing. | 08:17 |
| wgrant | Nice simple buildfarm-related branch at https://code.edge.launchpad.net/~wgrant/launchpad/faster-and-more-general-getBuildQueueSizes/+merge/28476, if anyone's interested... | 08:20 |
| noodles775 | Nice... more red than green :) | 08:21 |
| noodles775 | And much simpler query. Is it worth adding tests that ensure the other build types are considered (or possible yet? or worth waiting to land this when it is possible?) | 08:22 |
| wgrant | It's not possible yet. | 08:23 |
| wgrant | Hm. | 08:24 |
| jtv | stub: but anyway, I was looking at that MP for you... I'm tempted to ask whether json_data should be in Job, but that's got enough generic dead wood in it already. | 08:24 |
| wgrant | Actually, it might be. | 08:24 |
| wgrant | Since this is just BuildQueue, not BuildFarmJob. | 08:24 |
| noodles775 | Right - which means it's implied anyway (and doesn't need to be tested... as the BFJ table isn't even referenced)? | 08:25 |
| wgrant | If it works for one it will work for the others. It would still be nice to test, though. | 08:26 |
| wgrant | Let's see how easy the tests are to change. | 08:26 |
| stub | jtv: A case could certainly be made for putting json_data on Job. I can also make a case for putting the job type on job too. Probably not on this branch though. | 08:26 |
| noodles775 | And would ensure the implementation doesn't change in the future to exclude them... great, thanks wgrant. | 08:27 |
| * wgrant throws in a new recipe build job. | 08:27 | |
| jtv | stub: in that case, just for future reference, BranchJob has the json_data but AFAIK in most cases doesn't use it. | 08:28 |
| wgrant | How I hate to make lp.buildmaster depend on lp.code, though :( | 08:28 |
| stub | jtv: Hopefully this design will hold together until we can revisit the messaging infrastructure. | 08:29 |
| wgrant | stub: Is that ever going to happen? | 08:30 |
| jtv | stub: No such thing as a temporary system. I'm sure it'll hold together and we'll probably never get around to moving it over. :-) | 08:30 |
| jtv | stub: I don't get line 98 of the diff in the MP... Is it meant to say "job_type" (with an underscore instead of space) or is this a piece of syntax I'm not aware of? | 08:32 |
| stub | wgrant: Eventually. foundations got performance this round though, and other teams too busy chasing their features lists. I suspect bugs would have saved time if they had bit the bullet and did it with the bug heat stuff... | 08:33 |
| stub | jtv: Has an underscore here -- job_type integer NOT NULL, | 08:33 |
| stub | jtv: You installed some new fonts perchance? | 08:33 |
| jtv | stub: yes but this isn't using it... Hang on, I'll reload. | 08:34 |
| jtv | ...and now it renders correctly. | 08:34 |
| jtv | That's a bit worrisome. | 08:34 |
| jtv | stub: trying hard to come up with stuff to bitch about... are these jobs inserted and deleted by the same db role? | 08:37 |
| stub | I have no idea. They probably should add some security.cfg entries, yes. | 08:37 |
| stub | oh - there are some there. | 08:39 |
| stub | Ahh... bad dev. [write] is deprecated. I've added a new comment. | 08:41 |
| jtv | stub: it's just that often you get one role inserting them and another deleting them. | 08:50 |
| jtv | stub: I've found about all I can bitch about... are you content with that? | 08:51 |
| === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| wgrant | noodles775: It's actually not easy to test, since the SPRB and TTBJ factory methods are broken in various ways. | 09:24 |
| wgrant | (and fixing them causes widespread test failures) | 09:24 |
| wgrant | makeSourcePackageRecipeBuild creates an arch-agnostic build, when SPRBs aren't actually arch-agnostic any more. | 09:25 |
| wgrant | And makeTranslationTemplatesBuildJob creates a virt-agnostic build, which doesn't make sense (bug #598390) | 09:25 |
| _mup_ | Bug #598390: BuildQueue.virtualized and BuildFarmJob.virtualized should be NOT NULL <Soyuz:New> <https://launchpad.net/bugs/598390> | 09:25 |
| noodles775 | ok. Is it possible (not as nice) to use queued BinaryPackageBuild's and then in one test, just manually change the job_type of one of them (as long as you're not trying to use it for anything, and it's only the duration of that one test, it may test what you're after)? | 09:26 |
| wgrant | I guess I could create an SPRB and then manually mangle its processor. | 09:28 |
| wgrant | That's probably better. | 09:28 |
| wgrant | noodles775: Test there now. | 09:51 |
| noodles775 | wgrant: thanks... if it's ok, do you mind if adeuring does the review (hopefully the comments we've added will be helpful). I've just got my head in some other stuff atm. | 09:54 |
| wgrant | noodles775: Of course. | 09:55 |
| * adeuring is looking | 09:55 | |
| wgrant | adeuring: Can you have a look at https://code.edge.launchpad.net/~wgrant/launchpad/faster-and-more-general-getBuildQueueSizes/+merge/28476 when you have a moment? | 09:55 |
| adeuring | wgrant: sure | 09:55 |
| wgrant | adeuring: Thanks. | 09:55 |
| === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adeuring | wgrant: r=me; thanks for the additional test. | 10:23 |
| adeuring | wgrant: please remind me to land the branch once pQM opens again next week | 10:23 |
| wgrant | adeuring: Oh, PQM is closed already? | 10:26 |
| wgrant | Thanks. | 10:26 |
| adeuring | wgrant: yes, it was closed yetserday | 10:27 |
| * wgrant didn't see any emails. | 10:27 | |
| adeuring | wgrant: gahh, I mixed that up, PQM closes _today-- 22UTC... | 10:29 |
| * adeuring is starting the EC2 job | 10:29 | |
| wgrant | Thanks. | 10:30 |
| wgrant | Though I haven't seen any emails to that effect either -- I guess it must have been internal. | 10:30 |
| jelmer_ | adeuring: it's been extended to today 2200 UTC | 10:30 |
| adeuring | wgrant: right, it was announced on an internal list only... | 10:31 |
| === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews abgeändert | ||
| === matsubara-afk is now known as matsubara | ||
| === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews abgeändert | ||
| bac | hi adeuring | 14:03 |
| adeuring | hi bac | 14:03 |
| bac | sorry i failed to show up to review last week until you'd already gone. i'm still not used to the switch to friday | 14:03 |
| adeuring | bac: no problem --it was a quiet day :) | 14:04 |
| bac | adeuring: like today, it seems | 14:04 |
| adeuring | right | 14:04 |
| === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -,sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews abgeändert | ||
| === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews abgeändert | ||
| bac | adeuring: your MP has a huge diff. is that a mistake? | 14:44 |
| adeuring | bac: yes, I assume due to the wrong target branch... I re-submitted it. (and aked already deryck to review it) | 14:44 |
| bac | oh, ok | 14:45 |
| === matsubara is now known as matsubara-lunch | ||
| adeuring | wgrant: still around? | 15:35 |
| === salgado is now known as salgado-lunch | ||
| leonardr | adeuring, bac, request a review of this very small branch: https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/load-relative-uri/+merge/28511 | 16:13 |
| adeuring | leonardr: I can look | 16:13 |
| === adeuring changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: leonardr,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| adeuring | leonardr: purely a matter of taste: you could use "url[:1] == '/'" instead of "len(url) > 0 and url[0] == '/'" in line 91 of the diff. | 16:25 |
| adeuring | leonardr: r=me | 16:25 |
| === adeuring changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-lunch is now known as matsubara | ||
| === salgado-lunch is now known as salgado | ||
| mrevell | Hi, can a non-lunching reviewer take a look at my what's new branch please? https://code.edge.launchpad.net/~matthew.revell/launchpad/whatsnew-1006/+merge/28519 | 17:30 |
| leonardr | mrevell: couple questions | 17:37 |
| mrevell | sure | 17:37 |
| leonardr | what's the None by the surveymonkey link? shouldn't it be "take our survey"? or is it none because of the <strong> tag? | 17:38 |
| mrevell | leonardr, It seems to be due to the <strong> tag | 17:38 |
| leonardr | ok, i bet that's beautifulsoup related, so i can't really complain | 17:39 |
| leonardr | it looks like you added two news items, and removed two older news items, but kept the 'launchpad is now open source' news item because that's really important | 17:40 |
| leonardr | is that right? | 17:40 |
| mrevell | leonardr, That's right. I considered removing the open sourcing item but don't want to do that until we have some other mention of Launchpad being open source on the front page, and I don't have time to do that right now. | 17:41 |
| leonardr | mrevell: ok, r=me | 17:42 |
| mrevell | thanks leonardr | 17:42 |
| === deryck is now known as deryck[lunch] | ||
| === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jml | I have a soyuz-related patch up for review. | 18:20 |
| jml | https://code.edge.launchpad.net/~jml/launchpad/poppy-cleanup/+merge/28525 | 18:20 |
| * jml has been procrastinating | 18:21 | |
| jml | easy patch here:https://code.edge.launchpad.net/~jml/launchpad/git-and-hg/+merge/28527 | 18:45 |
| === deryck[lunch] is now known as deryck | ||
| bac | jml: both done | 19:37 |
| jml | bac, thanks. | 19:37 |
| bac | leonardr: it looks like you reviewed mrevell's branch here but didn't update the MP. i didn't see this conversation so i did the review again. | 19:39 |
| leonardr | bac: argh, sorry | 19:40 |
| leonardr | i'm still somewhat flaky from my illness | 19:40 |
| leonardr | (not literally) | 19:40 |
| bac | leonardr: np. hope you're feeling better | 19:41 |
| === 40FAA4R6V is now known as kiko | ||
| === salgado is now known as salgado-afk | ||
| === matsubara_ is now known as matsubara-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!