Allow properties migrator to fail application startup - #51855
Conversation
noojung
changed the title
spring-projects-issues
added
the
status: waiting-for-triage
noojung
changed the title
|
|
||
| private static final Log logger = LogFactory.getLog(PropertiesMigrationListener.class); | ||
|
|
||
| private static final String FAIL_ON_PROPERTY = "spring.tools.properties-migrator.fail-on"; |
There was a problem hiding this comment.
With an explicit value for the "don't fail case" (which I think is an improvement over the proposal in #40435), I think we need a different property name. fail-on: none doesn't read very well. Does it mean never fail or fail when there are no warnings or errors?
How about fail with values never, on-error, and on-warning?
There was a problem hiding this comment.
Thank you for the review.
That's a good point. I also think fail-on: none could be confusing, and your suggested names make the behavior clearer. I've updated the property to fail with never, on-error, and on-warning as suggested.
Could you please take another look?
|
|
||
| private static final Log logger = LogFactory.getLog(PropertiesMigrationListener.class); | ||
|
|
||
| private static final String FAIL_ON_PROPERTY = "spring.tools.properties-migrator.fail-on"; |
There was a problem hiding this comment.
We should also think about the prefix as we generally try to avoid - characters in prefixes. Rather than spring.tools.properties-migrator, perhaps spring.tools.properties.migrator.
There's also an outside change that we may want other properties that control property handling (the escape hatch mentioned in #27582 (comment) for example) and those should perhaps be grouped together. spring.properties.migrator might be better in that context and then we could have other spring.properties.* properties for related settings.
No need to change anything at this point @noojung, but we'll have to consider this before merging.
philwebb
added
type: enhancement
Add a new configuration property to prevent the application from starting when configuration keys that need to be migrated are found. With `on-error`, the application fails when keys that are no longer supported are found. With `on-warning`, it also fails when keys that have been renamed are found. The default, `never`, keeps the current behavior of only logging the report. See spring-projectsgh-51855 Signed-off-by: Hyunwoo Jung <hyunwoojung@kakao.com>
|
Thanks very much @noojung, I've changed the property to |
4 participants
This PR adds a
spring.tools.properties-migrator.fail-onproperty that allows the properties migrator to prevent the application from starting when configuration keys that need to be migrated are found.As suggested in #40435, a single property is used, with the following values:
none(default): preserve the existing migration and reporting behavior.error: fail when keys that are no longer supported are found.warning: fail when keys that have been renamed or are no longer supported are found.The migration report is still logged regardless of this setting.
See #40435