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 Liquibase config option allowDuplicatedChangesetIdentifiers #40493

Closed
joachimglink opened this issue May 7, 2024 · 10 comments · Fixed by #40603
Closed

add Liquibase config option allowDuplicatedChangesetIdentifiers #40493

joachimglink opened this issue May 7, 2024 · 10 comments · Fixed by #40603

Comments

@joachimglink
Copy link

Description

With Liquibase 4.25.1 a new config option was introduced: liquibase.allowDuplicatedChangesetIdentifiers (see https://docs.liquibase.com/parameters/allow-duplicated-changeset-identifiers.html)

This option allows that a change set is found multiple times without failing the Liquibase migration; it effectively just ignores duplicate changesets.
As of now, there is no Quarkus specific config property for this, so one needs to define it as environment variable or system property; neither is very nice to handle.

It would be great if Quarkus adds this config option to the current list of options.
Or: would it be possible to create a more generic config way so that we could define every Liquibase config property with one single Quarkus property using a map?

Thanks.

Implementation ideas

No response

@quarkus-bot
Copy link

quarkus-bot bot commented May 7, 2024

/cc @andrejpetras (liquibase), @geoand (liquibase), @gsmet (liquibase), @radcortez (config)

@geoand
Copy link
Contributor

geoand commented May 8, 2024

Does something like quarkus.liquibase.change-log-parameters.allowDuplicatedChangesetIdentifiers=true work?

@joachimglink
Copy link
Author

No, this is not working. I already tried this.
Seems that these parameters can only be used in the changelog definitions itself but they are not set as Liquibase config options.

@geoand
Copy link
Contributor

geoand commented May 8, 2024

If that's the case, then unfortunately I don't think we can do much, because from what I saw from the code, Liquibase has its own way of setting configuration that we can't hook into

@gsmet
Copy link
Member

gsmet commented May 13, 2024

I see we can set it as a system property: JAVA_OPTS=-Dliquibase.allowDuplicatedChangesetIdentifiers=<true|false>, maybe producing a SystemPropertyBuildItem would work?

@geoand
Copy link
Contributor

geoand commented May 13, 2024

Yeah, we could do that early enough to make it work, but it's very unfortunate to have to resort to that kind of hack

@gsmet
Copy link
Member

gsmet commented May 13, 2024

Yeah, I completely agree it's not pretty.

Not sure how to structure it but these options would basically need to be added in LiquibaseBuildTimeConfig. Then you would add a method in LiquibaseProcessor consuming the config (you already have examples of that in this class) and producing the appropriate SystemPropertyBuildItem.

It should be simple enough if you're willing to give it a try.

Be aware though that these will be fixed at build time. Overriding the property directly at runtime might work but you would have to test it.

@geoand
Copy link
Contributor

geoand commented May 13, 2024

I am pretty sure we can make it work for runtime properties as well.

Actually I have an idea on how to improve on your proposal.
I'll try and find some time for it later on this week

geoand added a commit to geoand/quarkus that referenced this issue May 13, 2024
…fiers

Because Liquibase has its own configuration system that
does not allow us to integrate with it, we need to configure it
using setting and resetting system properties.

Fixes: quarkusio#40493
@geoand
Copy link
Contributor

geoand commented May 13, 2024

#40603 is what I have in mind.

I tested it manually and it seems to work, but if someone knows how to test this in a test, please feel free to update the PR.

@joachimglink
Copy link
Author

That sounds great.
The System property approach was what I used as a workaround; not in such a nice way though.
Thanks for getting this resolved so quickly.

geoand added a commit to geoand/quarkus that referenced this issue May 14, 2024
geoand added a commit that referenced this issue May 14, 2024
Add configuration option for liquibase.allowDuplicatedChangesetIdentifiers
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants