Clean up loop handling

[Archelyst]

Lately, there have been some changes regarding the loop which resulted in kind of a messy situation. I'd suggest, instead of making a bunch of small uncoordinated changes, let's discuss how we'd actually want to have it.

The issues I see are

1. app.run() accepts a loop parameter which it says will be deprecated in 0.4.0, however, it already ignores that parameter now. Let's either completely remove it or if we accept it, it should do something.
2. app.run() takes the default loop (via _helper) and passes it to serve(). Hence, the code creating a loop in serve() will never execute.
3. As a consequence, Sanic never uses uvloop, except if one creates a loop beforehand and sets it as default loop. That's not documented at all and IMHO we should strive to using uvloop as a default.

While we're at it, we should fix _helper and it's usages. Some params are not used, another is misspelled.

I'd be happy to provide a PR, but let's agree on a bearing way first.

[Archelyst]

Let's also talk about testing in that context. I find it really hard to test a Sanic app. My current blocker is a RuntimeError('Event loop is closed') that gets raised whenever two sanic_endpoint_test()s are executed in the same test case. The problem here is that when Sanic stops, it closes the loop. In the next execution, it just takes the default loop and tries to work with that... closed.

I'm using pytest_async with quite some success, but here it doesn't help. By the way, you can observe the same failure with current master. Let us try to find a good way for testing (API tests and unit tests) and document that. Any suggestions?

[seemethere]

Yeah I'm observing the failure right now, I'm going to roll back the commit that cause it.

[seemethere]

Fixed the RuntimeError with 487e335

[seemethere]

No I understand the confusion, I think it stems from us trying to solve #275, but I don't think there's enough of a use-case to actually have a non-blocking way to run the server.

[Archelyst]

What I liked about the commit was test_loop_policy.py, could you re-add that?

[seemethere]

I have an idea for how to handle the loop policy / loop sharing, I'll post it up in another commit but needless to say it'll involve addressing concerns over us messing with asyncio global settings (Which I'm not a fan of anyway.)

I'll have the PR up later this week.

[r0fls]

Things are definitely a mess. This is what happens for me when I run the simple_server.py example with debug set to True:

$ python3 simple_server.py 
/Users/r0fls/Documents/code/foss/sanic/sanic/sanic.py:374: DeprecationWarning: Passing a loop will be deprecated in version 0.4.0 https://github.com/channelcat/sanic/pull/335 has more information.
  DeprecationWarning)
2017-01-31 17:40:23,988: DEBUG: 
                 ▄▄▄▄▄
        ▀▀▀██████▄▄▄       _______________
      ▄▄▄▄▄  █████████▄  /                 \
     ▀▀▀▀█████▌ ▀▐▄ ▀▐█ |   Gotta go fast!  |
   ▀▀█████▄▄ ▀██████▄██ | _________________/
   ▀▄▄▄▄▄  ▀▀█▄▀█════█▀ |/
        ▀▀▀▄  ▀▀███ ▀       ▄▄
     ▄███▀▀██▄████████▄ ▄▀▀▀▀▀▀█▌
   ██▀▄▄▄██▀▄███▀ ▀▀████      ▄██
▄▀▀▀▄██▄▀▀▌████▒▒▒▒▒▒███     ▌▄▄▀
▌    ▐▀████▐███▒▒▒▒▒▐██▌
▀▄▄▄▄▀   ▀▀████▒▒▒▒▄██▀
          ▀▀█████████▀
        ▄▄██▀██████▀█
      ▄██▀     ▀▀▀  █
     ▄█             ▐▌
 ▄▄▄▄█▌              ▀█▄▄▄▄▀▀▄
▌     ▐                ▀▀▄▄▄▀
 ▀▀▄▄▀

2017-01-31 17:40:23,989: INFO: Goin' Fast @ http://0.0.0.0:8000
2017-01-31 17:40:23,993: INFO: Starting worker [57696]
^C2017-01-31 17:40:25,130: INFO: Stopping worker [57696]
2017-01-31 17:40:25,131: INFO: Server Stopped
/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/base_events.py:508: ResourceWarning: unclosed event loop <_UnixSelectorEventLoop running=False closed=False debug=False>

Note that I'm not passing a loop, but I still get the deprecation warning (because run passes a loop to serve). Also, the ResourceWarning -- one of the loops isn't being closed, because we're creating a new separate loop in serve and also using get_event_loop in run.

[r0fls]

Both issues go away when I stop passing the loop from run to serve. This is probably not recommended anyway, so I think we should remove that.

[lvalladares]

I can confirm RuntimeError: Event loop is closed keeps happening when trying to use the loop after calling sanic_endpoint_test under sanic 0.3.1. In my case im using python unittest and i have a simple test that just calls to sanic_endpoint_test with some simple params, later in my tearDown method i do something like this: asyncio.get_event_loop().run_until_complete(coro) and im getting the following traceback:

Traceback (most recent call last):
File "/usr/share/nginx/html/sanicApp/tests_integracion/test_integracion.py", line 21, in tearDown
asyncio.get_event_loop().run_until_complete(config.conexion.execute(config.db.archivos.delete()))
File "uvloop/loop.pyx", line 1178, in uvloop.loop.Loop.run_until_complete (uvloop/loop.c:25152)
File "uvloop/loop.pyx", line 532, in uvloop.loop.Loop._check_closed (uvloop/loop.c:13856)
RuntimeError: Event loop is closed

if i just remove the sanic_endpoint_test call everything works fine

[seemethere]

@lvalladares Why would you need to use the loop after running sanic_endpoint_test?

[lvalladares]

@seemethere im using motor for async mongo database handling and before i make a sanic_endpoint_test i populate the database with some dummy data for the tests, after the test i want to drop all the data, because the python unittest module is sync after sanic_endpoint_test i call asyncio.get_event_loop().run_until_complete(db.archivos.delete()).

I think the use case can be any async cleanup operation after the test.

[r0fls]

@lvalladares does it work to get a new event loop afterwards and run it there? For example:

class WidgettestCase(unittest.TestCase):
    app = Sanic()

    def tearDown(self):
        loop = asyncio.new_event_loop()
        loop.run_until_complete(db.archivos.delete())

    def test_sanic(self):
        @app.route('/')
        def hello(request):
            return text('ok')

        req, res = app.test_client.get('/')
        assert res.body == b'ok'

[Archelyst]

What about use cases where I want to create sth via the API and then also read it? That's a pretty simple case.

[lvalladares]

@r0fls nop, doesnt work, i get the following error:

RuntimeError: Task <Task pending coro=<MiddlewareConexion.process_request() running at /usr/share/nginx/html/falcon/librerias/middlewareConexion.py:23> cb=[run_until_complete.<locals>.<lambda>()]> got Future <Future pending cb=[_chain_future.<locals>._call_check_cancel() at /usr/local/lib/python3.5/asyncio/futures.py:431]> attached to a different loop

i workaround it making all my calls on tearDown sync but this is very hard with some packages.

[seemethere]

Closing. I think we took care of most of the concerns here.

Will re-open if necessary.