Skip to content

Conversation

tausbn
Copy link
Contributor

@tausbn tausbn commented Aug 25, 2025

Our current modelling only treated psycopg2 insofar as it implemented PEP 249 (which does not define any notion of connection pool), which meant we were missing database connections that arose from such pools.

With these changes, we add support for the three classes relating to database pools that are defined in psycopg2. (Note that getAnInstance automatically looks at subclasses, which means this should also handle cases where the user has defined a new subclass that inherits from one of these three classes.)

Our current modelling only treated `psycopg2` insofar as it implemented
PEP 249 (which does not define any notion of connection pool), which
meant we were missing database connections that arose from such pools.

With these changes, we add support for the three classes relating to
database pools that are defined in `psycopg2`. (Note that
`getAnInstance` automatically looks at subclasses, which means this
should also handle cases where the user has defined a new subclass that
inherits from one of these three classes.)
joefarebrother
joefarebrother previously approved these changes Aug 25, 2025
Copy link
Contributor

@joefarebrother joefarebrother left a comment

Choose a reason for hiding this comment

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

Looks good, though I might add one or two unit test cases somewhere to cover the additional models.

@tausbn
Copy link
Contributor Author

tausbn commented Aug 25, 2025

Looks good, though I might add one or two unit test cases somewhere to cover the additional models.

Yup, I'm working on some tests now. I just wanted to put the code up ASAP so it could help the people who reported the issue to me.

@mbianchidev
Copy link
Member

Looks good, though I might add one or two unit test cases somewhere to cover the additional models.

Yup, I'm working on some tests now. I just wanted to put the code up ASAP so it could help the people who reported the issue to me.

And it definitely did, thank you so much for the great work!

Copy link
Contributor

@joefarebrother joefarebrother left a comment

Choose a reason for hiding this comment

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

👍

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

Successfully merging this pull request may close these issues.

3 participants