Skip to content
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

Sync change from Klutch upstream #35

Merged
merged 9 commits into from
Jan 15, 2025
Merged

Sync change from Klutch upstream #35

merged 9 commits into from
Jan 15, 2025

Conversation

abdulhaseeb3
Copy link
Contributor

WARNING: Only users listed in the CODEOWNERS file can approve PRs!

@abdulhaseeb3 abdulhaseeb3 changed the base branch from main to remove-resources January 7, 2025 08:09
@abdulhaseeb3 abdulhaseeb3 force-pushed the sync-changes branch 5 times, most recently from 5bc1534 to a700881 Compare January 10, 2025 12:34
Base automatically changed from remove-resources to main January 14, 2025 09:50
Our confighealth reconciler performs a health check HTTP request to the anynines service broker and writes the result to the providerconfig's status field.

Previously, one Golang context with a timeout was used for both the health check and the subsequent patching of providerconfig's status. However, if the health check request exceeded the timeout, the patching of the status field could never succeed. client-go's Patch() method would error with a "context deadline exceeded" error message.

This commit introduces a separate sub-context for the health check request, which has an explicit timeout, while using the parent context (which does not time out) for patching the status field.
Copy link
Contributor

@rfashwall-anynines rfashwall-anynines left a comment

Choose a reason for hiding this comment

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

LGTM

@abdulhaseeb3 abdulhaseeb3 merged commit 4266fe5 into main Jan 15, 2025
2 checks passed
@abdulhaseeb3 abdulhaseeb3 deleted the sync-changes branch January 15, 2025 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants