You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Santhosh Edukulla <sa...@citrix.com> on 2013/11/06 12:45:59 UTC
Review Request 15263: Added fix for bug 5056.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15263/
-----------------------------------------------------------
Review request for cloudstack and Girish Shilamkar.
Bugs: CLOUDSTACK-5056
https://issues.apache.org/jira/browse/CLOUDSTACK-5056
Repository: cloudstack-git
Description
-------
1. The library( remoteSSHClient ) in its current form has some issues. Fixed few of them. As part of that, did clean up of the code.
2. Added tcp timeout flag as an additional parameter to help establish the tcp connections and retry if failed. It retries even otherwise failed.
3. Renamed few variables and maintained uniform usage and convention.
4. Added few new member functions runCommand,createConnection
5. Current, way of creating connection and raising exception from constructor along with return was removed and added a new way of handling it.
6. Currently, result list was returned for execute command, but we dont know the status of command execution, whether the output contains error or output etc. Now, provided more information in the return with new implementation.
7. Added few codes for proper return status.
TODO:
1. Remove establishing connection from constructor altogether. This will effect the current modules using it.
2. Need to change is_server_ssh_ready function for proper and adequate checks, currently the name and functionality is little different. It can add more adequate checks before returning ssh object.
Diffs
-----
tools/marvin/marvin/codes.py b6580d0
tools/marvin/marvin/integration/lib/utils.py 4d048f0
tools/marvin/marvin/remoteSSHClient.py fea9b12
Diff: https://reviews.apache.org/r/15263/diff/
Testing
-------
Thanks,
Santhosh Edukulla
Re: Review Request 15263: Added fix for bug 5056.
Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15263/#review28534
-----------------------------------------------------------
commit cf7532e8480385bcc9c5dfd63fb82cd836282028
- SrikanteswaraRao Talluri
On Nov. 6, 2013, 1:44 p.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15263/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2013, 1:44 p.m.)
>
>
> Review request for cloudstack and Girish Shilamkar.
>
>
> Bugs: CLOUDSTACK-5056
> https://issues.apache.org/jira/browse/CLOUDSTACK-5056
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
>
> 1. The library( remoteSSHClient ) in its current form has some issues. Fixed few of them. As part of that, did clean up of the code.
> 2. Added tcp timeout flag as an additional parameter to help establish the tcp connections and retry if failed. It retries even otherwise failed.
> 3. Renamed few variables and maintained uniform usage and convention.
> 4. Added few new member functions runCommand,createConnection
> 5. Current, way of creating connection and raising exception from constructor along with return was removed and added a new way of handling it.
> 6. Currently, result list was returned for execute command, but we dont know the status of command execution, whether the output contains error or output etc. Now, provided more information in the return with new implementation.
> 7. Added few codes for proper return status.
>
>
> TODO:
> 1. Remove establishing connection from constructor altogether. This will effect the current modules using it.
> 2. Need to change is_server_ssh_ready function for proper and adequate checks, currently the name and functionality is little different. It can add more adequate checks before returning ssh object.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/codes.py b6580d0
> tools/marvin/marvin/integration/lib/utils.py 4d048f0
> tools/marvin/marvin/remoteSSHClient.py fea9b12
>
> Diff: https://reviews.apache.org/r/15263/diff/
>
>
> Testing
> -------
>
>
> File Attachments
> ----------------
>
> Added New File
> https://reviews.apache.org/media/uploaded/files/2013/11/06/0001-Added-fix-for-bug-5056.patch
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15263: Added fix for bug 5056.
Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15263/#review28494
-----------------------------------------------------------
Ship it!
- SrikanteswaraRao Talluri
On Nov. 6, 2013, 1:44 p.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15263/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2013, 1:44 p.m.)
>
>
> Review request for cloudstack and Girish Shilamkar.
>
>
> Bugs: CLOUDSTACK-5056
> https://issues.apache.org/jira/browse/CLOUDSTACK-5056
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
>
> 1. The library( remoteSSHClient ) in its current form has some issues. Fixed few of them. As part of that, did clean up of the code.
> 2. Added tcp timeout flag as an additional parameter to help establish the tcp connections and retry if failed. It retries even otherwise failed.
> 3. Renamed few variables and maintained uniform usage and convention.
> 4. Added few new member functions runCommand,createConnection
> 5. Current, way of creating connection and raising exception from constructor along with return was removed and added a new way of handling it.
> 6. Currently, result list was returned for execute command, but we dont know the status of command execution, whether the output contains error or output etc. Now, provided more information in the return with new implementation.
> 7. Added few codes for proper return status.
>
>
> TODO:
> 1. Remove establishing connection from constructor altogether. This will effect the current modules using it.
> 2. Need to change is_server_ssh_ready function for proper and adequate checks, currently the name and functionality is little different. It can add more adequate checks before returning ssh object.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/codes.py b6580d0
> tools/marvin/marvin/integration/lib/utils.py 4d048f0
> tools/marvin/marvin/remoteSSHClient.py fea9b12
>
> Diff: https://reviews.apache.org/r/15263/diff/
>
>
> Testing
> -------
>
>
> File Attachments
> ----------------
>
> Added New File
> https://reviews.apache.org/media/uploaded/files/2013/11/06/0001-Added-fix-for-bug-5056.patch
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15263: Added fix for bug 5056.
Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
> On Nov. 7, 2013, 5:05 p.m., SrikanteswaraRao Talluri wrote:
> > tools/marvin/marvin/remoteSSHClient.py, line 116
> > <https://reviews.apache.org/r/15263/diff/1/?file=379016#file379016line116>
> >
> > it should be self.keyPairFiles instead of keyPairFiles
> >
> > again keyPairFiles is not defined at this point and it is resulting in exception.
>
> Santhosh Edukulla wrote:
> It says self.keyPiarFiles only in the patch. I didnt see this line in patch1, did it changed after applying patch2 related to this?
but this diff reflects it as jus keyPairFiles . Anyways, if it is self.keyPairFiles after two patches, that is fine.
> On Nov. 7, 2013, 5:05 p.m., SrikanteswaraRao Talluri wrote:
> > tools/marvin/marvin/remoteSSHClient.py, line 115
> > <https://reviews.apache.org/r/15263/diff/1/?file=379016#file379016line115>
> >
> > self.keyPairFiles is not defined at this moment we are getting an exception here.
> >
> > remove this here
>
> Santhosh Edukulla wrote:
> self.keyPairFiles is defined in constructor and we are calling createConnection later, so its not an issue i believe.
agreed
- SrikanteswaraRao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15263/#review28380
-----------------------------------------------------------
On Nov. 6, 2013, 1:44 p.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15263/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2013, 1:44 p.m.)
>
>
> Review request for cloudstack and Girish Shilamkar.
>
>
> Bugs: CLOUDSTACK-5056
> https://issues.apache.org/jira/browse/CLOUDSTACK-5056
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
>
> 1. The library( remoteSSHClient ) in its current form has some issues. Fixed few of them. As part of that, did clean up of the code.
> 2. Added tcp timeout flag as an additional parameter to help establish the tcp connections and retry if failed. It retries even otherwise failed.
> 3. Renamed few variables and maintained uniform usage and convention.
> 4. Added few new member functions runCommand,createConnection
> 5. Current, way of creating connection and raising exception from constructor along with return was removed and added a new way of handling it.
> 6. Currently, result list was returned for execute command, but we dont know the status of command execution, whether the output contains error or output etc. Now, provided more information in the return with new implementation.
> 7. Added few codes for proper return status.
>
>
> TODO:
> 1. Remove establishing connection from constructor altogether. This will effect the current modules using it.
> 2. Need to change is_server_ssh_ready function for proper and adequate checks, currently the name and functionality is little different. It can add more adequate checks before returning ssh object.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/codes.py b6580d0
> tools/marvin/marvin/integration/lib/utils.py 4d048f0
> tools/marvin/marvin/remoteSSHClient.py fea9b12
>
> Diff: https://reviews.apache.org/r/15263/diff/
>
>
> Testing
> -------
>
>
> File Attachments
> ----------------
>
> Added New File
> https://reviews.apache.org/media/uploaded/files/2013/11/06/0001-Added-fix-for-bug-5056.patch
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15263: Added fix for bug 5056.
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On Nov. 7, 2013, 5:05 p.m., SrikanteswaraRao Talluri wrote:
> > tools/marvin/marvin/remoteSSHClient.py, line 115
> > <https://reviews.apache.org/r/15263/diff/1/?file=379016#file379016line115>
> >
> > self.keyPairFiles is not defined at this moment we are getting an exception here.
> >
> > remove this here
self.keyPairFiles is defined in constructor and we are calling createConnection later, so its not an issue i believe.
> On Nov. 7, 2013, 5:05 p.m., SrikanteswaraRao Talluri wrote:
> > tools/marvin/marvin/remoteSSHClient.py, line 116
> > <https://reviews.apache.org/r/15263/diff/1/?file=379016#file379016line116>
> >
> > it should be self.keyPairFiles instead of keyPairFiles
> >
> > again keyPairFiles is not defined at this point and it is resulting in exception.
It says self.keyPiarFiles only in the patch. I didnt see this line in patch1, did it changed after applying patch2 related to this?
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15263/#review28380
-----------------------------------------------------------
On Nov. 6, 2013, 1:44 p.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15263/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2013, 1:44 p.m.)
>
>
> Review request for cloudstack and Girish Shilamkar.
>
>
> Bugs: CLOUDSTACK-5056
> https://issues.apache.org/jira/browse/CLOUDSTACK-5056
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
>
> 1. The library( remoteSSHClient ) in its current form has some issues. Fixed few of them. As part of that, did clean up of the code.
> 2. Added tcp timeout flag as an additional parameter to help establish the tcp connections and retry if failed. It retries even otherwise failed.
> 3. Renamed few variables and maintained uniform usage and convention.
> 4. Added few new member functions runCommand,createConnection
> 5. Current, way of creating connection and raising exception from constructor along with return was removed and added a new way of handling it.
> 6. Currently, result list was returned for execute command, but we dont know the status of command execution, whether the output contains error or output etc. Now, provided more information in the return with new implementation.
> 7. Added few codes for proper return status.
>
>
> TODO:
> 1. Remove establishing connection from constructor altogether. This will effect the current modules using it.
> 2. Need to change is_server_ssh_ready function for proper and adequate checks, currently the name and functionality is little different. It can add more adequate checks before returning ssh object.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/codes.py b6580d0
> tools/marvin/marvin/integration/lib/utils.py 4d048f0
> tools/marvin/marvin/remoteSSHClient.py fea9b12
>
> Diff: https://reviews.apache.org/r/15263/diff/
>
>
> Testing
> -------
>
>
> File Attachments
> ----------------
>
> Added New File
> https://reviews.apache.org/media/uploaded/files/2013/11/06/0001-Added-fix-for-bug-5056.patch
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15263: Added fix for bug 5056.
Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15263/#review28380
-----------------------------------------------------------
tools/marvin/marvin/integration/lib/utils.py
<https://reviews.apache.org/r/15263/#comment55198>
change keyword 'keyPairFileLocation' to 'keyPairFiles'
tools/marvin/marvin/remoteSSHClient.py
<https://reviews.apache.org/r/15263/#comment55196>
self.keyPairFiles is not defined at this moment we are getting an exception here.
remove this here
tools/marvin/marvin/remoteSSHClient.py
<https://reviews.apache.org/r/15263/#comment55197>
it should be self.keyPairFiles instead of keyPairFiles
again keyPairFiles is not defined at this point and it is resulting in exception.
- SrikanteswaraRao Talluri
On Nov. 6, 2013, 1:44 p.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15263/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2013, 1:44 p.m.)
>
>
> Review request for cloudstack and Girish Shilamkar.
>
>
> Bugs: CLOUDSTACK-5056
> https://issues.apache.org/jira/browse/CLOUDSTACK-5056
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
>
> 1. The library( remoteSSHClient ) in its current form has some issues. Fixed few of them. As part of that, did clean up of the code.
> 2. Added tcp timeout flag as an additional parameter to help establish the tcp connections and retry if failed. It retries even otherwise failed.
> 3. Renamed few variables and maintained uniform usage and convention.
> 4. Added few new member functions runCommand,createConnection
> 5. Current, way of creating connection and raising exception from constructor along with return was removed and added a new way of handling it.
> 6. Currently, result list was returned for execute command, but we dont know the status of command execution, whether the output contains error or output etc. Now, provided more information in the return with new implementation.
> 7. Added few codes for proper return status.
>
>
> TODO:
> 1. Remove establishing connection from constructor altogether. This will effect the current modules using it.
> 2. Need to change is_server_ssh_ready function for proper and adequate checks, currently the name and functionality is little different. It can add more adequate checks before returning ssh object.
>
>
> Diffs
> -----
>
> tools/marvin/marvin/codes.py b6580d0
> tools/marvin/marvin/integration/lib/utils.py 4d048f0
> tools/marvin/marvin/remoteSSHClient.py fea9b12
>
> Diff: https://reviews.apache.org/r/15263/diff/
>
>
> Testing
> -------
>
>
> File Attachments
> ----------------
>
> Added New File
> https://reviews.apache.org/media/uploaded/files/2013/11/06/0001-Added-fix-for-bug-5056.patch
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 15263: Added fix for bug 5056.
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15263/
-----------------------------------------------------------
(Updated Nov. 6, 2013, 1:44 p.m.)
Review request for cloudstack and Girish Shilamkar.
Changes
-------
Added few corrections.
Bugs: CLOUDSTACK-5056
https://issues.apache.org/jira/browse/CLOUDSTACK-5056
Repository: cloudstack-git
Description
-------
1. The library( remoteSSHClient ) in its current form has some issues. Fixed few of them. As part of that, did clean up of the code.
2. Added tcp timeout flag as an additional parameter to help establish the tcp connections and retry if failed. It retries even otherwise failed.
3. Renamed few variables and maintained uniform usage and convention.
4. Added few new member functions runCommand,createConnection
5. Current, way of creating connection and raising exception from constructor along with return was removed and added a new way of handling it.
6. Currently, result list was returned for execute command, but we dont know the status of command execution, whether the output contains error or output etc. Now, provided more information in the return with new implementation.
7. Added few codes for proper return status.
TODO:
1. Remove establishing connection from constructor altogether. This will effect the current modules using it.
2. Need to change is_server_ssh_ready function for proper and adequate checks, currently the name and functionality is little different. It can add more adequate checks before returning ssh object.
Diffs
-----
tools/marvin/marvin/codes.py b6580d0
tools/marvin/marvin/integration/lib/utils.py 4d048f0
tools/marvin/marvin/remoteSSHClient.py fea9b12
Diff: https://reviews.apache.org/r/15263/diff/
Testing
-------
File Attachments (updated)
----------------
Added New File
https://reviews.apache.org/media/uploaded/files/2013/11/06/0001-Added-fix-for-bug-5056.patch
Thanks,
Santhosh Edukulla