txaio icon indicating copy to clipboard operation
txaio copied to clipboard

accept loop= kwarg

Open meejah opened this issue 7 years ago • 3 comments

See https://github.com/crossbario/autobahn-python/issues/747 for context.

asyncio allows passing in a loop= kwarg to the Future constructor. Currently, we always pass the One True Loop (whatever config.loop says).

txaio.create_future APIs should accept an (optional) loop= kwarg (or reactor= or loop_or_reactor=?). It's an error on Twisted if this is not the same object as config.loop. For asyncio, we pass it along to Future.

The affected APIs are:

  • txaio.create_future
  • txaio.as_future
  • txaio.call_later
  • txaio.make_batched_timer

meejah avatar Apr 06 '17 01:04 meejah

The tough ones are as_future and call_later as they don't have a nice way to just add a new loop=None kwarg.

We discussed a few options on IRC, and concluded:

  • breaking the API totally by adding a positional "loop" arg is Very Bad
  • pop()-ing "loop" from kwargs is Bad (and will lead to weird, surprising errors in code that happens to already take a loop kwarg)
  • using something like the testutils.replace_loop context manager is weird and might not work reliably anyway

So we will try by introducing an "instantiate an API object"-type method (final naming TBD) that has all the txaio API methods hanging off it and takes an (optional) custom loop argument. So code like this:

f0 = txaio.as_future(...)
f1 = txaio.call_later(...)

becomes:

api = txaio.create_api(loop=asyncio.new_event_loop())
f0 = api.as_future(...)
f1 = api.call_later(...)

This won't change any existing code, but code that wants to use many custom loops can opt-in by using the .create_api() call.

meejah avatar Apr 06 '17 16:04 meejah

Worth noting: you can already use an explicit event-loop (or reactor), but only one. This feature is for users of asyncio to use multiple event loops at once. Twisted doesn't (yet) support this.

meejah avatar Apr 06 '17 16:04 meejah

See https://github.com/crossbario/txaio/pull/98#issuecomment-292713470

meejah avatar Apr 29 '17 02:04 meejah