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

Implementation of MariaDB container #215

Merged
merged 1 commit into from
Sep 24, 2016
Merged

Conversation

bitbrain
Copy link
Contributor

This PR contains a slightly modified version of the MySQLContainer implementation due to the nature of MariaDB, which has its origins from MySQL.

@rnorth
Copy link
Member

rnorth commented Sep 11, 2016

@MyRealityCoding good idea - thanks. Will have a play and look over - but essentially would I be right in thinking this is just based on the MySQL module?

@bitbrain
Copy link
Contributor Author

@rnorth Due to the historical nature of MariaDB this would make a lot of sense.

@rnorth
Copy link
Member

rnorth commented Sep 13, 2016

@MyRealityCoding absolutely, that's exactly right :)

I'm more looking to know what the differences are between this and the MySQL class, so I know where to look when reviewing!

@bitbrain
Copy link
Contributor Author

bitbrain commented Sep 13, 2016

The main difference is the naming (instead of using mysql, mariadb is used). Also for the tests itself different specific version numbers are used, see here. And not to forget about the test dependency for the mariadb driver.

My aim was to have an independent module on its own to be as free as possible. The problem is, since it is copied from the mysql module, if there was an issue in one module we would have to fix it for both, which makes it very fragile. What would you recommend? We could also provide the mariadb container inside the mysql module (as a "sub" category of mysql) and parameterize the JUnit tests and apply them for both containers. Another solution could be to inherit from the MySQL module. There would be the problem how to use the Junit tests from MySQL without copy-pasting them.

@rnorth
Copy link
Member

rnorth commented Sep 14, 2016

Miguel,

Thanks - that sounds fine to me.

Let me deal with the duplicated code issue after merging. Really, I think
that the majority of the duplication is in the tests. There's a lot of
boilerplate shared even in between completely different DB modules. I think
I might extract out the test code to another part of the codebase- that way
I think we might see a dramatic reduction in the volume of code in each
specific module.

Thanks for this!

Richard
On Tue, 13 Sep 2016 at 14:30, ☕️ Miguel notifications@github.com wrote:

The main difference is the naming
https://github.com/testcontainers/testcontainers-java/pull/215/files#diff-c802ae3c6e4219c17538c5366c52c3fdR10
(instead of using mysql, mariadb is used). Also for the tests itself
different specific version numbers are used, see here
https://github.com/testcontainers/testcontainers-java/pull/215/files#diff-45eaf3e3a9854bdacf04daa4d1c466d6R31.
And not to forget about the test dependency
https://github.com/testcontainers/testcontainers-java/pull/215/files#diff-918c7b5ac5395dc05f0e392e8bc7712aR28
for the mariadb driver.

My aim was to have an independent module on its own to be as free as
possible. The problem is, since it is copied from the mysql module, if
there was an issue in one module we would have to fix it for both,
which makes it very fragile. What would you recommend?


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#215 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIET5S6W9DMl11ignLM7Y3eLxcCibCWks5qpqV9gaJpZM4JuyQZ
.

Richard

@rnorth rnorth added this to the 1.1.6 milestone Sep 24, 2016
@rnorth rnorth merged commit 30183eb into testcontainers:master Sep 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants