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 19:53:33 GMT
On Wed, Sep 5, 2012 at 10:23 AM, Wido den Hollander <wido@widodh.nl> wrote:
> On 09/05/2012 04:17 PM, Chip Childers wrote:
>>
>> 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?
>>
>
> Ubuntu and Debian both ship the MySQL connector in a package. I already
> added a dependency for that:
> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=commit;h=9064236879dc9f2f11538d891271545e1ff10e9c
>
> Wido

Great!  Then I think you're right Pradeep.  We can probably close out
this topic for now.

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