Skip to content

jir-ogc #554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jirhiker merged 1 commit into from
Feb 27, 2026
Merged

jir-ogc #554

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 37 additions & 14 deletions transfers/transfer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,19 @@
# environment variables already set by the runtime (e.g., Cloud Run jobs).
load_dotenv(override=False)

# In managed runtime environments, DB_DRIVER is occasionally omitted while
# CLOUD_SQL_* vars are present. Default to cloudsql in that case to avoid
# silently falling back to localhost/postgres settings.
if (
not (os.getenv("DB_DRIVER") or "").strip()
and (os.getenv("CLOUD_SQL_INSTANCE_NAME") or "").strip()
):
Comment on lines +41 to +47
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

Comment/condition mismatch: the comment says "CLOUD_SQL_* vars are present", but the fallback to cloudsql only checks CLOUD_SQL_INSTANCE_NAME. Either update the comment to match the actual condition, or expand the condition to check the required Cloud SQL vars you expect to be present (e.g., database/user) before forcing DB_DRIVER=cloudsql.

Copilot uses AI. Check for mistakes.
os.environ["DB_DRIVER"] = "cloudsql"

# Cloud SQL should use IAM auth by default unless explicitly disabled.
if (os.getenv("DB_DRIVER") or "").strip().lower() == "cloudsql":
os.environ.setdefault("CLOUD_SQL_IAM_AUTH", "true")

from alembic import command
from alembic.config import Config

Expand Down Expand Up @@ -690,20 +703,30 @@ def _transfer_parallel(
def main():
message("START--------------------------------------")

# Display database configuration for verification
db_name = os.getenv("POSTGRES_DB", "postgres")
db_host = os.getenv("POSTGRES_HOST", "localhost")
db_port = os.getenv("POSTGRES_PORT", "5432")
message(f"Database Configuration: {db_host}:{db_port}/{db_name}")

# Double-check we're using the development database
if db_name != "ocotilloapi_dev":
message(f"WARNING: Using database '{db_name}' instead of 'ocotilloapi_dev'")
if db_name in ("ocotilloapi_test", "nmsamplelocations_test"):
raise ValueError(
"ERROR: Cannot run transfer on test database! "
"Set POSTGRES_DB=ocotilloapi_dev in .env file"
)
db_driver = (os.getenv("DB_DRIVER") or "").strip().lower()
if db_driver == "cloudsql":
db_name = os.getenv("CLOUD_SQL_DATABASE", "")
instance_name = os.getenv("CLOUD_SQL_INSTANCE_NAME", "")
iam_auth = os.getenv("CLOUD_SQL_IAM_AUTH", "")
Comment on lines +707 to +710
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 27, 2026

Choose a reason for hiding this comment

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

P1 Badge Reinstate test-database block for Cloud SQL path

When DB_DRIVER=cloudsql, this branch only logs Cloud SQL settings and no longer enforces the prior guard that aborted on test databases. As a result, a run with CLOUD_SQL_DATABASE=ocotilloapi_test or nmsamplelocations_test now continues into transfer_all and can mutate/drop data that was previously protected by a fail-fast check. Add the same test-database validation in the Cloud SQL branch before the transfer starts.

Useful? React with 👍 / 👎.

message(
"Database Configuration: "
f"driver=cloudsql instance={instance_name} db={db_name} iam_auth={iam_auth}"
)
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The safety guard that blocks running transfers against known test DBs only runs in the non-Cloud SQL branch. When DB_DRIVER=cloudsql, db_name can still point at *_test and the script will proceed. Consider applying the same check to CLOUD_SQL_DATABASE (and failing fast for the same test DB names / non-dev DBs) so Cloud SQL executions get the same protection.

Suggested change
)
)
# Double-check we're using the development database for Cloud SQL as well
if db_name != "ocotilloapi_dev":
message(f"WARNING: Using database '{db_name}' instead of 'ocotilloapi_dev'")
if db_name in ("ocotilloapi_test", "nmsamplelocations_test"):
raise ValueError(
"ERROR: Cannot run transfer on test database! "
"Set CLOUD_SQL_DATABASE=ocotilloapi_dev in environment"
)

Copilot uses AI. Check for mistakes.
else:
# Display database configuration for verification
db_name = os.getenv("POSTGRES_DB", "postgres")
db_host = os.getenv("POSTGRES_HOST", "localhost")
db_port = os.getenv("POSTGRES_PORT", "5432")
message(f"Database Configuration: {db_host}:{db_port}/{db_name}")

# Double-check we're using the development database
if db_name != "ocotilloapi_dev":
message(f"WARNING: Using database '{db_name}' instead of 'ocotilloapi_dev'")
if db_name in ("ocotilloapi_test", "nmsamplelocations_test"):
raise ValueError(
"ERROR: Cannot run transfer on test database! "
"Set POSTGRES_DB=ocotilloapi_dev in .env file"
)

metrics = Metrics()

Expand Down