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

Aiohttp2 support+ssl fix #1

Merged
merged 10 commits into from
Apr 27, 2017
Merged

Aiohttp2 support+ssl fix #1

merged 10 commits into from
Apr 27, 2017

Conversation

a-urth
Copy link

@a-urth a-urth commented Apr 5, 2017

  • add aiohttp2 compatibility
  • change tox respectively
  • fix ssl exception handling
  • add ssl_context to aiohttp connector

@a-urth
Copy link
Author

a-urth commented Apr 5, 2017

And about aiodns support - since its not used by aiohttp without explicit AsyncResolver usage, only possibility for aiodns errors its if session with corresponding resolver will be passed as parameter.

I propose:

  • either to remove possibility to pass session as parameter, since original lib does not provide it;
  • or/and explicitly create AsyncResolver and ensure aiodns usage.

@hellysmile
Copy link
Member

@hellysmile
Copy link
Member

aio-libs/aiohttp#559

}
session_kwargs = {'auth': self.http_auth}

if AIOHTTP_2:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Как я токошо узнал начиная с 1.3 (или мб раньше) оно и так None по дефолту, а дефалтный в 5 минут ставится в request

SSLError)
from yarl import URL
from yarl import URL # noqa # isort:skip
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module level import not on the top because of if statement for aiohttp


_exc = str(exc)
# aiohttp wraps ssl error
if 'SSL: CERTIFICATE_VERIFY_FAILED' in _exc:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other way to check it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its just a message, so no.

from elasticsearch.connection import Connection
from elasticsearch.exceptions import (ConnectionError, ConnectionTimeout,

from .compat import AIOHTTP_2, create_future # isort:skip
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Own party level impoort please

@@ -54,51 +63,118 @@ def __init__(
connector=aiohttp.TCPConnector(
limit=maxsize,
use_dns_cache=kwargs.get('use_dns_cache', False),
ssl_context=ssl_context,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to double check one more time list of parameters from init here...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what exactly to double check and what for?

@hellysmile hellysmile merged commit 9121710 into master Apr 27, 2017
@hellysmile hellysmile deleted the aiohttp2-support+ssl-fix branch April 27, 2017 12:16
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