/srv/irclogs.ubuntu.com/2016/05/24/#launchpad.txt

=== 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
freewgrant: hey there, around?08:29
wgrantfree: Hi08:31
freewgrant: 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/27828708:32
wgrantfree: I didn't. Let me see.08:34
freewgrant: cheers08:35
wgrantfree: There's no better solution for that OptimisingTestSuite hack in __init__.py?08:35
freewgrant: not if we want to avoid to create a specialised test runner08:36
freewgrant: or introduce runner-specific idioms08:36
wgrantfree: Isn't that already runner-specific, by patching testtools.run?08:46
wgrantI might be slightly more comfortable with a custom entrypoint, but surely there's a better way :(08:46
freewgrant: testtools.run by itself is not test-runner specific, afaict08:47
freewgrant: testools is unittest.TestCase compatible08:47
wgrantfree: But isn't testtools.run the testtools testrunner?08:48
wgrantlifeless: Maybe you have suggestions on the best way to do this :)08:48
freewgrant: 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 fine08:52
freewgrant: 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 above08:57
freewgrant: 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
wgrantYeah, I ignored the makefile because the buildout seems to have bitrotten somewhat.09:00
freewgrant: 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
freewgrant: "make check" seems to work fine in trunk actually, except for some spurious output that my branch fixes09:04
wgrantfree: Branch looks fine apart from that. I'll see if Robert replies, if not then it'll do :)09:04
wgrantThanks.09:05
freewgrant: 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 it09:06
wgrantfree: Right, I remember running into this once in the past.09:06
wgrantQuite possibly this very case, in txlongpoll.09:06
wgrantSo I just gave up :(09:06
wgrantBut a custom runner seems sensible here, I think.09:06
freewgrant: fwiw if you have troubles with buildout, I think this is the culprit https://pastebin.canonical.com/157090/ (from README)09:09
wgrantfree: I ran into the setup_requires pbr mess and just fell back to pip+virtualenv without debugging.09:12
freewgrant: fyi I've pushed r91 which removes the monkey patching hack and fixes the buildout breakages that I could spot10:06
lifelesswgrant: 'sup?10:10
lifelessfree: have you filed a bug on the tearDown + inlinecallbacks issue?10:13
lifelessfree: seems like a genuine bug10:13
lifelessfree: wgrant: threw a review comment up10:14
lifelessfree: there is a standard way10:15
lifelessfree: https://docs.python.org/2.7/library/unittest.html#load-tests-protocol10:15
freelifeless: yeah but is it supported by testtools/subunit?10:15
freereading the comment10:16
freelifeless: ah yeah *that* way, but does it require to add that function in every file?10:16
lifelessfree: no, you add it to __init__.py10:17
freelifeless: ah nice10:17
lifelessfree: (needs unittest2 or python3.5 because of various bugs, but testtools depends on unittest2 for that reason)10:17
freelifeless: I see10:17
lifelessfree: https://github.com/testing-cabal/testrepository/blob/master/testrepository/tests/__init__.py#L5710:18
freelifeless: 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
lifelessfree: thanks10:19
freelifeless: is the load_tests convenience in testresources release? if not I might just copy it10:21
frees/realease/released/10:21
lifelessfree: thats the concrete one from testrepository :)10:22
lifelessfree: just copy what you need from it :)10:22
freelifeless: ah :)10:22
freelifeless: pushed r93 which drops the custom runner10:34
lifelessfree: one nit11:16
freelifeless: hm did you post it?11:17
=== dupondje_ is now known as dupondje
freewgrant: 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
cjwatsonfree: Slightly more sophisticated but not by much; https://git.launchpad.net/launchpad/tree/lib/lp/scripts/runlaunchpad.py#n24113:44
cjwatson(or less sophisticated, depending on your point of view)13:44
cjwatsonTxLongPollServer is TxLongPollFixture with service_config set to "[txlongpoll]\nfrontend_port: %d\n" % config.frontend_port13:46
cjwatsonAnd txlongpollfixture is https://pypi.python.org/pypi/txlongpollfixture13:47
freecjwatson: 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 latter13:52
freecjwatson: or perhaps you do use QueueManager elsewhere in other LP services for publishing AMQP messages to be consumed by txlongpoll?13:53
cjwatsonfree: I'm no expert here but I think we just use our own more generic AMQP code for that13:55
cjwatsonWe don't use QueueManager or FrontEndAjax, at least, and our only direct txlongpoll imports are in the code that starts the service13:56
freecjwatson: 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
cjwatsonRight, I don't think we care about that level of detail at the moment.13:58
freecjwatson: cool13:58
freecjwatson: are you depending on some twisted version or are you generally tracking the latest (e.g. with backported debs or buildout/virtualenv) ?13:59
cjwatsonfree: haha a sad story14:00
cjwatsonfree: 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 buildout14:00
freecjwatson: understandable14:01
cjwatsonfree: plan is to move to virtualenv/pip but it's fairly complicated14:01
freecjwatson: I'll avoid touching anything that requires >13 then, for the time being14:01
cjwatsonfree: anyway, don't worry about that from txlongpoll's point of view, we have no pressing reason to upgrade it14:01
cjwatsonfree: lots of our dependencies are blocked on this overall issue at the moment, one more would be no big deal14:02
freecjwatson: yeah is still nice to know that you're running trunk and you can upgrade txlongpoll at any time14:02
cjwatsontrue; I'm not sure I've ever touched any longpoll code in LP :)14:02
freecjwatson: so I'd prefer preventing you guys on having yet another block14:02
cjwatsonI'm hoping to be able to find time to finish off the virtualenv/pip migration in a couple of months14:03
cjwatsonwe 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
freecjwatson: yeah, it's not hard to keep backward compatibility in txlongpoll, even if I end up needing a few tweaks14:04
cjwatsonOK14:04
=== StevenK_ is now known as StevenK
lifelessfree: I thought so?19:59
lifelessfree: I clicked on a line, entered a commment and hit ctrl-enter20:00
cjwatsonlifeless: 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
cjwatsonit is possibly slightly odd but allows for easy batching of a whole review20:01
lifelessok done20:04
lifelesscjwatson: trap for new players; not specially weird, but was not clear20:04
cjwatsonlifeless: 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 it20:08
lifelesshttps://bugs.launchpad.net/launchpad/+bug/1585354 for your prioritising pleasure20:19
ubot5Launchpad bug 1585354 in Launchpad itself "behaviour of inline code review unclear to new users" [Undecided,New]20:19
freelifeless: replied, it's actually a real documentation fix20:31
lifelessfree: k20:50
blrcjwatson: lifeless: it is a bit awkward having to make a comment (e.g. 'see inline comments') to save an inline comment.22:02
blrnot really sure what a good solution would be however...22:03
blrcjwatson: perhaps the problem is the email threshold should be higher?22:04
cjwatsonI don't know what threshold you mean22:05

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