On 08/12/2013 14:54, Oliver Wulff wrote:
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.
Patch looks fine, thanks!
I would avoid proliferating of properties file that would make harder
times with generation from archetype, so I'd keep with adding entries in
persistence.properties.
I will wait till we come to an agreement before applying your patch.
Regards.
________________________________________
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/
