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

chore: improve log handling when container is stopping #2601

Merged
merged 4 commits into from
Jun 26, 2024

Conversation

ash2k
Copy link
Contributor

@ash2k ash2k commented Jun 22, 2024

What does this PR do?

I'm getting an error when my container cleanly stops:

container log error: EOF. Stopping log consumer: Headers out of sync

Please see individual commits. I tried to tidy up a bit, I hope this is fine.

Why is it important?

The error is a red herring and distracts users of the library.

Related issues

@ash2k ash2k requested a review from a team as a code owner June 22, 2024 04:14
Copy link

netlify bot commented Jun 22, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 354d95e
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/66764fb834a32c00082b1fdd
😎 Deploy Preview https://deploy-preview-2601--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.

docker.go Show resolved Hide resolved
@mdelapenya mdelapenya self-assigned this Jun 24, 2024
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Jun 24, 2024
@mdelapenya
Copy link
Member

@ash2k do you think it's possible to write a test replicating the scenario where the container is stopped and the error is not present anymore?

@ash2k
Copy link
Contributor Author

ash2k commented Jun 24, 2024

I'm not sure how to test this. Start a container, emit logs, check that there is no error written to stderr? Seems too complicated just to test if a log line is written. Are there any similar tests?

@ash2k
Copy link
Contributor Author

ash2k commented Jun 26, 2024

@mdelapenya Friendly ping. I'd like to get this merged as I'd like to send a few more PRs and ideally don't want to have more than one in flight.

Copy link
Member

@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.

LGTM, thanks!

@mdelapenya mdelapenya changed the title Improve log handling chore: improve log handling when container is stopping Jun 26, 2024
@mdelapenya mdelapenya merged commit cef96e4 into testcontainers:main Jun 26, 2024
106 checks passed
@ash2k ash2k deleted the improve-log-handling branch June 26, 2024 07:56
mdelapenya added a commit that referenced this pull request Jun 28, 2024
* main:
  chore(ci): pass docker install type to the nightly build payload (#2612)
  chore: run rootless mode in nighlty builds (#2611)
  chore(deps): bump github.com/hashicorp/go-retryablehttp (#2605)
  chore: improve log handling when container is stopping (#2601)
mdelapenya added a commit to thaJeztah/testcontainers-go that referenced this pull request Jun 28, 2024
* main:
  chore: test cleanups (testcontainers#2608)
  chore(ci): pass docker install type to the nightly build payload (testcontainers#2612)
  chore: run rootless mode in nighlty builds (testcontainers#2611)
  chore(deps): bump github.com/hashicorp/go-retryablehttp (testcontainers#2605)
  chore: improve log handling when container is stopping (testcontainers#2601)
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Jul 2, 2024
* main:
  chore(deps): bump github.com/docker/docker from v26.1.4 to v27.0.2 (testcontainers#2593)
  fix: Rename TC_HOST environment variable to TESTCONTAINERS_HOST_OVERRIDE (testcontainers#2536)
  chore: test cleanups (testcontainers#2608)
  chore(ci): pass docker install type to the nightly build payload (testcontainers#2612)
  chore: run rootless mode in nighlty builds (testcontainers#2611)
  chore(deps): bump github.com/hashicorp/go-retryablehttp (testcontainers#2605)
  chore: improve log handling when container is stopping (testcontainers#2601)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Changes that do not impact the existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants