Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define web.Application.on_startup() handler #1

Closed
wants to merge 1 commit into from

Conversation

f0t0n
Copy link
Owner

@f0t0n f0t0n commented Aug 20, 2016

What do these changes do?

According to discussion in #1092 next changes were made:

  • Define web.Application.on_startup() signal handler.
    • Run app.on_startup() along with the request handler within an event
      loop in web.run_app() and in worker.GunicornWebWorker().

Are there changes in behavior for the user?

Related issue number

aio-libs#1092

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

@f0t0n
Copy link
Owner Author

f0t0n commented Aug 20, 2016

Existing Problems

  • Tests in test_worker.py don't pass on the master branch and complain on pytest.skip():
―――――――――――――――――――――――――――――――――――――――――――― ERROR collecting tests/test_worker.py ―――――――――――――――――――――――――――――――――――――――――――――
Using @pytest.skip outside a test (e.g. as a test function decorator) is not allowed. Use @pytest.mark.skip or @pytest.mark.skipif instead.
  • Tests in test_run_app.py don't pass because loop is mocked with wrap parameter and loop.create_task(), loop.create_server() return mock (If I mock loop.create_task.return_value._source_traceback = [], it starts to complain on loop.create_server()):
―――――――――――――――――――――――――――――――――――――――――――――――――――――― test_run_app_http ―――――――――――――――――――――――――――――――――――――――――――――――――――――――

loop = <Mock spec='AbstractEventLoop' id='140501275451744'>

    def test_run_app_http(loop):
        loop = mock.Mock(spec=asyncio.AbstractEventLoop, wrap=loop)
        loop.call_later(0.01, loop.stop)

        app = web.Application(loop=loop)

>       web.run_app(app)

tests/test_run_app.py:14: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
aiohttp/web.py:287: in run_app
    loop=loop))
/usr/local/lib/python3.5/asyncio/tasks.py:607: in gather
    fut = ensure_future(arg, loop=loop)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

coro_or_future = <generator object Application.startup at 0x7fc900ac0200>

    def ensure_future(coro_or_future, *, loop=None):
        """Wrap a coroutine or an awaitable in a future.

        If the argument is a Future, it is returned directly.
        """
        if isinstance(coro_or_future, futures.Future):
            if loop is not None and loop is not coro_or_future._loop:
                raise ValueError('loop argument must agree with Future')
            return coro_or_future
        elif coroutines.iscoroutine(coro_or_future):
            if loop is None:
                loop = events.get_event_loop()
            task = loop.create_task(coro_or_future)
            if task._source_traceback:
>               del task._source_traceback[-1]
E               TypeError: 'Mock' object doesn't support item deletion

/usr/local/lib/python3.5/asyncio/tasks.py:541: TypeError

 tests/test_run_app.pytest_run_app_http ⨯                                                                         54% █████▍    

―――――――――――――――――――――――――――――――――――――――――――――――――――――― test_run_app_https ――――――――――――――――――――――――――――――――――――――――――――――――――――――

loop = <Mock spec='AbstractEventLoop' id='140501275453648'>

    def test_run_app_https(loop):
        loop = mock.Mock(spec=asyncio.AbstractEventLoop, wrap=loop)
        loop.call_later(0.01, loop.stop)

        app = web.Application(loop=loop)

        ssl_context = ssl.create_default_context()

>       web.run_app(app, ssl_context=ssl_context)

tests/test_run_app.py:29: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
aiohttp/web.py:287: in run_app
    loop=loop))
/usr/local/lib/python3.5/asyncio/tasks.py:607: in gather
    fut = ensure_future(arg, loop=loop)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

coro_or_future = <generator object Application.startup at 0x7fc9009ee3b8>

    def ensure_future(coro_or_future, *, loop=None):
        """Wrap a coroutine or an awaitable in a future.

        If the argument is a Future, it is returned directly.
        """
        if isinstance(coro_or_future, futures.Future):
            if loop is not None and loop is not coro_or_future._loop:
                raise ValueError('loop argument must agree with Future')
            return coro_or_future
        elif coroutines.iscoroutine(coro_or_future):
            if loop is None:
                loop = events.get_event_loop()
            task = loop.create_task(coro_or_future)
            if task._source_traceback:
>               del task._source_traceback[-1]
E               TypeError: 'Mock' object doesn't support item deletion

/usr/local/lib/python3.5/asyncio/tasks.py:541: TypeError

 tests/test_run_app.pytest_run_app_https ⨯                                                                        54% █████▍    

―――――――――――――――――――――――――――――――――――――――――――――― test_run_app_nondefault_host_port ―――――――――――――――――――――――――――――――――――――――――――――――

loop = <Mock spec='AbstractEventLoop' id='140501274873136'>, unused_port = 49459

    def test_run_app_nondefault_host_port(loop, unused_port):
        port = unused_port
        host = 'localhost'

        loop = mock.Mock(spec=asyncio.AbstractEventLoop, wrap=loop)
        loop.call_later(0.01, loop.stop)

        app = web.Application(loop=loop)

>       web.run_app(app, host=host, port=port)

tests/test_run_app.py:45: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
aiohttp/web.py:287: in run_app
    loop=loop))
/usr/local/lib/python3.5/asyncio/tasks.py:607: in gather
    fut = ensure_future(arg, loop=loop)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

coro_or_future = <generator object Application.startup at 0x7fc900adce60>

    def ensure_future(coro_or_future, *, loop=None):
        """Wrap a coroutine or an awaitable in a future.

        If the argument is a Future, it is returned directly.
        """
        if isinstance(coro_or_future, futures.Future):
            if loop is not None and loop is not coro_or_future._loop:
                raise ValueError('loop argument must agree with Future')
            return coro_or_future
        elif coroutines.iscoroutine(coro_or_future):
            if loop is None:
                loop = events.get_event_loop()
            task = loop.create_task(coro_or_future)
            if task._source_traceback:
