Skip to content

GH-6183: Fix relaxed properties for SI JMX config #6184

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

Closed
wants to merge 1 commit into from

Conversation

artembilan
Copy link
Member

@artembilan artembilan commented Jun 17, 2016

Fixes GH-6183 (#6183)

Since annotation based config like

@EnableIntegrationMBeanExport(defaultDomain = "${spring.jmx.default-domain:}")

is far away of RelaxedPropertyResolver in Spring Boot,
rework the IntegrationJmxConfiguration to configure required beans via raw JavaConfig

Cherry-pick to 1.4

Fixes spring-projectsGH-6183 (spring-projects#6183)

Since annotation based config like
```
@EnableIntegrationMBeanExport(defaultDomain = "${spring.jmx.default-domain:}")
```
is far away of `RelaxedPropertyResolver` in Spring Boot,
rework the `IntegrationJmxConfiguration` to configure required beans via raw JavaConfig

**Cherry-pick to 1.4**
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 17, 2016
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 18, 2016
@snicoll snicoll added this to the 1.3.6 milestone Jun 18, 2016
@snicoll
Copy link
Member

snicoll commented Jun 20, 2016

Looking at it, the original IntegrationMBeanExportConfiguration seems to do a bit more than what we have now. Either way, it's copy-pasting the code behind the annotation so I am worried about the maintenance burden of that code.

Maybe we should revisit this once SI has a public api to build the IntegrationMBeanExporter ? Thoughts @wilkinsona ?

@wilkinsona
Copy link
Member

It seems to do everything that our previous code did; it allows the default domain and server to be configured via the environment. An API could be a nice refinement but I think we should merge this as-is.

@snicoll snicoll self-assigned this Jun 20, 2016
@snicoll snicoll closed this in 3ea84f9 Jun 20, 2016
snicoll added a commit that referenced this pull request Jun 20, 2016
* pr/6184:
  Fix relaxed binding of SI JMX config
@artembilan
Copy link
Member Author

artembilan commented Jun 20, 2016

Thank you for taking a look, but, to be honest, we didn't reinvented the wheel in SI, and our IntegrationMBeanExporter environment and configuration is fully similar to MBeanExporter in the SF.

OTOH I did this new IntegrationJmxConfiguration like it is in the JmxAutoConfiguration, where that is a copy/pasting from the MBeanExportConfiguration behind EnableMBeanExport as well.

@artembilan
Copy link
Member Author

By the way. Don't forget to include it into 1.3.x 😄

@snicoll
Copy link
Member

snicoll commented Jun 20, 2016

@artembilan it seems I have issues with your stuff. Second time there is a glitch with a backport. This time I did it and forgot to push. Oh well 😢 - It's done now, thanks!

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

Successfully merging this pull request may close these issues.

4 participants