incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chip Childers <chip.child...@sungard.com>
Subject Re: Need some clarification on MYSQL connector (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)
Date Wed, 05 Sep 2012 14:17:38 GMT
On Wed, Sep 5, 2012 at 10:08 AM, Pradeep Soundararajan
<pradeep.soundararajan@citrix.com> wrote:
> 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?

I *think* that's right.  Let's see how the Maven build process shakes
out, since this is actually tied to that as well.  We know that
mysql-connector needs to be a system dependency, and not be
distributed with the ASF release.

One item that does come to mind though...  shouldn't it be included in
the DEB packages too?  Can you add that?

> Thanks,
>
> Pradeep.S
>
>
>
>
>
> -----Original Message-----
>
> From: Alex Huang [mailto:noreply@reviews.apache.org]<mailto:[mailto:noreply@reviews.apache.org]>
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
>
>>
>
>>
>
>

Mime
View raw message