Add Async IO support #26
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dszakallas
force-pushed
the
async-support
branch
2 times, most recently
from
568df92 to
7b075cb
Compare
dszakallas
force-pushed
the
async-support
branch
from
7b075cb to
14fc194
Compare
dszakallas
commented
Nov 10, 2017
sumologic/async.py
Outdated
|
|
||
| class SumoLogic(object): | ||
|
|
||
| def __init__(self, accessId, accessKey, endpoint=None, cookieFile='cookies.txt'): |
There was a problem hiding this comment.
there are a couple of rough edges here:
- cookies have to be added back if they are really needed
- remove explicitly setting the read timeout to infinity. I don't why but while fetching results for some search jobs I frequently got TimeOutErrors. I think this should be configurable at least.
dszakallas
commented
Nov 10, 2017
sumologic/async.py
Outdated
| self.endpoint = str(response.url).replace('/collectors', '') # dirty hack to sanitise URI and retain domain | ||
|
|
||
| async def delete(self, method, params=None): | ||
| await self._guard_endpoint() |
There was a problem hiding this comment.
I did it this way because I felt async constructors are unidiomatic. Are they?
dszakallas
commented
Nov 10, 2017
| @@ -0,0 +1,162 @@ | |||
| from copy import copy | |||
There was a problem hiding this comment.
I don't like it that the whole file is basically copied, but I couldn't find a better way. Is there?
dszakallas
commented
Nov 12, 2017
sumologic/async.py
Outdated
| response = await self.session.get('https://api.sumologic.com/api/v1/collectors') # Dummy call to get endpoint | ||
| self.endpoint = str(response.url).replace('/collectors', '') # dirty hack to sanitise URI and retain domain | ||
| async def _set_endpoint(self): | ||
| if self._endpoint is None: |
There was a problem hiding this comment.
double checked locking to avoid sending the request more than once
dszakallas
commented
Nov 12, 2017
sumologic/async.py
Outdated
|
|
||
| async def search_job(self, query, fromTime=None, toTime=None, timeZone='UTC'): | ||
| params = {'query': query, 'from': fromTime, 'to': toTime, 'timeZone': timeZone} | ||
| r = await self.post('/search/jobs', params) | ||
| return json.loads(await r.text()) | ||
| async with r: |
There was a problem hiding this comment.
prevents leaking response objects
dszakallas
commented
Nov 12, 2017
sumologic/async.py
Outdated
| return json.loads(await r.text()), r.headers['etag'] | ||
| async with r: | ||
| r = await self.get('/collectors/' + str(collector_id) + '/sources/' + str(source_id)) | ||
| return json.loads(await r.text()), r.headers['etag'] | ||
|
|
||
| async def create_source(self, collector_id, source): | ||
| return await self.post('/collectors/' + str(collector_id) + '/sources', source) |
There was a problem hiding this comment.
i cannot prevent leaking here without changing the API, as this returns the request object itself.
dszakallas
commented
Nov 12, 2017
sumologic/async.py
Outdated
| SumoLogic REST API endpoint changes based on the geo location of the client. | ||
| For example, If the client geolocation is Australia then the REST end point is | ||
| https://api.au.sumologic.com/api/v1 | ||
| async def __aenter__(self): |
There was a problem hiding this comment.
for safely closing the connection
There was a problem hiding this comment.
also it maybe wort exposing a close method.
raszi
reviewed
Nov 13, 2017
| async def search_metrics(self, query, fromTime=None, toTime=None, requestedDataPoints=600, maxDataPoints=800): | ||
| '''Perform a single Sumo metrics query''' | ||
|
|
||
| def millisectimestamp(ts): |
There was a problem hiding this comment.
I believe the common functions should be extracted to a util namespace to reduce code duplications.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added a proof of concept implementation for this lib whihc uses async http:
https://aiohttp.readthedocs.io/en/stable/index.html
I don't know yet how to include this code in a way that does not break interpreters below 3.5. I want to avoid putting it into a string and execing.
I am not very familiar with Python yet, so I appricieate any help on this. Thanks!