Refactor PostgresHook and associated runtime tests#66893
Open
SameerMesiah97 wants to merge 2 commits into
Open
Refactor PostgresHook and associated runtime tests#66893SameerMesiah97 wants to merge 2 commits into
SameerMesiah97 wants to merge 2 commits into
Conversation
Dev-iL
reviewed
May 14, 2026
Collaborator
Dev-iL
left a comment
There was a problem hiding this comment.
Thank you - this is a much-needed refactor I never got around to doing. Mostly LGTM with two inquiries/concerns.
6e2a98e to
2561091
Compare
Contributor
Author
|
I have addressed your feedback. |
Dev-iL
approved these changes
May 14, 2026
added 2 commits
May 22, 2026 22:20
…sycopg3. Consolidate duplicated insert/upsert and dialect tests into a shared base class while preserving version-specific behavior and lineage coverage.
2561091 to
01a514c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This change refactors psycopg-specific connection handling in
PostgresHookand consolidates duplicated runtime test coverage shared between psycopg2 and psycopg3 implementations.Connection setup responsibilities previously embedded directly inside
get_connhave been extracted into dedicated helper methods._get_cursor_confignow centralizes cursor configuration handling, while_create_connectionencapsulates driver-specific connection initialization, including adapter registration and notice handler setup.The accompanying test suite has also been reorganized to reduce duplicated behavioral coverage. Shared runtime tests have been moved into
_BasePostgresHookRuntimeTests, allowing psycopg2- and psycopg3-specific test classes to focus only on behavior that differs between implementations.Rationale
PostgresHook.get_connpreviously contained duplicated psycopg2- and psycopg3-specific connection setup logic. Maintaining separate inline code paths for cursor configuration, adapter registration, and notice handler initialization increases maintenance overhead and makes future changes harder to validate consistently across both drivers.Extracting
_get_cursor_configand_create_connectionsimplifies the control flow inget_conn, reduces branching complexity, and makes psycopg-specific behavior easier to reason about and extend independently.The test suite had a similar issue, with substantial overlap between psycopg2 and psycopg3 runtime behavioral tests. Consolidating identical coverage into
_BasePostgresHookRuntimeTestsimproves readability, reduces repeated CI execution for equivalent scenarios, and keeps implementation-specific tests scoped to behavior that actually differs.Tests
_BasePostgresHookRuntimeTests.execute_batch, and notice handling tests.copy_expertand notice handling coverage.Backwards Compatibility
This change only refactors test code and does not modify production behavior or public APIs.