Update Python support #6

ArjunNarendra · 2025-10-05T22:00:41Z

Slim down test suite to its essentials
Move common source code logic into core.py
Add support for SqlAlchemy

lossyrob · 2025-10-06T17:20:57Z

python/pyproject.toml

 dependencies = [
  "azure-identity>=1.13.0",
  "azure-core>=1.24.0",
+  "aiohttp>=3.8.0",


Is this required for all implementations? Otherwise it should go in the optional dependency that requires it.

Thanks for catching this! Both psycopg3 and sqlalchemy require it because they support asynchronous token retrieval via get_entra_conninfo_async, so aiohttp should go into their respective optional dependencies.

lossyrob · 2025-10-06T17:21:42Z

python/samples/psycopg2/getting_started/create_db_connection_psycopg2.py

-            connection_factory=entra_connection_factory
+            host=SERVER,
+            database=DATABASE,
+            connection_factory=SyncEntraConnection


Rename to remove the Sync naming everywhere for psycopg2 since we aren't implementing async.

Makes sense - done

lossyrob · 2025-10-06T17:26:27Z

python/samples/psycopg3/getting_started/create_db_connection_psycopg3.py

    try:
+        # We pass in the AsyncEntraConnection class to enable Entra authentication for the
+        # PostgreSQL database by acquiring an Azure access token, extracting a username from the token, and using
+        # the token itself (with the PostgreSQL scope) as the password.


This comment isn't clear and seems wrong.

"We pass in AsyncEntraConnection to enable Entra...by aquiring an Azure access token .. and using the token itself as the password". This is confusing. The latter part talks about the implementation details of how AsyncEntraConnection work. This might confuse users who are just looking to use the functionality - reading this quickly I might think I need to get a token and pass it in as the password. Suggest rewording to focus on the required parameter, and a brief high level description of how the pool uses the parameter, potentially with a link to psycopg documentation about that capability.

e.g. (needs better wording):

# We pass in the AsyncEntraConnection class to enable Entra authentication for accessing the # database. This is the connection class that will be used whenever the pool creates a new connection, # and this connection type will manage the Entra authentication token to ensure a valid token is used # when connecting.

I reworded this comment to focus more on what the code does rather than how it does it. I made this change in all relevant places across the three drivers. I also added links to the documentation for the user to learn more about these concepts.

lossyrob · 2025-10-06T17:34:05Z

python/src/azurepg_entra/core.py

 import logging
-import json
-import base64
+import jwt


The reason I implemented JWT parsing directly is:

A) adding dependencies as a library is always a disadvantage. The more libraries your particular library brings in, the more potential for conflicting versions with the user's environment, duplication with other libraries the user might be using, more contribution to code bloat, and more dependency management. For this reason, when implementing a library, the ideal is to use as few dependencies as possible. This of course needs to be balanced with hand-rolling code, which is often not ideal. However:

B) The JWT token content and the way to parse it out is very stable. This isn't a moving target - it's a bit complex of logic, but it's a standardized algorithm that won't change.

Given those two above I decided that hand-rolling the implementation (or I should say, having GPT roll it, which I have high confidence in its capability of doing), saves us a dependency and doesn't have high risk of producing less effective code than the library or a lot of code that needs maintenance. I still believe that to be the correct decision for this library.

I understand - thanks for the thorough explanation. Hand-rolling the library code makes sense in this case, although it was not intuitive to me at first. I have went back to using the hand-rolled code over using the jwt library.

lossyrob · 2025-10-06T17:34:39Z

python/pyproject.toml

  "azure-identity>=1.13.0",
  "azure-core>=1.24.0",
+  "aiohttp>=3.8.0",
+  "PyJWT>=2.0.0"


See comment in src/azurepg_entra/core.py.

Yep I got rid of both of these.

lossyrob · 2025-10-06T17:47:29Z

python/src/azurepg_entra/psycopg3/psycopg3_entra_id_extension.py

-
-For example:
-
-    from azure_pg_entra import AsyncEntraConnection


Fix import; make sure all examples work. Most robust way to do this is to copy out the examples and run them as code in e.g. a notebook to ensure correctness before PR is merged.

Also - similar comment above to file naming.

