Conversation
…ERVER_URL for Cloud SQL IAM authentication
There was a problem hiding this comment.
Pull request overview
This PR updates the staging and production CD GitHub Actions workflows to refine which environment variables are provided during DB migrations and to pass a new PYGEOAPI_SERVER_URL value into the deployed App Engine app.yaml environment.
Changes:
- Remove
PYGEOAPI_POSTGRES_*env vars from the Alembic migration step (staging + production). - Add
PYGEOAPI_SERVER_URLto the App Engineenv_variablesfor staging + production deployments.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/CD_staging.yml | Stops passing PYGEOAPI_POSTGRES_* to the Alembic step; adds PYGEOAPI_SERVER_URL to deployed app env. |
| .github/workflows/CD_production.yml | Mirrors the staging workflow changes for production deployment. |
| PYGEOAPI_POSTGRES_HOST: "${{ vars.PYGEOAPI_POSTGRES_HOST || '127.0.0.1' }}" | ||
| PYGEOAPI_POSTGRES_PORT: "${{ vars.PYGEOAPI_POSTGRES_PORT || '5432' }}" | ||
| PYGEOAPI_POSTGRES_PASSWORD: "${{ secrets.PYGEOAPI_POSTGRES_PASSWORD }}" | ||
| PYGEOAPI_SERVER_URL: "${{ vars.PYGEOAPI_SERVER_URL }}" |
There was a problem hiding this comment.
PYGEOAPI_SERVER_URL is used verbatim by core/pygeoapi.py (it only strips trailing slashes) and the default includes the mount path (/ogcapi). If the GitHub env var is set to just the site root (e.g., https://example.com), the generated OpenAPI servers URL will likely be missing the mount path. Consider enforcing/clarifying that this value must include the mount path (or adjust the app logic to append PYGEOAPI_MOUNT_PATH when present).
| PYGEOAPI_SERVER_URL: "${{ vars.PYGEOAPI_SERVER_URL }}" | |
| PYGEOAPI_SERVER_URL: "${{ format('{0}{1}', vars.PYGEOAPI_SERVER_URL, vars.PYGEOAPI_MOUNT_PATH || '/ogcapi') }}" |
| PYGEOAPI_POSTGRES_HOST: "${{ vars.PYGEOAPI_POSTGRES_HOST || '127.0.0.1' }}" | ||
| PYGEOAPI_POSTGRES_PORT: "${{ vars.PYGEOAPI_POSTGRES_PORT || '5432' }}" | ||
| PYGEOAPI_POSTGRES_PASSWORD: "${{ secrets.PYGEOAPI_POSTGRES_PASSWORD }}" | ||
| PYGEOAPI_SERVER_URL: "${{ vars.PYGEOAPI_SERVER_URL }}" |
There was a problem hiding this comment.
PYGEOAPI_SERVER_URL is used verbatim by core/pygeoapi.py (it only strips trailing slashes) and the default includes the mount path (/ogcapi). If the GitHub env var is set to just the site root (e.g., https://example.com), the generated OpenAPI servers URL will likely be missing the mount path. Consider enforcing/clarifying that this value must include the mount path (or adjust the app logic to append PYGEOAPI_MOUNT_PATH when present).
| PYGEOAPI_SERVER_URL: "${{ vars.PYGEOAPI_SERVER_URL }}" | |
| # Ensure PYGEOAPI_SERVER_URL includes the OGC API mount path (/ogcapi) | |
| PYGEOAPI_SERVER_URL: "${{ vars.PYGEOAPI_SERVER_URL }}/ogcapi" |
| - name: Run Alembic migrations on staging database | ||
| env: | ||
| DB_DRIVER: "cloudsql" | ||
| CLOUD_SQL_INSTANCE_NAME: "${{ secrets.CLOUD_SQL_INSTANCE_NAME }}" | ||
| CLOUD_SQL_DATABASE: "${{ vars.CLOUD_SQL_DATABASE }}" | ||
| CLOUD_SQL_USER: "${{ secrets.CLOUD_SQL_USER }}" | ||
| PYGEOAPI_POSTGRES_DB: "${{ vars.CLOUD_SQL_DATABASE }}" | ||
| PYGEOAPI_POSTGRES_USER: "${{ secrets.PYGEOAPI_POSTGRES_USER }}" | ||
| PYGEOAPI_POSTGRES_HOST: "${{ vars.PYGEOAPI_POSTGRES_HOST || '127.0.0.1' }}" | ||
| PYGEOAPI_POSTGRES_PORT: "${{ vars.PYGEOAPI_POSTGRES_PORT || '5432' }}" | ||
| PYGEOAPI_POSTGRES_PASSWORD: "${{ secrets.PYGEOAPI_POSTGRES_PASSWORD }}" | ||
| CLOUD_SQL_IAM_AUTH: true | ||
| run: | | ||
| uv run alembic upgrade head |
There was a problem hiding this comment.
The PR description still contains the template placeholders and doesn’t explain why the workflow env vars are changing (e.g., removal of PYGEOAPI_POSTGRES_* from the Alembic step and addition of PYGEOAPI_SERVER_URL). Please update the PR description so reviewers/operators understand the intended behavioral change and any required GitHub Environment variable updates.
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?