add couchbase module #876
add couchbase module #876
Conversation
…ervicesEnabled with gjson
…ure external port
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
I added a few minor comments fixing what #877 caused, but it's amazing you used the code generation tool for the module 👏
I'm going to start the review today, and want to double check with the rest of the team for commonalities with other implementations (e.g. Java). Did you use it by any case as the reference implementation for Go?
Other than that, thanks for the initial PR, I will be pinging you with questions though it.
The first ping is regarding the docs: as the module is a more elaborated than an example one, I'd recommend extending the docs, like in the LocalStack module (https://golang.testcontainers.org/modules/localstack/) where we expand on the API; or in the upcoming Apache Pulsar module (still in PR stage): https://deploy-preview-872--testcontainers-go.netlify.app/modules/pulsar/
mdelapenya
added
the
feature
# Conflicts: # wait/http.go
# Conflicts: # .github/dependabot.yml
|
Hi @mdelapenya, I added requested tests and Couchbase documentation. |
alihanyalcin
force-pushed
the
main
branch
2 times, most recently
from
8b73074
to
409570f
Compare
There was a problem hiding this comment.
I left a few minor comments regarding methods belonging to the container struct.
Other than that, once discussed, this PR LGTM, as it matches the Java implementation. Possibly @eddumelendez can verify the Java part.
Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>
modules/couchbase/couchbase.go
Outdated
| storageMode := "forestdb" | ||
| if c.config.isEnterprise { | ||
| storageMode = "memory_optimized" | ||
| } |
There was a problem hiding this comment.
I think it would be great to consider this in the new Go implementation
There was a problem hiding this comment.
@alihanyalcin do you think we can contribute that part? As this module will fall under the next release lifecycle, we could merge this PR as is, and you could do what @eddumelendez suggests in a follow-up. Wdyt?
There was a problem hiding this comment.
Hi, I have contributed the requested section. BTW, the enterprise edition of the Couchbase Server supports both the Plasma and Memory Optimized storage engines, while the community edition only supports the ForestDB storage engine.
https://docs.couchbase.com/server/current/rest-api/post-settings-indexes.html
There was a problem hiding this comment.
Thanks for getting it so fast! We cannot be experts in all the supported technologies so we are more than thankful for seeing this kind of improvements 👏👏👏
|
Kudos, SonarCloud Quality Gate passed!
|
There was a problem hiding this comment.
It took a while, but I think this PR LGTM
Thanks @alihanyalcin for your support and your time during the review. 🙇
|
@mdelapenya @eddumelendez A new vulnerability was found in the crypto dependency, which is used by testcontainers! I strongly recommend bumping that dependency, as SonarQube and other tools do not let pipelines through! |
|
@lmitelman could you open a separate report for this? We'll label with security to address it asap Thanks for reporting it! |
What does this PR do?
This module initialize couchbase cluster and create buckets for testing purposes. Similar to testcontainers-java couchbase module
Why is it important?
In java projects, it is really easy to write integration tests for couchbase. So, this module has nearly all functionality supported in testcontainers-java couchbase module.
Related issues