testcontainers-go icon indicating copy to clipboard operation
testcontainers-go copied to clipboard

Published 20 hours ago • verified publisher icon testcontainers

chore!: return error from Customize

Open stevenh opened this issue 1 year ago • 2 comments

What does this PR do?

Change ContainerCustomizer.Customize method to return an error so that options can handle errors gracefully instead of relying on panic or just a log entry, neither of which are user friendly.

Enable errcheck linter to ensure that errors that aren't handled are reported.

Run go mod tidy on k3s and weaviate to allow tests to be run using go 1.22.

Run gofumpt on a few files to satisfy golangci-lint.

Fix direct comparison with http.ErrServerClosed flagged by errcheck.

Closes #2266

BREAKING CHANGE: ContainerCustomizer.Customize now returns an error.

Why is it important?

Current implementation either hides errors in a log message or triggers a runtime panic, neither of which is ideal.

Related issues

Closes #2266

stevenh avatar Feb 25 '24 20:02 stevenh

Deploy Preview for testcontainers-go ready!

Name Link
Latest commit 0e7cb731eb3dc98871901b2a935d0b40103716f1
Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/662aa36db7d6b1000896ec8b
Deploy Preview https://deploy-preview-2267--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.

netlify[bot] avatar Feb 25 '24 20:02 netlify[bot]

Draft as I need to look at the failing tests, looks like the extra error checking is now making a bunch of them fail

stevenh avatar Feb 26 '24 09:02 stevenh

@stevenh If you agree I'm going to fetch this PR locally and start building on top of your code to make this into the next release. wdyt?

mdelapenya avatar Apr 24 '24 16:04 mdelapenya

@stevenh If you agree I'm going to fetch this PR locally and start building on top of your code to make this into the next release. wdyt?

Go for it!

stevenh avatar Apr 24 '24 19:04 stevenh

Thank you @stevenh for bringing this issue to the table, and tackling it from the roots.

We introduced a breaking change for module creators, but I think it's worth it for the sake of a more robust API design.

I'll try to do great job in the release notes explaining it and proposing the upgrade path 🤞

mdelapenya avatar Apr 26 '24 06:04 mdelapenya

Thanks for all your support @mdelapenya and finding the time to address the final issues with the PR, most appreciated!

stevenh avatar Apr 26 '24 10:04 stevenh