Add asyncpg SQLAlchemy engine factory for Entra authentication - #48368
Add asyncpg SQLAlchemy engine factory for Entra authentication#48368pabloacan wants to merge 4 commits into
Conversation
github-actions
Bot
added
Community Contribution
|
Thank you for your contribution @pabloacan! We will review the pull request and get back to you soon. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 8 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
…ra-authentication
There was a problem hiding this comment.
🟡 Not ready to approve
Conflicting connection arguments can override the fetched Entra credentials, and required documentation and test coverage remain incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds asyncpg-backed SQLAlchemy engine creation using asynchronous Microsoft Entra credentials.
Changes:
- Adds
create_asyncpg_engineand public export. - Adds asyncpg dependencies, documentation, and changelog entry.
- Adds mocked unit coverage for credential injection and failures.
File summaries
| File | Description |
|---|---|
tests/test_sqlalchemy_async.py |
Adds asyncpg integration tests. |
README.md |
Documents installation and usage. |
pyproject.toml |
Adds asyncpg and SQLAlchemy asyncio extras. |
dev_requirements.txt |
Adds asyncpg for development. |
CHANGELOG.md |
Records the new factory. |
sqlalchemy/asyncpg.py |
Implements the engine factory. |
sqlalchemy/__init__.py |
Exports the new API. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 4
- Review effort level: Medium
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| ```python | ||
| from azure.identity.aio import DefaultAzureCredential | ||
| from azure_postgresql_auth.sqlalchemy import create_asyncpg_engine |
| connection_kwargs = { | ||
| **connect_args, | ||
| "user": connect_args.get("user", entra_conninfo["user"]), | ||
| "password": connect_args.get("password", entra_conninfo["password"]), | ||
| } |
| class TestSqlalchemyAsyncpgEntraAuthentication: | ||
| """Tests for asyncpg SQLAlchemy Entra authentication integration.""" |
| """ | ||
|
|
||
| from .async_entra_connection import enable_entra_authentication_async | ||
| from .asyncpg import create_asyncpg_engine |
|
@microsoft-github-policy-service agree company="Industria de Diseño Textil, S.A." |
|
Unlike the existing This is intentional: SQLAlchemy's Using SQLAlchemy's |
There was a problem hiding this comment.
🟡 Not ready to approve
Credential precedence, SQLAlchemy compatibility, documentation, and required test coverage need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (5)
sdk/postgresql/azure-postgresql-auth/azure_postgresql_auth/sqlalchemy/asyncpg.py:74
- Always use the freshly acquired Entra principal and token here. As written, any username or password present in the URL or
connect_argssilently takes precedence, so a conventional URL such aspostgresql+asyncpg://user@host/dbauthenticates as that user and an accidentally retained password bypasses Entra authentication entirely.
"user": connect_args.get("user", entra_conninfo["user"]),
"password": connect_args.get("password", entra_conninfo["password"]),
sdk/postgresql/azure-postgresql-auth/README.md:216
- This standalone example calls
text(...)without importing it, so copying the documented example raisesNameErrorbefore any connection is made.
```python
from azure.identity.aio import DefaultAzureCredential
from azure_postgresql_auth.sqlalchemy import create_asyncpg_engine
sdk/postgresql/azure-postgresql-auth/tests/test_sqlalchemy_async.py:148
- The linked issue explicitly requires concurrent-creation unit coverage and a live test using
DefaultAzureCredential, but the new asyncpg test class only exercises one mocked connection and credential validation/failure. The existing concurrent/live tests below cover the older psycopg event-hook path, so they cannot detect asyncpg factory integration or concurrency regressions.
class TestSqlalchemyAsyncpgEntraAuthentication:
"""Tests for asyncpg SQLAlchemy Entra authentication integration."""
sdk/postgresql/azure-postgresql-auth/azure_postgresql_auth/sqlalchemy/init.py:26
- Exporting this new public helper leaves the module-level
RequirementsandFunctionsdocumentation above stale: it still lists only the two event-hook helpers and gives no asyncpg installation requirement. Update that overview so generated module documentation exposes the new supported integration.
from .async_entra_connection import enable_entra_authentication_async
from .asyncpg import create_asyncpg_engine
sdk/postgresql/azure-postgresql-auth/pyproject.toml:40
async_creatorwas added in SQLAlchemy 2.0.16, but this extra still permits 2.0.0–2.0.15. Environments resolved to those declared-supported versions will reject the new keyword when this factory callscreate_async_engine; raise the minimum to the first version that provides this API.
"sqlalchemy[asyncio]>=2.0.0",
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Medium
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
[Pilot] PR Pipeline Failure AnalysisA CI pipeline failed on this pull request. Here is an automated analysis of what went wrong and how to get the build green. What failedThe pylint validation check for
Recommended next steps
Raw pipeline analysis (azsdk ci analyze)
|
There was a problem hiding this comment.
🟡 Not ready to approve
The dependency floor is incompatible, credentials can bypass Entra authentication, and required concurrent/live coverage is missing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (4)
sdk/postgresql/azure-postgresql-auth/tests/test_sqlalchemy_async.py:156
- The linked issue requires concurrent connection-creation coverage and a live test for this asyncpg factory, but these tests invoke the captured creator only once with mocks. The existing concurrent/live cases below still exercise
enable_entra_authentication_async, so they cannot catch failures in SQLAlchemy'sasync_creatoradaptation or the asyncpg path. Add concurrent unit coverage and a live test usingcreate_asyncpg_enginewith the async credential fixture.
async def test_creates_asyncpg_connection_with_entra_credentials(
self, mock_connect, mock_get_conninfo, mock_create_engine
):
sdk/postgresql/azure-postgresql-auth/azure_postgresql_auth/sqlalchemy/init.py:26
- Exporting the new public helper makes this module docstring's requirements and function inventory incomplete: it still lists only the two event-hook helpers and says the SQLAlchemy extra installs
sqlalchemy>=2.0.0. Document the asyncpg extra andcreate_asyncpg_enginehere as well.
from .asyncpg import create_asyncpg_engine
sdk/postgresql/azure-postgresql-auth/pyproject.toml:40
async_creatorwas added in SQLAlchemy 2.0.16, so the declared>=2.0.0range allows versions where this factory fails whenasync_creatoris forwarded. Raise the minimum to 2.0.16.
"sqlalchemy[asyncio]>=2.0.0",
sdk/postgresql/azure-postgresql-auth/README.md:216
- This new example calls
text(...)without importing it, so running the documented snippet raisesNameError. Add the SQLAlchemy import to keep the example executable.
from azure.identity.aio import DefaultAzureCredential
from azure_postgresql_auth.sqlalchemy import create_asyncpg_engine
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| connection_kwargs = { | ||
| **connect_args, | ||
| "user": connect_args.get("user", entra_conninfo["user"]), | ||
| "password": connect_args.get("password", entra_conninfo["password"]), | ||
| } |
…ra-authentication
There was a problem hiding this comment.
🟡 Not ready to approve
The dependency floor, token precedence, documentation example, and required test coverage need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (3)
sdk/postgresql/azure-postgresql-auth/tests/test_sqlalchemy_async.py:148
- The linked issue explicitly requires concurrent connection-creation unit coverage and a live test using an asynchronous Azure Identity credential. These added tests invoke only one mocked creator at a time, while the existing live class below still exercises
enable_entra_authentication_asyncwith the synchronous credential fixture. Add a concurrent creator/pool test and a livecreate_asyncpg_enginetest usingasync_credential.
class TestSqlalchemyAsyncpgEntraAuthentication:
"""Tests for asyncpg SQLAlchemy Entra authentication integration."""
sdk/postgresql/azure-postgresql-auth/azure_postgresql_auth/sqlalchemy/asyncpg.py:79
- A password supplied in the URL or
connect_argscurrently overrides the freshly acquired Entra token. That makes every pool connection keep using a static or expired password even though a new token was retrieved, contradicting this factory's authentication contract and causing reconnections to fail. Always use the newly acquired token as the password; an explicit user override can remain if that is intentional.
connection_kwargs = {
**connect_args,
"user": connect_args.get("user", entra_conninfo["user"]),
"password": connect_args.get("password", entra_conninfo["password"]),
}
sdk/postgresql/azure-postgresql-auth/README.md:216
- This example calls
text(...)below but never imports it, so the documented copy/paste example raisesNameError. Include the SQLAlchemy import in this code block.
from azure.identity.aio import DefaultAzureCredential
from azure_postgresql_auth.sqlalchemy import create_asyncpg_engine
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| ] | ||
| sqlalchemy = [ | ||
| "sqlalchemy>=2.0.0", | ||
| "sqlalchemy[asyncio]>=2.0.0", |
Description
Adds
create_asyncpg_enginetoazure-postgresql-authfor SQLAlchemy async engines usingasyncpgandAsyncTokenCredentialimplementations such asazure.identity.aio.DefaultAzureCredential.The factory retrieves Microsoft Entra connection information asynchronously whenever SQLAlchemy opens a physical pooled connection, then passes the Entra principal and token to
asyncpg.connect.Adds the
asyncpgoptional dependency, documentation, release history, and unit coverage.Fixes #48365.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines
Tests:
15 passed, 5 deselectedwithtests/test_sqlalchemy_async.py -m 'not live_test_only'; live Azure tests were not run.