Skip to content

Reference #883: env timeout open k8s logstream #1154

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

Merged

Conversation

aeter
Copy link
Contributor

@aeter aeter commented Oct 3, 2024

References #883

Adds env variable to configure open logstream timeout.

Please note: when writing this PR: not sure where/how to document the env variable (other than in the code), so users can find it.

Testing:

$ PATH="${PWD}:${PATH}" go test ./pkg/workceptor/kubernetes_test.go
ok      command-line-arguments  0.014s
$ PATH="${PWD}:${PATH}" go test ./pkg/workceptor/kubernetes_test.go -count=1 -race -timeout 5m
ok      command-line-arguments  1.053s 

Copy link

sonarqubecloud bot commented Oct 3, 2024

Copy link

codecov bot commented Oct 3, 2024

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 41.48%. Comparing base (9192dc5) to head (4a81a46).
Report is 17 commits behind head on devel.

Files with missing lines Patch % Lines
pkg/workceptor/kubernetes.go 94.11% 1 Missing ⚠️
@@            Coverage Diff             @@
##            devel    #1154      +/-   ##
==========================================
+ Coverage   40.84%   41.48%   +0.64%     
==========================================
  Files          45       48       +3     
  Lines        9279     9875     +596     
==========================================
+ Hits         3790     4097     +307     
- Misses       5198     5487     +289     
  Partials      291      291              
Files with missing lines Coverage Δ
pkg/workceptor/kubernetes.go 28.57% <94.11%> (+0.92%) ⬆️

... and 7 files with indirect coverage changes

Components Coverage Δ
Go 41.00% <94.11%> (+0.15%) ⬆️
Receptorctl 49.31% <ø> (∅)
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JoelKle
Copy link

JoelKle commented Nov 18, 2024

thank you for the PR, @aeter! @AaronH88, @thom-at-redhat, @fosterseth - besides the small linting issues to fix, do we need anything else for this PR?

@aeter aeter force-pushed the 883-fix-insufficient-timeout-opening-k8s-stream branch from bb0b10f to 89a2cf7 Compare November 21, 2024 08:03
@AaronH88
Copy link
Contributor

Sorry for the delay in replying, This PR looks really good. I just have a request for a bit more defensive programming and it should be good to merge 😄

Signed-off-by: Adrian Nackov <adrian.nackov@mail.schwarz>
@aeter aeter force-pushed the 883-fix-insufficient-timeout-opening-k8s-stream branch from 89a2cf7 to 4a81a46 Compare December 10, 2024 12:56
@aeter
Copy link
Contributor Author

aeter commented Dec 10, 2024

Thanks! Updated per review, added more unit tests (for "-1", "0") at kubernetes_test.go, rebased with latest devel.

Copy link
Contributor

@AaronH88 AaronH88 left a comment

Choose a reason for hiding this comment

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

lgtm

Thanks so much for the PR

@AaronH88 AaronH88 merged commit ad9c4eb into ansible:devel Dec 11, 2024
26 checks passed
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.

3 participants