| === JanC is now known as Guest21659 | ||
| === JanC_ is now known as JanC | ||
| === nickoe_ is now known as nickoe | ||
| === benonsoftware is now known as Guest30663 | ||
| === \b is now known as benonsoftware | ||
| free | wgrant: hey there, around? | 08:29 |
|---|---|---|
| wgrant | free: Hi | 08:31 |
| free | wgrant: hi, I was wondering if you noticed this txlongpoll MP from a while back https://code.launchpad.net/~free.ekanayaka/txlongpoll/fast-rabbit-reset/+merge/278287 | 08:32 |
| wgrant | free: I didn't. Let me see. | 08:34 |
| free | wgrant: cheers | 08:35 |
| wgrant | free: There's no better solution for that OptimisingTestSuite hack in __init__.py? | 08:35 |
| free | wgrant: not if we want to avoid to create a specialised test runner | 08:36 |
| free | wgrant: or introduce runner-specific idioms | 08:36 |
| wgrant | free: Isn't that already runner-specific, by patching testtools.run? | 08:46 |
| wgrant | I might be slightly more comfortable with a custom entrypoint, but surely there's a better way :( | 08:46 |
| free | wgrant: testtools.run by itself is not test-runner specific, afaict | 08:47 |
| free | wgrant: testools is unittest.TestCase compatible | 08:47 |
| wgrant | free: But isn't testtools.run the testtools testrunner? | 08:48 |
| wgrant | lifeless: Maybe you have suggestions on the best way to do this :) | 08:48 |
| free | wgrant: yeah that's true, I think I have been running tests with a different runner and it was working because atm there's no assumption on the runner. But subclassing the testtools runner and making it mandatory sounds fine | 08:52 |
| free | wgrant: there is probably going to be a bit of annoyance for supporting subunit output, so I think I'll need to grow the branch a bit and basically create a subclass of testtools' runner that also handles subunit output. That's essentially what I meant above | 08:57 |
| free | wgrant: also looking better, at the moment my reading is that we don't really use testtools.run at all, since the check target in Makefile calls "bin/testpy -m subunit/run -- discover" and bin/testpy just sets sys.path. So we use the subunit runner. | 09:00 |
| wgrant | Yeah, I ignored the makefile because the buildout seems to have bitrotten somewhat. | 09:00 |
| free | wgrant: yeah, I guess that's the reason why just ran the tests with a random runner, which worked fine iirc (now looking again) | 09:02 |
| free | wgrant: "make check" seems to work fine in trunk actually, except for some spurious output that my branch fixes | 09:04 |
| wgrant | free: Branch looks fine apart from that. I'll see if Robert replies, if not then it'll do :) | 09:04 |
| wgrant | Thanks. | 09:05 |
| free | wgrant: looking at that now, I might add a custom runner to make things cleaner. Unfortunately this is a known limitation of testresources.OptimisingTestSuite, because there's no standard way of telling a runner to use it | 09:06 |
| wgrant | free: Right, I remember running into this once in the past. | 09:06 |
| wgrant | Quite possibly this very case, in txlongpoll. | 09:06 |
| wgrant | So I just gave up :( | 09:06 |
| wgrant | But a custom runner seems sensible here, I think. | 09:06 |
| free | wgrant: fwiw if you have troubles with buildout, I think this is the culprit https://pastebin.canonical.com/157090/ (from README) | 09:09 |
| wgrant | free: I ran into the setup_requires pbr mess and just fell back to pip+virtualenv without debugging. | 09:12 |
| free | wgrant: fyi I've pushed r91 which removes the monkey patching hack and fixes the buildout breakages that I could spot | 10:06 |
| lifeless | wgrant: 'sup? | 10:10 |
| lifeless | free: have you filed a bug on the tearDown + inlinecallbacks issue? | 10:13 |
| lifeless | free: seems like a genuine bug | 10:13 |
| lifeless | free: wgrant: threw a review comment up | 10:14 |
| lifeless | free: there is a standard way | 10:15 |
| lifeless | free: https://docs.python.org/2.7/library/unittest.html#load-tests-protocol | 10:15 |
| free | lifeless: yeah but is it supported by testtools/subunit? | 10:15 |
| free | reading the comment | 10:16 |
| free | lifeless: ah yeah *that* way, but does it require to add that function in every file? | 10:16 |
| lifeless | free: no, you add it to __init__.py | 10:17 |
| free | lifeless: ah nice | 10:17 |
| lifeless | free: (needs unittest2 or python3.5 because of various bugs, but testtools depends on unittest2 for that reason) | 10:17 |
| free | lifeless: I see | 10:17 |
| lifeless | free: https://github.com/testing-cabal/testrepository/blob/master/testrepository/tests/__init__.py#L57 | 10:18 |
| free | lifeless: re AsynchronousDeferredRunTest, probably a genuine bug, yes, I guess I'll file one and propose a branch. I've also noticed that it breaks with latest Twisted 16.2.0 (because of the move from twisted.python.log to twisted.logger) | 10:19 |
| lifeless | free: thanks | 10:19 |
| free | lifeless: is the load_tests convenience in testresources release? if not I might just copy it | 10:21 |
| free | s/realease/released/ | 10:21 |
| lifeless | free: thats the concrete one from testrepository :) | 10:22 |
| lifeless | free: just copy what you need from it :) | 10:22 |
| free | lifeless: ah :) | 10:22 |
| free | lifeless: pushed r93 which drops the custom runner | 10:34 |
| lifeless | free: one nit | 11:16 |
| free | lifeless: hm did you post it? | 11:17 |
| === dupondje_ is now known as dupondje | ||
| free | wgrant: does Launchpad invoke txlongpoll as standalone deamon with something like "twistd txlongpoll -c /some/config.yaml" or do you have a more sophisticated deployment? | 13:37 |
| cjwatson | free: Slightly more sophisticated but not by much; https://git.launchpad.net/launchpad/tree/lib/lp/scripts/runlaunchpad.py#n241 | 13:44 |
| cjwatson | (or less sophisticated, depending on your point of view) | 13:44 |
| cjwatson | TxLongPollServer is TxLongPollFixture with service_config set to "[txlongpoll]\nfrontend_port: %d\n" % config.frontend_port | 13:46 |
| cjwatson | And txlongpollfixture is https://pypi.python.org/pypi/txlongpollfixture | 13:47 |
| free | cjwatson: ah I see, that's on the "unsophisticated" side with respect to the quesiton above. I mainly wanted to know if you were using lower-level APIs like QueueManager and FrontEndAjax directly or via the plugin, and it seems the latter | 13:52 |
| free | cjwatson: or perhaps you do use QueueManager elsewhere in other LP services for publishing AMQP messages to be consumed by txlongpoll? | 13:53 |
| cjwatson | free: I'm no expert here but I think we just use our own more generic AMQP code for that | 13:55 |
| cjwatson | We don't use QueueManager or FrontEndAjax, at least, and our only direct txlongpoll imports are in the code that starts the service | 13:56 |
| free | cjwatson: okay thanks, I was considering changing the API of QueueManager slightly to leverage some new twisted features like ClientService and simpliyfing the code. Also, it'd help a bit Landscape which uses that stuff a bit more intrusively. Just wanted to know what the impact on LP would be. Anyway, nothing fully concrete for now. | 13:57 |
| cjwatson | Right, I don't think we care about that level of detail at the moment. | 13:58 |
| free | cjwatson: cool | 13:58 |
| free | cjwatson: are you depending on some twisted version or are you generally tracking the latest (e.g. with backported debs or buildout/virtualenv) ? | 13:59 |
| cjwatson | free: haha a sad story | 14:00 |
| cjwatson | free: we're stuck on a patched version of 13.0.0 at the moment because newer versions end up pulling in pbr which is hell with buildout | 14:00 |
| free | cjwatson: understandable | 14:01 |
| cjwatson | free: plan is to move to virtualenv/pip but it's fairly complicated | 14:01 |
| free | cjwatson: I'll avoid touching anything that requires >13 then, for the time being | 14:01 |
| cjwatson | free: anyway, don't worry about that from txlongpoll's point of view, we have no pressing reason to upgrade it | 14:01 |
| cjwatson | free: lots of our dependencies are blocked on this overall issue at the moment, one more would be no big deal | 14:02 |
| free | cjwatson: yeah is still nice to know that you're running trunk and you can upgrade txlongpoll at any time | 14:02 |
| cjwatson | true; I'm not sure I've ever touched any longpoll code in LP :) | 14:02 |
| free | cjwatson: so I'd prefer preventing you guys on having yet another block | 14:02 |
| cjwatson | I'm hoping to be able to find time to finish off the virtualenv/pip migration in a couple of months | 14:03 |
| cjwatson | we need to be able to move to new twisted for improved crypto support, new zope for all kinds of reasons, new testtools, etc. | 14:03 |
| free | cjwatson: yeah, it's not hard to keep backward compatibility in txlongpoll, even if I end up needing a few tweaks | 14:04 |
| cjwatson | OK | 14:04 |
| === StevenK_ is now known as StevenK | ||
| lifeless | free: I thought so? | 19:59 |
| lifeless | free: I clicked on a line, entered a commment and hit ctrl-enter | 20:00 |
| cjwatson | lifeless: that stages an inline comment, but you also have to submit the form as a whole (can be an empty comment - there should be a checkbox for whether to include inline comments) | 20:01 |
| cjwatson | it is possibly slightly odd but allows for easy batching of a whole review | 20:01 |
| lifeless | ok done | 20:04 |
| lifeless | cjwatson: trap for new players; not specially weird, but was not clear | 20:04 |
| cjwatson | lifeless: I agree; bug welcome. It does IIRC say "Unsaved comment" but there should probably be a clearer indication of what you need to do to save it | 20:08 |
| lifeless | https://bugs.launchpad.net/launchpad/+bug/1585354 for your prioritising pleasure | 20:19 |
| ubot5 | Launchpad bug 1585354 in Launchpad itself "behaviour of inline code review unclear to new users" [Undecided,New] | 20:19 |
| free | lifeless: replied, it's actually a real documentation fix | 20:31 |
| lifeless | free: k | 20:50 |
| blr | cjwatson: lifeless: it is a bit awkward having to make a comment (e.g. 'see inline comments') to save an inline comment. | 22:02 |
| blr | not really sure what a good solution would be however... | 22:03 |
| blr | cjwatson: perhaps the problem is the email threshold should be higher? | 22:04 |
| cjwatson | I don't know what threshold you mean | 22:05 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!