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

[probes.external] Refactor the external probe code #695

Merged
merged 2 commits into from
Mar 22, 2024

Conversation

manugarg
Copy link
Contributor

@manugarg manugarg commented Mar 9, 2024

Changes:

  • Split out server handling code into a different file
  • Write tests for serverutils
  • Use context to cancel various loops, but we'll still not be able to cancel the pending read.

@manugarg manugarg added this to the v0.13.4 milestone Mar 15, 2024
@manugarg manugarg merged commit 527a9eb into master Mar 22, 2024
9 checks passed
@manugarg manugarg deleted the external_refactor branch March 22, 2024 01:18
manugarg added a commit that referenced this pull request May 21, 2024
We should not be writing to buffered stdout and stderr. This change
was introduced recently in v0.13.4:
  github.com//pull/695
manugarg added a commit that referenced this pull request May 21, 2024
We should not be writing to buffered stdout and stderr. This change
was introduced recently in v0.13.4:
  github.com//pull/695
stvnrhodes added a commit to stvnrhodes/cloudprober that referenced this pull request Jun 18, 2024
I noticed that https://pkg.go.dev/github.com/cloudprober/cloudprober/probes/external/serverutils#ServeContext doesn't take a context. This has been true since it was added in cloudprober#695, so it looks like an oversight. This change updates the function to properly take a context (and to work with its example code.
manugarg pushed a commit that referenced this pull request Jun 19, 2024
I noticed that https://pkg.go.dev/github.com/cloudprober/cloudprober/probes/external/serverutils#ServeContext doesn't take a context. This has been true since it was added in #695, so it looks like an oversight. This change updates the function to properly take a context (and to work with its example code.
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.

1 participant