Skip to content

Ensure data.sql / schema.sql support is disabled automatically when using Flyway/Liquibase #22741

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

Open
philwebb opened this issue Aug 5, 2020 · 12 comments
Labels
status: blocked An issue that's blocked on an external project change status: pending-design-work Needs design work before any code can be developed theme: datasource Issues relating to data sources type: enhancement A general enhancement
Milestone

Comments

@philwebb
Copy link
Member

philwebb commented Aug 5, 2020

See #20920 for background.

@ThomasVitale
Copy link

If this issue is open to community contribution, I'm available to work on it.

@wilkinsona
Copy link
Member

@ThomasVitale Thanks very much for the offer. That'd be great, yes please. Let us know if you have any questions.

@ThomasVitale
Copy link

@wilkinsona thank you and sorry for the late answer. I looked into the code and it seems that the DataSourceInitializer class is the one responsible for handling the schema.sql and data.sql files. Right now, there's a condition about whether to run the scripts that is based on the value of spring.datasource.initialization-mode and the data source type (embedded or not). So, if I use H2 and Flyway (and I don't explicitly set the initialization mode to false), then the scripts are run.

I thought about two possible solutions that I'd like to validate with you.
A) Extending the condition in DataSourceInitializer to include a check for Flyway/Liquibase.
B) Preventing the loading of DataSourceInitializationConfiguration if either Flyway or Liquibase is present, so that we don't register the DataSourceInitializerPostProcessor bean at all.

@wilkinsona
Copy link
Member

@ThomasVitale Thanks for taking a look.

I think, ideally, DataSourceInitializationConfiguration would back off when Flyway or Liquibase is going to run. Unfortunately, after a brief look at the code, I'm not sure of the best way to make that happen. The auto-configuration for Flyway and Liquibase runs after the DataSource auto-configuration so we can't simply look for a Flyway or Liquibase bean in the context when deciding whether or not to auto-configure data source initialization.

@bclozel bclozel modified the milestones: 2.4.x, 3.x Sep 28, 2020
@bclozel bclozel added the status: pending-design-work Needs design work before any code can be developed label Sep 28, 2020
@ThomasVitale
Copy link

Good point! Both FlywayAutoConfiguration and LiquibaseAutoConfiguration are loaded after DataSourceAutoConfiguration, so we can't check the beans for building the conditional. What about the properties? The autoconfiguration for Flyway and Liquibase is only loaded if spring.flyway.enabled=true and spring.liquibase.enabled=true, respectively. What if we checked for the presence of Flyway/Liquibase in the classpath and whether the corresponding property is true? Though, it would probably not be the cleanest solution.

@wilkinsona
Copy link
Member

What if we checked for the presence of Flyway/Liquibase in the classpath and whether the corresponding property is true? Though, it would probably not be the cleanest solution.

I agree that may not be the cleanest. I think we'd be in danger of duplicating the Flyway and Liquibase auto-configuration conditions in the DataSource initialization auto-configuration.

The core team discussed this one yesterday and decided that it's something that should be tackled as part of a broader revamp of DataSource initialization that we'd like to do in 3.0. Thanks anyway for taking a look.

@wilkinsona wilkinsona added the theme: datasource Issues relating to data sources label Jan 27, 2021
@lbruun
Copy link

lbruun commented Jan 28, 2021

I think there's a use-case which is being missed here. Both Flyway and Liquibase add some metadata tables for their own use into the database. If you want to control where these tables are located then there's a chicken-and-egg problem.

So yes, users should not mix database population methods and it is good that the documentation now makes that clear. However, for the reason mentioned above user may have a legitimate need for executing some SQL code before Flyway or Liquibase kicks in.

I think this can easily be added as a feature on both Flyway and Liquibase autoconfiguration. For example I'm imagining that the Liquibase autoconfiguration would be looking for files pre-liquibase.sql (or pre-liquibase-{platform}.sql) and then execute them if they exist, very similar as to what the DataSource initialization already does today.

For example, my pre-liquibase-postgresql.sql file may look like this:

