Allow to configure StartMojo's wait and maxAttempts attributes from the command-line #26422
Conversation
|
@marcus-nl Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@marcus-nl Thank you for signing the Contributor License Agreement! |
spring-projects-issues
added
the
status: waiting-for-triage
There was a problem hiding this comment.
Thanks for the PR. I don't think that all properties of a goal should be exposed, in particular when a certain consistency is requested as it is the case here.
It is useful to increase the timeout on an CI environment, where it can take a longer time to start than usual.
No doubt that exposing that via a property is useful but you can achieve that too by configuring the plugin with a property of your own and set that property in your CI environment.
I am, in particular, not keen to expose separate properties for thing that must have the same value. Thoughts?
|
|
||
| /** | ||
| * The port to use to expose the platform MBeanServer if the application is forked. | ||
| */ | ||
| @Parameter | ||
| private int jmxPort = 9001; | ||
| @Parameter(property = "spring-boot.start.jmxPort", defaultValue = "9001") |
There was a problem hiding this comment.
The JMX port that is used to start and stop the app can't be different so exposing two properties for this doesn't feel right to me.
| @@ -61,29 +61,29 @@ | |||
| * The JMX name of the automatically deployed MBean managing the lifecycle of the | |||
| * spring application. | |||
| */ | |||
| @Parameter | |||
| private String jmxName = SpringApplicationAdminClient.DEFAULT_OBJECT_NAME; | |||
| @Parameter(property = "spring-boot.start.jmxName", defaultValue = SpringApplicationAdminClient.DEFAULT_OBJECT_NAME) | |||
There was a problem hiding this comment.
the JMX name that is used to start and stop the app can't be different so exposing two properties for this doesn't feel right to me.
snicoll
added
the
status: waiting-for-feedback
|
Hi @snicoll, Thanks for the feedback.
True, but doesn't that apply to every configuration option? I think the timeout settings are particularly good candidates to give their own properties because their values may quite realistically differ in different circumstances. Not having to change the pom.xml (even to add your own property) makes it a lot more convenient.
You are right. Alternative combined names could be something like "spring-boot.management.jmxName" (/Port) or "spring-boot.jmx.name" (port)? Or just leave them propertly-less, since these seem less likely to need customization. I would suggest keeping the default values in the annotation though, so they show up in the generated documentation. |
spring-projects-issues
added
status: feedback-provided
|
Thanks for the quick follow-up.
Agreed. The core of my review is that we shouldn't expose all properties. I agree those are good candidates.
We don't really have any other use cases like this so I'd rather not expose it at all if we can. The default values is good feedback and should be done regardless. Would you have the time to update your PR in that direction if you agree? |
…es. Keep default values for Start/StopMojo's jmxName and jmsPort in @parameter.
|
Done |
snicoll
added
type: enhancement
snicoll
changed the title
wait and maxAttempts attributes from the command-line
philwebb
changed the title
wait and maxAttempts attributes from the command-line
|
@marcus-nl thank you for making your first contribution to Spring Boot. |
This allows overriding the settings for the Maven StartMojo (
spring-boot:start) from the command line, most notablywaitandmaxAttempts, but also the other missing ones and for StopMojo as well. It is useful to increase the timeout on an CI environment, where it can take a longer time to start than usual. I've also moved the default values to the@Parameterannotation, so that these show up in the documentation.