Skip to content

Add failure analyzer for missing Liquibase changelog #22320

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

Conversation

fernandezseb
Copy link
Contributor

This PR closes #22159.

I created a FailureAnalyzer that analyzes exceptions of type liquibase.exception.ChangeLogParseException in case they are triggered by a FileNotFoundException and shows a message accordingly.

I don't consider this PR final because I do have some more questions and remarks:

  • In order to satisfy the checkStyle I had to add a @since Javadoc tag, I didn't know what to put there so I put 2.4.0 as a guess.
  • I think that the FailureAnalysis would be even better if it would tell the user at which path it is looking for the changelog, however I couldn't figure out the best way to get a hold of this path in the LiquibaseChangelogMissingFailureAnalyzer class.
  • If needed I will also add a test class to test the functionality of the new FailureAnalyzer added in this PR.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 13, 2020
@wilkinsona
Copy link
Member

wilkinsona commented Jul 14, 2020

Thanks very much, @fernandezseb. This is looking good.

In order to satisfy the checkStyle I had to add a @since Javadoc tag, I didn't know what to put there so I put 2.4.0 as a guess.

Thanks. 2.4.0 sounds good to me.

I think that the FailureAnalysis would be even better if it would tell the user at which path it is looking for the changelog, however I couldn't figure out the best way to get a hold of this path in the LiquibaseChangelogMissingFailureAnalyzer class.

I think we can probably extract it from the exception's message. The message should be Error parsing <<location>> so it should be possible to strip off Error parsing and be left with the location.

If needed I will also add a test class to test the functionality of the new FailureAnalyzer added in this PR.

Yes, please. Some tests would be much appreciated. It would be good to verify that extracting the path from the message works as expected. That should allow us to catch any changes that Liquibase make in the future that would break the extraction.

@wilkinsona wilkinsona added this to the 2.3.x milestone Jul 14, 2020
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 14, 2020
@wilkinsona wilkinsona marked this pull request as draft July 14, 2020 14:16
@wilkinsona wilkinsona modified the milestones: 2.3.x, 2.4.x Jul 14, 2020
@fernandezseb fernandezseb force-pushed the liquibase-missing-changelog branch 3 times, most recently from 4821b3a to a33709d Compare July 15, 2020 19:48
@fernandezseb fernandezseb force-pushed the liquibase-missing-changelog branch from a33709d to 3c4fe96 Compare July 15, 2020 20:47
@fernandezseb
Copy link
Contributor Author

fernandezseb commented Jul 15, 2020

Thanks for the feedback @wilkinsona!

In the meantime I made some changes:

  • Added the path where liquibase is looking for the changelog file to the description.
  • Added a test case that attempts to create SpringLiquibase in order to trigger the error from the Liquibase.
  • Changed the access modifier from the FailureAnalyzer to package-private from public, thus not requiring the @since tag anymore in the Javadoc, so I also removed this.

@fernandezseb fernandezseb marked this pull request as ready for review July 15, 2020 21:19
@wilkinsona wilkinsona self-assigned this Jul 17, 2020
@wilkinsona wilkinsona modified the milestones: 2.4.x, 2.4.0-M2 Jul 17, 2020
@wilkinsona
Copy link
Member

Thanks very much, @fernandezseb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide better diagnostics when Liquibase fails due to a missing changelog
3 participants