getodk / aggregate

ODK Aggregate is a Java server that stores, analyzes, and presents survey data collected using ODK Collect. Contribute and make the world a better place! ✨🗄✨
https://docs.opendatakit.org/aggregate-intro/
Other
74 stars 227 forks source link

Issue 429 Fix the JDBC conf file the installer creates #432

Closed ggalmazor closed 5 years ago

ggalmazor commented 5 years ago

Closes #429

This PR brings changes to the installer that ensure that all the jdbc properties are set by the installer.

What has been done to verify that this works as intended?

Build the installer (linux-x64) and run it, then deploy the resulting WAR into a local Tomcat. Verify that Aggregate launches without changing anything.

Why is this the best possible solution? Were any other approaches considered?

This PR adds some required adaptations from v1.x to v2.x into the installer

In v1.x, the odk-settings.xml for each platform comes with a hard-coded driver classname, which deviates from the same file in the code repo, which uses whatever value is defined in the jdbc.driverClassNameprops key injdbc.properties`

In v2.x, we have made an effort to normalize all the different configuration assets, and we changed the odk-settings.xml that the installer uses, but forgot to set the prop key in the jdbc settings file.

This PR takes care of that.

Are there any risks to merging this code? If so, what are they?

Nope. It only affects to the installer.

Do we need any specific form for testing your changes? If so, please attach one

Nope.

Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.

Nope.

ggalmazor commented 5 years ago

@yanokwa I was thinking about how to release this change since it only affects to the installer. We will be releasing this fix on v2.0.1 for sure, but maybe we should backport it to v2.0 by replacing the installers. What do you think?

ggalmazor commented 5 years ago

Installers:

yanokwa commented 5 years ago

I don't like the idea of the same version numbers of the installers having different behavior. I think we should release in v2.0.1 and then we have a few options

ggalmazor commented 5 years ago

Closing in favour of #422, which has the commit from this PR. This will make testing the installer changes easier.