Hi there JIRA raised and patch applied: https://issues.apache.org/jira/browse/SYNCOPE-460
WRT the BasicDataSource properties... what do you think about adding only connectionProperties which could be a file with the BasicDataSource properties. Oli ------ Oliver Wulff Blog: http://owulff.blogspot.com Solution Architect http://coders.talend.com Talend Application Integration Division http://www.talend.com ________________________________________ From: Francesco Chicchiriccò [ilgro...@apache.org] Sent: 06 December 2013 16:21 To: dev@syncope.apache.org Subject: Re: BasicDataSource usage in persistence.xml On 06/12/2013 16:17, Guido Wimmel wrote: > Hi, > > I wouldn't keep Oliver from providing a patch wrt. his original request, > if he'd like to ;-) > Though if not, I can possibly also provide something - maybe next week. > > I'm only unsure about which of the connection pool settings exactly should > be externalized into properties, I'd say _any_ available property. > and about the isolation level setting (maybe one > could make this configurable as well, with default READ_COMMITTED?). +1 For whoever will take care of this: please (1) consider *all* persistence.properties found under core/ (2) include commons-dbcp 1.4 as direct dependency either in root pom.xml (<dependencyManagement/>) and core/pom.xml. Regards. > Gesendet: Freitag, 06. Dezember 2013 um 15:38 Uhr > Von: "Oliver Wulff" <owu...@talend.com> > An: "dev@syncope.apache.org" <dev@syncope.apache.org> > Betreff: RE: BasicDataSource usage in persistence.xml > Will do ;-) > > Cheers > Oli > ________________________________________ > From: Francesco Chicchiriccò [ilgro...@apache.org] > Sent: 06 December 2013 13:22 > To: dev@syncope.apache.org > Subject: Re: BasicDataSource usage in persistence.xml > > On 06/12/2013 12:49, Guido Wimmel wrote: >> Hi Oliver, >> >> FYI: we use such a setup (with dbcp.BasicDataSource) in our project, thus >> avoiding the need to >> configure a data source in the container. This works fine; also with our own >> unit/integration tests. >> >> We also used the standard MySQL isolation level (REPEATABLE_READ), as we had >> problems with our standard >> MySQL configuration related to Binary Logs ("InnoDB is limited to >> row-logging when transaction isolation level >> is READ_COMMITTED or READ_UNCOMMITTED"). This also works fine, but probably >> the explicit setting of >> READ_COMMITTED as a transaction level was chosen on purpose. >> >> We externalized some more properties of BasicDataSource (testOnBorrow, >> testOnReturn, removeAbandoned, >> initialSize, maxActive, minIdle, removeAbandoned, >> timeBetweenEvictionRunsMillis, minEvictableIdleTimeMillis, >> validationQuery). IMO, at least the size parameters (initialSize, maxActive, >> minIdle) and the validation query >> (which can be database specific) should be configurable via properties. >> BasicDataSource should also probably be a direct >> Maven dependency then. >> >> I'd be +1 for this change, as it would spare us the need of overwriting >> persistenceContext.xml ;-) > Well, what are you still waiting for providing a patch, then? ;-) > >>> Von: "Francesco Chicchiriccò" <ilgro...@apache.org> >>>> Hi there >>> Hi Oliver, >>>> Wouldn't it make sense to use a BasicDataSource instead of >>>> DriverManagerDataSource as it is also recommended from Spring: >>>> http://docs.spring.io/spring/docs/current/spring-framework-reference/html/jdbc.html#jdbc-connections >>> Short answer: Yes, why not? >>> Long answer follows. >>> >>> The localDataSource bean in persistenceContext.xml is there for >>> providing a DataSource for non-production usage: unit and integration >>> tests, various development profiles [1], embedded mode [2] and >>> standalone distribution [3]. >>> >>> As you can see from syncopeContext.xml, things are arranged so that at >>> first a JNDI lookup for a proper DataSource is performed: this becomes >>> of fundamental importance in production environments, as we suggest [4]. >>> >>> Moreover, usage of BasicDataSource implies commons-dbcp dependency, >>> which is currently indirectly pulled in by Activiti >>> (org.activiti:activiti-spring, exactly). >>> >>> Hence, I am +-0 for this change. >>> >>>> Instead of this: >>>> >>>> <bean id="localDataSource" >>>> class="org.springframework.jdbc.datasource.IsolationLevelDataSourceAdapter"> >>>> <property name="targetDataSource"> >>>> <bean class="org.springframework.jdbc.datasource.DriverManagerDataSource"> >>>> <property name="driverClassName" value="${jpa.driverClassName}"/> >>>> <property name="url" value="${jpa.url}"/> >>>> <property name="username" value="${jpa.username}"/> >>>> <property name="password" value="${jpa.password}"/> >>>> </bean> >>>> </property> >>>> <property name="isolationLevelName" value="ISOLATION_READ_COMMITTED"/> >>>> </bean> >>>> >>>> >>>> we can configure this bean: >>>> >>>> <bean id="localDataSource" >>>> class="org.springframework.jdbc.datasource.IsolationLevelDataSourceAdapter"> >>>> <property name="targetDataSource"> >>>> <bean class="org.apache.commons.dbcp.BasicDataSource" >>>> destroy-method="close"> >>>> <property name="driverClassName" value="${jpa.driverClassName}"/> >>>> <property name="url" value="${jpa.url}"/> >>>> <property name="username" value="${jpa.username}"/> >>>> <property name="password" value="${jpa.password}"/> >>>> </bean> >>>> </property> >>>> <property name="isolationLevelName" value="ISOLATION_READ_COMMITTED"/> >>>> </bean> >>> If you like such change, please file an issue and attach a patch there. >>> >>> Regards. >>> >>> [1] >>> http://syncope.apache.org/building.html#More_build_profiles[http://syncope.apache.org/building.html#More_build_profiles] >>> [2] >>> https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+in+embedded+mode[https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+in+embedded+mode] >>> [3] >>> https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+standalone+distribution[https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+standalone+distribution] >>> [4] >>> https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+in+real+environments#RunSyncopeinrealenvironments-Usedatasource[https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+in+real+environments#RunSyncopeinrealenvironments-Usedatasource] >>> >>> -- >>> Francesco Chicchiriccò >>> >>> Tirasa - Open Source Excellence >>> http://www.tirasa.net/ >>> >>> ASF Member, Apache Syncope PMC chair, Apache Cocoon PMC Member >>> http://people.apache.org/~ilgrosso/
