Skip to content

[couchbase] Only expose ports for enabled services#4595

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

[couchbase] Only expose ports for enabled services#4595
eddumelendez merged 2 commits intotestcontainers:masterfrom
daschl:ports-exposed

Conversation

@daschl
Copy link
Copy Markdown
Member

@daschl daschl commented Oct 20, 2021

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
Copy link
Copy Markdown
Member

rnorth commented Oct 22, 2021

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

Copy link
Copy Markdown
Member

@kiview kiview left a comment

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
Copy link
Copy Markdown
Member Author

daschl commented Oct 22, 2021

@rnorth @kiview done!

@daschl
Copy link
Copy Markdown
Member Author

daschl commented Oct 22, 2021

The failing test is unrelated I think.

@kiview
Copy link
Copy Markdown
Member

kiview commented Oct 22, 2021

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