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] Add support for Couchbase Analytics. #4592

Merged
merged 2 commits into from Oct 21, 2021

Conversation

daschl
Copy link
Member

@daschl daschl commented Oct 19, 2021

This changeset adds support for the couchbase analytics service, but
does not enable it by default because it is a little more resource
hungry and some users may not need it out of the box.

This changeset adds support for the couchbase analytics service, but
does not enable it by default because it is a little more resource
hungry and some users may not need it out of the box.
@@ -177,6 +202,8 @@ protected void configure() {
QUERY_SSL_PORT,
SEARCH_PORT,
SEARCH_SSL_PORT,
ANALYTICS_PORT,
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering if those should be added depending on enabled services. On the other hand, there is no harm in exposing unused ports and it simplifies the code I suppose.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that’s a good idea - I‘ll do that in a follow-up PR since it’s a different optimization

Copy link
Member

@rnorth rnorth left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks @daschl

@rnorth rnorth merged commit a541886 into testcontainers:master Oct 21, 2021
@rnorth rnorth modified the milestone: next Oct 21, 2021
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

3 participants