Repository navigation
fix(dbng): bound database readiness polling - #1277
WilliamK112 wants to merge 2 commits into
Conversation
Use the shared Scalingo retry utility to cap readiness polling at 30 minutes while preserving retries for transitional states and databases that are not visible yet. Stop immediately for stopped databases and API errors, and cover the behavior with focused tests.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 962c8515e1
ℹ️ 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".
| db, err := client.Preview().DatabaseShow(ctx, appID) | ||
| retryer := retry.New( | ||
| retry.WithWaitDuration(databaseStatusPollInterval), | ||
| retry.WithMaxDuration(databaseStatusMaxWait), |
There was a problem hiding this comment.
When a DatabaseShow call runs across the 30-minute mark, this option does not actually bound the wait: Retryer.Do creates a separate timeout context but invokes method(ctx) with the original context and returns a successful result before consulting that timeout (vendor/github.com/Scalingo/go-utils/retry/retry.go:163,177-180). Consequently, an in-flight lookup can continue past the deadline and can even report success afterward; this is significant here because Preview DatabaseShow performs a list plus per-database population requests. Derive and pass a 30-minute context to the lookup so the provisioning deadline also cancels an active poll.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fc116ade. The 30-minute provisioning limit now creates a child context before the retry loop and passes that same context into every DatabaseShow call, so an in-flight lookup is canceled when the deadline expires. I added a regression test whose lookup blocks until the supplied context is canceled.
Validation passed:
go test ./dbng- cancellation regression test repeated 50 times
go test ./...go vet ./...
Summary
go-utils/retryFixes #1271
Validation
go test ./dbnggo test ./...go vet ./...gofmtandgit diff --checkThe repository-equivalent golangci-lint command was also attempted locally, but the analyzer remained CPU-bound without producing output on this resource-constrained machine; PR CI will run the same official configuration.
Checklist
CHANGELOG.md