Also - I think SyncEntraConnection isn't right, it doesn't follow the naming convention of the connection pooling itself. It's ConnectionPool and AsyncConnectionPool not SyncConnectionPool and AsyncConnectionPool. Let's follow the naming.

I would split the two classes up into two files, entra_connection.py and async_entra_connection.py.

I actually removed the examples because I thought the examples in the samples folder were sufficient (although I verified the examples in the samples folder). I have also modified the class and file naming according to your suggestions.

lossyrob · 2025-10-06T17:48:40Z

python/src/azurepg_entra/sqlalchemy/__init__.py

 """
 SQLAlchemy integration for Azure PostgreSQL with Entra ID authentication.
+
+This module provides seamless integration between SQLAlchemy and Azure Entra ID


-seamless

overused marketing word

Got rid of it.

lossyrob · 2025-10-06T17:49:54Z

python/src/azurepg_entra/sqlalchemy/sqlalchemy_entra_id_extension.py

+                logger.debug("User and password already present, skipping Entra authentication")
+        except Exception as e:
+            logger.error(f"Failed to get Entra credentials: {e}")
+            raise


Needs specific error

Got rid of it because we do error handling inside the method

lossyrob · 2025-10-06T17:50:50Z

python/src/azurepg_entra/sqlalchemy/sqlalchemy_entra_id_extension.py

+                if not has_password and "password" in entra_creds:
+                    cparams["password"] = entra_creds["password"]
+
+                logger.debug(f"Provided Entra credentials for user: {entra_creds.get('user', 'unknown')}")


If we add debug logging here, we should add it to the other implementations as well.

I removed debug logging here. I was only using it in one other place in core.py.

lossyrob · 2025-10-06T17:53:21Z

python/src/azurepg_entra/sqlalchemy/sqlalchemy_entra_id_extension.py

+                # For async engines, we need to handle the async credential fetching
+                try:
+                    # Try to get the current event loop
+                    asyncio.get_running_loop()


This feels out of band.

Is there no way to add an async event handler? I see engine.sync_engine, is there an engine.async_engine that would allow us to do a async def provide_token_async? We don't want to be managing loops and thread pools in the function.

Makes sense. As we discussed, I will simplify this and use get_entra_conninfo even in the async case.

lossyrob · 2025-10-09T18:06:10Z

.github/workflows/pr.yml

+      - name: Run tests
+        working-directory: ./python
+        run: |
+          pytest tests/ -v --tb=short


Consolidate this into a script that can be run locally to give devs the confidence that CI will pass.

Consider "scripts to rule them all" - see pgts or other Matt/Rob repositories for examples of modified implementation.

Need to consider how to handle these scripts and GitHub Actions based on the different languages in this project. Can we run workflows just on changes to the language folders? Should there be scripts/ under each language? Design challenge.

I have added GitHub actions for python and dotnet that are triggered when changes are made to their respective folders. I have also added corresponding scripts for developers to run before they push up a PR.

lossyrob · 2025-10-09T18:07:11Z

python/README.md

-                print(f"Pool connection as: {user} at {time}")
+                print(f"Connected as: {user} at {time}")
+    finally:
+        pool.close()


This should move back to with instead of try/catch

lossyrob · 2025-10-09T18:07:30Z

python/README.md

-                print(f"Pool connection as: {user} at {time}")
+                print(f"Async connected as: {user} at {time}")
+    finally:
+        await pool.close()


Same as above - use with

lossyrob · 2025-10-09T18:08:50Z

python/README.md

 ## SQLAlchemy Integration

-SQLAlchemy integration uses psycopg3 as the backend driver with automatic Entra ID authentication.
+SQLAlchemy integration uses psycopg3 as the backend driver with automatic Entra ID authentication through event listeners.


Worth linking to the sqlalchemy docs for the event hooks

lossyrob · 2025-10-09T18:11:12Z

python/README.md

        print(f"Async SQLAlchemy connected as: {user} at {time}")

    # Async ORM usage
+    from sqlalchemy.ext.asyncio import async_sessionmaker


Good to have this type of usage as an example; move to samples and have brief samples in README to give reader context, and link out to samples for more broad treatment.

I moved the example with sessionmaker to samples and focused more on how to use/enable the Entra libraries in README and saved the more rehearsed aspects like querying the database to the samples.

lossyrob · 2025-10-09T19:04:17Z

python/src/azurepg_entra/sqlalchemy/async_entra_connection.py

+                UsernameExtractionError,
+                ScopePermissionError,
+            ) as e:
+                print(repr(e))


lossyrob · 2025-10-09T19:04:47Z

python/src/azurepg_entra/sqlalchemy/async_entra_connection.py

+                TokenDecodeError,
+                UsernameExtractionError,
+                ScopePermissionError,
+            ) as e:


catch bare Exception

lossyrob · 2025-10-09T19:06:22Z

python/src/azurepg_entra/sqlalchemy/async_entra_connection.py

+
+    This function registers an event listener that automatically provides
+    Entra ID credentials for each database connection if they are not already set.
+


Clarify here and elsewhere that although this is async, event handlers don't provide async so the fetch token call will still be sync

lossyrob · 2025-10-09T19:06:52Z

python/src/azurepg_entra/sqlalchemy/entra_connection.py

+            ) as e:
+                print(repr(e))


lossyrob · 2025-10-09T19:18:19Z

python/tests/azure/data/postgresql/sqlalchemy/test_entra_id_extension.py

-# Example usage and test runner
+
+
+class TestEnableEntraAuthentication:


As discussed, think through a new testing strategy that can reduce mocking code that should be targeted by tests.

Ideas:

docker Postgres

mock at the last possible moment - get_entra_token could be patched to return test db password to mimick token generation

…s and exception handling

lossyrob · 2025-10-13T18:23:06Z

scripts/run-python-checks.ps1

@@ -0,0 +1,117 @@
+param(


scripts should be in scripts/

…management

Arjun Narendra added 4 commits October 3, 2025 13:51

Use connection_factory parameter for SyncEntraConnection

7448f51

Remove user-defining creds

004fb8c

Update Python support (source code, sample program, tests)

e7f836b

Update sqlalchemy tests

99ef432

lossyrob requested changes Oct 6, 2025

View reviewed changes

ArjunNarendra added 7 commits October 7, 2025 22:08

Major refactoring including adding exception handling and renaming

ab1db2c

Add ruff, mypy, and GitHub workflow for PR

e3987eb

Add types-psycopg2 dependency for static type checking

f66f56d

Add psycopg-pool to dev dependencies

9079dec

Add sample programs to test token refresh capabilities

3967f04

Add ruff and mypy checks

2a150e5

Fixed ruff formatting issues

8cb5c5f

lossyrob approved these changes Oct 9, 2025

View reviewed changes

Make major updates based on Rob's feedback including to GitHub Action…

e2a036b

…s and exception handling

lossyrob reviewed Oct 13, 2025

View reviewed changes

scripts/run-python-checks.ps1

@@ -0,0 +1,117 @@

param(

Copy link

Collaborator

lossyrob Oct 13, 2025

Choose a reason for hiding this comment

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

scripts should be in scripts/

ArjunNarendra added 7 commits October 13, 2025 20:52

Minor modifications such as file renaming and sample program context …

0f0d796

…management

Fix formatting and workflow script

4b2f09d

Fix Python workflow file

597504a

Update Python workflow script

093ae17

Fix file naming in solution file

6db50b5

Fix dotnet script

c7cd50f

Update scripting to only focus on Python and not .NET

a62a171

lossyrob merged commit ff71687 into main Oct 16, 2025
2 checks passed

lossyrob deleted the user/an/python-revisions branch October 16, 2025 18:09


		This function registers an event listener that automatically provides
		Entra ID credentials for each database connection if they are not already set.

		# Example usage and test runner


		class TestEnableEntraAuthentication:

Update Python support #6

Update Python support #6

Uh oh!

Conversation

ArjunNarendra commented Oct 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArjunNarendra Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArjunNarendra Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArjunNarendra Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArjunNarendra Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArjunNarendra Oct 6, 2025 •

edited

Loading

ArjunNarendra Oct 6, 2025 •

edited

Loading

ArjunNarendra Oct 8, 2025 •

edited

Loading

ArjunNarendra Oct 10, 2025 •

edited

Loading