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

internal/testcontainersdocker: close unused client #1441

Merged
merged 1 commit into from Aug 6, 2023

Conversation

AlexanderYastrebov
Copy link
Contributor

What does this PR do?

Closes unused docker client.

Why is it important?

Docker client uses idle connections and if it is not closed it triggers goroutine leak detector.
I've created #1358 previously but while it is in discussion I propose to fix this obvious case which would also be enough for our project, see zalando/skipper#2436

Related issues

@AlexanderYastrebov AlexanderYastrebov requested a review from a team as a code owner August 6, 2023 15:01
@sonarcloud
Copy link

sonarcloud bot commented Aug 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@netlify
Copy link

netlify bot commented Aug 6, 2023

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 4e20026
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/64cfb5e10438ef000888ae05
😎 Deploy Preview https://deploy-preview-1441--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

Good catch!

@mdelapenya mdelapenya self-assigned this Aug 6, 2023
@mdelapenya mdelapenya added the bug An issue with the library label Aug 6, 2023
@mdelapenya mdelapenya merged commit d1c291e into testcontainers:main Aug 6, 2023
106 checks passed
@AlexanderYastrebov AlexanderYastrebov deleted the close-client branch August 6, 2023 16:36
AlexanderYastrebov pushed a commit to zalando/skipper that referenced this pull request Aug 6, 2023
* update testcontainers to fix goroutine leak (see testcontainers/testcontainers-go#1441)
  [changes](testcontainers/testcontainers-go@v0.20.1...d1c291e)
* enable redis tests back

Closes #2496

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando/skipper that referenced this pull request Aug 6, 2023
* update testcontainers to fix goroutine leak (see testcontainers/testcontainers-go#1441)
  [changes](testcontainers/testcontainers-go@v0.20.1...d1c291e)
* enable redis tests back

Closes #2496

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando/skipper that referenced this pull request Aug 6, 2023
* update testcontainers to fix goroutine leak (see testcontainers/testcontainers-go#1441)
  [changes](testcontainers/testcontainers-go@v0.20.1...d1c291e)
* enable redis tests back

Updates #2458
Closes #2496

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
@AlexanderYastrebov
Copy link
Contributor Author

Thank you.

AlexanderYastrebov added a commit to zalando/skipper that referenced this pull request Aug 6, 2023
* update testcontainers to fix goroutine leak (see testcontainers/testcontainers-go#1441)
  [changes](testcontainers/testcontainers-go@v0.20.1...d1c291e)
* enable redis tests back

Updates #2458
Closes #2496

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue with the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants