Conversation
Re-attempt of bcgov#1972 (reverted in bcgov#1991/bcgov#1994/603f2977) after root-causing the prior ImportError: namex-pay and solr-names-updater depended on DBConfig/setup_search_path_event_listener via a re-export through the namex API package's git-branch dependency, so any drift between their locked namex commit and namex/__init__.py's exports broke the import. - api, solr-synonyms-api, solr-admin-app: add setup_pg8000_close_event_listener alongside the existing setup_search_path_event_listener call. solr-admin-app's cloud-sql-connector pin is bumped to a rev that actually exports the function. - namex-pay, solr-names-updater: switch from importing DBConfig/listeners via the namex package to depending on cloud-sql-connector directly (matching solr-synonyms-api's existing pattern), removing the fragile re-export coupling that caused the previous breakage.
panish16
requested review from
davemck513,
mengdong19,
ozamani9gh and
stevenc987
as code owners
September 15, 2026 22:42
…ling sbc-connect-common#78 merged after this branch was first opened — it broadens setup_pg8000_close_event_listener's exception handling, which previously only suppressed pg8000.exceptions.InterfaceError and let other exception types (e.g. ValueError: "write to closed file") for the same close-during-teardown situation fall through unsuppressed. Bumps the pinned rev across all 5 apps so they pick up the fix rather than needing a second follow-up PR right after this one merges.
solr-names-updater, solr-synonyms-api These three apps depend on both namex (git@main) and cloud-sql-connector directly. namex's own pyproject.toml is still pinned to the pre-bcgov#78 commit (b92e98f4) on bcgov/namex's actual main branch, since this PR hasn't merged yet. Bumping these three apps' own direct pin to the new commit (9ba15bc1) created two conflicting resolutions for the same package - real dependency conflict, not a Poetry bug, confirmed via: Because namex-pay depends on namex (1.50.3) @ .../namex.git@main which depends on cloud-sql-connector @ ...@b92e98f4..., cloud-sql-connector is required. So, because namex-pay depends on cloud-sql-connector @ ...@9ba15bc1..., version solving failed. solr-names-updater-ci/verify-build only "passed" after a Poetry version bump because its Dockerfile trusts the lock file without re-resolving; namex-pay's Dockerfile calls `poetry update` unconditionally and hit the real conflict immediately. Reverting both the pin and the now-unneeded Poetry version bumps restores the pre-conflict, verified-working state. api and solr-admin-app don't depend on namex, so their bump to 9ba15bc1 stays - no conflict there. Follow-up once this PR merges: bcgov/namex@main's own pin becomes 9ba15bc1, and a trivial one-line bump to these three apps' direct pin will then be conflict-free, since both paths will agree.
|
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
1 participant
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.
Summary
Re-attempt of #1972, which was reverted (#1991, #1994, and a full revert) after it broke production with
ImportError: cannot import name 'setup_pg8000_close_event_listener' from 'namex'in the solr-names-updater and solr-synonyms-api Cloud Run services.Root cause:
namex-payandsolr-names-updaterdidn't depend oncloud-sql-connectordirectly. They gotDBConfig/setup_search_path_event_listenerby re-exporting through a git-branch (main) dependency on thenamexAPI package itself. Any drift between their lockednamexcommit and whatapi/namex/__init__.pyactually exported broke the import — that's exactly what happened last time.This PR fixes the underlying coupling instead of just re-adding the listener call:
setup_pg8000_close_event_listeneralongside the existingsetup_search_path_event_listenercall.solr-admin-app'scloud-sql-connectorpin is bumped to a rev that actually exports the function (it was previously floating onbranch = "main"at a stale locked commit that predated the function).DBConfig/listeners via thenamexpackage to depending oncloud-sql-connectordirectly (same patternsolr-synonyms-apialready uses), removing the fragile re-export coupling entirely. This makes all 5 apps independent at both build and deploy time — no more relying onnamex's git-branch pin staying in sync.Test plan
Ran each app's test suite locally in a Linux container (matching the CI environment) against a real Postgres instance, including full Alembic migrations:
api: 348 passed, 6 failed (pre-existing, unrelated Solr-search mock/assertion issues — confirmed unrelated to this change), 367 skippedsolr-synonyms-api: 36 passed, 0 failednamex-pay: 23 passed, 0 failed — confirms the decoupledcloud-sql-connectordependency works end-to-end with real migrationssolr-names-updater: 36 passed, 0 failedsolr-admin-app: 4 passed, 17 failed — all failures are pre-existing (a Flask-SQLAlchemy 3.x app-context scoping bug in this app's own test fixtures, and tests requiring a live Solr server), unrelated to this change and consistent with this app's CI having been failing to even start for monthsImportErroranywhere — the exact failure mode from the previous incident