-
Notifications
You must be signed in to change notification settings - Fork 227
implement support for asyncio [double-plus-WIP] #252
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
Conversation
362d36c
to
228d980
Compare
elasticapm/__init__.py
Outdated
if sys.version_info >= (3, 5): | ||
from elasticapm.contrib.asyncio.traces import async_capture_span as capture_span # noqa: F401 | ||
else: | ||
from elasticapm.traces import capture_span # noqa: F401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least two spaces before inline comment
elasticapm/__init__.py
Outdated
from elasticapm.traces import set_transaction_name, set_user_context, tag # noqa: F401 | ||
from elasticapm.traces import set_transaction_result # noqa: F401 | ||
|
||
if sys.version_info >= (3, 5): | ||
from elasticapm.contrib.asyncio.traces import async_capture_span as capture_span # noqa: F401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least two spaces before inline comment
elasticapm/__init__.py
Outdated
|
||
if sys.version_info >= (3, 5): | ||
from elasticapm.contrib.asyncio.traces import async_capture_span # noqa: F401 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line at end of file
elasticapm/__init__.py
Outdated
@@ -23,3 +24,7 @@ | |||
from elasticapm.traces import capture_span, set_context, set_custom_context # noqa: F401 | |||
from elasticapm.traces import set_transaction_name, set_user_context, tag # noqa: F401 | |||
from elasticapm.traces import set_transaction_result # noqa: F401 | |||
|
|||
if sys.version_info >= (3, 5): | |||
from elasticapm.contrib.asyncio.traces import async_capture_span # noqa: F401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'elasticapm.contrib.asyncio.traces.async_capture_span' imported but unused
at least two spaces before inline comment
inline comment should start with '# '
017d8e1
to
12cd582
Compare
2248bf3
to
d9dc287
Compare
@beniwohli can you rebase against master when you can, I'd like to test this on twisted |
Signed-off-by: Benjamin Wohlwend <[email protected]>
this allows us to defer the costly frame processing until the end of the span, at which time we can discard it if span time threshold hasn't been reached Signed-off-by: Benjamin Wohlwend <[email protected]>
this allows the use of `async_capture_span` as an async decorator Signed-off-by: Benjamin Wohlwend <[email protected]>
Signed-off-by: Benjamin Wohlwend <[email protected]>
Signed-off-by: Benjamin Wohlwend <[email protected]>
@ciarancourtney done :) let me know how it goes! |
Sorry I misunderstood the PR, I thought it added support for twisted. FYI I'm using |
…locals this is a partial "import" of the asyncio work done in elastic#252, sans the asyncio specific extensions
…locals (elastic#291) this is a partial "import" of the asyncio work done in elastic#252, sans the asyncio specific extensions closes elastic#291
Hi, do you still plan to release this? |
@menecio I think that this MR is a bit outdated and not closed... Support for asyncio was added in v4 of apt-agent-python. |
@approxit unfortunately, that is not quite correct. We did merge some preliminary support for asyncio in 4.0 (like the usage of |
Closing this, as most parts have been merged or are in the process of being merged in other PRs. Progress on asyncio can be tracked in #598 |
DO NOT TRY THIS AT HOME JUST YET
Note: in the v2 API implementation PR, we removed the
AsyncIOHTTPTransport
. It might make sense to re-add it with this PR.