-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support init script at JdbcDatabaseContainer level #575
Support init script at JdbcDatabaseContainer level #575
Conversation
* @param databaseDelegate database delegate for script execution | ||
* @param initScriptPath the resource to load the init script from | ||
*/ | ||
public static void runInitScript(DatabaseDelegate databaseDelegate, String initScriptPath) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method can also be used in ContainerDatabaseDriver.runInitScriptIfRequired(...) to replace almost all of the method's code, but I didn't do it to have diff smaller.
Not really your fault, but I just realised the intendation is this file is totally messed up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please also add another test to SimplePostgreSQLTest
, just to be sure? 🙂
modules/database-commons/src/main/java/org/testcontainers/ext/ScriptUtils.java
Outdated
Show resolved
Hide resolved
@kiview Added new test to SimplePostgreSQLTest and refactored another - to have tests completely isolated and not have several postgresql containers (if in second test would use postresql.withInitScript as local variable). |
We're shortly going to be merging #574, which changes our build system to Gradle. This is in part intended to make contributions of modules easier (per #564), but unfortunately means that for a short while your PR is going to show merge conflicts with the master branch. I just want to let you know we don't want to create new work for you: we'll take care of the merge conflicts shortly. Please don't worry - we're grateful for your PR and want to help integrate it soon. Thank you. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this. |
Sorry this has waited so long - I'm not sure why we haven't just merged it yet. I'll assign to myself and will aim to fix the merge conflict and push this through soon. |
I just resolved the conflict and merged with master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently init script can be applied only through ContainerDatabaseDriver which doesn't support private Docker registries. This limitation also denies generic container approach of usage.
More information about the problem is in the corresponding issue: #532
So I've implemented an init script support for all Jdbc containers.
Resolve #532