Re: Review Request: S3-backed Secondary Storage

2013-01-08 Thread Rohit Yadav
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review15168 --- Already shipped, pl. close it as submitted. Thanks. - Rohit Yadav

Re: Review Request: S3-backed Secondary Storage

2013-01-02 Thread edison su
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review15014 --- Ship it! Ship It! - edison su On Dec. 21, 2012, 12:10 p.m., John

Re: Review Request: S3-backed Secondary Storage

2012-12-21 Thread John Burwell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/ --- (Updated Dec. 21, 2012, 12:10 p.m.) Review request for cloudstack and edison su.

Re: Review Request: S3-backed Secondary Storage

2012-12-20 Thread John Burwell
> +import com.cloud.network.dao.IPAddressDao; > +import com.cloud.network.dao.NetworkDao; > +import com.cloud.network.dao.PhysicalNetworkDao; > +import com.cloud.network.dao.PhysicalNetworkTrafficTypeDao; > +import com.cloud.network.dao.PhysicalNetworkTrafficTypeVO; > import com.clo

RE: Review Request: S3-backed Secondary Storage

2012-12-19 Thread Edison Su
ese imports are necessary? > -Original Message- > From: John Burwell [mailto:nore...@reviews.apache.org] On Behalf Of John > Burwell > Sent: Wednesday, December 19, 2012 1:40 PM > To: Edison Su > Cc: cloudstack; John Burwell > Subject: Re: Review Request: S3-backed Seconda

Re: Review Request: S3-backed Secondary Storage

2012-12-19 Thread John Burwell
> On Dec. 17, 2012, 5:51 p.m., edison su wrote: > > Hi John, thanks for the patch. Few comments: 1. I can't apply the patch > > against master top, there is conflict in pom.xml. 2. Could you remove the > > unnecessary change in ConfigurationManagerImpl file? Edison, Which change in Configurat

Re: Review Request: S3-backed Secondary Storage

2012-12-17 Thread edison su
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review14588 --- Hi John, thanks for the patch. Few comments: 1. I can't apply the pat

Re: Review Request: S3-backed Secondary Storage

2012-12-14 Thread Rohit Yadav
> On Dec. 14, 2012, 9:58 p.m., Rohit Yadav wrote: > > John, don't put all the patches on one review. Pl. close this one as > > submitted and open a new review (possibly backed by a jira issue) for your > > changes: > > Makes the Marvin database configuration optional > > Adds S3 template search

Re: Review Request: S3-backed Secondary Storage

2012-12-14 Thread John Burwell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/ --- (Updated Dec. 14, 2012, 11:54 p.m.) Review request for cloudstack and edison su.

Re: Review Request: S3-backed Secondary Storage

2012-12-14 Thread John Burwell
> On Dec. 14, 2012, 9:58 p.m., Rohit Yadav wrote: > > John, don't put all the patches on one review. Pl. close this one as > > submitted and open a new review (possibly backed by a jira issue) for your > > changes: > > Makes the Marvin database configuration optional > > Adds S3 template search

Re: Review Request: S3-backed Secondary Storage

2012-12-14 Thread John Burwell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/ --- (Updated Dec. 14, 2012, 11:48 p.m.) Review request for cloudstack and edison su.

Re: Review Request: S3-backed Secondary Storage

2012-12-14 Thread Rohit Yadav
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review14524 --- John, don't put all the patches on one review. Pl. close this one as

Re: Review Request: S3-backed Secondary Storage

2012-12-14 Thread John Burwell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/ --- (Updated Dec. 14, 2012, 9:49 p.m.) Review request for cloudstack. Changes

Re: Review Request: S3-backed Secondary Storage

2012-12-14 Thread Rohit Yadav
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review14508 --- Was applied on master: commit b70c1a5a84ee014d980c07555d341f16ed515cb

Re: Review Request: S3-backed Secondary Storage

2012-12-13 Thread edison su
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review14496 --- Ship it! Ship It! - edison su On Nov. 26, 2012, 9:44 p.m., John B

Re: Review Request: S3-backed Secondary Storage

2012-12-07 Thread Rohit Yadav
> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote: > > John, thanks for patch. Much better now. Can you check, why the code that > > configure db in the testclient is commented? > > tools/marvin/marvin/deployDataCenter.py > > For the actual functional review, Edison can help on storage and > >

Re: Review Request: S3-backed Secondary Storage

