Fixed the problem of AsyncAioPikaManager repeatedly creating a connec… #692
Conversation
|
This is my first time to contribute code😄. give me some advice. |
| return queue | ||
| def initialize(self): | ||
| loop = asyncio.get_event_loop() | ||
| loop.create_task(self._initialize()) |
There was a problem hiding this comment.
This is a really strange way to initialize. It is also bound to race conditions, since you also call _initialize() in the _listen() method. I mean, I understand why you are doing it, but it isn't a robust solution, I think.
There was a problem hiding this comment.
What do you suggest? I want to do this well.
| channel = await self._channel(connection) | ||
| exchange = await self._exchange(channel) | ||
| await exchange.publish( | ||
| await self.listener_exchange.publish( |
There was a problem hiding this comment.
This will fail if the connection to the queue breaks and needs to be re-established. The Redis manage, which is the one that the vast majority of people use, has connection retries, maybe we want that here as well?
| self.listener_channel, exchange | ||
| ) | ||
| retry_sleep = 1 | ||
| await self._initialize() |
There was a problem hiding this comment.
This is problematic, because you have one more _initialize() call above. Both will set the same variables.
miguelgrinberg
force-pushed
the
main
branch
3 times, most recently
from
8e19980 to
82cc2cb
Compare
miguelgrinberg
force-pushed
the
main
branch
2 times, most recently
from
b069fc5 to
ed5679a
Compare
|
Given the lack of response and the fact that this PR is now outdated, I will close it. Please open a new PR and address my feedback if you want me to add this. |
No description provided.