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

Issue with session #36

Closed
muminoff opened this issue Feb 22, 2017 · 21 comments
Closed

Issue with session #36

muminoff opened this issue Feb 22, 2017 · 21 comments

Comments

@muminoff
Copy link
Contributor

Hey @szastupov,

Was using until the new version bumped, never had an issue. After getting installed new version from pip, my code throws error. Guessing that it might have problem with aiohttp session.

My code:

# Asyncio
import asyncio

# UVLoop
import uvloop

# Asyncpg
from asyncpg import create_pool as create_pg_pool

# Aiobotocore
from aiobotocore import get_session as boto_session

# Bot
from bot import bot

# Misc
import os
from urllib.parse import urlparse

# Use uvloop
asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())


async def run_bot():
    await bot.loop()


async def make_pg_pool():
    dsn = os.environ.get('DATABASE_URL')
    return await create_pg_pool(
        dsn=dsn,
        min_size=10,
        max_size=20)

async def make_s3_client(loop):
    aws_access_key_id = os.environ.get('AWS_ACCESS_KEY_ID')
    aws_secret_access_key = os.environ.get('AWS_SECRET_ACCESS_KEY')
    session = boto_session(loop=loop)
    return session.create_client(
        's3', region_name='us-east-1',
        aws_access_key_id=aws_access_key_id,
        aws_secret_access_key=aws_secret_access_key)

# Main event loop
loop = asyncio.get_event_loop()

# Attach postgres connection pool to bot
pg_pool = loop.run_until_complete(make_pg_pool())
setattr(bot, 'pg_pool', pg_pool)

# Attach s3 client to bot
s3_client = loop.run_until_complete(make_s3_client(loop))
setattr(bot, 's3_client', s3_client)


if __name__ == '__main__':
    import sys
    if len(sys.argv) == 2 and sys.argv[1] == 'loop':
        loop.run_until_complete(run_bot())
    else:
        bot.run_webhook(os.environ.get('APP_URL') + 'webhook')

Exception:

$ python main.py
Creating a client session outside of coroutine
client_session: <aiohttp.client.ClientSession object at 0x7f2fc36aa4e0>
DEBUG:botocore.loaders:Loading JSON file: /home/vb/.env/lib/python3.5/site-packages/botocore/data/endpoints.json
DEBUG:botocore.loaders:Loading JSON file: /home/vb/.env/lib/python3.5/site-packages/botocore/data/s3/2006-03-01/service-2.json
DEBUG:botocore.loaders:Loading JSON file: /home/vb/.env/lib/python3.5/site-packages/botocore/data/_retry.json
DEBUG:botocore.client:Registering retry handlers for service: s3
DEBUG:botocore.hooks:Event creating-client-class.s3: calling handler <function add_generate_presigned_post at 0x7f2fc3eac048>
DEBUG:botocore.hooks:Event creating-client-class.s3: calling handler <function add_generate_presigned_url at 0x7f2fc3ea6400>
DEBUG:botocore.args:The s3 config key is not a dictionary type, ignoring its value of: None
DEBUG:botocore.endpoint:Setting s3 timeout as (60, 60)
DEBUG:botocore.client:Defaulting to S3 virtual host style addressing with path style addressing fallback.
DEBUG:aiotg:api_call setWebhook, {'url': 'https://vb.muminoff.uz/webhook'}
Traceback (most recent call last):
  File "main.py", line 61, in <module>
    bot.run_webhook(os.environ.get('APP_URL') + 'webhook')
  File "/home/vb/project/aiotg/bot.py", line 118, in run_webhook
    loop.run_until_complete(self.set_webhook(webhook_url, **options))
  File "uvloop/loop.pyx", line 1203, in uvloop.loop.Loop.run_until_complete (uvloop/loop.c:25632)
  File "uvloop/future.pyx", line 146, in uvloop.loop.BaseFuture.result (uvloop/loop.c:109361)
  File "uvloop/future.pyx", line 101, in uvloop.loop.BaseFuture._result_impl (uvloop/loop.c:108900)
  File "uvloop/future.pyx", line 372, in uvloop.loop.BaseTask._fast_step (uvloop/loop.c:112669)
  File "/home/vb/project/aiotg/bot.py", line 248, in api_call
    response = await self.session.post(url, data=params)
  File "/home/vb/.env/lib/python3.5/site-packages/aiohttp/client.py", line 582, in __await__
    resp = yield from self._coro
  File "/home/vb/.env/lib/python3.5/site-packages/aiohttp/client.py", line 200, in _request
    with timer:
  File "/home/vb/.env/lib/python3.5/site-packages/aiohttp/helpers.py", line 750, in __enter__
    raise RuntimeError('Timeout context manager should be used '
RuntimeError: Timeout context manager should be used inside a task
ERROR:asyncio:Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x7f2fc0514940>

Faced to this, this and this issues while searching with RuntimeError: Timeout context manager should be used inside a task.

Information:

$ python --version
Python 3.5.2

$ pip freeze
aiobotocore==0.2.1
aiohttp==1.3.3
aiotg==0.7.19
async-timeout==1.1.0
asyncpg==0.8.4
botocore==1.5.0
chardet==2.3.0
docutils==0.13.1
hashids==1.2.0
jmespath==0.9.1
multidict==2.1.4
pkg-resources==0.0.0
python-dateutil==2.6.0
six==1.10.0
uvloop==0.8.0
Wand==0.4.4
yarl==0.9.8

Any hints to fix or workaround?

@muminoff
Copy link
Contributor Author

cc @asvetlov

@muminoff
Copy link
Contributor Author

muminoff commented Feb 22, 2017

PS: I might be using aiobotocore wrong as it was coded in rush. I think, it has no relation to current issue.

@muminoff
Copy link
Contributor Author

I think, it is more related with uvloop project.
Because, if I remove asyncio.set_event_loop_policy(uvloop.EventLoopPolicy()), it is working.

$ python main.py
Creating a client session outside of coroutine
client_session: <aiohttp.client.ClientSession object at 0x7fdb5995c4e0>
DEBUG:botocore.loaders:Loading JSON file: /home/vb/.env/lib/python3.5/site-packages/botocore/data/endpoints.json
DEBUG:botocore.loaders:Loading JSON file: /home/vb/.env/lib/python3.5/site-packages/botocore/data/s3/2006-03-01/service-2.json
DEBUG:botocore.loaders:Loading JSON file: /home/vb/.env/lib/python3.5/site-packages/botocore/data/_retry.json
DEBUG:botocore.client:Registering retry handlers for service: s3
DEBUG:botocore.hooks:Event creating-client-class.s3: calling handler <function add_generate_presigned_post at 0x7fdb5a15f048>
DEBUG:botocore.hooks:Event creating-client-class.s3: calling handler <function add_generate_presigned_url at 0x7fdb5a158400>
DEBUG:botocore.args:The s3 config key is not a dictionary type, ignoring its value of: None
DEBUG:botocore.endpoint:Setting s3 timeout as (60, 60)
DEBUG:botocore.client:Defaulting to S3 virtual host style addressing with path style addressing fallback.
DEBUG:aiotg:api_call setWebhook, {'url': 'https://vb.muminoff.uz/webhook'}
======== Running on http://127.0.0.1:8000 ========
(Press CTRL+C to quit)

@szastupov
Copy link
Owner

It's hard to tell to be honest, maybe if it works without uvloop then it's not aiotg problem :) Are you sure you need uvloop? I bet you'll be fine with default event loop.

