FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

postgres: use wait_for in disconnect by blueyed · Pull Request #97 · encode/databases · GitHub

This repository was archived by the owner on Aug 19, 2025. It is now read-only.
/ databases Public archive

postgres: use wait_for in disconnect - #97

Open
blueyed wants to merge 3 commits into
encode:masterfrom
blueyed:wait_for
ConversationCommits (3)ChecksFiles changed
Open

postgres: use wait_for in disconnect#97
blueyed wants to merge 3 commits into
encode:masterfrom
blueyed:wait_for

Conversation

blueyed commented May 7, 2019

Copy link
Copy Markdown
Contributor

blueyed commented May 7, 2019

Copy link
Copy Markdown
Contributor Author

Note that there is also a timeout for acquire, which gets used by release then also: https://github.com/MagicStack/asyncpg/blob/92c2d81256a1efd8cab12c0118d74ccd1c18131b/asyncpg/pool.py#L560-L564

lovelydinosaur commented Jun 20, 2019

Copy link
Copy Markdown
Member

This is a good call yup.
Presumably we also want to add the timeout to acquire right?
Any thoughts about adding timeout to the API someplace, and using that value?

vmarkovtsev suggested changes Apr 29, 2020
View reviewed changes

vmarkovtsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hey @blueyed I can take care of finishing this PR if you don't have time right now. WDYT?

Comment thread databases/backends/postgres.py
async def disconnect(self) -> None:
assert self._pool is not None, "DatabaseBackend is not running"
await self._pool.close()
await asyncio.wait_for(self._pool.close(), timeout=30)

vmarkovtsev Apr 29, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

As @tomchristie wrote, it is a good idea to make the timeout configurable, otherwise, 30 seconds can be too much for everybody.

blueyed commented Jun 4, 2021

Copy link
Copy Markdown
Contributor Author

@vmarkovtsev

I can take care of finishing this PR if you don't have time right now. WDYT?

Sure, please go ahead.
(Sorry for the late reply, of course)

taybin commented Sep 8, 2021

Copy link
Copy Markdown
Contributor

Any progress on this?

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
vmarkovtsev vmarkovtsev requested changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL