On Wed, Sep 5, 2012 at 10:08 AM, Pradeep Soundararajan <[email protected]> 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:[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 > >> > >> > >
