| wgrant | wallyworld: BugTaskSearchParams.setTarget maybe of interest. | 01:33 |
|---|---|---|
| wallyworld | wgrant: context? | 01:34 |
| wgrant | + all_branches = getUtility(IAllBranches).visibleByUser(user) | 01:34 |
| wgrant | + wanted_branches = all_branches.visibleByUser(user).withIds( | 01:34 |
| wgrant | + *branch_ids) | 01:34 |
| wgrant | Also, that's got a duplicate visibleByUser | 01:34 |
| wgrant | wallyworld: The preloading branch | 01:34 |
| wallyworld | i copied that code from elsewhere | 01:34 |
| wgrant | You currently branch based on pillar interface and call setProduct/setDistribution | 01:34 |
| wgrant | Rather than setTarget | 01:34 |
| wallyworld | ok. i just cut and pasted it. will fix along with the other typo. thanks | 01:35 |
| wgrant | Cargo-culting Launchpad code is a little less likely to be optimal than in other projects, sadly. | 01:36 |
| wgrant | Our codebase is in a bit of a sorry state. | 01:36 |
| wallyworld | it's getting better though | 01:40 |
| wgrant | Wow | 02:14 |
| wgrant | https://lp-oops.canonical.com/oops.py/?oopsid=OOPS-b0f528b0f920a20462267635b6697b7a | 02:14 |
| wgrant | SQL time: 4032 ms | 02:15 |
| wgrant | Non-sql time: 32597 ms | 02:15 |
| wgrant | Ah, it's the old "Product.has_release_files actually retrieves every detail about everything relating to every release file" trick. | 02:16 |
| wgrant | wallyworld: Thanks :) | 02:46 |
| wallyworld | np. i hate typos | 02:47 |
| wgrant | Oh. | 04:34 |
| wgrant | lifeless: I still fundamentally reject the way bugsummary journalling is done, but you weren't wrong about there being a lot of fat... | 04:53 |
| wgrant | There we go, 20 times faster already | 04:55 |
| * StevenK stabs stacking | 04:56 | |
| wgrant | Yes. | 04:56 |
| wgrant | That is the correct response to stacking in Launchpad :( | 04:56 |
| wgrant | Tag addition to an 80 task bug in 20ms instead of 650ms. | 04:57 |
| wgrant | And there's probably another 5-10ms or so still to trim. | 04:58 |
| StevenK | I can't figure out where a branch is created with the stacked_on set | 04:58 |
| wgrant | I guess there really was no thought put to optimisation during the mad scramble to unbreak prod | 04:58 |
| wgrant | StevenK: That would never happen on production. | 04:58 |
| wgrant | Only in a test. | 04:58 |
| StevenK | wgrant: I need a DB trigger then? | 04:58 |
| === Guest8786 is now known as Ursinha | ||
| wgrant | StevenK: No. The things that update the stacked_on field will be easily findable in the application code. | 04:59 |
| wgrant | You can put guards there. | 04:59 |
| wgrant | Code's code isn't insane. | 04:59 |
| wgrant | Bugs' code is, which is why I used triggers for the early stuff. | 04:59 |
| wgrant | Branches are also far far simpler. The only vague complications they have are retargeting and restacking. | 05:00 |
| StevenK | Let me paste a diff and I can explain the problem. | 05:00 |
| wgrant | The existing privacy model is sufficiently crap that we don't have to migrate it or work around it. | 05:00 |
| StevenK | http://pastebin.ubuntu.com/990144/ is my diff | 05:00 |
| StevenK | lp.code.model.tests.test_branch.TestBranchPrivacy.test_public_stacked_on_private_is_private fails since it's not private | 05:01 |
| wgrant | You need to define rules around what happens when stacked_on is changed, and implement them. | 05:02 |
| wgrant | Nasty lifeless writing n+1 query code in DB triggers :) | 05:04 |
| wgrant | Although I'm not quite sure why it's so slow. | 05:04 |
| wgrant | Since it's all static so it should be preplanned. | 05:04 |
| wgrant | Perhaps bulk inserts optimise heap insertion somehow. | 05:04 |
| StevenK | wgrant: So <branch namespace>.createBranch() is called, and then branch.branchChanged() is called? | 05:09 |
| wgrant | StevenK: That's how it normally happens, yes. | 05:09 |
| wgrant | branchChanged should hopefully be the only place stacked_on is changed on production. | 05:09 |
| StevenK | Looks like it | 05:09 |
| StevenK | Also looks like I'll need to change makeBranch() to do the same. | 05:10 |
| lifeless | wgrant: thats great news | 05:20 |
| lifeless | wgrant: what did you change to make it faster ? | 05:20 |
| wgrant | lifeless: First I removed the fixed_upstream thing, which I turned off display of by feature flag a couple of months back. That saved about 50% of the time. Then I fixed summarise/unsummarise/flush to do bulk inserts instead of a FOR loop. | 05:27 |
| wgrant | That saved the rest of the time. | 05:27 |
| lifeless | interesting | 05:30 |
| wgrant | Yes. | 05:30 |
| lifeless | thanks for suspending your disbelief long enough to investigate | 05:30 |
| wgrant | Heh | 05:30 |
| lifeless | well, a little tease is in order ;) | 05:30 |
| wgrant | Thanks for poking me in this direction; it will indeed be a bit quicker to implement than the redesign. | 05:30 |
| wgrant | naturally :) | 05:31 |
| lifeless | anytime | 05:31 |
| wgrant | It's not entirely implausible that eg. FK constraints are optimised in a batch INSERT, I guess. | 05:31 |
| wgrant | Since most of the FKs will be duplicates. | 05:31 |
| adeuring | good mornin | 07:51 |
| === frankban changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: frankban* (rvba) | Firefighting: - | Critical bugs: 3.47*10^2 | ||
| === almaisan-away is now known as al-maisan | ||
| StevenK_ | frankban: Oooh, is this your first shift? | 08:35 |
| frankban | yes StevenK_ | 08:35 |
| StevenK_ | Hm, where did that _ come from? | 08:36 |
| StevenK_ | frankban: However, congrats for standing up as a reviewer. | 08:36 |
| === StevenK_ is now known as StevenK | ||
| frankban | StevenK: thanks | 08:37 |
| cjwatson | Ooh, a victim! :-) | 09:51 |
| cjwatson | frankban: I've got https://code.launchpad.net/~cjwatson/launchpad/packageset-score/+merge/105915 for you, if you fancy it | 09:52 |
| cjwatson | (But not if you have more urgent stuff) | 09:52 |
| frankban | cjwatson: on it in a minute | 09:52 |
| nigelb | heh, a victim. | 09:54 |
| jamestunnicliffe | gmb: Any news on that review? https://code.launchpad.net/~dooferlad/launchpad/upcomingwork_show_incomplete_bp/+merge/105846 | 09:54 |
| gmb | jamestunnicliffe, I haven't had a chance to look at it yet; it's on my list for this morning. | 10:02 |
| jamestunnicliffe | gmb: Cool, thanks. | 10:03 |
| === al-maisan is now known as almaisan-away | ||
| danilos | mrevell, just to confirm, we have our call today? | 12:20 |
| mrevell | danilos, If you and James can make it, that would be very helpful for us. | 12:21 |
| danilos | mrevell, I can for sure, let's invite James as well | 12:21 |
| danilos | mrevell, oh, and hi btw :) | 12:22 |
| mrevell | Hello! :) | 12:22 |
| cjwatson | Good grief the whole Archive/ArchivePermission edifice isn't half wordy. | 12:50 |
| deryck | rick_h_, hey, man. | 13:20 |
| rick_h_ | deryck: howdy | 13:20 |
| rick_h_ | deryck: nvm email I sent last night, problems solved I think | 13:20 |
| rick_h_ | will bring it up in call | 13:20 |
| deryck | rick_h_, ah, ok. was about to say I couldn't find email to respond and then respond here. :) But we can chat on call then. | 13:21 |
| wgrant | cjwatson: Hm, why do you have a setScore method? | 13:26 |
| cjwatson | wgrant: score should be visible read-only on the API and have a restricted setter; happy to take suggestions on a better way to do it, I was probably cargo-culting from nearby code | 13:31 |
| * czajkowski peers at wgrant | 13:32 | |
| czajkowski | wgrant: it's not a competition who can stay up the latest you know :) | 13:33 |
| wgrant | cjwatson: Attributes have separate read and write permissions. | 13:33 |
| wgrant | cjwatson: eg. look at Builder | 13:34 |
| wgrant | <allow | 13:34 |
| wgrant | interface="lp.buildmaster.interfaces.builder.IBuilder"/> | 13:34 |
| wgrant | <allow | 13:34 |
| wgrant | interface="lp.soyuz.interfaces.buildrecords.IHasBuildRecords"/> | 13:34 |
| wgrant | <require | 13:34 |
| wgrant | permission="launchpad.Edit" | 13:34 |
| wgrant | set_schema="lp.buildmaster.interfaces.builder.IBuilder"/> | 13:34 |
| cjwatson | So maybe I can avoid the grotty IPackagesetRestricted altogether? | 13:36 |
| wgrant | interface= is for getting attributes on the interface (including methods), set_schema= for setting attributes, attributes= for reading specific non-interface attributes, set_attributes= for writing them. | 13:36 |
| wgrant | cjwatson: You probably need a separate permission. | 13:36 |
| cjwatson | Right, but I'm using launchpad.Moderate for that. | 13:37 |
| wgrant | Unless anyone who can edit a packageset should be able to edit its score.; | 13:37 |
| cjwatson | No. Build scores control access to a site-wide resource. | 13:37 |
| cjwatson | There's a test for that :) | 13:37 |
| wgrant | Right, so it needs a separate permission. But you could indeed do away with the interface, and just use set_attributes | 13:37 |
| cjwatson | But perhaps I can just do set_attributes="score". Right. | 13:37 |
| wgrant | On large interfaces we usually use sub-interfaces for read permissions. | 13:38 |
| wgrant | But write permissions often end up as set_attributes because they need to be split differently. | 13:38 |
| wgrant | Like this. | 13:38 |
| cjwatson | Does the strictest <require/> apply, or is there a problem with <require permission="launchpad.Edit" interface="lp.soyuz.interfaces.packageset.IPackagesetEdit"/> <require permission="launchpad.Moderate" set_attributes="score"/>? | 13:39 |
| wgrant | That ZCML will fail to execute. | 13:39 |
| wgrant | Conflicts like that are fatal. | 13:40 |
| cjwatson | Ugh, so I have to enumerate all the non-Moderate attributes? A separate interface suddenly looks nicer after all. | 13:41 |
| wgrant | Indeed. And a separate interface works here, since everything is world-readable. | 13:41 |
| wgrant | Things get messy when you have some stuff read-restricted, and subsets of that write-restricted, so you'd end up needing like 10 interfaces. | 13:41 |
| cjwatson | But I can still make it be set_schema= rather than having a setScore method, I guess. | 13:42 |
| wgrant | Yep. | 13:42 |
| wgrant | czajkowski: It's not yet midnight :) | 13:42 |
| wgrant | czajkowski: Some of us weren't up at 4am... | 13:43 |
| czajkowski | yeah wont be doing that again | 13:45 |
| wgrant | I am glad :) | 13:45 |
| cjwatson | wgrant: Not entirely sure how to write the setter, though. I can't use @score.setter here, can I? Doesn't look like storm implements that. | 13:45 |
| wgrant | cjwatson: No need for a setter. | 13:45 |
| wgrant | It's not a property, is it? | 13:46 |
| cjwatson | Duh. Good point. | 13:46 |
| wgrant | Right. | 13:46 |
| wgrant | You should have just to change the permission and set readonly=False on the interface. | 13:46 |
| wgrant | And it will all magically work. | 13:46 |
| wgrant | But also tests. | 13:46 |
| cjwatson | But then the restricted interface has nothing in it. | 13:46 |
| wgrant | You move score to that interface. | 13:46 |
| wgrant | The <allow interface="BlahRestricted" /> | 13:47 |
| wgrant | *Then | 13:47 |
| cjwatson | s/move/copy/ maybe? I need it in the read-only one too. | 13:47 |
| wgrant | No | 13:47 |
| wgrant | You have to have it in only one. | 13:47 |
| cjwatson | Oh I se | 13:47 |
| cjwatson | e | 13:47 |
| wgrant | So you <allow> read and <require> write | 13:47 |
| wgrant | For that interface. | 13:47 |
| cjwatson | I think the existing tests I have should cover this | 13:48 |
| cjwatson | test_score_packageset_forbids_non_buildd_admin and test_score_packageset_allows_buildd_admin | 13:48 |
| stub | deryck: Has all the translations opening been completes as far as you know? | 13:56 |
| deryck | stub, hi. As far as I know, it has *not* be complete. I filed an RT and never heard back. And I need to hand that off to jam and the blue squad now actually. | 13:57 |
| wgrant | Um | 13:57 |
| cjwatson | wgrant: Something like http://bazaar.launchpad.net/~cjwatson/launchpad/packageset-score/revision/15260 then? | 13:57 |
| wgrant | Handing an extremely fragile process that almost always breaks off to a squad that's new to Launchpad doesn't sound like the wisest of plans. | 13:58 |
| wgrant | cjwatson: Much nicer, thanks. | 13:58 |
| stub | deryck: Ta. There are some droppings in the DB I was wondering about from one of the scripts. | 13:59 |
| deryck | stub, yeah, I gave webops a script to clean that up. | 14:00 |
| stub | deryck: ok. Is that in your RT? | 14:00 |
| deryck | stub, yes | 14:00 |
| stub | Have the number handy? | 14:00 |
| cjwatson | wgrant: The only downside is that it shows up as writeable in the apidoc. | 14:00 |
| cjwatson | But I guess it is, sort of. | 14:00 |
| deryck | wgrant, maybe not the wisest of plans. but there is literally no one that understands the process, except jtv. so if he isn't doing it, anyone of us is a good as another. | 14:01 |
| wgrant | cjwatson: Yeah, permissions have always been an issue there. | 14:02 |
| wgrant | deryck: Perhaps. | 14:02 |
| wgrant | I'm not sure Blue is likely to agree :P | 14:03 |
| czajkowski | poor smurfs | 14:03 |
| jcsackett | "The smurfs"; that sound much friendlier than some of the other squad nicknames. :-P | 14:10 |
| czajkowski | jcsackett: lol | 14:11 |
| deryck | rick_h_, hey. give me 5 minutes to settle in again and we can chat | 14:59 |
| rick_h_ | deryck: sounds good | 14:59 |
| mrevell | danilos, Are we Mumbling? | 15:00 |
| mrevell | danilos, Does James have access to Mumble? | 15:00 |
| mrevell | well, our Mumble server | 15:00 |
| danilos | mrevell, flacoste, jamestunnicliffe: let's meet up in https://plus.google.com/hangouts/_/extras/linaro.org/lp-meetup (James doesn't have access to our mumble server) | 15:00 |
| flacoste | danilos: welcome to the 21st century :-) | 15:02 |
| danilos | flacoste, heh, thanks :) | 15:04 |
| danilos | though, 21st century has suddenly left me with no sound output | 15:04 |
| jamestunnicliffe | danilos: pulseaudio -k solves everything :-) | 15:04 |
| danilos | jamestunnicliffe, I was just restarting it system wide | 15:05 |
| sinzui | rick_h_, I suspect the combo-loader is broke the translation import queue. Can you take a few minutes to give your thoughts: https://bugs.launchpad.net/launchpad/+bug/1000282 | 15:08 |
| _mup_ | Bug #1000282: Uncaught TypeError on translations series +imports <combo-loader> <javascript> <Launchpad itself:Triaged> < https://launchpad.net/bugs/1000282 > | 15:08 |
| rick_h_ | sinzui: looking, have a call in a moment, but will check it out | 15:09 |
| rick_h_ | sinzui: off the bat I'd say it was a requires: issue. A module isn't requiring something it needs to, or a lint issue, if a module fails to parse due to invalid JS? I'll look deeper in a second | 15:10 |
| rick_h_ | but those are my initial thoughts | 15:10 |
| danilos | mrevell, flacoste, jamestunnicliffe: hangout hanged for me again, I'll reboot to get to a clean state | 15:10 |
| flacoste | danilos: hang out | 15:10 |
| flacoste | we are switching to the conf system | 15:10 |
| danilos | ok | 15:11 |
| flacoste | danilos: conf code: 116 862 4336 | 15:11 |
| danilos | flacoste, ack, in and waiting for the leader | 15:12 |
| flacoste | danilos: are you in? | 15:13 |
| danilos | mrevell, I am | 15:13 |
| danilos | mrevell, that's right | 15:14 |
| danilos | 12.04 has started popping up a bunch of crash reports | 15:14 |
| danilos | mrevell, there is a lot, I'll reboot | 15:14 |
| danilos | jamestunnicliffe, flacoste, mrevell: https://launchpad.net/~linaro-infrastructure/+upcomingwork | 15:21 |
| danilos | flacoste, mrevell: https://docs.google.com/spreadsheet/ccc?key=0AvOsYPy8e7yUdGkyRmx2WGFwT3NnSjdHVW04Q1pvSmc | 15:21 |
| flacoste | mrevell: danilos' email is "Per-milestone metadata in blueprints" dated from 12-03-21 | 15:25 |
| noodles775 | Hi people, we're just trying to use staging.lp for some QA, and wanted to check whether it's just a quick outage, or is it likely to be a while before it's up again? | 15:40 |
| czajkowski | noodles775: let me find out | 15:42 |
| noodles775 | Thanks czajkowski | 15:43 |
| czajkowski | noodles775: just checking | 15:46 |
| danilos | flacoste, mrevell: https://blueprints.launchpad.net/linaro-infrastructure-misc/+spec/lp-workitems-qa-tracking | 15:49 |
| danilos | jamestunnicliffe, ^ | 15:49 |
| czajkowski | noodles775: back up there now | 15:55 |
| noodles775 | czajkowski: thanks! | 16:00 |
| timrc | how do I view an oops from the web again? forgot the subdomain | 16:08 |
| czajkowski | timrc: https://lp-oops.canonical.com/ | 16:10 |
| timrc | czajkowski, thanks | 16:12 |
| timrc | anyone else getting timeouts? I tried, https://launchpad.net/ubuntu/+search?text=live-build, and it resulted in this oops: https://lp-oops.canonical.com/?oopsid=OOPS-e439dc4c04ec939b5798cebb1d5c4177 | 16:13 |
| adeuring | abentley: https://code.launchpad.net/~adeuring/launchpad/put-retry-jobs-into-celery-queue/+merge/106012 | 16:23 |
| rick_h_ | sinzui: do you have a sample url for that JS error? have a branch to fix it, but want to make sure I qa the right place | 16:27 |
| rick_h_ | sinzui: nvm, got it | 16:35 |
| sinzui | rick_h_, sorry, I was picking my son up from school: qastaging doesn't have the combo loader on at the moment so this works https://translations.qastaging.launchpad.net/+imports where as production does not | 16:46 |
| rick_h_ | sinzui: yea, got it, MP is up now | 16:46 |
| rick_h_ | waiting for the diff to update | 16:46 |
| sinzui | fab | 16:47 |
| rick_h_ | https://code.launchpad.net/~rharding/launchpad/meta_1000282/+merge/106013 if you're feeling like moving along quiclly | 16:47 |
| sinzui | my pleasure | 16:47 |
| rick_h_ | sinzui: I tried to note in the bug and the MP why it failed and how to debug that in the future hopefully | 16:47 |
| rick_h_ | let me know if it doesn't make sense | 16:47 |
| sinzui | ah | 16:48 |
| rick_h_ | but thanks for catching this, exactly the type of things hard to find automatically | 16:48 |
| sinzui | rick_h_, what are the odds that the comment is irrelevant now | 16:48 |
| rick_h_ | sinzui: not sure, I didn't look at how they were using things tbh | 16:49 |
| sinzui | rick_h_, understood. We have a problem when we do not document the version that is broken. I fixed a few bugs recently by removing our hacks because YUI does work | 16:49 |
| rick_h_ | sinzui: heh, yea. In a quick glance through the code I don't see what's 'strange' but like you say, no reference to when this was added. I guess I could annotate/match YUI versions/etc | 16:50 |
| rick_h_ | would be great if they had a link to a bug/etc | 16:50 |
| sinzui | rick_h_, you have my approval to land the fix. I will see if I can track down the version in annotate and report a bug if needed | 16:51 |
| rick_h_ | sinzui: yea, I'll remove the comment | 16:51 |
| rick_h_ | it doesn't say anything other than why oop/event are added to req | 16:51 |
| rick_h_ | and honestly, those are on every page anyway | 16:51 |
| rick_h_ | and if someone comes along, they're not going to remove those/get passing tests if it really is required | 16:51 |
| sinzui | excellent point | 16:52 |
| rick_h_ | ok, so removing comment, will get to land shortly, try to get fix out asap. Sorry to break the page | 16:52 |
| rick_h_ | guess I should apologize to czajkowski on that one | 16:52 |
| cjwatson | I seem to recall hearing before that using launchpadlib_for in tests is awfully slow, and it does seem to be true here. Is there an example somewhere of a reasonably fast and current preferred style for webservice unit tests? | 16:53 |
| czajkowski | rick_h_: eh ? | 16:54 |
| rick_h_ | czajkowski: the combo loader stuff broke the translations import queue page which is in the maint. watch stuff | 16:54 |
| rick_h_ | assuming you're doing that these days as I know we used to before you started | 16:54 |
| czajkowski | rick_h_: ahhhh | 16:54 |
| cjwatson | (I don't actually need to test launchpadlib as such, only what's exposed on the webservice.) | 16:55 |
| james_w | cjwatson, there's webservice_for I think | 16:55 |
| james_w | direct requests, rather than via lplib | 16:55 |
| cjwatson | webservice_for_person? | 16:56 |
| james_w | cjwatson, that's the one | 16:57 |
| cjwatson | Thanks, I'll give that a try | 16:57 |
| james_w | from lp.testing.pages import webservice_for_person | 16:57 |
| === frankban changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 3.47*10^2 | ||
| daker | hello | 17:22 |
| czajkowski | daker: aloha there | 17:22 |
| czajkowski | daker: coming to submit patches :-) | 17:23 |
| daker | hhhh ツ | 17:23 |
| abentley | adeuring: I believe the recommended way of creating a naive datetime from a UTC datetime is datetime.reaplace(tzinfo=None) | 17:24 |
| czajkowski | daker: folks will help if you get stuck | 17:24 |
| daker | i hope ツ | 17:24 |
| czajkowski | daker: feel free to ask here or on the mailing list, and they're very friendly | 17:24 |
| adeuring | abentley: maybe. but then we simply assume that the time delta is correct... | 17:25 |
| abentley | adeuring: How do you mean? | 17:26 |
| === matsubara is now known as matsubara-lunch | ||
| adeuring | abentley: lets me try your suggestion first... | 17:26 |
| abentley | s/reaplace/replace/ | 17:27 |
| abentley | adeuring: Catching AttributeError is too broad, because db_class.makeInstance might raise an AttributeError. | 17:28 |
| adeuring | abentley: http://paste.ubuntu.com/991017/ | 17:28 |
| abentley | adeuring: I recommend getattr(db_class, 'makeInstance', None) | 17:28 |
| adeuring | abentley: right, that is better. but check the paste above: replace(tzinfo=None) is not usable. | 17:29 |
| abentley | adeuring: I thought if Celery was using naive datetimes, they would represent UTC. They represent local time? | 17:31 |
| adeuring | abentley: yes | 17:32 |
| adeuring | abentley: perhaps that can be configured | 17:32 |
| abentley | adeuring: How about datetime.astimezone then? | 17:32 |
| adeuring | now_utc.astimezone(tz=None) | 17:34 |
| adeuring | -> astimezone() argument 1 must be datetime.tzinfo, not None | 17:34 |
| adeuring | ... a TypeError | 17:34 |
| abentley | adeuring: astimezone can be used to convert from UTC to the local timezone, and then you can strip off the tzinfo with replace. | 17:36 |
| adeuring | abentley: well, ok. But is that really much better than to use the delta? | 17:37 |
| abentley | adeuring: I think it is much better than using datetime.now() twice, since the current date is not relevant to the conversion you're performing. | 17:39 |
| adeuring | well, ok... | 17:39 |
| adeuring | abentley: do you know how figure out the local timezone? I find the example in the Python docs a bit cumbersome... | 17:42 |
| adeuring | (class LocalTimezone) | 17:42 |
| abentley | adeuring: No, I don't know how to do that. | 17:42 |
| adeuring | abentley: so.... with the delta, we don't have to deal with this ;) | 17:43 |
| abentley | adeuring: Yes, but we'd be doing a lossy conversion. | 17:44 |
| abentley | adeuring: And unnecessary system calls. | 17:44 |
| adeuring | abentley: well, maybe. But anyway, there is a different details that concerns me a bit there: It might make sense to add another 10 seconds or so to the delat, to allow for time skew between the DB time and the time seen by celeryd | 17:45 |
| adeuring | s/delat/delta/ | 17:46 |
| abentley | adeuring: I agree that's a reason for concern. | 17:47 |
| abentley | adeuring: I wonder if it makes sense to use the delta from the current DB time? | 17:48 |
| adeuring | abentley: maybe. But I would prefer a system call to another DB request... | 17:49 |
| abentley | adeuring: Using a system call doesn't address the concern of skew between the db and the celeryd system time. | 17:50 |
| adeuring | abentley: right, I'll try that. | 17:51 |
| abentley | adeuring: So AFAICT, the DB time isn't relevant. | 17:51 |
| abentley | adeuring: acquireLease doesn't use DB time. | 17:52 |
| adeuring | abentley: ah, right. | 17:52 |
| abentley | adeuring: I believe there's no possibility of skew. | 17:53 |
| adeuring | abentley: unless we have workers on more than one machine | 17:53 |
| abentley | adeuring: Right. We don't have that, and we're not expecting to have that in the near future, at least for a single queue. | 17:54 |
| abentley | adeuring: Still, I guess a fudge factor doesn't hurt here. | 17:55 |
| adeuring | abentley: right, one second should be enough | 17:56 |
| abentley | adeuring: sure. | 17:57 |
| adeuring | abentley: the example from the Python docs (class Localtime in the description of the datetime module) make _four_ calls to time.mktime() if the class is used in astimezone.astimezone(Localtime()) ;) | 18:07 |
| === matsubara-lunch is now known as matsubara | ||
| adeuring | abentley: any progress with the review? | 18:50 |
| abentley | adeuring: No, I was waiting on your changes. | 18:50 |
| adeuring | abentley: so... what should we do woth the time delta? That's my main question. I think it does not make sense to use the example from the python docs to get local timezone. | 18:51 |
| adeuring | but UI leave this decision to you... | 18:51 |
| abentley | adeuring: I agree that the example from the python docs doesn't make sense, but I find it hard to accept the proposition that calling datetime.now multiple times is the cleanest way to get a naive datetime. | 18:54 |
| adeuring | abentley: agreed, this not elegant but straightforward and easy to understand... | 18:55 |
| adeuring | and I don't think that this is an all-important aspect of what we want to achieve. | 18:56 |
| adeuring | abentley: but I am open to better suggestions | 18:57 |
| abentley | adeuring: No, respecting the lease eta was something we weren't trying to achieve at all. | 18:57 |
| adeuring | abentley: well, the other option would be to drop the lease completely. but I am not sure if it is reasonable to retry jobs vers fast | 18:57 |
| adeuring | ...very fast | 18:58 |
| abentley | adeuring: It's not reasonable, but the lease isn't the correct value to use here. If we're actually implementing retry delays, we should be doing exponential backoff so that outages of up to a day don't prevent the job from completing. | 19:01 |
| adeuring | abentley: ok, makes sense. But this is nothing I am going to implement before friday | 19:02 |
| adeuring | abentley: anyway, hereis the current diff: http://paste.ubuntu.com/991164/ | 19:06 |
| abentley | adeuring: So I suggest making it a hardcoded 10-minute delay. That will easily allow leases to expire, and give a bit of time for problems to clear. | 19:06 |
| adeuring | abentley: ok | 19:07 |
| adeuring | abentley: current diff: http://paste.ubuntu.com/991191/ | 19:19 |
| abentley | adeuring: Since 'generator' has a specific meaning in Python, how about 'factory'? | 19:20 |
| adeuring | abentley: sure | 19:20 |
| abentley | adeuring: It looks like you're using a retry delay of 1 second, rather than 10 minutes. | 19:21 |
| adeuring | abentley: gahh, fixed: retry_delay = timedelta(minutes=10) | 19:21 |
| abentley | adeuring: Are the Celery docs wrong? They say "eta must be a datetime object, specifying an exact date and time (including millisecond precision, and timezone information)" | 19:25 |
| adeuring | abentley: yes. try it for yourself | 19:25 |
| adeuring | i tried datetime objects with timezone myself... | 19:26 |
| abentley | We should file a bug on the documentation, then. | 19:27 |
| adeuring | yes | 19:28 |
| abentley | adeuring: Okay, r=me with your latest changes. | 19:31 |
| adeuring | abentley: thanks | 19:31 |
| abentley | adeuring: we should always supply an ETA, not just when there is a lease. | 19:34 |
| adeuring | abentley: why? | 19:35 |
| abentley | adeuring: If we're retrying, then we must have acquired a lease, because we always acquire a lease before running, and we can't retry until we run. | 19:36 |
| abentley | adeuring: But if there was some scenario where we could retry without having acquired a lease, we would still want to wait because it doesn't make sense to retry immediately, as you've pointed out. | 19:37 |
| adeuring | abentley: yes, that's the logic there: If a lease exists, this is an attemt to retry a job, so we need an ETA: but we don't need that when the job is run fpot the first time | 19:37 |
| adeuring | abentley: ok, i see your point, but for now i give. simply too late here... | 19:37 |
| abentley | adeuring: I'm not retracting the r=me, but I think we should fix that. | 19:39 |
| adeuring | abentley: ok, that's right | 19:39 |
| daker-cloud | czajkowski: the CPU is getting hot :) | 20:32 |
| czajkowski | daker-cloud: do ask questions if you get stuck ok | 21:36 |
| czajkowski | lotta lp dev folks around | 21:36 |
| daker-cloud | ok now i am running make schema | 21:37 |
| === spm` is now known as spm | ||
| daker-cloud | czajkowski: https://plus.google.com/u/0/101694416703170881163/posts/Ad3hvn385Po :) | 21:59 |
| sinzui | jcsackett, mumble? | 22:04 |
| czajkowski | daker-cloud: cool, what are you looking to fix | 22:06 |
| daker-cloud | not sure czajkowski, do i need a mentor ? | 22:10 |
| daker-cloud | this thing is huge | 22:10 |
| czajkowski | so lp doesnt have mentors, they do have code reviewers, and you could post to the launchpad-dev mailing list with abug you are working on or want to work on to get some feedback | 22:12 |
| czajkowski | daker-cloud: or ask people in here like sinzui wgrant jcsackett and others who are around in your timezone | 22:12 |
| daker-cloud | ok | 22:14 |
| wgrant | czajkowski: I'm American now!? | 22:28 |
| czajkowski | wgrant: no, you just dont sleep | 22:28 |
| === matsubara is now known as matsubara-afk | ||
| wgrant | True. | 22:32 |
| StevenK | wallyworld_: I've got a deployment edit all ready, let me know when your QA is done. | 23:07 |
| wallyworld_ | StevenK: done | 23:08 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!