You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@cloudstack.apache.org by Pradeep Soundararajan <pr...@citrix.com> on 2012/09/05 16:08:03 UTC

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)

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

>

>

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)

Posted by Chip Childers <ch...@sungard.com>.

On Wed, Sep 5, 2012 at 10:23 AM, Wido den Hollander <wi...@widodh.nl> wrote:
> On 09/05/2012 04:17 PM, Chip Childers wrote:
>>
>> On Wed, Sep 5, 2012 at 10:08 AM, Pradeep Soundararajan
>> <pr...@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
>>>
>>>
>>>>
>>>
>>>>
>>>
>>>
>
>

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)

Posted by Wido den Hollander <wi...@widodh.nl>.

On 09/05/2012 04:17 PM, Chip Childers wrote:
> On Wed, Sep 5, 2012 at 10:08 AM, Pradeep Soundararajan
> <pr...@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

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

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)

Posted by Chip Childers <ch...@sungard.com>.

On Wed, Sep 5, 2012 at 10:08 AM, Pradeep Soundararajan
<pr...@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
>
>>
>
>>
>
>