Add failure analyzer for Flyway's bootstrap failure #16015
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
snicoll
changed the title
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.springframework.boot.diagnostics.analyzer; |
There was a problem hiding this comment.
This failure analyzer should belong with the rest of the Flyway support. For insance HikariDriverConfigurationFailureAnalyzer is in the jdbc package of the auto-configuration module. Can you please move this class to the auto-configuration module as well (in the flyway package)?
There was a problem hiding this comment.
Sounds good. I have moved this class under flyway package of auto-configuration module.
|
|
||
| /** | ||
| * A {@code FailureAnalyzer} that performs analysis of failures caused by a | ||
| * {@code IllegalStateException}. |
There was a problem hiding this comment.
We can't really do that. The purpose of a failure analyzer is to tracker a specific exception. This is a sign that we need to provide such exception if it doesn't exist yet to avoid magic string comparison.
There was a problem hiding this comment.
I understand. I have created a new exception as advised.
| @Override | ||
| protected FailureAnalysis analyze(Throwable rootFailure, IllegalStateException cause) { | ||
| if (cause.getMessage().startsWith(MISSING_SCRIPT_MESSAGE)) { | ||
| String location = StringUtils.substringBetween(cause.getMessage(), "[", "]"); |
There was a problem hiding this comment.
Rather than parsing the error message, please create a FlywayMigrationScriptNotFoundException or something that could contain the original locations as an argument?
There was a problem hiding this comment.
There is also a case of no location configured at all which would fit here and would require a slightly different failure analysis.
There was a problem hiding this comment.
The FlywayMigrationScriptNotFoundException is now taking location as an argument.
I have added a separate comment on this thread about failure in case of no location configured.
| private final FlywayMigrationScriptMissingFailureAnalyzer analyzer = new FlywayMigrationScriptMissingFailureAnalyzer(); | ||
|
|
||
| @Test | ||
| public void analysisForFlywayScriptMissingFailureWhenIllegalStateExceptionIsForFlyway() { |
There was a problem hiding this comment.
This should change with the case where none of the locations exist and one case where no location is provided (see FlywayAutoConfiguration).
snicoll
added
the
status: waiting-for-feedback
…ule and add new flywayMigrationScriptMissing exception Fixes gh-15674
|
Thanks for the review comments. I have made the suggested changes for the no location exists scenario. About the second scenario "where no location is provided", |
snicoll
added
type: enhancement
* pr/16015: Polish "Add failure analyzer for Flyway's bootstrap failure" Add failure analyzer for Flyway's bootstrap failure
|
@anandshastri1990 thank you very much for making your first contribution to Spring Boot. Your contribution is now merged on |
No description provided.