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

Add sharding support to MongoDBContainer #6727

Merged
merged 11 commits into from
Mar 14, 2023

Conversation

evanchooly
Copy link
Contributor

@evanchooly evanchooly commented Mar 3, 2023

Adds sharding support but in its current form may not quite be The Testcontainers Way just yet. Happy to work with whomever to rearrange it to its correct form.

@evanchooly evanchooly requested a review from a team as a code owner March 3, 2023 22:07
@eddumelendez eddumelendez added this to the next milestone Mar 3, 2023
Copy link
Member

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @evanchooly ! I've left some comments. I already have those changes locally but can not push them to your branch (probably because is the main branch 🤔 )

@evanchooly
Copy link
Contributor Author

Thanks for your contribution @evanchooly ! I've left some comments. I already have those changes locally but can not push them to your branch (probably because is the main branch thinking )

yeah, i noticed that after i pushed. a brainfart. should've done it on a branch. maybe with these edits, i'll move it off to a branch and recreate. but maybe it's not worth the effort.

@evanchooly
Copy link
Contributor Author

i think i have all the requested changes made.

Copy link
Member

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

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

Check fails because of an unused import. Also, please run ./gradlew :mongodb:check --no-daemon

@evanchooly
Copy link
Contributor Author

I wasn't clear from your comments earlier if you wanted mongo:6 to be the default or mongo:4.0.4. I just changed it back, though.

Copy link
Member

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

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

@evanchooly thanks! Just last comments before to merge it. Sorry about this, I'd applied them but do not have access.

apply PR feedback
@eddumelendez eddumelendez merged commit 34aa967 into testcontainers:main Mar 14, 2023
@eddumelendez
Copy link
Member

Thank you so much, @evanchooly ! For your contribution and patience :) This is now merge in main branch and it will be part of the next release.

@evanchooly
Copy link
Contributor Author

yay! next time i'll use a branch. ;)

@volomas
Copy link

volomas commented Jun 8, 2023

Hello @evanchooly
I've run into issue when using testcontainers with this change.
GenericContainer has public "withCommand" method which client can use like so:
var mongoDBContainer = new MongoDBContainer(dockerImageName).withCommand("--replSet", "rs0"); mongoDBContainer.start();
But the thing is that "configure" calls "withCommand", basically erasing everything client has provider.
I my case, I wanted to provide custom replicaSet name, but on this version it's always "docker-rs"

Potential fix: #7164

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