Fix runtime heartbeat review gaps - #282
Conversation
Co-Authored-By: Codex <noreply@openai.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86b7f04a47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RUNTIME_HEARTBEAT_MARKET_TIMEZONE: ${{ vars.SCHWAB_MARKET_TIMEZONE }} | ||
| RUNTIME_HEARTBEAT_PUBLICATION_GRACE_MINUTES: ${{ vars.RUNTIME_HEARTBEAT_PUBLICATION_GRACE_MINUTES || '30' }} | ||
| RUNTIME_HEARTBEAT_SCHEDULER_AWARE: ${{ vars.RUNTIME_HEARTBEAT_SCHEDULER_AWARE || 'true' }} | ||
| RUNTIME_HEARTBEAT_SCHEDULER_LOCATION: ${{ vars.RUNTIME_HEARTBEAT_SCHEDULER_LOCATION || vars.CLOUD_RUN_REGION || 'us-central1' }} |
There was a problem hiding this comment.
When CLOUD_SCHEDULER_LOCATION differs from CLOUD_RUN_REGION, the deployment workflow creates or updates jobs in the former, but this heartbeat passes only the latter as the scheduler-location fallback. Any target with a two-field schedule then looks for its deployed job in the wrong region, reports an unable to resolve effective five-field scheduler cron policy error, and fails every scheduled heartbeat. Include vars.CLOUD_SCHEDULER_LOCATION in this fallback, consistent with .github/workflows/sync-cloud-run-env.yml.
Useful? React with 👍 / 👎.
| enabled_value = _target_field( | ||
| item, | ||
| defaults, | ||
| "runtime_target_enabled", | ||
| "RUNTIME_TARGET_ENABLED", | ||
| ) | ||
| if enabled_value is None: | ||
| enabled_value = item.get("RUNTIME_TARGET_ENABLED") | ||
| enabled_value = runtime_target.get("runtime_target_enabled") |
There was a problem hiding this comment.
For configurations with defaults.runtime_target_enabled=false and a target opting in through its nested runtime_target.runtime_target_enabled=true, _target_field returns the default before the nested value is consulted. Consequently this function reports that no targets are enabled, main() silently skips the heartbeat, and the analogous precedence in cloud_run_runtime_guard.py also excludes the service. Treat the per-target nested runtime value as an override of defaults, just as top-level per-target fields are.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
python -m pytest -q(226 passed)python -m ruff check --exclude external .uv lock --checkactionlint .github/workflows/execution-report-heartbeat.yml .github/workflows/runtime-guard.yml