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

fix: make this library compatible with SQL #62

Merged
merged 3 commits into from
Mar 15, 2021

Conversation

ylacaute
Copy link
Contributor

@ylacaute ylacaute commented Mar 8, 2021

Adding the cassandra extension in the classpath currently breaks any existing Liquibase config working with SQL databases. Indeed, Liquibase initializes many things by searching in the classPath, and some generators are taken from Cassandra extension even when we are working on a true SQL database.

This commit fixes this unexpected behaviour and allows to configure a single project working with a true SQL database and Cassandra, at the same time.

┆Issue is synchronized with this Jira Bug by Unito

…he pom should not break Liquibase working with SQL databases)
@ylacaute
Copy link
Contributor Author

ylacaute commented Mar 9, 2021

I understand PRs can take some times. How soon will you be able to check it ?

@KushnirykOleh
Copy link
Contributor

Hello @ylacaute. Thank you for submitting this PR. Your changes definitely makes sense, i'll try to test it by the end of this week.

@KushnirykOleh
Copy link
Contributor

Hey, @ylacaute i changed your test to be Spock one, as extension already use Spock and it's general direction where whole Liquibase test infrastructure is moving. Also made check in test a bit more obvious. If you are fine with those changes we can merge this PR

@sync-by-unito sync-by-unito bot changed the title fix: make this library compatible with SQL fix: make this library compatible with SQL Mar 12, 2021
@sync-by-unito
Copy link

sync-by-unito bot commented Mar 12, 2021

➤ Kristyl Gomes commented:

Assigning this ticket to Oleh Kushniryk who has reviewed and improved the PR.

3 similar comments
@sync-by-unito
Copy link

sync-by-unito bot commented Mar 12, 2021

➤ Kristyl Gomes commented:

Assigning this ticket to Oleh Kushniryk who has reviewed and improved the PR.

@sync-by-unito
Copy link

sync-by-unito bot commented Mar 12, 2021

➤ Kristyl Gomes commented:

Assigning this ticket to Oleh Kushniryk who has reviewed and improved the PR.

@sync-by-unito
Copy link

sync-by-unito bot commented Mar 12, 2021

➤ Kristyl Gomes commented:

Assigning this ticket to Oleh Kushniryk who has reviewed and improved the PR.

@ylacaute
Copy link
Contributor Author

ylacaute commented Mar 12, 2021

Yes of course ! If you prefer to keep spoke, do as you please ! I undertand you want to keep a single test framework.
By the way, why spoke tests are not launched when you do a mvn test ?

@ylacaute
Copy link
Contributor Author

ylacaute commented Mar 12, 2021

Also, please note that my comment on my test is not very good. This PR fixes 2 others conflicts with generators (I think, don't remember how many), maybe we should remove the comment which is incomplete and finaly not very usefull.

@kristyldatical
Copy link
Contributor

Hi @ylacaute -- the spock tests are not run via mvn test right now because we didn't finish the pieces of adding support via maven surefire and gmavenplus plugins to the Cassandra pom.xml file. This is happening in the background and hope to finish next week.

@KushnirykOleh KushnirykOleh merged commit 6581662 into liquibase:main Mar 15, 2021
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.

3 participants