-
Notifications
You must be signed in to change notification settings - Fork 80
feat(server): get kubernetes resource by informer #213
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e20519564f
ℹ️ 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".
| cached = informer.get(sandbox_id) | ||
| if cached: | ||
| return cached |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skip stale cache reads when informer is unsynced
This returns cached data before checking informer sync state, so after any watch/list failure (where _has_synced is flipped to false in WorkloadInformer._run) get_workload can still serve stale objects and never fall back to a direct API read for that sandbox ID. In practice, a sandbox deleted or changed while the informer is degraded can still be reported as existing here, which breaks read-after-delete and status accuracy for the agent-sandbox path.
Useful? React with 👍 / 👎.
| cached = informer.get(sandbox_id) | ||
| if cached: | ||
| return cached |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid serving cache entries after informer desync
The batchsandbox provider has the same stale-read path: cached objects are returned unconditionally before consulting has_synced, so once the informer loses sync it may continue returning outdated entries for existing cache keys and skip the API fallback. This can surface deleted BatchSandbox resources as still present until a successful resync, causing incorrect 200/404 behavior in production reads.
Useful? React with 👍 / 👎.
Summary
Testing
Breaking Changes
Checklist