Copilot commented on code in PR #2366:
URL: https://github.com/apache/age/pull/2366#discussion_r3039024136
##########
drivers/python/age/age.py:
##########
@@ -333,9 +341,9 @@ def __init__(self):
# Connect to PostgreSQL Server and establish session and type extension
environment.
def connect(self, graph:str=None, dsn:str=None, connection_factory=None,
cursor_factory=ClientCursor,
- load_from_plugins:bool=False, **kwargs):
+ load_from_plugins:bool=False, skip_load:bool=False, **kwargs):
Review Comment:
`connection_factory` is accepted and forwarded through the public API, but
it is never used: `Age.connect()` ignores it when calling
`psycopg.connect(...)`. Either pass it through (if supported by psycopg) or
remove/deprecate it to avoid a misleading no-op parameter.
```suggestion
load_from_plugins:bool=False, skip_load:bool=False,
**kwargs):
if connection_factory is not None:
raise TypeError(
"connection_factory is not supported by Age.connect(); "
"use psycopg.connect() options supported by this driver
instead."
)
```
##########
drivers/python/README.md:
##########
@@ -89,9 +89,26 @@ SET search_path = ag_catalog, "$user", public;
* Make sure to give your non-superuser db account proper permissions to the
graph schemas and corresponding objects
* Make sure to initiate the Apache Age python driver with the
```load_from_plugins``` parameter. This parameter tries to
load the Apache Age extension from the PostgreSQL plugins directory located
at ```$libdir/plugins/age```. Example:
- ```python.
+ ```python
ag = age.connect(host='localhost', port=5432, user='dbuser',
password='strong_password',
- dbname=postgres, load_from_plugins=True, graph='graph_name)
+ dbname='postgres', load_from_plugins=True,
graph='graph_name')
+ ```
+
+### Managed PostgreSQL Usage (Azure, AWS RDS, etc.)
+* On managed PostgreSQL services where the AGE extension is loaded server-side
via ```shared_preload_libraries```,
+ the ```LOAD 'age'``` command may fail because the binary is not at the
expected file path. Use the ```skip_load```
+ parameter to skip the ```LOAD``` statement while still performing all other
setup:
+ ```python
+ ag = age.connect(host='myserver.postgres.database.azure.com', port=5432,
+ user='dbuser', password='strong_password',
+ dbname='postgres', skip_load=True, graph='graph_name')
+ ```
+* **Connection pools:** If you manage connections externally (e.g. via
```psycopg_pool.ConnectionPool```),
+ use ```configure_connection()``` instead, which skips ```LOAD 'age'``` by
default and is designed for
+ per-connection setup callbacks:
+ ```python
+ from age import configure_connection
+ configure_connection(conn, graph_name='graph_name')
Review Comment:
README references `configure_connection()` (and claims it skips `LOAD 'age'`
by default), but there is no `configure_connection` symbol anywhere under
`drivers/python`. This will mislead users; either implement/export
`configure_connection` or remove/update this section to document the supported
approach (e.g., calling `setUpAge(..., skip_load=True)` on each pooled
connection).
```suggestion
run AGE setup on each newly created connection and pass
```skip_load=True``` so the driver skips
```LOAD 'age'``` while still performing the remaining setup:
```python
from age import setUpAge
setUpAge(conn, 'graph_name', skip_load=True)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
