break: return error from Customize request option #2267

stevenh · 2024-02-25T20:35:30Z

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

netlify · 2024-02-25T20:35:47Z

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	`0e7cb73`
🔍 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... Use your smartphone camera to open QR code link.

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

stevenh · 2024-02-26T09:35:24Z

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

modules/k3s/go.mod

modules/weaviate/go.mod

network/network.go

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. Fixes #2266 BREAKING CHANGE: `ContainerCustomizer.Customize` now returns an error.

Fix captured loop variable in mongodb test reported by govet.

Fix formatting in test file reported by gci during linting.

Add missing error returns for implementations of CustomizeRequestOption.

* main: (21 commits) feat: optimizes file copies to and from containers (#2450) fix(exec): updates the `Multiplexed` opt to combine stdout and stderr (#2452) Upgrade neo4j module to use features from v0.29.1 of testcontainers-go (#2463) bug:Fix AMQPS url (#2462) chore: more compose updates in comments chore: use "docker compose" (v2) instead of "docker-compose" (v1) (#2464) chore(deps): bump github/codeql-action from 2.22.12 to 3.24.9 (#2459) refactor: Add Weaviate modules tests (#2447) feat(exitcode): Add exit code sugar method (#2342) feat: add module to support InfluxDB v1.x (#1703) feat: authenticate docker on PullImage (#2446) feat: add distribution-registry module (#2341) chore(deps): Bumping ChromaGo client version (#2402) chore(deps): bump github.com/docker/docker from 25.0.3+incompatible to 25.0.5+incompatible (#2444) feat: support passing io.Reader as ContainerFile (#2401) chore: bump ryuk to latest (#2395) feat(MustConn): Add MustConnectionString on (some) dbs (#2343) fix: typo in ci-test-go.yml (#2394) feat: support for waiting for response headers (#2349) chore(deps): bump google.golang.org/protobuf from 1.32.0 to 1.33.0 (#2392) ...

mdelapenya · 2024-04-24T16:21:58Z

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

* main: (22 commits) feat: forward host ports to a container using an SSH tunnel (#2471) Update follow_logs.md with adding missing package (#2513) fix: don't retry on permanent APIClient errors (#2506) feat: support overriding the default recreate options for compose (#2511) feat: support passing io.Reader for compose files when creating a compose instance (#2509) chore: add funding button for testcontainers (#2510) feat: support Ryuk for the compose module (#2485) chore(deps): bump golang.org/x/net in modules (minio, gcloud, weaviate, compose, qdrant, couchbase, k3s, milvus, mockserver, pulsar, kafka) (#2505) fix: fallback to URL-path when parsing auth config URL without scheme (#2488) fix(postgres): Fix the non-default dbname error (#2489) feat: Bump default postgres version (#2481) support Dolt (#2177) chore: create TLS certs in a consistent manner (#2478) chore(deps): bump idna from 3.6 to 3.7 (#2480) Elasticsearch disable CA retrieval when ssl is disabled (#2475) fix: handle dockerignore exclusions properly (#2476) chore: prepare for next minor development cycle (0.31.0) chore: use new version (v0.30.0) in modules and examples Fix url creation to handle query params when using HTTP wait strategy (#2466) fix: data race on container run (#2345) ...

stevenh · 2024-04-24T19:00:12Z

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

* main: docs: document the SSHd tunnel (#2514) fix: enhance host configuration port binding (#2512)

stevenh

Can only comment, as my PR, but some small items to consider @mdelapenya

modules/neo4j/config.go

port_forwarding.go

* main: docs: add documentation for Exec method (#2451)

modules/mongodb/mongodb_test.go

stevenh

Looks good to me

mdelapenya · 2024-04-26T06:03:06Z

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 🤞

stevenh · 2024-04-26T10:47:00Z

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

* main: (34 commits) break: return error from Customize request option (testcontainers#2267) fix: wrong copy paste (testcontainers#2515) docs: add documentation for Exec method (testcontainers#2451) docs: document the SSHd tunnel (testcontainers#2514) fix: enhance host configuration port binding (testcontainers#2512) feat: forward host ports to a container using an SSH tunnel (testcontainers#2471) Update follow_logs.md with adding missing package (testcontainers#2513) fix: don't retry on permanent APIClient errors (testcontainers#2506) feat: support overriding the default recreate options for compose (testcontainers#2511) feat: support passing io.Reader for compose files when creating a compose instance (testcontainers#2509) chore: add funding button for testcontainers (testcontainers#2510) feat: support Ryuk for the compose module (testcontainers#2485) chore(deps): bump golang.org/x/net in modules (minio, gcloud, weaviate, compose, qdrant, couchbase, k3s, milvus, mockserver, pulsar, kafka) (testcontainers#2505) fix: fallback to URL-path when parsing auth config URL without scheme (testcontainers#2488) fix(postgres): Fix the non-default dbname error (testcontainers#2489) feat: Bump default postgres version (testcontainers#2481) support Dolt (testcontainers#2177) chore: create TLS certs in a consistent manner (testcontainers#2478) chore(deps): bump idna from 3.6 to 3.7 (testcontainers#2480) Elasticsearch disable CA retrieval when ssl is disabled (testcontainers#2475) ...

* main: (44 commits) feat: expose JSON representation of a container with Inspect (testcontainers#2534) chore(deps): bump test-summary action to v2.3 (testcontainers#2535) chore(deps): bump jinja2 from 3.1.3 to 3.1.4 (testcontainers#2533) Update devcontainer image (testcontainers#2531) chore(influxdb): include more characters in wait for log regex (testcontainers#2532) fix(compose): avoid race conditions when caching services (testcontainers#2528) chore(deps): bump golangci/golangci-lint-action from 3.7.0 to 5.1.0 (testcontainers#2525) chore(deps): bump mkdocs-material from 8.2.7 to 9.1.21 (testcontainers#2524) chore(compose): return error in options (testcontainers#2520) chore(deps): bump github.com/compose-spec/compose-go/v2 from v2.0.0-rc8 to v2.1.0 (testcontainers#2519) chore(deps): bump github.com/containerd/containerd from 1.7.12 to 1.7.15 (testcontainers#2517) break: return error from Customize request option (testcontainers#2267) fix: wrong copy paste (testcontainers#2515) docs: add documentation for Exec method (testcontainers#2451) docs: document the SSHd tunnel (testcontainers#2514) fix: enhance host configuration port binding (testcontainers#2512) feat: forward host ports to a container using an SSH tunnel (testcontainers#2471) Update follow_logs.md with adding missing package (testcontainers#2513) fix: don't retry on permanent APIClient errors (testcontainers#2506) feat: support overriding the default recreate options for compose (testcontainers#2511) ...

Needed after testcontainers/testcontainers-go#2267

stevenh force-pushed the chore/options-error branch 3 times, most recently from 0a87c6f to 4c0c443 Compare February 25, 2024 20:49

stevenh marked this pull request as ready for review February 25, 2024 20:53

stevenh requested a review from a team as a code owner February 25, 2024 20:53

stevenh mentioned this pull request Feb 25, 2024

[Enhancement]: support errors from container option #2266

Closed

stevenh marked this pull request as draft February 26, 2024 09:34

mdelapenya reviewed Mar 11, 2024

View reviewed changes

modules/k3s/go.mod Outdated Show resolved Hide resolved

modules/weaviate/go.mod Outdated Show resolved Hide resolved

network/network.go Show resolved Hide resolved

stevenh added 4 commits March 12, 2024 20:24

fix(mongodb): captured loop variable

a3aa85b

Fix captured loop variable in mongodb test reported by govet.

fix(k3s): test formatting

634ae00

Fix formatting in test file reported by gci during linting.

chore: add missing Customize error returns

3542c5c

Add missing error returns for implementations of CustomizeRequestOption.

stevenh force-pushed the chore/options-error branch from 4c0c443 to 3542c5c Compare March 12, 2024 21:16

mdelapenya added 4 commits April 24, 2024 18:24

chore: update modules with the new error API

30468dc

fix: use logger

d284e23

fix: update modulegen tests

eeb7f13

mdelapenya added 2 commits April 24, 2024 22:48

chore: fix lint

b955846

chore: handle customise errors

58a3e4b

mdelapenya added the breaking change Causing compatibility issues. label Apr 25, 2024

mdelapenya added 5 commits April 25, 2024 12:03

Merge branch 'main' into chore/options-error

7bedac8

* main: docs: document the SSHd tunnel (#2514) fix: enhance host configuration port binding (#2512)

chore: update new host port access option

48c8691

docs: move new error API to the right doc

45705ba

docs: move customise request to the bottom

efe1c2d

docs: sync functional options

67bd389

mdelapenya changed the title ~~chore!: return error from Customize~~ break: return error from Customize request option Apr 25, 2024

mdelapenya self-assigned this Apr 25, 2024

mdelapenya marked this pull request as ready for review April 25, 2024 11:26

stevenh commented Apr 25, 2024

View reviewed changes

modules/neo4j/config.go Outdated Show resolved Hide resolved

port_forwarding.go Outdated Show resolved Hide resolved

port_forwarding.go Outdated Show resolved Hide resolved

mdelapenya added 2 commits April 25, 2024 17:38

chore: avoid panics in localstack

f26f891

Merge branch 'main' into chore/options-error

f381720

* main: docs: add documentation for Exec method (#2451)

mdelapenya reviewed Apr 25, 2024

View reviewed changes

modules/mongodb/mongodb_test.go Show resolved Hide resolved

mdelapenya added 3 commits April 25, 2024 18:03

chore: require no error in test

18df9d0

chore: ignore error in copy API, using nolint:errcheck

70f1eac

chore: convert log into error

0e7cb73

stevenh commented Apr 25, 2024

View reviewed changes

mdelapenya merged commit 14d6929 into testcontainers:main Apr 26, 2024
104 checks passed

mdelapenya mentioned this pull request Apr 26, 2024

feat(postgres): ssl for postgres #2473

Open

stevenh deleted the chore/options-error branch April 26, 2024 10:46

This was referenced Apr 30, 2024

chore(compose): return error in options #2520

Merged

feat: Add support to seed data when using RunBigQueryContainer #2521

Closed

mdelapenya mentioned this pull request May 10, 2024

fix: add import to module generation #2537

Merged

wormi4ok mentioned this pull request May 20, 2024

Update CustomizeRequestOption func signature wiremock/wiremock-testcontainers-go#39

Open

vaihtovirta mentioned this pull request Jun 13, 2024

Bump Testcontainers to 0.31.0 wiremock/wiremock-testcontainers-go#40

Merged

6 tasks

mdelapenya added a commit to mdelapenya/dynamodb-local-testcontainers-go that referenced this pull request Aug 5, 2024

chore: use new testcontainers APIs

c2ae848

Needed after testcontainers/testcontainers-go#2267

mdelapenya mentioned this pull request Aug 5, 2024

chore: update to testcontainers-go v0.33.0 abhirockzz/dynamodb-local-testcontainers-go#7

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

break: return error from Customize request option #2267

break: return error from Customize request option #2267

stevenh commented Feb 25, 2024

netlify bot commented Feb 25, 2024 •

edited

Loading

stevenh commented Feb 26, 2024

mdelapenya commented Apr 24, 2024

stevenh commented Apr 24, 2024

stevenh left a comment

stevenh left a comment

mdelapenya commented Apr 26, 2024

stevenh commented Apr 26, 2024

break: return error from Customize request option #2267

break: return error from Customize request option #2267

Conversation

stevenh commented Feb 25, 2024

What does this PR do?

Why is it important?

Related issues

netlify bot commented Feb 25, 2024 • edited Loading

✅ Deploy Preview for testcontainers-go ready!

stevenh commented Feb 26, 2024

mdelapenya commented Apr 24, 2024

stevenh commented Apr 24, 2024

stevenh left a comment

Choose a reason for hiding this comment

stevenh left a comment

Choose a reason for hiding this comment

mdelapenya commented Apr 26, 2024

stevenh commented Apr 26, 2024

netlify bot commented Feb 25, 2024 •

edited

Loading