-- Create application schema
CREATE SCHEMA IF NOT EXISTS medusa;

and I would then set property:

spring.liquibase.default-schema=medusa

If I wanted to separate application tables from Liquibase meta-tables (wouldn't recommend it), I would then have my pre-liquibase-postgresql.sql file look like this:
:

-- Create application schema
CREATE SCHEMA IF NOT EXISTS medusa;
CREATE SCHEMA IF NOT EXISTS medusa_meta;

and properties set as:

spring.liquibase.default-schema=medusa
spring.liquibase.liquibase-schema=medusa_meta

In recap: There's a legitimate need to be able to execute some SQL code before Flyway or Liquibase. Such feature would be trivial to implement but I advocate it should part of Flyway / Liquibase autoconfiguration rather than trying to squeeze it into the general DataSource initialization. If such "pre" feature existed on Flyway / Liquibase autoconfiguration then it would be safe for Spring Boot to force spring.datasource.initialization-mode=never if Flyway / Liquibase is being used .. and the discussion would be simpler.

Having this "pre" feature on Flyway / Liquibase autoconfiguration would also enable some use-cases where the same database (catalog) is being used for different test environments... which I would say is quite common. The only way to separate such environments would be by using different schemas but then that necessitates the ability to ensure the schema exist prior to executing Flyway / Liquibase.

@wilkinsona
Copy link
Member

Assuming I have understood your use case correctly, it is already supported by Flyway: https://flywaydb.org/documentation/concepts/migrations.html#schema-creation. I think that within the database migration tool itself is the right place for this sort of functionality as it's applicable to anyone using Flyway not just those using Flyway with Spring Boot.

@lbruun
Copy link

lbruun commented Jan 28, 2021

@wilkinsona. Yes, I think you've understood correctly. Unfortunately I'm using using Liquibase, not Flyway, and wasn't aware of this feature in Flyway. There's no equivalent in Liquibase. Flyway has the "advantage" that it doesn't attempt to be database engine agnostic. Liquibase does indeed attempt to be database engine agnostic and such a feature would open a myriad of questions for them, AFAIU. I can see it has been discussed on several occasions in Liquibase Forum (Ref1, Ref2). It seems like Nathan, the original developer of Liquibase, would like to keep it "clean" and is therefore hesitant.

I'll ponder a bit if I agree with you or not that it doesn't belong in Spring Boot. :-) I'll go back to thinking.

@ThomasVitale
Copy link

In Flyway, you can also define callbacks (defined in SQL or Java) that let you hook into the Flyway lifecyle and execute specific procedures (for example, before the baseline migration is run or before automatically creating non-existent schemas). https://flywaydb.org/documentation/concepts/callbacks

@itstannus
Copy link

@wilkinsona. Yes, I think you've understood correctly. Unfortunately I'm using using Liquibase, not Flyway, and wasn't aware of this feature in Flyway. There's no equivalent in Liquibase. Flyway has the "advantage" that it doesn't attempt to be database engine agnostic. Liquibase does indeed attempt to be database engine agnostic and such a feature would open a myriad of questions for them, AFAIU. I can see it has been discussed on several occasions in Liquibase Forum (Ref1, Ref2). It seems like Nathan, the original developer of Liquibase, would like to keep it "clean" and is therefore hesitant.

I'll ponder a bit if I agree with you or not that it doesn't belong in Spring Boot. :-) I'll go back to thinking.

@ibrunn I am also working with Liquibase , did you find the solution for this problem?

@lbruun
Copy link

lbruun commented Aug 5, 2021

@itstannus : Yeah, my solution is the Pre-Liquibase Spring Boot module. But I think you already found it. :-)

In a few words it is a Spring Boot autoconfig which essentially fires SQL scripts and guaranteed to execute before Spring Boot invokes Liquibase itself.

@scottfrederick scottfrederick added the status: blocked An issue that's blocked on an external project change label Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: blocked An issue that's blocked on an external project change status: pending-design-work Needs design work before any code can be developed theme: datasource Issues relating to data sources type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

7 participants