2012-12-03 Thread Prasanna Santhanam
> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote: > > John, thanks for patch. Much better now. Can you check, why the code that > > configure db in the testclient is commented? > > tools/marvin/marvin/deployDataCenter.py > > For the actual functional review, Edison can help on storage and > >

Re: Review Request: S3-backed Secondary Storage

2012-12-03 Thread John Burwell
> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote: > > John, thanks for patch. Much better now. Can you check, why the code that > > configure db in the testclient is commented? > > tools/marvin/marvin/deployDataCenter.py > > For the actual functional review, Edison can help on storage and > >

Re: Review Request: S3-backed Secondary Storage

2012-11-30 Thread Rohit Yadav
> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote: > > John, thanks for patch. Much better now. Can you check, why the code that > > configure db in the testclient is commented? > > tools/marvin/marvin/deployDataCenter.py > > For the actual functional review, Edison can help on storage and > >

Re: Review Request: S3-backed Secondary Storage

2012-11-30 Thread John Burwell
> On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote: > > John, thanks for patch. Much better now. Can you check, why the code that > > configure db in the testclient is commented? > > tools/marvin/marvin/deployDataCenter.py > > For the actual functional review, Edison can help on storage and > >

Re: Review Request: S3-backed Secondary Storage

2012-11-30 Thread Rohit Yadav
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review13904 --- John, thanks for patch. Much better now. Can you check, why the code

RE: Review Request: S3-backed Secondary Storage

2012-11-26 Thread Edison Su
> -Original Message- > From: John Burwell [mailto:nore...@reviews.apache.org] On Behalf Of John > Burwell > Sent: Monday, November 26, 2012 2:54 PM > To: cloudstack; Rohit Yadav; John Burwell > Subject: Re: Review Request: S3-backed Secondary Storage > > >

Re: Review Request: S3-backed Secondary Storage

2012-11-26 Thread John Burwell
> On Nov. 21, 2012, 4:32 a.m., Rohit Yadav wrote: > > Read the coding conventions and checkout the diff so you'll know. > > > > Watch out for the red lines and extra indents. > > For example, fix for following files with tabwidth=4 with spaces, no tabs; > > fix trailing spaces; > > > > api/src

Re: Review Request: S3-backed Secondary Storage

2012-11-26 Thread John Burwell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/ --- (Updated Nov. 26, 2012, 9:44 p.m.) Review request for cloudstack. Changes

Re: Review Request: S3-backed Secondary Storage

2012-11-20 Thread Rohit Yadav
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review13659 --- Read the coding conventions and checkout the diff so you'll know. Wa

Re: Review Request: S3-backed Secondary Storage

2012-11-19 Thread John Burwell
> On Nov. 19, 2012, 1:51 p.m., David Nalley wrote: > > utils/pom.xml, line 106 > > > > > > How is this dependency licensed? The Amazon AWS SDK for Java is licensed under the Apache 2.0 license. (http://aws.amazon.com

Re: Review Request: S3-backed Secondary Storage

2012-11-19 Thread John Burwell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/ --- (Updated Nov. 19, 2012, 5:16 p.m.) Review request for cloudstack. Changes

Re: Review Request: S3-backed Secondary Storage

2012-11-19 Thread John Burwell
> On Nov. 19, 2012, 8:03 a.m., Rohit Yadav wrote: > > John, can you fix indendations at some places, trailing whitespaces etc. > > see: > > http://docs.cloudstack.org/CloudStack_Documentation/Design_Documents/Coding_Conventions Rohit, Which files are have the issue? Thanks, -John - John -

Re: Review Request: S3-backed Secondary Storage

2012-11-19 Thread David Nalley
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review13564 --- api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java

Re: Review Request: S3-backed Secondary Storage

2012-11-19 Thread Rohit Yadav
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review13557 --- John, can you fix indendations at some places, trailing whitespaces e

Re: Review Request: S3-backed Secondary Storage

2012-11-19 Thread Rohit Yadav
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review13556 --- John, can you fix indendations at some places, trailing whitespaces e

Re: Review Request: S3-backed Secondary Storage

2012-11-18 Thread John Burwell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/ --- (Updated Nov. 19, 2012, 6:37 a.m.) Review request for cloudstack. Description

Review Request: S3-backed Secondary Storage

2012-11-18 Thread John Burwell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/ --- Review request for cloudstack. Description --- Backs NFS-based secondary st