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

Add support for async I/O #1203

Closed
wants to merge 30 commits into from
Closed

Add support for async I/O #1203

wants to merge 30 commits into from

Conversation

sethmlarson
Copy link
Contributor

@sethmlarson sethmlarson commented Apr 17, 2020

  • Revises the generation of API from the JSON spec:
    • Generates the elasticsearch/_async/client/... code using normal workflow
    • Uses unasync to translate async code to sync code in elasticsearch/client/...
  • On Python 3.6+ import the AsyncElasticsearch, AsyncTransport, and AIOHttpConnection into the elasticsearch namespace.
  • Add async helpers on Python 3.6+
    • Use unasync to generate these?
  • Async unit tests
    • Move to pytest
    • Might be able to use unasync here too (nope)
    • On CI use a random gen seed, maybe on date, that "stripes" the test suite on Jenkins between all the connection classes
  • Trim down the AsyncTransport class, doesn't need to override much
  • AsyncTransport sniffing via async
    • May need to be different than the sync variant because most users will create AsyncElasticsearch in the global scope, outside an event loop.
    • Instead we can have sniff_on_start=True mean that sniffing happens in the background on the first perform_request() or call to __aenter__()
    • Can attempt to grab a currently running loop in constructor but if we don't find one fallback on the above method?
  • Create documentation for AsyncElasticsearch and how to use
  • Point to the new implementation on the elasticsearch-async repository
  • Add aiohttp as an extra elasticsearch[async]?

Copy link
Member

@danielmitterdorfer danielmitterdorfer left a comment

Choose a reason for hiding this comment

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

Great to see asyncio support in the main code base! I left a couple of comments and suggestions mostly in the implementation of the new connection class.

elasticsearch/_async/client/__init__.py Outdated Show resolved Hide resolved
elasticsearch/_async/client/__init__.py Outdated Show resolved Hide resolved
elasticsearch/_async/http_aiohttp.py Outdated Show resolved Hide resolved
elasticsearch/_async/http_aiohttp.py Outdated Show resolved Hide resolved
elasticsearch/_async/http_aiohttp.py Outdated Show resolved Hide resolved
elasticsearch/_async/http_aiohttp.py Outdated Show resolved Hide resolved
elasticsearch/_async/http_aiohttp.py Outdated Show resolved Hide resolved
elasticsearch/_async/http_aiohttp.py Show resolved Hide resolved
elasticsearch/_async/http_aiohttp.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@sethmlarson
Copy link
Contributor Author

Closed in favor of #1230, thanks for all the early review comments!

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.

3 participants