Cbioportal: DB settings confusing

[pieterlukasse]

The documentation in https://github.com/cBioPortal/cbioportal/wiki/portal.properties-Reference#database-settings is a bit confusing as it seems that some db properties are only used by the data loading scripts and not by the portal code (which takes config from spring?).

[holtgrewe]

I have spent the last two days setting up cBioPortal for private deployment on Flynn (roughly equivalent to Heroku).

This would be much easier if cBioPortal adhered to the Twelve Factor App Principles, that is giving configuration through environment variables. I have an internal patch that makes GlobalProperties consider the environment variables for each queried property. This works well for the contact email address, for example.

However, in cBioPortal, the connection information appears to be compiled in, so I have to recompile the app everything I change the database settings (which is easy and thus can happen frequently in a Heroku-like deployment).

For me, a "golden path" for database configuration would be to read a database URL environment variable, (cmp. dj-database-url) by default, and maybe fallback to a portal.properties using this setting. Nothing would be baked in during compilation ;)

[pieterlukasse]

I agree with @holtgrewe , just not sure what are the best follow-up steps. @inodb, @morungos : any ideas? Maybe you guys are already thinking about changing some of this?

[holtgrewe]

Another place where the data source appears is in applicationContext-business.xml.

Also, this is the place where I attempted to resolve the the mysql timeout issue. This brought me over the timeout of 10 minutes but I'm not certain that the portal will continue to work overnight.

@@ -77,10 +77,12 @@
             <property name="password" value="${db.password}"/>
             <property name="minIdle" value="0"/>
             <property name="maxIdle" value="10"/>
             <property name="maxActive" value="100"/>
             <property name="poolPreparedStatements" value="true"/>
+            <property name="testOnBorrow" value="true" />
+            <property name="validationQuery" value="SELECT 1" />
         </bean>
        <bean class="org.springframework.beans.factory.config.MethodInvokingFactoryBean">
            <property name="staticMethod" value="org.mskcc.cbio.portal.dao.JdbcUtil.setDataSource"/>
            <property name="arguments">
                <list>

[holtgrewe]

I google'd around and found some useful information.

It should be possible to interpret the DATABASE_URL environment variable and fall back to whatever is in the portal.properties otherwise. The relevant part of JdbcUtil.java should be adaptable in a similar way, or (even better?) somehow get the same object injected as from the applicationContext-business.xml.

[inodb]

@holtgrewe thanks for the 12 factor app principles link, that's super useful!

I think it would be great to use an environment variable for the db. I checked with @ersinciftci and it seems that there's no current effort ongoing to change this.

For heroku I added a spring config variable to switch between jndi and dbcp at run time, see: https://github.com/cBioPortal/cbioportal/pull/605. I would assume something similar is possible for the database. The heroku devcenter solutions @holtgrewe pointed out look decent to me.

[jjgao]

@pieterlukasse @inodb should we keep this open?

[fedde-s]

It's still a major issue, and has not been resolved.

[fedde-s]

It makes both manual configuration rather fragile in a way that can't be automated in a clean way by the Dockerisation efforts. Especially since large parts of the data loading pipeline actually ignore changes to the configuration after the Java code has been built, as I brought up in the ‘cloud’ channel on Slack. That makes for at least three, maybe six, files that must at all times contain the same database credentials for cBioPortal to function.

[inodb]

For just running the portal, env variables work. Just put e.g. ${db.user:CBIOPORTAL_DB_USER} in the properties during compilation. I haven't tested if this also solves it for data loading, but I imagine there might be some issues there. Agreed that the current state is still confusing.

[fedde-s]

An undocumented seventh place that should also hold the same credentials to make the parts of the stack work together is not the solution to the confusingly long list. The supported way to change the defaults baked in at compile time is src/main/resources/portal.properties, that was the part that was already clear.

[pieterlukasse]

I would like to see a solution where we stick to maven.properties for compile time relevant properties and have a default of $PORTAL_HOME/portal.properties for all other properties.

[pieterlukasse]

@inodb , @fedde-s I know you guys have made some progress since last year. Can we close this issue? What is the current status?

[jjgao]

I am closing this. Please create a new issue if there are remaining issues.