Skip to content

update delivery api to use custom schema / force custom schema via po…#171

Merged
bolyachevets merged 2 commits intomainfrom
feature-schema-follow-up
Jun 27, 2025
Merged

update delivery api to use custom schema / force custom schema via po…#171
bolyachevets merged 2 commits intomainfrom
feature-schema-follow-up

Conversation

@bolyachevets
Copy link
Collaborator

@bolyachevets bolyachevets commented Jun 27, 2025

…ol event listerns

Currently deployed in dev.

For other environments (i.e. test, sandbox, prod):

  • make sure notifyuser has permissions to create new schemas, i.e. GRANT CREATE ON DATABASE notify TO notifyuser;

  • make sure NOTIFY_DATABASE_SCHEMA env var is added to db migration job

  • After deployment rerun readonly.sql and readwrite.sql to apply grants to new 'notify' schema in notify-db

@bolyachevets bolyachevets requested a review from Copilot June 27, 2025 02:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for configuring a custom database schema (DB_SCHEMA), applies that schema on connector-based and pooled SQLAlchemy connections, and updates the environment vault to surface the new schema variable.

  • Introduces DB_SCHEMA config and environment variable
  • Executes SET search_path immediately after connect and on engine checkout
  • Adds pool options for the API service, mirrors that behavior in the delivery service

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

File Description
notify-delivery/src/notify_delivery/config.py Added DB_SCHEMA and removed redundant DB_NAME/DB_USER lookups
notify-delivery/src/notify_delivery/init.py Applies search_path on initial connect and checkout events
notify-delivery/devops/vaults.gcp.env Exposed NOTIFY_DATABASE_SCHEMA in environment vault
notify-api/src/notify_api/init.py Mirrors delivery service’s schema logic and adds pool options
Comments suppressed due to low confidence (2)

notify-service/notify-delivery/devops/vaults.gcp.env:9

  • Please update the README or deployment docs to document the new NOTIFY_DATABASE_SCHEMA environment variable and its default value (public).
NOTIFY_DATABASE_SCHEMA="op://database/$APP_ENV/notify-db-gcp/NOTIFY_DATABASE_SCHEMA"

notify-service/notify-api/src/notify_api/init.py:70

  • Indentation appears off under if db_config.schema:cursor = conn.cursor() should be indented one level further to be inside the block, or the block braces adjusted.
            cursor = conn.cursor()

@bolyachevets bolyachevets requested a review from pwei1018 June 27, 2025 15:08
@bolyachevets bolyachevets merged commit cbb25e2 into main Jun 27, 2025
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants