feat: support databend module by hantmac · Pull Request #2779 · testcontainers/testcontainers-go

hantmac · 2024-09-13T03:25:06Z

What does this PR do?

Support new module: databend

netlify · 2024-09-13T03:25:23Z

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	`7a24c55`
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/66eccf0ee96ad1000832bc3d
😎 Deploy Preview	https://deploy-preview-2779--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.

mdelapenya

@hantmac thanks for adding this new module, much appreciated! The go code looks good, although we need to add all the metadata for the module to be included in the CI and in the docs.

Could you please run the module generator code for it? https://golang.testcontainers.org/modules/#creating-a-new-module

Thanks!

mdelapenya · 2024-09-13T11:55:39Z

@hantmac I went ahead and added the scaffolding. Could you please enrich the docs with the new functional options so that users find them in our docs site 🙏 ?

Thanks in advance

modules/databend/databend.go

hantmac · 2024-09-13T12:45:32Z

@hantmac I went ahead and added the scaffolding. Could you please enrich the docs with the new functional options so that users find them in our docs site 🙏 ?

Thanks in advance

HI @mdelapenya , would you like to explain what is new functional options or where should I found it?

hantmac · 2024-09-13T12:56:02Z

@mdelapenya HI bro, I saw you have add the docs and some CI workflow information, thanks very much! Is there some other things I need to do?

docs/modules/databend.md

modules/databend/databend.go

stevenh

Thanks for working on this, a few bugs to fix and a few questions, where I'd like some clarification.

modules/databend/databend.go

modules/databend/databend_test.go

modules/databend/examples_test.go

modulegen/_template/ci.yml.tmpl

docs/modules/databend.md

Co-authored-by: Steven Hartland <[email protected]>

mdelapenya

We are close 😊 Just a few comments from my side plus Steven's.

Thanks for your patience during the review 🙏

mdelapenya · 2024-09-14T01:59:56Z

modules/databend/databend.go

