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

Implement connection service file functionality #1223

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

Conversation

AndrewJackson2020
Copy link
Contributor

This PR attempts to implement connection service file functionality into asyncpg.

Connection service files are a libpq feature that allows details of postgres connection strings to be stored in a file that is referenced via a service parameter in the connection string. This makes it very easy to ensure/enforce consistent connection strings across a codebase.

https://www.postgresql.org/docs/17/libpq-pgservice.html

@AndrewJackson2020
Copy link
Contributor Author

Another complication wrt the connection service functionality: apparently connection service files also allow ldap lookups.

https://www.postgresql.org/docs/current/libpq-ldap.html

Co-authored-by: Elvis Pranskevichus <[email protected]>
@AndrewJackson2020
Copy link
Contributor Author

I believe the test failure is due to a flaky test. Please let me know if there are any other changes that need to be made to this PR.

if ssl is None:
ssl = val

if 'sslcert' in service_params:
Copy link
Member

Choose a reason for hiding this comment

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

This and other ssl params below are missing the if <param> is None: check.

@@ -2074,6 +2074,7 @@ async def _do_execute(
async def connect(dsn=None, *,
host=None, port=None,
user=None, password=None, passfile=None,
service=None,
Copy link
Member

Choose a reason for hiding this comment

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

Oh, I think we should also let you specify the service file location via the servicefile kwarg (a-la passfile).

Copy link
Member

Choose a reason for hiding this comment

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

Also please add a .. versionadded:: 0.31.0 tag in the docstring below mentioning the new kwargs.

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

Successfully merging this pull request may close these issues.

2 participants