Support all labkey.xml/ROOT.xml settings via application.properties #680
Conversation
| @@ -44,6 +48,26 @@ mail.smtpUser=@@smtpUser@@ | |||
| # mail.smtpSocketFactoryClass=@@smtpSocketFactoryClass@@ | |||
| # mail.smtpAuth=@@smtpAuth@@ | |||
|
|
|||
| # Optional - JMS configuration for remote ActiveMQ message management for distributed pipeline jobs | |||
| # https://www.labkey.org/Documentation/wiki-page.view?name=jmsQueue | |||
There was a problem hiding this comment.
Have we captured in a TODO list somewhere that we'll need to update these docs for embedded?
There was a problem hiding this comment.
Yes, there's docs handoff task in the story on the board. I'll add pointers to these two pages as definitely needing attention in the spec.
| @@ -262,10 +293,22 @@ private List<ContextResource> getDataSourceResources(ContextProperties props) th | |||
| dataSourceResource.setProperty("accessToUnderlyingConnectionAllowed", getPropValue(props.getAccessToUnderlyingConnectionAllowed(), i, ACCESS_TO_CONNECTION_ALLOWED_DEFAULT, "accessToUnderlyingConnectionAllowed")); | |||
| dataSourceResource.setProperty("validationQuery", getPropValue(props.getValidationQuery(), i, VALIDATION_QUERY_DEFAULT, "validationQuery")); | |||
|
|
|||
| // These two properties are handled different, as separate parameters | |||
There was a problem hiding this comment.
| // These two properties are handled different, as separate parameters | |
| // These two properties are handled differently, as separate parameters |
| { | ||
| ldapResource.setProperty("credentials", ldapProps.getCredentials()); | ||
| } | ||
| if (ldapProps.isUseSsl()) |
There was a problem hiding this comment.
Seems like this if is not needed since we can just as well set the property to false. (Same for isUseTls below.)
There was a problem hiding this comment.
True. I was aiming for to create the resource most similarly to how it would have been configured in the XML world, but this simplification should be safe.
| ldapResource.setProperty("sslProtocol", ldapProps.getSslProtocol()); | ||
| } | ||
| return ldapResource; | ||
| } |
There was a problem hiding this comment.
Thanks for taking care of this, @labkey-jeckels . I know I'm late to the party, but it seems unfortunate that LabKeyServer must know about every possible property used by every module. Shouldn't we introduce a registration mechanism for modules to handle the properties they care about? Similar to startup properties.
There was a problem hiding this comment.
I don't like needing centralized knowledge of these properties either, but this code has no idea about modules. And the module code doesn't know anything about application.properties or how to find it.
Some of these properties, such as disabling 2FA, could be handled via startup properties. That may be worth doing.
JMS and LDAP could probably be handled differently too but this was the approach I found that was as close to the old mechanism as possible, and easiest to manage while we're in transition between standalone and embedded.
Rationale
We're enabling full compatibility for configuration via embedded Tomcat.
Related Pull Requests
Changes