Skip to content

add multi initScripts for JDBC containers #1997

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.

Sign up for GitHub

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

Jump to bottom
Closed
png-tech wants to merge 1 commit into from
Closed

add multi initScripts for JDBC containers #1997

png-tech wants to merge 1 commit into from

Conversation

@png-tech
Copy link

@png-tech png-tech commented Oct 23, 2019

I need to use many initScripts for JDBC containers.
first script - sql structure
second script - test-data

usage example:

try (JdbcDatabaseContainer mysql = new MySQLContainer()
                .withDatabaseName("test-database")
                .withUsername(DB_USR)
                .withPassword(DB_PASSWD)
                .withInitScript("structure.sql")  // sql-structure
                .withInitScript("data1.sql") // test data with insert sql-script
        ) {
            mysql.start();
            
            // ....
}

@png-tech png-tech force-pushed the add-jdbc-many-scripts branch from 00eafe1 to 1798cb2 Compare October 23, 2019 06:20
@rnorth rnorth self-assigned this Nov 1, 2019
rnorth
rnorth requested changes Jan 24, 2020
View reviewed changes
Copy link
Member

@rnorth rnorth left a comment

Choose a reason for hiding this comment

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

Sorry this has been stuck for so long. As you can see we're rather swamped 😔

I think that to be as similar to other APIs as possible, the method ought to be of the form:

public SELF withInitScript(String... initScriptPath) {

and possibly also:

public SELF withInitScript(Iterable<String> initScriptPath) {

We should also have a test to guard against future regressions.

@stale
Copy link

stale bot commented Apr 25, 2020

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.

@stale stale bot added the stale label Apr 25, 2020
@png-tech png-tech force-pushed the add-jdbc-many-scripts branch from 1798cb2 to 446f3c7 Compare April 27, 2020 08:42
@stale stale bot removed the stale label Apr 27, 2020
@png-tech
Copy link
Author

png-tech commented Apr 27, 2020 

public SELF withInitScript(String... initScriptPath) {

ok

@png-tech png-tech force-pushed the add-jdbc-many-scripts branch from 446f3c7 to 553e5c8 Compare April 27, 2020 10:15
@rnorth
Copy link
Member

rnorth commented May 1, 2020

Sorry, we also have #2578 - can we just converge on that PR? I don't think it's perfect but it's most of the way towards solving binary compatibility and has the advantage of having a test.

(FWIW re binary compatibility the problem is that changing the parameter of a method from String[] to String will break other libraries that have been compiled against earlier versions of this one).

@rnorth rnorth closed this May 1, 2020
savinov added a commit to savinov/testcontainers-java that referenced this pull request Oct 18, 2023
@savinov
Support multiple init scripts in JdbcDatabaseContainer
69bdb96 
Inspired by PRs: testcontainers#2578 and testcontainers#1997
Closes testcontainers#2232
@savinov savinov mentioned this pull request Oct 18, 2023
Merged
savinov added a commit to savinov/testcontainers-java that referenced this pull request Oct 19, 2023
@savinov
Support multiple init scripts in JdbcDatabaseContainer
1059fb9 
Inspired by PRs: testcontainers#2578 and testcontainers#1997
Closes testcontainers#2232
savinov added a commit to savinov/testcontainers-java that referenced this pull request Jan 22, 2024
@savinov
Support multiple init scripts in JdbcDatabaseContainer
a30f8ec 
Inspired by PRs: testcontainers#2578 and testcontainers#1997
Closes testcontainers#2232
@sdavids sdavids mentioned this pull request May 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rnorth rnorth rnorth requested changes

@bsideup bsideup Awaiting requested review from bsideup

@kiview kiview Awaiting requested review from kiview

Assignees

@rnorth rnorth

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@png-tech @rnorth