+}
+
+// WithDatabase sets the name of the database to use.
+func WithDatabase(database string) DatabendOption {


If the user pass this option, where does the Run function applies it to the container? See https://github.com/testcontainers/testcontainers-go/pull/2779/files#diff-05e51be59eadcbdc57a90dccc140946369a930b7bec7e9e987096ebed8beb9beR78 where the default db name is used.

We also need to add it to the docs site.

The db name will add to DSN when call ConnectionString, so that user can use the db.

To confirm my understanding QUERY_DEFAULT_USER and QUERY_DEFAULT_PASSWORD configure the container at startup, which are then also used along side the database setting to allow ConnectionString to return the details for the client to connect to. There is no needed for default DB because the container doesn't need one?

The container doesn't need default DB ENV, but the connection dsn need a default one.

Does that mean a hardcoded DB is always created?

Yep, the default dababase exists when Databend start.

So I think the WithDatabase function maybe not suitable for Databend?

@hantmac then, I'd remove the withDatabase support and add it in a follow up: adding the option and creating the new database using the commands you posted above, as part of the container lifecycle hooks (postStarts) in order to create that database. Wdyt?

@mdelapenya I agree with you.

@hantmac perfect then. Let's remove it to make progress and merge this new module 🚀 , and postpone the work for the database support on a new iteration.

Thank you for all this hard work during the review 🙇

modules/databend/examples_test.go

modules/databend/databend.go

stevenh

Just need the imports fixing into the correct blocks as identified by the linter

hantmac · 2024-09-17T03:55:46Z

Just need the imports fixing into the correct blocks as identified by the linter

Thanks! @stevenh , fixed the ci lint.

Co-authored-by: Steven Hartland <[email protected]>

mdelapenya

@hantmac let's remove all references to the WithDatabase, and once there. I think we are good to go.

…tcontainers-go into feat/support-databend-module

hantmac · 2024-09-19T03:10:25Z

@hantmac let's remove all references to the WithDatabase, and once there. I think we are good to go.

@mdelapenya Have removed the references to the WithDatabase, thanks for your patience.

…tcontainers-go into feat/support-databend-module

We'll tackle that in a separate PR

hantmac · 2024-09-20T01:59:02Z

@mdelapenya It's strange the mssql CI test failed. q.q

mdelapenya

LGTM, thanks for your hard work adding this new module. Very useful!

mdelapenya · 2024-09-20T08:54:17Z

@mdelapenya It's strange the mssql CI test failed. q.q

The MSSQL failures have been resolved in #2786

* main: feat: support databend module (testcontainers#2779) chore: golangci-lint 1.61.0 (testcontainers#2787)

hantmac · 2024-09-20T08:58:06Z

@mdelapenya Thanks for your help! BTW, I add the testcontainer-rs module in this pr testcontainers/testcontainers-rs-modules-community#207, is there anybody who can help to review it?

mdelapenya · 2024-09-20T08:59:28Z

@hantmac could you please add this module to the modules catalog? https://testcontainers.com/modules/?language=go

There are instructions in this repo to elaborate the PR.

Regarding the Rust module, let me ping the community team on Slack

hantmac · 2024-09-20T09:01:29Z

@hantmac could you please add this module to the modules catalog? https://testcontainers.com/modules/?language=go

There are instructions in this repo to elaborate the PR.

Regarding the Rust module, let me ping the community team on Slack

@mdelapenya Of course, I will add it.

That's greate! Thank you!

* main: feat: support databend module (#2779) chore: golangci-lint 1.61.0 (#2787) fix(mssql): bump Docker image version (#2786) fix: handle 127 error code for podman compatibility (#2778) fix: do not override ImageBuildOptions.Labels when building from a Dockerfile (#2775) feat(mongodb): Wait for mongodb module with a replicaset to finish (#2777) fix(postgres): Apply default snapshot name if no name specified (#2783)

hantmac · 2024-09-21T06:59:46Z

There are instructions in this repo to elaborate the PR.

@mdelapenya Hi bro, I add the module to the modules catalog in this pr testcontainers/community-module-registry#75

* main: chore: use a much smaller image for testing (testcontainers#2795) fix: parallel containers clean race (testcontainers#2790) fix(registry): wait for (testcontainers#2793) fix: container timeout test (testcontainers#2792) docs: document redpanda options (testcontainers#2789) feat: support databend module (testcontainers#2779) chore: golangci-lint 1.61.0 (testcontainers#2787) fix(mssql): bump Docker image version (testcontainers#2786) fix: handle 127 error code for podman compatibility (testcontainers#2778) fix: do not override ImageBuildOptions.Labels when building from a Dockerfile (testcontainers#2775) feat(mongodb): Wait for mongodb module with a replicaset to finish (testcontainers#2777) fix(postgres): Apply default snapshot name if no name specified (testcontainers#2783) fix: resource clean up for tests and examples (testcontainers#2738) ci: add generate for mocks (testcontainers#2774) fix: docker config error handling when config file does not exist (testcontainers#2772)

* main: chore: use a much smaller image for testing (testcontainers#2795) fix: parallel containers clean race (testcontainers#2790) fix(registry): wait for (testcontainers#2793) fix: container timeout test (testcontainers#2792) docs: document redpanda options (testcontainers#2789) feat: support databend module (testcontainers#2779) chore: golangci-lint 1.61.0 (testcontainers#2787) fix(mssql): bump Docker image version (testcontainers#2786) fix: handle 127 error code for podman compatibility (testcontainers#2778) fix: do not override ImageBuildOptions.Labels when building from a Dockerfile (testcontainers#2775) feat(mongodb): Wait for mongodb module with a replicaset to finish (testcontainers#2777) fix(postgres): Apply default snapshot name if no name specified (testcontainers#2783) fix: resource clean up for tests and examples (testcontainers#2738)

* main: (29 commits) fix: template for code generation (#2800) fix: update module path (#2797) fix: container logging deadlocks (#2791) chore: use a much smaller image for testing (#2795) fix: parallel containers clean race (#2790) fix(registry): wait for (#2793) fix: container timeout test (#2792) docs: document redpanda options (#2789) feat: support databend module (#2779) chore: golangci-lint 1.61.0 (#2787) fix(mssql): bump Docker image version (#2786) fix: handle 127 error code for podman compatibility (#2778) fix: do not override ImageBuildOptions.Labels when building from a Dockerfile (#2775) feat(mongodb): Wait for mongodb module with a replicaset to finish (#2777) fix(postgres): Apply default snapshot name if no name specified (#2783) fix: resource clean up for tests and examples (#2738) ci: add generate for mocks (#2774) fix: docker config error handling when config file does not exist (#2772) docs: refine heading badges in README (#2770) feat(wait): for file (#2731) ...

feat: support databend module

0274491

hantmac requested a review from a team as a code owner September 13, 2024 03:25

mdelapenya requested changes Sep 13, 2024

View reviewed changes

chore: add project scaffolding

1a51a84

chore: more file

8473e23

hantmac marked this pull request as draft September 13, 2024 12:24

chore: run make lint

6eddbaf

mdelapenya reviewed Sep 13, 2024

View reviewed changes

modules/databend/databend.go Outdated Show resolved Hide resolved

fix: proper pinned version

af447e3

fix comment

4d3f405

hantmac marked this pull request as ready for review September 13, 2024 12:54

mdelapenya reviewed Sep 13, 2024

View reviewed changes

docs/modules/databend.md Show resolved Hide resolved

add the detail documents

efd6342

hantmac requested a review from mdelapenya September 13, 2024 15:43

mdelapenya reviewed Sep 13, 2024

View reviewed changes

modules/databend/databend.go Outdated Show resolved Hide resolved

mdelapenya self-assigned this Sep 13, 2024

mdelapenya added the enhancement New feature or request label Sep 13, 2024

stevenh requested changes Sep 13, 2024

View reviewed changes

hantmac and others added 2 commits September 14, 2024 08:55

Update modules/databend/databend.go

ff632e7

Co-authored-by: Steven Hartland <[email protected]>

fix review comment

1a09c9a

hantmac force-pushed the feat/support-databend-module branch from 497371c to 1a09c9a Compare September 14, 2024 01:33

hantmac requested review from mdelapenya and stevenh September 14, 2024 01:35

mdelapenya requested changes Sep 14, 2024

View reviewed changes

hantmac added 2 commits September 14, 2024 15:38

fix

b281577

default database

77c4c42

fix databend tests

8b729bf

stevenh requested changes Sep 17, 2024

View reviewed changes

fix golangci-lint

0db9906

hantmac and others added 4 commits September 18, 2024 00:08

Merge branch 'main' into feat/support-databend-module

2d9c1d4

Merge branch 'main' into feat/support-databend-module

6e2401c

Update modules/databend/databend.go

b78a4cf

Co-authored-by: Steven Hartland <[email protected]>

Merge branch 'main' into feat/support-databend-module

97016db

mdelapenya requested changes Sep 18, 2024

View reviewed changes

hantmac added 3 commits September 19, 2024 11:09

remove WithDatabase

74dead1

Merge branch 'feat/support-databend-module' of github.com:hantmac/tes…

3ff0b52

…tcontainers-go into feat/support-databend-module

Merge branch 'main' into feat/support-databend-module

e3e7489

hantmac and others added 4 commits September 19, 2024 11:11

fix

7f7bcce

Merge branch 'feat/support-databend-module' of github.com:hantmac/tes…

5a54a32

…tcontainers-go into feat/support-databend-module

chore: rollback pinned version

0922ab7

We'll tackle that in a separate PR

Merge branch 'main' into feat/support-databend-module

7a24c55

mdelapenya approved these changes Sep 20, 2024

View reviewed changes

mdelapenya merged commit c1bb0bb into testcontainers:main Sep 20, 2024

mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Sep 20, 2024

Merge branch 'main' into etcd-module

aa1cea5

* main: feat: support databend module (testcontainers#2779) chore: golangci-lint 1.61.0 (testcontainers#2787)

Uh oh!

Conversation

hantmac commented Sep 13, 2024

What does this PR do?

Uh oh!

netlify bot commented Sep 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for testcontainers-go ready!

Uh oh!

mdelapenya left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdelapenya commented Sep 13, 2024

Uh oh!

Uh oh!

hantmac commented Sep 13, 2024

Uh oh!

hantmac commented Sep 13, 2024

Uh oh!

Uh oh!

Uh oh!

stevenh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdelapenya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdelapenya Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevenh left a comment

Choose a reason for hiding this comment

Uh oh!

hantmac commented Sep 17, 2024

Uh oh!

mdelapenya left a comment

Choose a reason for hiding this comment

Uh oh!

hantmac commented Sep 19, 2024

Uh oh!

hantmac commented Sep 20, 2024

Uh oh!

mdelapenya left a comment

Choose a reason for hiding this comment

Uh oh!

netlify bot commented Sep 13, 2024 •

edited

Loading

mdelapenya left a comment •

edited

Loading

mdelapenya Sep 18, 2024 •

edited

Loading