=== 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
[08:29] <free> wgrant: hey there, around?
[08:31] <wgrant> free: Hi
[08:32] <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:34] <wgrant> free: I didn't. Let me see.
[08:35] <free> wgrant: cheers
[08:35] <wgrant> free: There's no better solution for that OptimisingTestSuite hack in __init__.py?
[08:36] <free> wgrant: not if we want to avoid to create a specialised test runner
[08:36] <free> wgrant: or introduce runner-specific idioms
[08:46] <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:47] <free> wgrant: testtools.run by itself is not test-runner specific, afaict
[08:47] <free> wgrant: testools is unittest.TestCase compatible
[08:48] <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:52] <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:57] <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
[09:00] <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:02] <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:04] <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:05] <wgrant> Thanks.
[09:06] <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:09] <free> wgrant: fwiw if you have troubles with buildout, I think this is the culprit https://pastebin.canonical.com/157090/ (from README)
[09:12] <wgrant> free: I ran into the setup_requires pbr mess and just fell back to pip+virtualenv without debugging.
[10:06] <free> wgrant: fyi I've pushed r91 which removes the monkey patching hack and fixes the buildout breakages that I could spot
[10:10] <lifeless> wgrant: 'sup?
[10:13] <lifeless> free: have you filed a bug on the tearDown + inlinecallbacks issue?
[10:13] <lifeless> free: seems like a genuine bug
[10:14] <lifeless> free: wgrant: threw a review comment up
[10:15] <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:16] <free> reading the comment
[10:16] <free> lifeless: ah yeah *that* way, but does it require to add that function in every file?
[10:17] <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:18] <lifeless> free: https://github.com/testing-cabal/testrepository/blob/master/testrepository/tests/__init__.py#L57
[10:19] <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:21] <free> lifeless: is the load_tests convenience in testresources release? if not I might just copy it
[10:21] <free> s/realease/released/
[10:22] <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:34] <free> lifeless: pushed r93 which drops the custom runner
[11:16] <lifeless> free: one nit
[11:17] <free> lifeless: hm did you post it?
=== dupondje_ is now known as dupondje
[13:37] <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:44] <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:46] <cjwatson> TxLongPollServer is TxLongPollFixture with service_config set to "[txlongpoll]\nfrontend_port: %d\n" % config.frontend_port
[13:47] <cjwatson> And txlongpollfixture is https://pypi.python.org/pypi/txlongpollfixture
[13:52] <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:53] <free> cjwatson: or perhaps you do use QueueManager elsewhere in other LP services for publishing AMQP messages to be consumed by txlongpoll?
[13:55] <cjwatson> free: I'm no expert here but I think we just use our own more generic AMQP code for that
[13:56] <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:57] <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:58] <cjwatson> Right, I don't think we care about that level of detail at the moment.
[13:58] <free> cjwatson: cool
[13:59] <free> cjwatson: are you depending on some twisted version or are you generally tracking the latest (e.g. with backported debs or buildout/virtualenv) ?
[14:00] <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:01] <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:02] <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:03] <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:04] <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
=== StevenK_ is now known as StevenK
[19:59] <lifeless> free: I thought so?
[20:00] <lifeless> free: I clicked on a line, entered a commment and hit ctrl-enter
[20:01] <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:04] <lifeless> ok done
[20:04] <lifeless> cjwatson: trap for new players; not specially weird, but was not clear
[20:08] <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:19] <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:31] <free> lifeless: replied, it's actually a real documentation fix
[20:50] <lifeless> free: k
[22:02] <blr> cjwatson: lifeless: it is a bit awkward having to make a comment (e.g. 'see inline comments') to save an inline comment.
[22:03] <blr> not really sure what a good solution would be however...
[22:04] <blr> cjwatson: perhaps the problem is the email threshold should be higher?
[22:05] <cjwatson> I don't know what threshold you mean