>               del task._source_traceback[-1]
E               TypeError: 'Mock' object doesn't support item deletion

/usr/local/lib/python3.5/asyncio/tasks.py:541: TypeError

 tests/test_run_app.pytest_run_app_nondefault_host_port ⨯                                                         54% █████▍    

――――――――――――――――――――――――――――――――――――――――――――――――― test_run_app_custom_backlog ――――――――――――――――――――――――――――――――――――――――――――――――――

loop = <Mock spec='AbstractEventLoop' id='140501275019416'>

    def test_run_app_custom_backlog(loop):
        loop = mock.Mock(spec=asyncio.AbstractEventLoop, wrap=loop)
        loop.call_later(0.01, loop.stop)

        app = web.Application(loop=loop)

>       web.run_app(app, backlog=10)

tests/test_run_app.py:57: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
aiohttp/web.py:287: in run_app
    loop=loop))
/usr/local/lib/python3.5/asyncio/tasks.py:607: in gather
    fut = ensure_future(arg, loop=loop)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

coro_or_future = <generator object Application.startup at 0x7fc9009ee258>

    def ensure_future(coro_or_future, *, loop=None):
        """Wrap a coroutine or an awaitable in a future.

        If the argument is a Future, it is returned directly.
        """
        if isinstance(coro_or_future, futures.Future):
            if loop is not None and loop is not coro_or_future._loop:
                raise ValueError('loop argument must agree with Future')
            return coro_or_future
        elif coroutines.iscoroutine(coro_or_future):
            if loop is None:
                loop = events.get_event_loop()
            task = loop.create_task(coro_or_future)
            if task._source_traceback:
>               del task._source_traceback[-1]
E               TypeError: 'Mock' object doesn't support item deletion

/usr/local/lib/python3.5/asyncio/tasks.py:541: TypeError

@asvetlov
Copy link

Please don't touch worker in this PR but implement on_startup signal support in web.Application and web.run_app.
Documentation and tests are required.
Worker support can be done in separate PR. This way makes review process easier.

f0t0n added a commit that referenced this pull request Aug 20, 2016
Reverts changes in `worker.GunicornWebWorker` according to
#1 (comment)

They could be introduced in a separate PR.
@f0t0n
Copy link
Owner Author

f0t0n commented Aug 20, 2016

@asvetlov Agreed. Just reverted.

@f0t0n
Copy link
Owner Author

f0t0n commented Aug 20, 2016

tests are required.

I still can't get existing tests passing. And have no idea how to beat this problem.

@asvetlov
Copy link

Hmm.
What command do you use for test run?
I believe

pip install -U -r requirements-dev.txt
make test

should work.
At least it works on my local environment as well as on travis and appveyor.

@f0t0n
Copy link
Owner Author

f0t0n commented Aug 20, 2016

I followed documentation: http://aiohttp.readthedocs.io/en/stable/contributing.html

pip install -r requirements-dev.txt 
make vtest

Then tried make clean && make install && make vtest.

Now I've ran

make clean
pip install -U -r requirements-dev.txt
make test

And got same errors output.

@f0t0n
Copy link
Owner Author

f0t0n commented Aug 20, 2016

For the error with test_worker probably it's somehow related to the latest changes in pytest?

pytest.skip() now raises an error when used to decorate a test function, as opposed to its original intent (to imperatively skip a test inside a test function)...

pytest-dev/pytest#1519

@f0t0n
Copy link
Owner Author

f0t0n commented Aug 20, 2016

For an experiment I've just changed a version of pytest in requrements to pytest==2.9.2 and the test_worker.py didn't fail.

Though the problem with that mocked loop still exists since it's not related.

@asvetlov
Copy link

I've changed run_app tests to don't use mocked loop but mock explicitly loop.create_server()

@f0t0n
Copy link
Owner Author

f0t0n commented Aug 21, 2016

Cool! Thanks, Andrew!

I gonna rebase and continue to work on this.

f0t0n added a commit that referenced this pull request Aug 21, 2016
Reverts changes in `worker.GunicornWebWorker` according to
#1 (comment)

They could be introduced in a separate PR.
f0t0n added a commit that referenced this pull request Aug 22, 2016
Reverts changes in `worker.GunicornWebWorker` according to
#1 (comment)

They could be introduced in a separate PR.
@f0t0n f0t0n force-pushed the feature/application-on-startup branch 10 times, most recently from e06fc3c to 9302e7e Compare August 22, 2016 16:47
Changes:

- Define `web.Application.on_startup()` signal handler.
- Run `app.on_startup()` along with the request handler within an event
loop in `web.run_app()`.
- Tests for `Application.on_startup` signal
- Tests for multiple tasks to run on startup, including two long running
- Extend tests on `web.run_app()`: ensure that `Application.startup()`
is called within `web.run_app()`.
- Add documentation for `Application.on_startup` signal handler:
    - Describe possible use cases
    - Minor: fix typo

Notes:

In the `tests/test_run_app.py` increased delay for `loop.call_later` from
`0.01s` to `0.02s` since with the old value loop used to stop
before coroutines gathered for `on_startup` finished, that caused
an exception.
@f0t0n f0t0n force-pushed the feature/application-on-startup branch from 9302e7e to c279825 Compare August 22, 2016 16:54
@f0t0n f0t0n closed this Aug 22, 2016
@f0t0n f0t0n deleted the feature/application-on-startup branch August 22, 2016 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants