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
[2] 
https://cwiki.apache.org/confluence/display/SYNCOPE/Run+Syncope+in+embedded+mode
[3] 
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

--
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/

Reply via email to