Skip to content

Refactor Katib Context Management: Replace context.TODO() with Propagated context.Context#2649

Open
invo-coder19 wants to merge 1 commit intokubeflow:masterfrom
invo-coder19:refactor/context-management
Open

Refactor Katib Context Management: Replace context.TODO() with Propagated context.Context#2649
invo-coder19 wants to merge 1 commit intokubeflow:masterfrom
invo-coder19:refactor/context-management

Conversation

@invo-coder19
Copy link
Copy Markdown

There are ~30 instances in production code (excluding tests and generated files) where context.TODO() is used as a placeholder. This is not suitable for production, as it prevents proper context propagation, including cancellation, deadlines, and request-scoped values.

In controller logic, the reconciler already receives a context.Context via:

Reconcile(ctx context.Context, ...)

However, this context was not consistently passed down through the call chain and into Kubernetes API calls.

Changes
Replaced all occurrences of context.TODO() in production code with the appropriate propagated ctx.
Ensured the ctx received in Reconcile is threaded through all downstream function calls.
Updated Kubernetes client/API calls to use the propagated context.
Benefits
Enables proper cancellation and timeout handling.
Aligns with Go best practices for context usage.
Improves observability and tracing capabilities.
Removes placeholder code that could lead to unintended behavior in production.

@github-actions
Copy link
Copy Markdown

🎉 Welcome to the Kubeflow Katib repo! 🎉

Thanks for opening your first PR! We're excited to have you onboard 🚀

Next steps:

Feel free to ask questions in the comments. Thanks again for contributing! 🙏

@google-oss-prow
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign gaocegege for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant