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

couchbase: allow to configure bucket replicas and default to 0. #5840

Merged
merged 1 commit into from Sep 14, 2022

Conversation

daschl
Copy link
Member

@daschl daschl commented Sep 13, 2022

This change allows the user to configure the number of bucket replicas and defaults it to 0 since most of the time the user ever only needs no replicas since it is a single container.

Note: technically this is a behavioral change since before the number of replicas was 1 (if not defined) but this causes issues when testing transactions (as seen in #5835).

Fixes #5835

@daschl daschl requested a review from a team as a code owner September 13, 2022 11:25
@@ -47,7 +69,7 @@ public BucketDefinition withFlushEnabled(final boolean flushEnabled) {
/**
* Sets a custom bucket quota (100MB by default).
*
* @param quota the quota to set for the bucket.
* @param quota the quota to set for the bucket in megabytes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: The quote is in mebibytes (MiB)

@dnault
Copy link
Contributor

dnault commented Sep 14, 2022

Hey @kralphs -- is this something you'd like to add to your Couchbase module for testcontainers-node?

This change allows the user to configure the number of bucket replicas
and defaults it to 0 since most of the time the user ever only needs
no replicas since it is a single container.

Note: technically this is a behavioral change since before the number
of replicas was 1 (if not defined) but this causes issues when
testing transactions (as seen in testcontainers#5835).

Fixes testcontainers#5835
@daschl
Copy link
Member Author

daschl commented Sep 14, 2022

Minor fixup, replaced megabytes with mebibytes per review. @eddumelendez this should be good to go now then.

@kiview kiview merged commit 1f3a1f7 into testcontainers:master Sep 14, 2022
@kralphs
Copy link

kralphs commented Sep 14, 2022

Hey @kralphs -- is this something you'd like to add to your Couchbase module for testcontainers-node?

Shouldn't be a heavy lift so I'll get around to that soon. #5835 doesn't seem to apply in the Node.js SDK. We are already successfully running tests within transactions using KV ops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: There is no way to test Transactions with CouchbaseContainer
5 participants