On the other hand, the latest changes introduced some warnings (I fixed them on 3.6 but they're still present on 3.5) so I might just as well just revert last session related changes.

I really liked the old aiohttp api, and I'm getting frustrated with increasing complexity of asyncio ecosystem :(

@asvetlov
Copy link

  1. All ClientSession operations should be performed inside a coroutine. I don't see how given example breaks the rule. Unfortunately I cannot reproduce the issue, the example requires very complex setup.
    But ClientSession itself works with uvloop pretty well.
  2. Old aiohttp API will be removed in next release, thus I strongly discourage its using.

@muminoff
Copy link
Contributor Author

Thank you, @asvetlov, for your quick response. Well, above example just uses code from the current project, in my opinion, it would be better if you take a look here.

I can write simple example to reproduce issue easier.

@muminoff
Copy link
Contributor Author

@asvetlov, however as I described above, commenting asyncio.set_event_loop_policy(uvloop.EventLoopPolicy()) line in my code just lets everything work as before. Using uvloop seems problematic in my case.

@muminoff
Copy link
Contributor Author

muminoff commented Feb 22, 2017

Weird. Following code is working.

import os
from aiotg import Bot
import uvloop
import asyncio

asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())

bot = Bot(api_token=os.environ["API_TOKEN"])


@bot.command(r"/echo (.+)")
def echo(chat, match):
    return chat.reply(match.group(1))


bot.run_webhook(webhook_url='https://ukvzmkewrh.localtunnel.me')

Versions:

aiohttp==1.3.3
uvloop==0.8.0
aiotg==0.7.20

Output:

$ python webhook.py 
Creating a client session outside of coroutine
client_session: <aiohttp.client.ClientSession object at 0x101215358>
======== Running on http://0.0.0.0:8080 ========
(Press CTRL+C to quit)

Update: added output

@asvetlov
Copy link

Most likely the problem is inside your code not asyncio, aiohttp or uvloop :)

@muminoff
Copy link
Contributor Author

@asvetlov so you mean warning message Creating a client session outside of coroutine is normal?

@muminoff
Copy link
Contributor Author

If it is not normal, then it is related to aiotg (sorry to mention you again @szastupov)

@shadrus
Copy link
Contributor

shadrus commented Feb 22, 2017

@szastupov I see some "ugly hacks" on closing session, but revert session related commits is not a right way, isn't it? Aiotg should use it.

@szastupov
Copy link
Owner

@shadrus sure, we have no choice :) But I'm definitely not going to force users to use async with Bot(...) as bot: ... form. It's better to have those ugly hacks as long as they don't break anything.

@szastupov
Copy link
Owner

@muminoff can't reproduce "Creating a client session outside of coroutine" on a standard asyncio loop.

@muminoff
Copy link
Contributor Author

@szastupov I am running it on Python 3.5.2.

@szastupov
Copy link
Owner

@asvetlov is there any way to use aiohttp.ClientSession without forcing users to use async with? I just can't break the API...

otherwise I'll just copy old wrappers and construct new session on each request

@asvetlov
Copy link

is there any way to use aiohttp.ClientSession without forcing users to use async with?

You can use just session = aiohttp.ClientSession() but the call should be done from coroutine.
Creating a session in global context (read as "on import time") is really bad idea.

otherwise I'll just copy old wrappers and construct new session on each request

Be ready for breaking your hacks by new aiohttp releases. You could copy the whole aiohttp though...

aiohttp has long enough deprecation period (a year and half). If you wouldn't get rid of deprecated code be ready to consequences.

@szastupov
Copy link
Owner

Ok, thanks. Maybe I should create ClientSession in a lazy manner 🤔

@szastupov
Copy link
Owner

I've just added lazy initialisation and the warning should go away, @muminoff could you try it from master?

@muminoff
Copy link
Contributor Author

@szastupov tested from master branch, seems OK now.

@szastupov
Copy link
Owner

Ok, cool. Just released it to pypi

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

No branches or pull requests

4 participants