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

Allow configuration of user settings to support couchbase community version #777

Merged
merged 4 commits into from
Sep 23, 2018
Merged

Allow configuration of user settings to support couchbase community version #777

merged 4 commits into from
Sep 23, 2018

Conversation

kstrek
Copy link
Contributor

@kstrek kstrek commented Jul 10, 2018

Hello,
Currently CouchbaseContainer does not support community version. The problem is that for each bucket there is user created with bucket_admin role, which is not supported in community.
It would be sufficient to change role to just admin, but to make sure I wouldn't break someone's code (in unlikely case when someone relies on this specific role), and to make it more flexible I extended API (without breaking changes) to allow specifying user settings for each bucket.

Regards

@rnorth
Copy link
Member

rnorth commented Jul 10, 2018

Thanks @kstrek, especially for taking care to avoid breaking changes. Basically I think this looks fine to me.

Before merging, though I think it would make sense to have a test that covers use of custom UserSettings - is that something you could add?

@kstrek
Copy link
Contributor Author

kstrek commented Jul 10, 2018

I agree test would be useful, I tried to write one before. However, it would unfortunately require creating another CouchbaseCointainer instance, besides one created as static field of AbstractCouchbaseTest in already existing test, which seems to be impossible due to port contention. I don't really see any simple way of doing this, but i'm open to suggestions 😉

@bsideup bsideup modified the milestones: next, 1.8.3 Jul 31, 2018
@klara-l
Copy link
Contributor

klara-l commented Aug 17, 2018

Hi @kstrek can you please rebase onto master? There were a lot of changes in the couchbase container. It how should be easier for you to write a test in either BaseCouchbaseContainerTest for integration tests or CouchbaseContainerTest for unit tests.

And for api stability: due to changes in the couchbase we were forced to introduce some breaking changes. So this might be a good change to clean up a bit further.

…ners-java into couchbase_support_for_community_version

# Conflicts:
#	modules/couchbase/src/main/java/org/testcontainers/couchbase/CouchbaseContainer.java
@kstrek
Copy link
Contributor Author

kstrek commented Sep 11, 2018

Here it is, sorry it took that long.

I did not introduce any new changes besides rebase and new test. Back then I thought of separating bucket and user initialization (eg. to allow creating one user for multiple buckets and so on), but as I think of it now there isn't really any use case for that as those containers should be used to test application code, not database capabilities.

P.S. Seems like travis ci pipeline failed because of some random container startup timeout. I'm not able to rerun it.

@bsideup bsideup modified the milestones: 1.9.0-rc1, next Sep 12, 2018
@rnorth rnorth modified the milestones: 1.9.0, next Sep 20, 2018
@kiview kiview merged commit 05b3c01 into testcontainers:master Sep 23, 2018
@bsideup
Copy link
Member

bsideup commented Sep 23, 2018

Thanks @kstrek! :)

@kstrek kstrek deleted the couchbase_support_for_community_version branch September 23, 2018 20:01
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.

None yet

5 participants