Refactor Producer #15
Conversation
| future = asyncio.open_connection(self.host, self.port, loop=self._loop) | ||
| self._reader, self._writer = yield from asyncio.wait_for( | ||
| future, self._config['request_timeout_ms']/1000) | ||
| self._read_task = asyncio.ensure_future(self._read(), loop=self._loop) |
There was a problem hiding this comment.
Not backward compatible, not all version of python 3.4.x support ensure_future
There was a problem hiding this comment.
We can add something like https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/helpers.py#L17-L20
|
Can we split this PR in smaller ones? It is hard to review... |
| raise KafkaUnavailableError("All servers failed to process request") | ||
| def close(self): | ||
| if self._sync_task: | ||
| self._sync_task.cancel() |
There was a problem hiding this comment.
Proper task cancel is required, you need to yield from self._sync_task somewhere
|
Honestly I did not dig deep inside proposed changes -- the patch is huge. |
|
Sorry, I'm kinda used to making "pre-review" pull-requests into separate branches (it's just so easy to read it on GitHub), this should not have been reviewed yet, we're still prototyping. Sorry |
No description provided.