Hello Hugo/Chip,
These are the review comments I have received so far.
- For build-cloud.properties, that line should be optional. This means add a
'#' in front of it and others an make it required when copying it into override.
- The change for developer.xml to read in the build-cloud.xml should be done as
a precondition to all build targets.
- The change does not take into account that the file may be on the class path
and so the property value should only be optional. Here it made it explicit.
- Also, we need to add the mysql connector as a rpm dependency. That change is
not in.
Hugo >> I did a quick fix to get the build working again, just pushed the
commit to master. The mysql connector rpm is already installed on the Jenkins
server, I just modified the scripts to it is picked up by the build.
>> I can add it to the spec file for rpm based distro's. I think it should
be added to the client rpm.
I see that you have added the class path explicitly in order to resolve this
temporarily.
# Add mysql jar from mysql-connector-java package to CP
# for Jenkins
CP=${CP}${PATHSEP}/usr/share/java/mysql-connector-java.jar
Also, I have seen that you have added that rpm in ‘cloud.spec’ as well.
Since you are calling mysql-connector explicitly from ‘deploy-db-dev.sh’
class-path, the changes from ‘build-cloud.properties’ and ‘developer.xml’ are
not required from my patch. Please correct me if I am wrong.
If everything is fine, shall we close this topic. Please share if anyone have
any comments?
Thanks,
Pradeep.S
-----Original Message-----
From: Alex Huang
[mailto:[email protected]]<mailto:[mailto:[email protected]]>
On Behalf Of Alex Huang
Sent: Friday, August 31, 2012 8:37 PM
To: David Nalley; Alex Huang
Cc: cloudstack; Pradeep Soundararajan
Subject: Re: Review Request: CS-15694:Remove MYSQL connector. Added the
explicit class path for mysql connector which will call the jar from the
desired location
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6752/#review10943
-----------------------------------------------------------
Also, we need to add the mysql connector as a rpm dependency. That change is
not in.
- Alex Huang
On Aug. 24, 2012, 10:19 a.m., Pradeep Soundararajan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6752/
> -----------------------------------------------------------
>
> (Updated Aug. 24, 2012, 10:19 a.m.)
>
>
> Review request for cloudstack, David Nalley and Alex Huang.
>
>
> Description
> -------
>
> CS-15694:Remove MYSQL connector. Added the explicit class path for mysql
> connector which will call the jar from the desired location
>
>
> Diffs
> -----
>
> build/build-cloud.properties 552de7f74db7b715da70cc48ff4dc8945fa066f8
> build/developer.xml f2e5aa6463ec849a3e97343e82423ff0ac622222
> setup/db/deploy-db-dev.sh f149e9efd029bd311b1d247e21764b9103fd01d9
>
> Diff: https://reviews.apache.org/r/6752/diff/
>
>
> Testing
> -------
>
> Executed: "ant build-all deploy-server deploydb" successfully.
>
>
> Thanks,
>
> Pradeep Soundararajan
>
>
