Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix/22 #64

wants to merge 4 commits into from

Conversation

pwoolvett
Copy link
Contributor

@pwoolvett pwoolvett commented May 6, 2022

fixes #22
fixes #42

@@ -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()
Copy link

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?

Copy link
Contributor Author

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
@skyforeverRivian
Copy link

recently, my project facing some issue

File "*****/python3.9/site-packages/broadcaster/_base.py", line 62, in connect
    await self._backend.connect()
AttributeError: 'Broadcast' object has no attribute '_backend'

seems like this PR doesn't resolve slow backend initialization issue

@logankaser
Copy link

Why didn't this merge? If I fixed its conflicts, would it be accepted?

@alex-oleshkevich
Copy link
Member

@logankaser let's have it split into multiple PRs. I believe there is only one issue related to postgres left.

@logankaser
Copy link

I think there are two:

  1. This issue, the connection is not guarded by a lock, so it can be accessed while the connection is active.
    Can be with using a asyncio lock like this PR did, or better something like https://github.com/permitio/broadcaster/blob/master/broadcaster/_backends/postgres.py
  2. Postgres backend: handle connection loss #66 Postgres backend cannot gracefully handle connection loss

@alex-oleshkevich
Copy link
Member

#66 looks like a good merge candidate, but I did not test it yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants