Added async support - #134
Conversation
|
Amazing, thanks for the PR 👍🏻 Could you rebase your PR please? |
done |
| __credits__ = ["Thierry Schellenbach, mellowmorning.com, @tschellenbach"] | ||
| __license__ = "BSD-3-Clause" | ||
| __version__ = "5.1.1" | ||
| __version__ = "6.0.0" |
There was a problem hiding this comment.
no need to change this, we'll bump it when we release it
| ## 6.0.0 - 2021-01-19 | ||
|
|
||
| * Add asynchronous code support | ||
| * Change ``%s`` to f-strings | ||
|
|
There was a problem hiding this comment.
No need to add this, we'll add it when we release it
| ) as response: | ||
|
|
||
| logger.debug( | ||
| f"stream api call {response}, headers {headers} data {data}", |
There was a problem hiding this comment.
We shouldn't leak the JWT into the logs. Can we either remove headers or filter out Authorization header?
There was a problem hiding this comment.
I removed Authorization from headers
| timeout=self.timeout, | ||
| verify_ssl=False, |
There was a problem hiding this comment.
verify ssl must be true.
| timeout=self.timeout, | |
| verify_ssl=False, | |
| timeout=self.timeout, |
| base_url=base_url, | ||
| location=location, | ||
| ) | ||
| self.session = aiohttp.ClientSession |
There was a problem hiding this comment.
I don't think we need session variable at all. Why not just:
async with aiohttp.ClientSession() as session:
async with session.request(| params=default_params, | ||
| timeout=self.timeout, | ||
| ) | ||
| logger.debug(f"stream api call {response.url}, headers {headers} data {data}") |
There was a problem hiding this comment.
we leak the jwt here as well.
| key = os.environ["STREAM_KEY"] | ||
| secret = os.environ["STREAM_SECRET"] | ||
| key = "n8udba92h9hf" | ||
| secret = "5mjaw2a5bynt3fzdnxy2pwxh7fw89gg2dq3mm4jy56vhrwnadfst7yssatmaxhkv" |
| key = "n8udba92h9hf" | ||
| secret = "5mjaw2a5bynt3fzdnxy2pwxh7fw89gg2dq3mm4jy56vhrwnadfst7yssatmaxhkv" |
| sys.exit(1) | ||
|
|
||
| return stream.connect(key, secret, location="qa", timeout=30) | ||
| return stream.connect(key, secret, location="qa", timeout=30, use_async=use_async) |
There was a problem hiding this comment.
as a future improvement client could be a fixture instead of a global variable :)
|
Will try to merge it! |
Summary:
Submitter checklist:
CHANGELOGupdated or N/AMerger checklist:
Dependencies: