-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
Fix/22 #64
base: master
Are you sure you want to change the base?
Fix/22 #64
Conversation
* group together fixtures for backend testing * Add per-backend setup to allow slow initialization. This is specially important as kafka subscribe can return while the consumer is not ready. See related test setup upstream: - https://github.com/aio-libs/aiokafka/blob/2c54e10c57760f779961a8c2f5df8ad609ef6983/tests/test_consumer.py#L433 - https://github.com/aio-libs/aiokafka/blob/2c54e10c57760f779961a8c2f5df8ad609ef6983/tests/_testutil.py#L376 - https://github.com/aio-libs/aiokafka/blob/2c54e10c57760f779961a8c2f5df8ad609ef6983/tests/_testutil.py#L364 fixes encode#42
Signed-off-by: Pablo Woolvett <[email protected]>
broadcaster/_backends/postgres.py
Outdated
@@ -13,6 +13,8 @@ def __init__(self, url: str): | |||
|
|||
async def connect(self) -> None: | |||
self._conn = await asyncpg.connect(self._url) | |||
self._event = asyncio.Event() |
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.
Wouldn't it be simpler to use asyncio.Lock
?
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.
Simpler, and better.
This fix adds an asyncio.Lock to avoid `asyncpg.exceptions._base.InterfaceError: cannot perform operation: another operation is in progress` fixes encode#22
recently, my project facing some issue
seems like this PR doesn't resolve slow backend initialization issue |
Why didn't this merge? If I fixed its conflicts, would it be accepted? |
@logankaser let's have it split into multiple PRs. I believe there is only one issue related to postgres left. |
I think there are two:
|
#66 looks like a good merge candidate, but I did not test it yet. |
Any update on this issue? |
test: add concurrent checks
This is specially important as kafka subscribe can return while the consumer
is not ready. See related test setup upstream:
chore: update remnants, packages
fix(backend/kafka): consumer unsubscribe not awaitable
fix(backend/postgres): allow concurrent pubs
This fix adds a lock (asyncio.Event based) to avoid
asyncpg.exceptions._base.InterfaceError: cannot perform operation: another operation is in progress
fixes #22
fixes #42