Skip to content

[couchbase] Only expose ports for enabled services - #4595

Merged
eddumelendez merged 2 commits into
testcontainers:masterfrom
daschl:ports-exposed
Aug 23, 2022
Merged

eddumelendez merged 2 commits into
testcontainers:masterfrom
daschl:ports-exposed

Conversation

@daschl

@daschl daschl commented Oct 20, 2021

Copy link
Copy Markdown
Member

Based on your previous PR comment @kiview, this one is best merged after the analytics one since I built it on top.

@daschl daschl changed the title Ports exposed [couchbase] Only expose ports for enabled services Oct 20, 2021
@rnorth

rnorth commented Oct 22, 2021

Copy link
Copy Markdown
Member

@daschl we merged #4592 - do you want to resolve conflicts in this one?

@kiview kiview left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM after resolving the conflicts.

This changeset is a minor cleanup, but it makes sense to
only actually expose the ports for the services which are
enabled and not all of them all the time.
@daschl

daschl commented Oct 22, 2021

Copy link
Copy Markdown
Member Author

@rnorth @kiview done!

@daschl

daschl commented Oct 22, 2021

Copy link
Copy Markdown
Member Author

The failing test is unrelated I think.

@kiview

kiview commented Oct 22, 2021

Copy link
Copy Markdown
Member

Yeah, that was a weird one. I re-triggered the build, just to be sure.

@eddumelendez
eddumelendez requested a review from a team August 23, 2022 20:53
@eddumelendez eddumelendez added this to the next milestone Aug 23, 2022
@eddumelendez

Copy link
Copy Markdown
Member

Eventing ports have been added and conflicts were resolved.

@eddumelendez

Copy link
Copy Markdown
Member

Thanks for the PR @daschl ! and sorry about the long wait but it is merged now :)

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.

4 participants