-
Notifications
You must be signed in to change notification settings - Fork 419
Refactor args parsing and config management for connect and create_pool #130
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
Conversation
@@ -625,8 +627,14 @@ def create_pool(dsn=None, *, | |||
An :exc:`~asyncpg.exceptions.InterfaceError` will be raised on any | |||
attempted operation on a released connection. | |||
""" | |||
if not issubclass(connection_class, connection.Connection): |
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.
We should add Connection
to __init__
, so that asyncpg.Connection
is a thing.
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.
ok
1c8c2e0
to
b4da559
Compare
asyncpg/connect_utils.py
Outdated
|
||
if not addrs: | ||
raise ValueError( | ||
'could not determine a list of database addresses to connect to') |
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.
s/a list of database addresses/the database address/
asyncpg/connect_utils.py
Outdated
not all(isinstance(v, str) for v in server_settings.values())): | ||
raise ValueError( | ||
'server_settings is expected to be None or ' | ||
'a Mapping[str, str]') |
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.
s/Mapping/Dict/
tests/test_pool.py
Outdated
@@ -351,6 +351,30 @@ class Error(Exception): | |||
async with pool.acquire() as con: | |||
await con.fetchval('SELECT 1') | |||
|
|||
async def test_pool_config_persistance(self): |
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.
"persistence"
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.
lgtm
Added: * `connection_class` parameter to connect() and create_pool() to specify a custom Connection object. * `server_settings` parameter to connect() and create_pool() to set custom server connection settings. Fixed: * Connection pool to always create Connections with correct configuration. Before, the pool could lose parameters like `statement_cache_size`. Backwards Compatibility: * connect() and create_pool() no longer accept arbitrary keyword arguments for server settings. Use new `server_settings` argument for that.
Added:
connection_class
parameter to connect() and create_pool() tospecify a custom Connection object.
server_settings
parameter to connect() and create_pool() toset custom server connection settings.
Fixed:
configuration. Before, the pool could lose parameters like
statement_cache_size
.Backwards Compatibility:
keyword arguments for server settings. Use new
server_settings
argument for that.