[00:24] desperately seeking reviewers [00:25] lifeless: Where? [00:25] I don't see a lifeless review pending. [00:25] https://code.launchpad.net/~lifeless/launchpad/bug-739799/+merge/54287 [00:26] Oh, it didn't actually exist yet. [00:26] I see. [00:26] lifeless: Still 100ms? :/ [00:28] wgrant: see the oops: 9000ms cold, new query is 900ms cold, 100ms hot [00:30] No thumper, and no StevenK yet I guess :( [00:30] Hmm? === wgrant changed the topic of #launchpad-dev to: Performance Tuesday | https://dev.launchpad.net/ | firefighting: - | On call reviewer: wgrant | https://code.launchpad.net/launchpad-project/+activereviews [00:31] StevenK: https://code.launchpad.net/~lifeless/launchpad/bug-739799/+merge/54287 [00:31] next thing is to permit bulk_load chainging [00:31] and we'll have a poor man persistence layer [00:31] Indeed. [00:32] I also need to look at a helper for collections. [00:33] wgrant: Done. [00:34] StevenK: Thanks. [00:34] StevenK, lifeless: https://code.launchpad.net/~cjwatson/launchpad/tar-xz/+merge/54215 [00:37] wgrant / sinzui can you confirm jc is still on Cve:+index ? [00:37] lifeless: He mentioned it on the call this morning. [00:37] kk [00:37] So I think so. [00:38] sob [00:38] I don't want to touch 27 / 26 ProductSet:CollectionResource:#projects [00:38] It's not that bad. [00:38] Just needs a few things preloaded. [00:38] A couple of them are collections, though. [00:38] its a nuisance determining all the traversals being hopped to [00:39] Indeed. [00:39] We may want to fix lazr.restful. [00:39] To provide a hook. [00:39] productseries should be sufficient to make the timeout go away [00:40] That was my impression, but officialbugtag might be bad too. [00:44] StevenK, lifeless: Thanks. [00:46] sinzui: [00:46] Traceback (most recent call last): [00:46] File "bin/sprite-util", line 54, in [00:46] url_prefix_substitutions={'/@@/': '../images/'}) [00:46] File "/home/robertc/launchpad/lp-branches/working/lib/lp/services/spriteutils.py", line 94, in __init__ [00:46] css_template_file, group_name, url_prefix_substitutions) [00:46] File "/home/robertc/launchpad/lp-branches/working/lib/lp/services/spriteutils.py", line 101, in _loadCSSTemplate [00:46] self.css_object = cssutils.parseFile(css_template_file) [00:46] File "/home/robertc/launchpad/lp-sourcedeps/eggs/cssutils-0.9.6-py2.6.egg/cssutils/__init__.py", line 156, in parseFile [00:46] return CSSParser().parseFile(*a, **k) [00:46] File "/home/robertc/launchpad/lp-sourcedeps/eggs/cssutils-0.9.6-py2.6.egg/cssutils/parse.py", line 130, in parseFile [00:46] return self.parseString(open(filename, 'rb').read(), [00:46] IOError: [Errno 2] No such file or directory: '/home/robertc/launchpad/lp-branches/working/lib/canonical/launchpad/icing/style-3-0.css.in' [00:51] * StevenK glares at run_script. Something looks to be running DELETE FROM FeatureFlag first [00:53] see jtv's recent landing? [00:53] StevenK: wgrant: any idea about this issue I have ? [00:54] or should I back 12638 out? [00:55] lifeless: Reproduced. [00:55] bah, not that rev. other rev [00:55] lifeless: We have another 3 hours left on buildbot. [00:55] no, actually, that rev [00:55] Let's debug slightly first. [00:56] its trying to build the style-3.0 regardless [00:57] Ah. [00:57] I wonder if bin/sprite-util didn't get rebuilt... [00:57] so the easiest fix is to rename it [00:58] indeed [00:58] Or touch something that buildout depends on. [00:58] removing it [00:58] and it fails to run [00:58] 'make' doesn't know that bin/sprite-util should be rebuilt [00:58] bin/buildout fixed [00:58] style-3-0.css.in should not exit /bin/buildout will fix the utils [00:59] lifeless: Fixed? [00:59] wgrant: made 'make' work [00:59] Looks like the buildout templates were not regenerated during the deploy [00:59] we need to land a dependency rule to rerun buildout [00:59] it may|maynot fail on deploy [00:59] lifeless: It shouldn't fail on deploy. [01:00] wgrant: we don't start with a clean tree [01:00] wgrant: do we explicitly run buildout? [01:00] Then why does it take 30 minutse to run buildout? [01:00] what [01:00] ? [01:01] bbr [01:01] brb [01:01] sinzui: Your change to sprite-util.in wasn't enough to cause buildout to be rerun. [01:02] sinzui: So bin/sprite-util is still old. [01:02] ./bin/buildout will remake the bin dir from the new bin templates [01:02] Right, but until then everyone's tree is broken. [01:03] Maybe I should just upgrade loggerhead. [01:03] Kill two birds with one stone. [01:03] Oh, no. [01:03] versions.cfg, not sourcedeps.conf :( [01:03] Mine wasn't. I think this is case of different setups. I branch and make new trees every tine [01:04] sinzui: I normally do, but sometimes run stuff from devel. [01:04] And as of yesterday I'm trying bzr-colo, so all my one tree is broken. [01:04] thanks for trying it [01:04] i do use it for lp development fwiw, but not as often as you will be [01:04] I stopped running from devel when we switched to eggs. It became unstrust worthy [01:04] what happened? [01:04] I miss simple python and debs [01:05] sinzui: Indeed. [01:05] poolie: Makefile + buildout == confusion [01:05] Compounded by running with a single tree. [01:05] sinzui, what's the alternative? [01:05] to running from devel, i mean [01:06] my frustration here is that i only touch lp stuff every 1-3 weeks and so every time i do, i need to fight with dependencies [01:08] so minimally [01:08] someone needs to mail the dev list [01:08] secondly, we should just fix Makefile to invoke buildout here [01:08] poolie: yes :( I run rocketfuel-setup the moment I start my day, merge and rebuild all my trees. I can review projects, check spam and answer questions, before all that work is complete some days [01:09] wgrant, so the breakage is not specifically from bzr-colo? [01:09] poolie: No. [01:09] poolie: colo is working OK, but the fact that it shares trees means that our screwed up build system is more exposed. [01:17] jcsackett: I have a change to lib/lp/bugs/browser/cve.py that may trivially fix the CVE:+index bug, though I'm not sure if it is/isn't related [01:19] jcsackett: actually nvm, I was on (hopeful) crack [01:23] huh [01:23] sinzui: looks to me like there is only one user of ProductSet.search : the API. [01:24] sinzui: if I'm right, could make it siiiiimpler === jtv-afk is now known as jtv [01:25] morning folks [01:36] wgrant: fyi, offiical bug tags may be an issue, but they are also locked behind a mixing behind a property hidden in a dilemma. [01:36] wgrant: e.g. TooHardBin [01:36] lifeless: Perhaps. [01:38] wgrant: certainly not low hanging fruit [01:52] hi StevenK, are you here? [02:09] lifeless: I would expect ProductSetView to use IProductSet.search(). I see it uses IPillarNameSet. I think this is because users do not care care that we divide things into distros, projects, and project groups [02:09] sinzui: I had such expectations too [02:09] sinzui: I found a total of 1 use: bugalsoaffects [02:16] can has review? https://code.launchpad.net/~lifeless/launchpad/bug-727020/+merge/54290 [02:18] jtv: I am now. What are you after? [02:18] StevenK: I figured it out for myself now. [02:19] jtv: Nice work. :-) [02:20] Well I still haven't solved my problem. :) The difficulty was finding out where Sources gets generated. Turns out the answer depends on the archive purpose. In my case it should be done by a-f. [02:31] StevenK: https://code.launchpad.net/~lifeless/launchpad/bug-727020/+merge/54290, if you would be so kind [02:34] wgrant: how is Archive:+index looking perf wise now, do you think ? [02:34] lifeless: Done. [02:35] thanks [02:35] lifeless: getBuild(Status)?Summar(y|ies)(ForSource(Id)?s(AndArchive)?)? is still being problematic. [02:35] Across +index, +packages, +copy-packages and +delete-packages [02:36] wgrant: want me to look ? [02:37] or I can fix ScopedCollection:CollectionResource:#person-page-resource [02:37] hmm, might do that one [02:37] lifeless: I'm looking. [02:37] Well, was looking before, and will be looking again in a while, I imagine... [02:38] lifeless: It needs to be rewritten to not execute four 300ms queries, basically. [02:38] wgrant: thats only 1.2 seconds [02:38] lifeless: The queries are at best 300ms. They're often much more. [02:40] jtv: Hi! :-) [02:40] uh-oh [02:40] yes? [02:40] jtv: Is there anything special I need to do to see feature flags in a script? [02:41] StevenK: if it's a LaunchpadScript, no. [02:41] If it's not a LaunchpadScript, you're still on your own. [02:41] I don't think it is. Can you point me at the scaffolding I need to set up? [02:42] Surprisingly, you can find the answer in LaunchpadScript. :) [02:42] Have a look at lp.services.scripts.base: LaunchpadScript.run [02:42] In a nutshell: [02:42] from lp.services.features import install_feature_controller, make_script_feature_controller [02:43] Ah ha, it's a JobCronScript, which bases from LaunchpadCronScript, which is based off LaunchpadScript. [02:43] install_script_feature_controller(make_script_feature_controller("identifying script name")) [02:43] Well then there you go. [02:43] The feature controller is installed only during "run." [02:43] After that, it restores whatever controller was previously active (which probably means "none"). [02:44] If you call main directly in a test, it uses whatever controller the test had set up. [02:44] I call the cronscript directly using run_script [02:45] Then it should just work (apart from ZCML taking #$%@ ages to parse during script startup) [02:45] It insists the feature flag isn't set [02:46] I shall worry about it after lunch [02:47] StevenK: my branch landed only recently, so make sure it's included. [02:50] jtv: This is with latest devel merged in [02:52] jtv: Is it because the test set up uses a fixture? [02:52] Uses a fixture for what? [02:52] self.useFixture(FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: u'on'})) [02:54] Does FeatureFixture actually change your feature-flag settings? Because it'd have to do that to have any influence on your script run. [02:54] jtv: Running the cronscript test by itself with LP_DEBUG_SQL=1, it runs "DELETE FROM FeatureFlag" [02:55] The job gets added, then there is a pause, which is ZCML and such like, and then two statements get executed, the first is the delete, and the second is checking the feature flag which is now unset [02:55] No idea where that comes from; does the FeatureFixture also INSERT your test flag (and do you commit before the script runs)? [02:56] My impression was that the FeatureFixture is an in-memory trick only, in which case it's not going to do anything for scripts. [02:56] jtv: So this is using the testcase you added :-) [02:56] How does that follow? [02:57] If FeatureFixture is in memory only, how do I go about enabling the flag? [02:57] its not in memory only AFAIK [02:57] it was discussed, not done. [02:57] IMBW [02:57] check the code [02:58] lifeless: So how can I tell what is running the DELETE? [02:58] LP_DEBUG_SQL_EXTRA=1 [02:58] I'm already running that [02:59] I was right, it is the fixture. [02:59] it should show a backtrace then [02:59] It's cleaning itself up before the script runs [03:00] StevenK: pastebin the code? [03:01] lifeless: It's in an MP: https://code.launchpad.net/~stevenk/launchpad/dsdj-runner/+merge/54159 ; I'm just pushing latest changes [03:02] Unless I cause the fixture to cleanup by running transaction.commit() [03:02] Anyway, lunch [03:02] (Finally) [03:09] grrrrr [03:09] self._fd.close() [03:09] IOError: [Errno 28] No space left on device [03:09] trying to setup a test env [03:09] because sill factory triggers [03:09] File "/home/robertc/launchpad/lp-sourcedeps/eggs/zope.sendmail-3.7.1-py2.6.egg/zope/sendmail/delivery.py", line 136, in createDataManager [03:09] msg.close() [03:10] File "/home/robertc/launchpad/lp-sourcedeps/eggs/zope.sendmail-3.7.1-py2.6.egg/zope/sendmail/maildir.py", line 136, in close [03:30] StevenK: looks like the fixture DELETE should happen at the end of the test, not before === jamesh_ is now known as jamesh [03:50] wgrant: I'm driving myself up the walls trying to find out why a-f isn't generating any Sources files for me. One thing I notice is that there's no trace of binary packages in the setup that the test publisher has produced for me… is that normal? [03:52] I do get a _main_source referring to an existing dsc file… what else is needed? [03:52] jtv: What does the a-f conf look like? [03:52] * jtv finds pastebin [03:53] wgrant: I hope apt.conf is the one you need… http://paste.ubuntu.com/583629/ [03:53] jtv: That's the one. [03:54] So, that should work. But I wonder if it's not working because you have no DASes. [03:54] I'm not sure anyone has tested that. [03:54] So the test publisher doesn't set any up? [03:54] Not if you just makeDistroSeries, no. [03:54] Also, the contents.header that that config refers to doesn't exist prior to the a-f run. [03:54] It's not designed to placate buggy a-f stuff. [03:54] Right, that's fine. [03:55] Trying it with a DAS in place, just for the hell of it… [03:56] It shouldn't make a difference, but you never know. [03:58] I'm just getting more error messages out of a-f. :) [03:58] It now also says "could not open file" about the Packages files. [03:58] Could you pastebin the full output? [03:58] Or throw the branch my way? [04:00] wgrant: the branch is lp:~jtv/launchpad/bug-55798 ; run ./bin/test lp.soyuz.scripts.tests.test_publish_ftpmaster -t publishes [04:00] I've set it up to tar up directory contents in /tmp, though not all of that is committed (because I don't want it in my final branch) [04:04] lifeless: It looks to me that the commit is setting it off [04:09] wgrant: another fairly wild guess: if none of this has been tested with distros other than Ubuntu, could something be reacting badly to the test distro not having (or populating) all the pockets that ubuntu has? [04:09] StevenK: the commit will commit all pending stuff [04:09] StevenK: which will include a cleanout of *prior* featureflags [04:09] StevenK: and should include an add of yours [04:10] StevenK: check the FeatureFixture code [04:10] jtv: Sorry, got distracted by uncontrollable anti-Microsoft rage. [04:10] Let's see. [04:10] wgrant: never apologize for that. It's Microsoft's fault and don't stop until they're gone. [04:12] argh [04:12] activemembers unions with adminmembers. why [04:14] lifeless: That doesn't explain what I see with LP_DEBUG_SQL_EXTRA. http://pastebin.ubuntu.com/583636/ [04:15] StevenK: thats after the test ran [04:16] File "/home/steven/launchpad/lp-sourcedeps/eggs/testtools-0.9.8-py2.6.egg/testtools/runtest.py", line 154, in _run_cleanups [04:16] StevenK: so its irrelevant to you === Ursinha is now known as Ursinha-afk [04:16] Drat, pasted the wrong bit [04:17] lifeless: http://pastebin.ubuntu.com/583637/ [04:18] lifeless: The traceback from the commit is half-way through the test [04:25] jtv: Ah [04:27] StevenK: still in *test* cleanups [04:27] File "/home/steven/launchpad/lp-sourcedeps/eggs/testtools-0.9.8-py2.6.egg/testtools/runtest.py", line 154, in _run_cleanups [04:30] lifeless: So the code for the statement is before the statement, http://pastebin.ubuntu.com/583640/ shows the entire test, and you can plainly see it runs the clean ups after line 6 of the test [04:33] StevenK: this suggests that that commit is failing [04:33] StevenK: raising an exception, which triggers cleanups [04:33] StevenK: i promise you, you've left your entire test according the backtraces you are showing me [04:34] StevenK: if I was debugging this, I would break in with pdb about now [04:34] Project db-devel build #477: FAILURE in 5 hr 1 min: https://lpci.wedontsleep.org/job/db-devel/477/ [04:35] argh [04:35] ---------------------------------------------------------------------- [04:35] SELECT 1 FROM (SELECT PersonTransferJob.json_data, PersonTransferJob.id, PersonTransferJob.job, PersonTransferJob.job_type, PersonTransferJob.major_person, PersonTransferJob.minor_person FROM Job, PersonTransferJob WHERE PersonTransferJob.job_type = 1 AND PersonTransferJob.job = Job.id AND Job.status IN (0, 1, 4) AND (PersonTransferJob.minor_person = 251673 OR PersonTransferJob.major_person = 251673) LIMIT 1) AS "_tmp" LIMIT 1 [04:35] ---------------------------------------------------------------------- [04:35] (repeat for the batch size) [04:35] SELECT 1 FROM (SELECT PersonTransferJob.json_data, PersonTransferJob.id, PersonTransferJob.job, PersonTransferJob.job_type, PersonTransferJob.major_person, PersonTransferJob.minor_person FROM Job, PersonTransferJob WHERE PersonTransferJob.job_type = 1 AND PersonTransferJob.job = Job.id AND Job.status IN (0, 1, 4) AND (PersonTransferJob.minor_person = 251672 OR PersonTransferJob.major_person = 251672) LIMIT 1) AS "_tmp" LIMIT 1 [04:35] jtv: Ran 1 tests with 0 failures and 0 errors in 32.236 seconds. [04:35] wgrant: yes, but no Sources files. Check /tmp/var_archive.tar.gz. [04:37] jtv: It's empty, but it's there. [04:37] (because I fixed it) [04:37] wgrant: you do know how to keep the tension up, don't you? [04:37] Indeed. [04:37] How did you fix it, and why is it empty? [04:37] You had your paths wrong [04:37] config.archivepublisher.root on prod is /srv/launchpad.net/ubuntu-archive, not /srv/launchpad.net. [04:37] And what's all this PPA business? [04:40] lp:~wgrant/launchpad/jtv-bug-55798 [04:40] What is what PPA business? [04:41] Your test and cron.publish-ftpmaster talk about PPAs. [04:41] cron.publish-ftpmaster does not go near PPAs. [04:41] Oh, right, there was that alternate codepath that could never have worked. [04:42] You speak in riddles. [04:42] - ARCHIVEROOT_PARTNER=$PUBLISHER_ROOT/ppa/$DISTRONAME-partner [04:42] All my test did was create some ppa directories that appeared to be needed for anything to run. [04:42] You added that. [04:43] Possibly because the original did this: [04:43] - ARCHIVEROOT_PARTNER=/srv/launchpad.net/ppa/ubuntu-partner [04:43] How is that different? [04:43] That branch has the minor issue of not making any sense at all. [04:43] So I removed it. [04:43] What branch? [04:44] -if [ "$LPCONFIG" = "$PRODUCTION_CONFIG" ]; then [04:44] - ARCHIVEROOT_PARTNER=/srv/launchpad.net/ubuntu-archive/ubuntu-partner [04:44] - GNUPGHOME=/srv/launchpad.net/ubuntu-archive/gnupg-home [04:44] -else [04:44] - # GNUPGHOME does not need to be set, keys can come from ~/.gnupg. [04:44] - ARCHIVEROOT_PARTNER=/srv/launchpad.net/ppa/ubuntu-partner [04:44] -fi [04:44] That's how it is in devel now. [04:44] The second part of that is broken. [04:44] You mean "branch" as in "if"? [04:44] Yes. [04:44] Sorry. [04:45] bah [04:45] wgrant: sinzui is gone, can you show https://bugs.launchpad.net/launchpad/+bug/739961 to him tomorrow ? [04:45] <_mup_> Bug #739961: person collections in API late evaluate PersonTransferJob < https://launchpad.net/bugs/739961 > [04:45] wgrant: Okay, but does this change really affect the Sources issue? I can see how the _other_ change affects it. [04:45] lifeless: Sure. [04:46] review plox https://code.launchpad.net/~lifeless/launchpad/bug-727540/+merge/54293 [04:46] jtv: I don't recall exactly. But the partner dists move was crashing until I fixed that. [04:48] wgrant: your change does mean that ARCHIVEROOT becomes /srv/launchpad.net/ubuntu instead of /srv/launchpad.net/ubuntu-archive/ubuntu as it currently is, doesn't it? [04:49] jtv: Yes. [04:49] Er. [04:49] Er. [04:49] Where does cron.publish-ftpmaster get this info? [04:49] It duplicates what it finds in the lazr config. [04:49] So it should be fine, then. [04:49] Which is one of the things that made this untestable. [04:50] Because it will see config.archivepublisher.root == /srv/launchpad.net/ubuntu-archive [04:50] But that's different yet again. [04:51] On development systems, by the way, it's all in /var/tmp/archive [04:53] Right. [04:53] But how is it different yet again? [04:53] Assuming that you give cron.publish-ftpmaster config.archivepublisher.root, it should behave the same on prod as the one in devel does. [04:55] StevenK: perhaps you could review https://code.launchpad.net/~lifeless/launchpad/bug-727540/+merge/54293 ? its very simple and has its diff now [04:55] StevenK: I'm going to grab dinner, after that I can perhaps pull your branch down and have a poke for you [04:58] wgrant: except that the script duplicates it all in its variables. You just said the production config said /srv/launchpad.net/ubuntu-archive, but you changed the script to use /srv/launchpad.net/ubuntu instead. [04:58] This is all so confusing. [04:58] lifeless: So where is the eager loading? [04:59] jtv: The production dists tree lives in /srv/launchpad.net/ubuntu-archive/ubuntu/dists [04:59] jtv: The dev one traditionally lives in /var/tmp/archive/ubuntu/dists [04:59] oic! [05:00] So then you missed a change: [05:00] PUBLISHER_ROOT="${PUBLISHER_ROOT:-/srv/launchpad.net/ubuntu-archive}" [05:00] Right? [05:00] StevenK: _members(need_validity..) etc [05:01] wgrant: in other words, the "ubuntu-archive" goes into the $PUBLISHER_ROOT instead of being added in ARCHIVEPARENT. [05:01] And in fact PUBLISHER_ROOT and ARCHIVEPARENT are the same thing [05:01] jtv: I did enough to make the tests pass. [05:01] (which the script previously hid by using $ARCHIVEROOT/..) [05:02] I didn't actually look at what changes you really made. [05:02] Enough to get the tests to pass with a sane path structure, that is. [05:02] It's pernicious because there's no way of testing it. Eventually it'll all go; right now I'm concerned with the "chalk outline" that the new version must fill. [05:03] Righ. [05:03] Rhymes with "Sigh." :) [05:03] * jtv looks up production config [05:04] Also note that Julian's config work will change how this works, but the values should stay the same. [05:04] Just your tests will need to change, if they decide to override configs. [05:15] They don't—wouldn't work for external script runs. [05:16] Well, it can. [05:16] We have facilities for that now. [05:16] Indeed you probably should. [05:17] (eg. the librarian and DB fixtures push config overlays then write them to disk) [05:23] That may become necessary later as we move this script to python and create more appropriate tests. Then again, at that point it won't involve script runs any more so we should be able to do more in-memory. [05:25] wgrant: you were right by the way: the archivepublisher's "root" setting on production is /srv/launchpad.net/ubuntu-archive. [05:25] jtv: Yup. [05:26] Thanks for helping me with this. I'm reluctant to drop the ppa stuff until I've got something of a proper test hammered out, but it'd be good to streamline things then. [05:26] Right. [05:27] Food. === jtv is now known as jtv-eat [05:35] Right, something odd is going on, since I've added the flag manually, and the cron script still can't see it. [05:39] * StevenK doesn't understand feature flags [05:40] StevenK: does your script install a flag handler? [jtv's work was in this area] [05:41] lifeless: jtv added it to LaunchpadScript's run() method from what I can see, and my script (through 2 layers of indirection) bases off it [05:44] lifeless: Unless I'm wrong. What's a flag handler? [05:46] sadness [05:47] 1112 timeouts [05:47] StevenK: featurecontroller or whatever [05:47] install_feature_controller(make_script_feature_controller(self.name)) [05:47] Is that enough? [05:51] lifeless: ^ [05:51] yeah, should be [05:51] StevenK: whats the feature controller present when your code does a flag lookup ? [05:51] How do I check that? [05:52] print ? [05:52] Sure, but print *what* ? :-) [05:52] lp.services.features.mumble [05:55] Guessing get_relevant_feature_controller(), which gives None [05:56] lifeless: ^ Which is likely sub-optimal [05:58] StevenK: that is indeed likely to be a problem [05:58] StevenK: is your script threaded? [05:58] lifeless: I have no idea, but I doubt it. [06:07] timeouts top to bottom - ugh, ugh, ugh, fixed fixed fixed ugh ugh in-progress ugh ugh ugh ugh [06:08] StevenK: so, keep spattering prints around - like after install_feature_controller - until you can see what goes wrong [06:10] ok seriously did someone rename style-3-0.css.in? [06:10] huwshimi: run bin/buildout [06:14] lifeless: So directly after install_feature_controller, it's still None [06:15] so, either you're looking in the wrong place, or that method is failing ;) [06:16] I don't get what install_feature_controller is supposed to do [06:19] StevenK: it's a simple setter for the feature controller, though it only applies to the running thread. === jtv-eat is now known as jtv [06:19] I'm close to ripping this out for now [06:20] lifeless: That didn't help. I think it has been renamed. [06:20] I don't understand it, and it seems quite opaque [06:20] huwshimi: delete bin/sprite-util [06:20] huwshimi: then run it [06:20] Or make clean; make [06:21] StevenK: let's start by establishing that you're talking to the right feature controller. [06:21] But that's a bigger hammer [06:22] StevenK: if get_relevant_feature_controller returns None right after install_feature_controller, then are you sure you didn't pass it a None? [06:22] 'install_feature_controller(make_script_feature_controller(self.name))' is the call [06:23] Which is the line you added to LaunchpadScript [06:23] Yes [06:24] Oh actually you don't need to install it; make_script_feature_controller calls install_feature_controller. [06:24] After which in fact it returns None. [06:24] Oh, so it's broken [06:24] :-P [06:24] Yup. [06:24] Fixing. [06:25] It's fine, I can do it [06:25] You can replace install_feature_controller(make_script_feature_controller(self.name)) with just make_script_feature_controller; or better yet, [06:25] make make_script_feature_controller return the controller instead of installing it. [06:27] We don't need the controller, so if it installs it, that's enough [06:27] jtv: This was the failing test for me, so I can just fix the bug in my branch if you wish [06:27] True, it's just nice to export just a single function to set the thing. [06:27]