Capture Manager should use contexts #317
Assignees
Labels
good first issue
Good for newcomers
lang/go
The Go Programming Language
priority/2
P2
scope/M
Change is Medium
type/enhancement
New feature or request
Comments
rbtr
added
type/enhancement
7 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 10, 2025
# Description Refactoring the capture manager and capture commands to create a context hierarchy by passing it around, rather than creating a new one in each method with `context.TODO`. ## Related Issue #317 ## Checklist - [x] I have read the [contributing documentation](https://retina.sh/docs/contributing). - [x] I signed and signed-off the commits (`git commit -S -s ...`). See [this documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification) on signing commits. - [x] I have correctly attributed the author(s) of the code. - [x] I have tested the changes locally. - [x] I have followed the project's style guidelines. - [ ] I have updated the documentation, if necessary. - [ ] I have added tests, if applicable. ## Additional Notes https://go.dev/blog/context --- Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more information on how to contribute to this project. --------- Signed-off-by: Kamil <[email protected]>
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
CaptureManager is often responsible for interacting with multiple APIs, including the k8s apiserver. It's apparent from PRs that add new functionality to CaptureManager that there is many
context.TODO()s in places where these interactions take place (see: https://github.com/microsoft/retina/pull/295/files#diff-99babd138c7f336f6abec8bd499b3593610bad046a34d47e8d448a7600040c7bR104) . This could be added with a bit of work, binding to theos.Signalchannel. While this might ultimately yield the same result, it at least establishes a context heirarchy that will be more easily iterated on in the future.The text was updated successfully, but these errors were encountered: