)]}'
{"nova/tests/test_powervm.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        self.powervm \u003d fake_get_powervm_operator()"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"class PowerVMDriverTestCase(test.TestCase):"},{"line_number":104,"context_line":"    \"\"\"Unit tests for PowerVM connection calls.\"\"\""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAKX%2F%2F5Gc%3D","line":103,"updated":"2012-07-18 16:36:42.000000000","message":"So, this test case actually isn\u0027t testing anything at all. All it is doing is testing the Fakes you defined above -- and maybe the LPAR class constructor :)\n\nAt a minimum, we\u0027d want to see unit tests that validated the interfaces between the PowerVMDriver, IVMOperator, and LPAR classes. You may think that because most of the actual meat of the calls in IVMOperator are remote SSH or FTP calls, that unit testing these interfaces is impossible. However, you can take a look at the other driver test cases to see how to use the stubout module for stubbing out methods (or closures of methods).\n\nIn particular, see here:\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/test_libvirt.py#L124\n\nFor examples of tracking executable calls.","commit_id":"698a8b15fd5684e9fe9dfc3c05f6a699f48f2c5c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":97,"context_line":"class FakePowerVMDriver(powervm_driver.PowerVMDriver):"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def __init__(self, read_only\u003dFalse):"},{"line_number":100,"context_line":"        self.powervm \u003d fake_get_powervm_operator()"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"class PowerVMDriverTestCase(test.TestCase):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2BrE%3D","line":100,"updated":"2012-07-27 00:37:45.000000000","message":"OK, so here you actually don\u0027t need to do this. You want to test the *actual* PowerVMDriver interface with a *fake* IVMOperator mock object. The best way to do so is like so:\n\n from nova.virt.powervm import operator\n ...\n class PowerVMDriverTestCase(test.TestCase):\n     \"\"\"Unit tests for PowerVM connection calls.\"\"\"\n \n     def setUp(self):\n         super(PowerVMDriverTestCase, self).setUp()\n         self.stubs.Set(operator, \u0027get_powervm_operator\u0027,\n             fake_get_powervm_operator)\n         self.powervm_connection \u003d powervm_driver.PowerVMDriver(read_only\u003dTrue)\n\nYou can then get rid of the FakePowerVMDriver class above entirely. The base unit test case class in Nova\u0027s setUp method adds a set of stubs from the stubout library to the object. You use those stubs to stub out methods you need to -- in this case, the simple method that returns an operator object (faked, in this case). The base test case class\u0027 tearDown() method automatically unsets any stubs back to their original state.","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                        info[\u0027max_mem\u0027] \u003d\u003d 2048 and \\"},{"line_number":130,"context_line":"                        info[\u0027mem\u0027] \u003d\u003d 1024 and \\"},{"line_number":131,"context_line":"                        info[\u0027num_cpu\u0027] \u003d\u003d 2 and \\"},{"line_number":132,"context_line":"                        info[\u0027cpu_time\u0027] \u003d\u003d 939395)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def test_destroy(self):"},{"line_number":135,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2Bqs%3D","line":132,"updated":"2012-07-27 00:37:45.000000000","message":"Please remove all trailing backslashes above. When inside parens, backslashes are not needed. Better yet, break the above into multiple self.assertEqual() statements, like so:\n\n self.assertEqual(power_state.RUNNING, info[\u0027state\u0027])\n self.assertEqual(2048, info[\u0027max_mem\u0027)\n ... etc, etc ...","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        pass"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def copy_image_file(self, sourcePath, remotePath):"},{"line_number":83,"context_line":"        finalPath \u003d \u0027/home/images/rhel62.raw.7e358754160433febd6f3318b7c9e335\u0027"},{"line_number":84,"context_line":"        size \u003d 4294967296"},{"line_number":85,"context_line":"        return finalPath, size"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Qo%3D","line":83,"updated":"2012-08-03 18:36:57.000000000","message":"why is this path so specific? is that going to cause problems during testing in trying to actually find that file?","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"98ecf285a2479115baecf993824257a8a335b0b2","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        pass"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def copy_image_file(self, sourcePath, remotePath):"},{"line_number":83,"context_line":"        finalPath \u003d \u0027/home/images/rhel62.raw.7e358754160433febd6f3318b7c9e335\u0027"},{"line_number":84,"context_line":"        size \u003d 4294967296"},{"line_number":85,"context_line":"        return finalPath, size"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5J8%3D","line":83,"in_reply_to":"AAAALH%2F%2F5Qo%3D","updated":"2012-08-03 20:59:26.000000000","message":"Yeah. It can be whatever path here it is not going to impact on the tests. The intention is to fake that the file was copied to that path...","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c3bd1862df8194e267d4da2161a310827c7ab285","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        self.assertTrue(info[\u0027max_mem\u0027] \u003d\u003d 2048)"},{"line_number":136,"context_line":"        self.assertTrue(info[\u0027mem\u0027] \u003d\u003d 1024)"},{"line_number":137,"context_line":"        self.assertTrue(info[\u0027num_cpu\u0027])"},{"line_number":138,"context_line":"        self.assertTrue(info[\u0027cpu_time\u0027] \u003d\u003d 939395)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def test_destroy(self):"},{"line_number":141,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5VQ%3D","line":138,"updated":"2012-08-03 17:44:03.000000000","message":"These should be assertEqual() calls, as Jay recommended.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"15250afe9b69f890b1573a2ece593c56f53ea5ae","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        image_meta \u003d {}"},{"line_number":147,"context_line":"        image_meta[\u0027id\u0027] \u003d \u0027666\u0027"},{"line_number":148,"context_line":"        self.powervm_connection.spawn(context.get_admin_context(),"},{"line_number":149,"context_line":"                                      self.instance, image_meta)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def test_destroy(self):"},{"line_number":152,"context_line":"        self.powervm_connection.destroy(self.instance, None)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAALH%2F%2FxqM%3D","line":149,"updated":"2012-08-13 16:34:12.000000000","message":"I don\u0027t see an assertions in this test case? What are we attempting to verify here?","commit_id":"953a0435cf56a0fcd798bd189a32e6b60b1d7d08"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"15250afe9b69f890b1573a2ece593c56f53ea5ae","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        self.stubs.Set(operator.PowerVMOperator, \u0027instance_exists\u0027,"},{"line_number":157,"context_line":"                       fake_instance_exists)"},{"line_number":158,"context_line":"        name \u003d self.instance[\u0027name\u0027]"},{"line_number":159,"context_line":"        self.assertFalse(self.powervm_connection.instance_exists(name))"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    def test_get_info(self):"},{"line_number":162,"context_line":"        info \u003d self.powervm_connection.get_info(self.instance)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAALH%2F%2FxqQ%3D","line":159,"updated":"2012-08-13 16:34:12.000000000","message":"Hmm, this isn\u0027t actually testing the destroy() method at all. Instead, it is just testing the return of the fake_instance_exists() method above.  I believe what you need to do is fake out only the calls which would do remote work in the IVM and assert that the other calls are called properly, including the PowerVMOperator\u0027s instance_exists() call.","commit_id":"953a0435cf56a0fcd798bd189a32e6b60b1d7d08"}],"nova/virt/powervm/command.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":15,"context_line":"#    under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\"\"\"PowerVM manager commands.\"\"\""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class BaseCommand(object):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6kU%3D","line":18,"updated":"2012-07-16 21:43:03.000000000","message":"for all of the commands in here, is the command executed in a root user context? If so, it would probably be best to re-use the existing nova.utils.execute() and ssh_execute method(s) with the run_as_root param. That way, you automatically get support for things like exception handling, automatic retries, etc...","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"}],"nova/virt/powervm/common.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":15,"context_line":"#    under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from ftplib import FTP"},{"line_number":18,"context_line":"import os"},{"line_number":19,"context_line":"import paramiko"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9U8%3D","line":17,"updated":"2012-07-11 22:31:21.000000000","message":"Nova\u0027s HACKING guidelines dictate that only modules should be imported, not objects...","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from ftplib import FTP"},{"line_number":18,"context_line":"import os"},{"line_number":19,"context_line":"import paramiko"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"SSH_KEY_HOST \u003d \u0027host\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9Vg%3D","line":19,"updated":"2012-07-11 22:31:21.000000000","message":"Please separate std library imports from 3rd part imports with a newline -- see HACKING document in root for more details.","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"SSH_KEY_HOST \u003d \u0027host\u0027"},{"line_number":23,"context_line":"SSH_KEY_USERNAME \u003d \u0027username\u0027"},{"line_number":24,"context_line":"SSH_KEY_PASSWORD \u003d \u0027password\u0027"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def ssh_connect(connectionDic):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9Uo%3D","line":24,"updated":"2012-07-11 22:31:21.000000000","message":"These seem a bit overboard, but OK :)","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":24,"context_line":"SSH_KEY_PASSWORD \u003d \u0027password\u0027"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def ssh_connect(connectionDic):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Method to connect to remote system using ssh protocol."},{"line_number":30,"context_line":"    It returns an active ssh connection."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9VE%3D","line":27,"updated":"2012-07-11 22:31:21.000000000","message":"You will need to adhere to the variable naming conventions of the Nova project (again, see HACKING guidelines).\n\ns/connectionDic/connection_dic\n\nor:\n\ns/connectionDic/connection\n\nwould be my recommendation, that way if in the future you decide to, say, use an object instead of a dictionary, you don\u0027t have to go changing all your variable names in the code...","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        print \u0027Caught exception: %s: %s\u0027 % (e.__class__, e)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"def ftp_put_command(connectionDic, localPath, remoteDir):"},{"line_number":81,"context_line":"    \"\"\""},{"line_number":82,"context_line":"    Method to transfer a file via ftp"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9UY%3D","line":80,"updated":"2012-07-11 22:31:21.000000000","message":"See comment above about variable naming... no camelCasing for local variables. Only underscore_casing.","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        fileD.close()"},{"line_number":96,"context_line":"        ftp.close()"},{"line_number":97,"context_line":"    except Exception, e:"},{"line_number":98,"context_line":"        print \u0027Caught exception: %s: %s\u0027 % (e.__class__, e)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9UE%3D","line":98,"updated":"2012-07-11 22:31:21.000000000","message":"This needs to be handled properly. You will need to:\n\na) Call LOG.exception() on the error.\nb) re-raise the exception or handle it more gracefully.","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        return self.password"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def port(self):"},{"line_number":46,"context_line":"        return self.port"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def ssh_connect(connection):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6kI%3D","line":46,"updated":"2012-07-16 21:43:03.000000000","message":"You don\u0027t use these methods at all... only the property selectors. Probably easier to remove these methods.","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    :param connection: an active paramiko.SSHClient connection."},{"line_number":73,"context_line":"    :param command: string containing the command to run."},{"line_number":74,"context_line":"    :returns: List -- a list of lines on stdout."},{"line_number":75,"context_line":"    \"\"\""},{"line_number":76,"context_line":"    chan \u003d ssh_connection._transport.open_session()"},{"line_number":77,"context_line":"    chan.exec_command(command)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6jg%3D","line":74,"updated":"2012-07-16 21:43:03.000000000","message":"This isn\u0027t the case.. returns a tuple of (lines, exitcode)","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"1b9c28410114f7cd3812b1be338f84e3c5c8cf96","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    stdout \u003d chan.makefile(\u0027rb\u0027, bufsize)"},{"line_number":80,"context_line":"    return_code \u003d chan.recv_exit_status()"},{"line_number":81,"context_line":"    output \u003d stdout.read().splitlines()"},{"line_number":82,"context_line":"    return output, return_code"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"def ssh_command_as_root(ssh_connection, command):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6B4%3D","line":82,"updated":"2012-07-17 16:26:19.000000000","message":"Agree with Jay, use built ins in nova whenever possible","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    stdout \u003d chan.makefile(\u0027rb\u0027, bufsize)"},{"line_number":80,"context_line":"    return_code \u003d chan.recv_exit_status()"},{"line_number":81,"context_line":"    output \u003d stdout.read().splitlines()"},{"line_number":82,"context_line":"    return output, return_code"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"def ssh_command_as_root(ssh_connection, command):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6jc%3D","line":82,"updated":"2012-07-16 21:43:03.000000000","message":"Instead of doing this directly (and not handling return codes with exceptions), I would advise reusing the nova.utils.ssh_execute() method here...","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    :param connection: an active paramiko.SSHClient connection."},{"line_number":89,"context_line":"    :param command: string containing the command to run."},{"line_number":90,"context_line":"    :returns: List -- a list of lines on stdout."},{"line_number":91,"context_line":"    :raises: PowerVMCommandError"},{"line_number":92,"context_line":"    \"\"\""},{"line_number":93,"context_line":"    chan \u003d ssh_connection._transport.open_session()"},{"line_number":94,"context_line":"    chan.exec_command(\u0027ioscli oem_setup_env\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6j4%3D","line":91,"updated":"2012-07-16 21:43:03.000000000","message":"This does not seem to be the case?","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    chan \u003d ssh_connection._transport.open_session()"},{"line_number":67,"context_line":"    # This command is required to be executed"},{"line_number":68,"context_line":"    # in order to become root."},{"line_number":69,"context_line":"    chan.exec_command(\u0027ioscli oem_setup_env\u0027)"},{"line_number":70,"context_line":"    bufsize \u003d -1"},{"line_number":71,"context_line":"    stdin \u003d chan.makefile(\u0027wb\u0027, bufsize)"},{"line_number":72,"context_line":"    stdout \u003d chan.makefile(\u0027rb\u0027, bufsize)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5QU%3D","line":69,"updated":"2012-08-03 18:36:57.000000000","message":"question: is anything needed to turn off root on the system, or does it automatically drop back to normal afterwards.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"98ecf285a2479115baecf993824257a8a335b0b2","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    chan \u003d ssh_connection._transport.open_session()"},{"line_number":67,"context_line":"    # This command is required to be executed"},{"line_number":68,"context_line":"    # in order to become root."},{"line_number":69,"context_line":"    chan.exec_command(\u0027ioscli oem_setup_env\u0027)"},{"line_number":70,"context_line":"    bufsize \u003d -1"},{"line_number":71,"context_line":"    stdin \u003d chan.makefile(\u0027wb\u0027, bufsize)"},{"line_number":72,"context_line":"    stdout \u003d chan.makefile(\u0027rb\u0027, bufsize)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5KI%3D","line":69,"in_reply_to":"AAAALH%2F%2F5QU%3D","updated":"2012-08-03 20:59:26.000000000","message":"the \u0027exit\u0027 below does it.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    exit_status \u003d chan.recv_exit_status()"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    if exit_status !\u003d -1:"},{"line_number":79,"context_line":"        LOG.debug(_(\u0027Result was %s\u0027) % exit_status)"},{"line_number":80,"context_line":"        if check_exit_code and exit_status !\u003d 0:"},{"line_number":81,"context_line":"            raise nova_exception.ProcessExecutionError(exit_code\u003dexit_status,"},{"line_number":82,"context_line":"                                                       stdout\u003dstdout,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Qc%3D","line":79,"updated":"2012-08-03 18:36:57.000000000","message":"I think that needs to be something more specific about what just failed. A message like this in the logs isn\u0027t going to help anyone with a busy nova system.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"98ecf285a2479115baecf993824257a8a335b0b2","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    exit_status \u003d chan.recv_exit_status()"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    if exit_status !\u003d -1:"},{"line_number":79,"context_line":"        LOG.debug(_(\u0027Result was %s\u0027) % exit_status)"},{"line_number":80,"context_line":"        if check_exit_code and exit_status !\u003d 0:"},{"line_number":81,"context_line":"            raise nova_exception.ProcessExecutionError(exit_code\u003dexit_status,"},{"line_number":82,"context_line":"                                                       stdout\u003dstdout,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5KA%3D","line":79,"in_reply_to":"AAAALH%2F%2F5Qc%3D","updated":"2012-08-03 20:59:26.000000000","message":"This was copied from nova.utils.ssh_execute (actually, it deservers a comment here saying that). A more detailed information about the error goes below if check_exit_code is True.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        f.close()"},{"line_number":106,"context_line":"        ftp.close()"},{"line_number":107,"context_line":"    except Exception:"},{"line_number":108,"context_line":"        LOG.exception(_(\u0027File transfer to PowerVM manager failed\u0027))"},{"line_number":109,"context_line":"        raise exception.PowerVMFileTransferFailed()"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5QQ%3D","line":108,"updated":"2012-08-03 18:36:57.000000000","message":"I\u0027d include the file names that failed the transfer, there might be something useful about what they were to help debugging.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"}],"nova/virt/powervm/constants.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    POWERVM_SHUTDOWN: power_state.SHUTDOWN,"},{"line_number":26,"context_line":"}"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"POWERVM_CPU_INFO \u003d [\u0027ppc64\u0027, \u0027powervm\u0027, \u00273940\u0027]"},{"line_number":29,"context_line":"POWERVM_HYPERVISOR_TYPE \u003d \u0027powervm\u0027"},{"line_number":30,"context_line":"POWERVM_HYPERVISOR_VERSION \u003d \u00277.1\u0027"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5QM%3D","line":28,"updated":"2012-08-03 18:36:57.000000000","message":"this should be a tuple (\u0027ppc64\u0027, \u0027powervm\u0027, \u00273940\u0027) instead of a list, because tuples are immutable, and it\u0027s supposed to be a constant.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"}],"nova/virt/powervm/driver.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from nova.openstack.common import cfg"},{"line_number":21,"context_line":"from nova.openstack.common import log as logging"},{"line_number":22,"context_line":"from nova.virt import driver"},{"line_number":23,"context_line":"from nova.virt.libvirt.driver import HostState"},{"line_number":24,"context_line":"from operator import PowerVMOperator"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9To%3D","line":23,"updated":"2012-07-11 22:31:21.000000000","message":"Please only import modules, not objects, in the Nova project.","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from nova.openstack.common import log as logging"},{"line_number":22,"context_line":"from nova.virt import driver"},{"line_number":23,"context_line":"from nova.virt.libvirt.driver import HostState"},{"line_number":24,"context_line":"from operator import PowerVMOperator"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9Ts%3D","line":24,"updated":"2012-07-11 22:31:21.000000000","message":"Only use absolute imports... no relative imports.","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    @property"},{"line_number":78,"context_line":"    def host_state(self):"},{"line_number":79,"context_line":"        if not self._host_state:"},{"line_number":80,"context_line":"            self._host_state \u003d HostState(self._session)"},{"line_number":81,"context_line":"        return self._host_state"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def init_host(self, host):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9Tc%3D","line":80,"updated":"2012-07-11 22:31:21.000000000","message":"I don\u0027t see where self._session is set?","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        :param block_device_info: Information about block devices to be"},{"line_number":138,"context_line":"                                  attached to the instance."},{"line_number":139,"context_line":"        \"\"\""},{"line_number":140,"context_line":"        instance[\u0027power_state\u0027] \u003d power_state.RUNNING"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def destroy(self, instance, network_info, block_device_info\u003dNone):"},{"line_number":143,"context_line":"        \"\"\"Destroy (shutdown and delete) the specified instance."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9TY%3D","line":140,"updated":"2012-07-11 22:31:21.000000000","message":"This looks like it\u0027s a stub ... this can\u0027t be complete, right?","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"1b9c28410114f7cd3812b1be338f84e3c5c8cf96","unresolved":false,"context_lines":[{"line_number":51,"context_line":"FLAGS.register_opts(powervm_opts)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def get_connection(read_only):"},{"line_number":55,"context_line":"    return PowerVMDriver(read_only)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6Bw%3D","line":54,"updated":"2012-07-17 16:26:19.000000000","message":"this should be removed, not needed with new driver approaches","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":51,"context_line":"FLAGS.register_opts(powervm_opts)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"class PowerVMDriver(driver.ComputeDriver):"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    \"\"\"PowerVM Implementation of Compute Driver.\"\"\""},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAKX%2F%2F5F4%3D","line":54,"updated":"2012-07-18 16:36:42.000000000","message":"Clearly there are a number of critical methods here that are still incomplete -- like spawn(), etc. What is your plan to address these? Are you planning a phased approach or are you wanting to submit a full patch with all functionality complete?","commit_id":"698a8b15fd5684e9fe9dfc3c05f6a699f48f2c5c"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":15,"context_line":"#    under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from decimal import Decimal"},{"line_number":18,"context_line":"import re"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from nova.compute import power_state"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2BqU%3D","line":17,"updated":"2012-07-27 00:37:45.000000000","message":"Please only import modules, not objects (see HACKING guidelines) :)","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    \"\"\"PowerVM Implementation of Compute Driver.\"\"\""},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def __init__(self, read_only\u003dFalse):"},{"line_number":67,"context_line":"        super(PowerVMDriver, self).__init__()"},{"line_number":68,"context_line":"        self.powervm \u003d operator.get_powervm_operator()"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2BqQ%3D","line":66,"updated":"2012-07-27 00:37:45.000000000","message":"I don\u0027t see where this read_only parameter is used? Please remove it if not needed...","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        lpar_instances \u003d self.list_instances()"},{"line_number":100,"context_line":"        if instance_name not in lpar_instances:"},{"line_number":101,"context_line":"            return False"},{"line_number":102,"context_line":"        return True"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def list_instances(self):"},{"line_number":105,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2Bpw%3D","line":102,"updated":"2012-07-27 00:37:45.000000000","message":"You can simplify the above like so:\n\n return instance_name in self.list_instances()\n\nIn addition, if PowerVM has a way of looking up an instance by name, that would be more efficient than getting all the instances and looking through the list of instances :)","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        data \u003d {}"},{"line_number":134,"context_line":"        data[\u0027vcpus\u0027] \u003d total_procs"},{"line_number":135,"context_line":"        data[\u0027vcpus_used\u0027] \u003d total_procs - avail_procs"},{"line_number":136,"context_line":"        data[\u0027cpu_info\u0027] \u003d [\u0027ppc64\u0027, \u0027powervm\u0027, \u00273940\u0027]"},{"line_number":137,"context_line":"        data[\u0027disk_total\u0027] \u003d 500"},{"line_number":138,"context_line":"        data[\u0027disk_used\u0027] \u003d 40"},{"line_number":139,"context_line":"        data[\u0027disk_available\u0027] \u003d 450"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2Bps%3D","line":136,"updated":"2012-07-27 00:37:45.000000000","message":"Is this really a hard-coded value? If so, better to use a constant (in your constants.py file). Here and below for any real constants.","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        file_path \u003d \u0027%s/%s.%s.gz\u0027 % (FLAGS.powervm_img_local_path,"},{"line_number":164,"context_line":"                                     image_meta[\u0027name\u0027],"},{"line_number":165,"context_line":"                                     image_meta[\u0027disk_format\u0027])"},{"line_number":166,"context_line":"        LOG.debug(_(\"Fetching image \u0027%s\u0027 from glance\") % image_meta[\u0027name\u0027])"},{"line_number":167,"context_line":"        images.fetch_to_raw(context, image_meta[\u0027id\u0027], file_path,"},{"line_number":168,"context_line":"                            instance[\u0027user_id\u0027],"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2Bpk%3D","line":165,"updated":"2012-07-27 00:37:45.000000000","message":"Instead of creating filepaths manually like this, the Pythonic way to do this is:\n\n import os\n ...\n file_name \u003d \u0027.\u0027.join([image_meta[\u0027name\u0027], image_meta[\u0027disk_format\u0027], \u0027gz\u0027])\n file_path \u003d os.path.join(FLAGS.powervm_img_local_path, file_name)\n\nThe above is compatible across platforms.\n\nAlso, image_meta[\u0027name\u0027] is an *optional* attribute. It could be None, so it would probably be better to use image_meta[\u0027id\u0027], which is unique and mandatory. If you used id, you wouldn\u0027t need to store the disk_format as part of the name, either...","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        file_path \u003d \u0027%s/%s.%s.gz\u0027 % (FLAGS.powervm_img_local_path,"},{"line_number":164,"context_line":"                                     image_meta[\u0027name\u0027],"},{"line_number":165,"context_line":"                                     image_meta[\u0027disk_format\u0027])"},{"line_number":166,"context_line":"        LOG.debug(_(\"Fetching image \u0027%s\u0027 from glance\") % image_meta[\u0027name\u0027])"},{"line_number":167,"context_line":"        images.fetch_to_raw(context, image_meta[\u0027id\u0027], file_path,"},{"line_number":168,"context_line":"                            instance[\u0027user_id\u0027],"},{"line_number":169,"context_line":"                            project_id\u003dinstance[\u0027project_id\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2Bpg%3D","line":166,"updated":"2012-07-27 00:37:45.000000000","message":"See previous comment about name possibly being None...","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        mem_min \u003d mem - 512"},{"line_number":179,"context_line":"        if mem_min \u003c 1:"},{"line_number":180,"context_line":"            mem_min \u003d mem"},{"line_number":181,"context_line":"        mem_max \u003d mem + 1024"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        # CPU"},{"line_number":184,"context_line":"        cpus \u003d instance[\u0027vcpus\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2BpA%3D","line":181,"updated":"2012-07-27 00:37:45.000000000","message":"Best to put constant numbers like the above in constants. Something like this?\n\n MIN_CPUS \u003d 1\n MIN_GUEST_MEM \u003d 512\n BURST_GUEST_MEM \u003d 1024\n\n mem_min \u003d min(mem, MIN_GUEST_MEM)\n mem_max \u003d mem + BURST_GUEST_MEM\n\n cpus_min \u003d min(cpus, MIN_CPUS)","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":217,"context_line":"        self.powervm.copy_file_to_device(remote_file_name, disk_name)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        LOG.debug(_(\"Activating the LPAR instance \u0027%s\u0027\") % inst_name)"},{"line_number":220,"context_line":"        self.powervm.start_lpar(inst_name)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    def destroy(self, instance, network_info, block_device_info\u003dNone):"},{"line_number":223,"context_line":"        \"\"\"Destroy (shutdown and delete) the specified instance."}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAALH%2F%2F%2Bo4%3D","line":220,"updated":"2012-07-27 00:37:45.000000000","message":"I\u0027d like to see more robust handling of any exceptions that may occur. For instance, the base compute driver abstract class method says this:\n\n\n        Once this successfully completes, the instance should be\n        running (power_state.RUNNING).\n\n        If this fails, any partial instance should be completely\n        cleaned up, and the virtualization platform should be in the state\n        that it was before this call began.\n\nAt a minimum, I\u0027d like to see how the cleanup of partial boots is handled...","commit_id":"e58407e7ea1656f5a763c8bc62d213e3beab5bcd"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        # and cleanup the inconsistencies."},{"line_number":84,"context_line":"        for db_instance in instances:"},{"line_number":85,"context_line":"            task_state \u003d db_instance[\u0027task_state\u0027]"},{"line_number":86,"context_line":"            if db_instance[\u0027name\u0027] not in self.list_instances() \\"},{"line_number":87,"context_line":"                and task_state \u003d\u003d task_states.DELETING:"},{"line_number":88,"context_line":"                db.instance_update(context, db_instance[\u0027uuid\u0027],"},{"line_number":89,"context_line":"                                   {\u0027vm_state\u0027: vm_states.DELETED,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5P4%3D","line":86,"updated":"2012-08-03 18:36:57.000000000","message":"Agreed, pull up the deleting check to an outer conditional. It also makes it more clear.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c3bd1862df8194e267d4da2161a310827c7ab285","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        # and cleanup the inconsistencies."},{"line_number":84,"context_line":"        for db_instance in instances:"},{"line_number":85,"context_line":"            task_state \u003d db_instance[\u0027task_state\u0027]"},{"line_number":86,"context_line":"            if db_instance[\u0027name\u0027] not in self.list_instances() \\"},{"line_number":87,"context_line":"                and task_state \u003d\u003d task_states.DELETING:"},{"line_number":88,"context_line":"                db.instance_update(context, db_instance[\u0027uuid\u0027],"},{"line_number":89,"context_line":"                                   {\u0027vm_state\u0027: vm_states.DELETED,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Uo%3D","line":86,"updated":"2012-08-03 17:44:03.000000000","message":"I haven\u0027t looked at the implementation of self._powervm, but is this likely to be a heavy operation? Any reason not to grab a list of instances before the loop instead of each time through?","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        return len(self.list_instances())"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def instance_exists(self, instance_name):"},{"line_number":101,"context_line":"        lpar_instance \u003d self._powervm.instance_exists(instance_name)"},{"line_number":102,"context_line":"        return (lpar_instance is not None)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def list_instances(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5P0%3D","line":101,"updated":"2012-08-03 18:36:57.000000000","message":"Agreed. exists should be a boolean, there should be a get function for this. You can always build exists out of the get function, but for clarity they shouldn\u0027t be used interchanably","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c3bd1862df8194e267d4da2161a310827c7ab285","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        return len(self.list_instances())"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def instance_exists(self, instance_name):"},{"line_number":101,"context_line":"        lpar_instance \u003d self._powervm.instance_exists(instance_name)"},{"line_number":102,"context_line":"        return (lpar_instance is not None)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def list_instances(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5UQ%3D","line":101,"updated":"2012-08-03 17:44:03.000000000","message":"Hmm instance_exists() returns an instance? That seems like it should return a boolean, based on the name...","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"98ecf285a2479115baecf993824257a8a335b0b2","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        return len(self.list_instances())"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def instance_exists(self, instance_name):"},{"line_number":101,"context_line":"        lpar_instance \u003d self._powervm.instance_exists(instance_name)"},{"line_number":102,"context_line":"        return (lpar_instance is not None)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def list_instances(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5R8%3D","line":101,"in_reply_to":"AAAALH%2F%2F5UQ%3D","updated":"2012-08-03 20:59:26.000000000","message":"I thought it was returning True/False because of the (lpar_instance is not None) which actually is a test... maybe that\u0027s python misunderstanding?","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c3bd1862df8194e267d4da2161a310827c7ab285","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def instance_exists(self, instance_name):"},{"line_number":101,"context_line":"        lpar_instance \u003d self._powervm.instance_exists(instance_name)"},{"line_number":102,"context_line":"        return (lpar_instance is not None)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def list_instances(self):"},{"line_number":105,"context_line":"        return self._powervm.list_instances()"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Uc%3D","line":102,"updated":"2012-08-03 17:44:03.000000000","message":"Parens are not required here","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                raise utils.LoopingCallDone"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        timer \u003d utils.LoopingCall(_wait_for_boot)"},{"line_number":155,"context_line":"        return timer.start(interval\u003d0.5).wait()"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def destroy(self, instance, network_info, block_device_info\u003dNone):"},{"line_number":158,"context_line":"        \"\"\"Destroy (shutdown and delete) the specified instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Po%3D","line":155,"updated":"2012-08-03 18:36:57.000000000","message":"Is it possible for us to get into a state where this never returns? given that we\u0027re running on eventlet that would start starving us out on other opperations?\n\nIs there any reason why we need to have the status synchronously.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c3bd1862df8194e267d4da2161a310827c7ab285","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                raise utils.LoopingCallDone"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        timer \u003d utils.LoopingCall(_wait_for_boot)"},{"line_number":155,"context_line":"        return timer.start(interval\u003d0.5).wait()"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def destroy(self, instance, network_info, block_device_info\u003dNone):"},{"line_number":158,"context_line":"        \"\"\"Destroy (shutdown and delete) the specified instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5So%3D","line":155,"updated":"2012-08-03 17:44:03.000000000","message":"So, what do we do if it times out and the LPAR never reaches the proper state? Shouldn\u0027t we attempt some cleanup?","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":157,"context_line":"    def destroy(self, instance, network_info, block_device_info\u003dNone):"},{"line_number":158,"context_line":"        \"\"\"Destroy (shutdown and delete) the specified instance.\"\"\""},{"line_number":159,"context_line":"        if self.instance_exists(instance[\u0027name\u0027]):"},{"line_number":160,"context_line":"            self._powervm.destroy(instance[\u0027name\u0027])"},{"line_number":161,"context_line":"        else:"},{"line_number":162,"context_line":"            LOG.warn(_(\u0027During destroy, instance was not found\u0027),"},{"line_number":163,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Pg%3D","line":160,"updated":"2012-08-03 18:36:57.000000000","message":"Do we know this always succeeds?","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"}],"nova/virt/powervm/exception.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    message \u003d _(\"No space left on any volume group\")"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"class PowerVMLPARAttributeNotFound(Exception):"},{"line_number":41,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6jY%3D","line":40,"updated":"2012-07-16 21:43:03.000000000","message":"Why not NovaException?","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"}],"nova/virt/powervm/lpar.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the"},{"line_number":14,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":15,"context_line":"#    under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"import shlex"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9TU%3D","line":16,"updated":"2012-07-11 22:31:21.000000000","message":"Please add docstrings to this module and the classes in it that indicate the purpose of the classes and the module. To the reader of the code, it\u0027s not possible to understand what an LPAR is.","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import shlex"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"class Base(object):"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    def __init__(self):"},{"line_number":23,"context_line":"        self.attributes \u003d {}"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9TA%3D","line":20,"updated":"2012-07-11 22:31:21.000000000","message":"This class is not necessary (and neither is Profile IMHO). Just use Python\u0027s collections library and things like MutableMapping class as the base class to inherit from, and use the standard dict way of accessing attributes:\n\nhttp://docs.python.org/library/collections.html#collections.MutableMapping","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        try:"},{"line_number":27,"context_line":"            return self.attributes[attribute]"},{"line_number":28,"context_line":"        except Exception, e:"},{"line_number":29,"context_line":"            print \u0027Caught exception: %s: %s\u0027 % (e.__class__, e)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def set_attrib(self, attribute, value):"},{"line_number":32,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9TQ%3D","line":29,"updated":"2012-07-11 22:31:21.000000000","message":"All these exceptions need to be properly logged and handled.","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            print \u0027Caught exception: %s: %s\u0027 % (e.__class__, e)"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    @staticmethod"},{"line_number":236,"context_line":"    def load_from_conf_data(conf_data):"},{"line_number":237,"context_line":"        try:"},{"line_number":238,"context_line":"            # config_data can contain comma separated values within"},{"line_number":239,"context_line":"            # double quotes, example: virtual_serial_adapters"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9S4%3D","line":236,"updated":"2012-07-11 22:31:21.000000000","message":"Do not use this. Replace with openstack.common.iniparser, which already handles this kind of stuff for you :)\n\nhttps://github.com/openstack/openstack-common/blob/master/openstack/common/iniparser.py","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"1b9c28410114f7cd3812b1be338f84e3c5c8cf96","unresolved":false,"context_lines":[{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def load_from_conf_data(conf_data):"},{"line_number":32,"context_line":"    # config_data can contain comma separated values within"},{"line_number":33,"context_line":"    # double quotes, example: virtual_serial_adapters"},{"line_number":34,"context_line":"    # and virtual_scsi_adapters attributes. So can\u0027t simply"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6Bo%3D","line":31,"updated":"2012-07-17 16:26:19.000000000","message":"Where is this intended to be used?","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"e0cb5a6e5a6f3ab03d004b94da192179e2887522","unresolved":false,"context_lines":[{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def load_from_conf_data(conf_data):"},{"line_number":32,"context_line":"    # config_data can contain comma separated values within"},{"line_number":33,"context_line":"    # double quotes, example: virtual_serial_adapters"},{"line_number":34,"context_line":"    # and virtual_scsi_adapters attributes. So can\u0027t simply"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F54k%3D","line":31,"in_reply_to":"AAAAKX%2F%2F6Bo%3D","updated":"2012-07-18 01:50:22.000000000","message":"It is used in the operator.BaseOperator.get_lpar method line: 65","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":163,"context_line":"    def __delitem__(self, key):"},{"line_number":164,"context_line":"        if key in self.attributes:"},{"line_number":165,"context_line":"            del self.attributes[key]"},{"line_number":166,"context_line":"        elif key in self.profile:"},{"line_number":167,"context_line":"            del self.profile[key]"},{"line_number":168,"context_line":"        else:"},{"line_number":169,"context_line":"            LOG.exception(\u0027LPAR attribute not found\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6i8%3D","line":166,"updated":"2012-07-16 21:43:03.000000000","message":"What this means is that is a key is in both attributes and profile, only the key in attributes will be deleted. I\u0027m pretty sure that\u0027s not what you want. :)","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"e0cb5a6e5a6f3ab03d004b94da192179e2887522","unresolved":false,"context_lines":[{"line_number":163,"context_line":"    def __delitem__(self, key):"},{"line_number":164,"context_line":"        if key in self.attributes:"},{"line_number":165,"context_line":"            del self.attributes[key]"},{"line_number":166,"context_line":"        elif key in self.profile:"},{"line_number":167,"context_line":"            del self.profile[key]"},{"line_number":168,"context_line":"        else:"},{"line_number":169,"context_line":"            LOG.exception(\u0027LPAR attribute not found\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F52Y%3D","line":166,"in_reply_to":"AAAAKX%2F%2F6i8%3D","updated":"2012-07-18 01:50:22.000000000","message":"Not all keys are in both. Actually, most of keys differ the only keys that are in both are: name and lpar_id if I didn\u0027t forget any other.","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":173,"context_line":"        if key in self.attributes:"},{"line_number":174,"context_line":"            return self.attributes[key]"},{"line_number":175,"context_line":"        elif key in self.profile:"},{"line_number":176,"context_line":"            return self.profile[key]"},{"line_number":177,"context_line":"        else:"},{"line_number":178,"context_line":"            LOG.exception(\u0027LPAR attribute not found\u0027)"},{"line_number":179,"context_line":"            raise exception.PowerVMLPARAttributeNotFound()"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6i0%3D","line":176,"updated":"2012-07-16 21:43:03.000000000","message":"Similarly, if the same key is in attributes AND profile, the profile value for key will never be returned because of the above. Is this what you want?","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                continue"},{"line_number":198,"context_line":"            if key in self.profile:"},{"line_number":199,"context_line":"                conf_data.append(\u0027%s\u003d%s\u0027 % (key, value))"},{"line_number":200,"context_line":"        return \u0027,\u0027.join(conf_data)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6gI%3D","line":200,"updated":"2012-07-16 21:43:03.000000000","message":"After looking more into this code, I\u0027m convinced that inheriting from collections.MutableMapping is actually not the correct way to address this. Instead, I think something like this would be better:\n\nhttp://paste.openstack.org/show/19450/","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"e0cb5a6e5a6f3ab03d004b94da192179e2887522","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                continue"},{"line_number":198,"context_line":"            if key in self.profile:"},{"line_number":199,"context_line":"                conf_data.append(\u0027%s\u003d%s\u0027 % (key, value))"},{"line_number":200,"context_line":"        return \u0027,\u0027.join(conf_data)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F52M%3D","line":200,"in_reply_to":"AAAAKX%2F%2F6gI%3D","updated":"2012-07-18 01:50:22.000000000","message":"Yeah. It looks even more elegant.","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"966828fda15ca4eed37c0d7f9d7d0761e809bfa5","unresolved":false,"context_lines":[{"line_number":140,"context_line":"    def to_string(self, exclude_attribs\u003d[]):"},{"line_number":141,"context_line":"        conf_data \u003d []"},{"line_number":142,"context_line":"        for (key, value) in self.all_attrs.items():"},{"line_number":143,"context_line":"            print \u0027key: %s, value %s\u0027 % (key, value)"},{"line_number":144,"context_line":"            if key in exclude_attribs:"},{"line_number":145,"context_line":"                continue"},{"line_number":146,"context_line":"            conf_data.append(\u0027%s\u003d%s\u0027 % (key, value))"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAKX%2F%2F5Fo%3D","line":143,"updated":"2012-07-18 16:36:42.000000000","message":"No print statements please :) If you need to debug, please either LOG.debug() something here or simply remove the temporary debug statements...","commit_id":"698a8b15fd5684e9fe9dfc3c05f6a699f48f2c5c"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def load_from_conf_data(conf_data):"},{"line_number":31,"context_line":"    # config_data can contain comma separated values within"},{"line_number":32,"context_line":"    # double quotes, example: virtual_serial_adapters"},{"line_number":33,"context_line":"    # and virtual_scsi_adapters attributes. So can\u0027t simply"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5PY%3D","line":30,"updated":"2012-08-03 18:36:57.000000000","message":"which config file is this loading from? If this a non openstack one, it\u0027s probably good to explain what this config file is, and why we have our own parser in here.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"}],"nova/virt/powervm/operator.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from nova.virt.powervm.common import ssh_interactive"},{"line_number":24,"context_line":"from nova.virt.powervm.lpar import LPAR"},{"line_number":25,"context_line":"import os"},{"line_number":26,"context_line":"import re"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"FLAGS \u003d flags.FLAGS"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9Sw%3D","line":26,"updated":"2012-07-11 22:31:21.000000000","message":"These imports need to align with HACKING guidelines please","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d3bb429b1b11e71299ea224eb13e411762b07e9","unresolved":false,"context_lines":[{"line_number":384,"context_line":"        \"\"\""},{"line_number":385,"context_line":"        try:"},{"line_number":386,"context_line":"            conn \u003d self.__connection if conn is None else conn"},{"line_number":387,"context_line":"            output \u003d ssh_command(conn, command + \u0027;echo $?\u0027)"},{"line_number":388,"context_line":"            return_code \u003d output.pop()"},{"line_number":389,"context_line":"            if int(return_code) \u003d\u003d CMD_SUCCESS:"},{"line_number":390,"context_line":"                return output"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAKX%2F%2F9Sc%3D","line":387,"updated":"2012-07-11 22:31:21.000000000","message":"Why would SSH be needed to execute a command on the host? nova-compute (which will be running this virt module) is already on the host...","commit_id":"23675087715743ecb669223f73bd0a9542c9ba17"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"1b9c28410114f7cd3812b1be338f84e3c5c8cf96","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        :param resource_type: the type of resources to list"},{"line_number":58,"context_line":"        :returns: LPAR object"},{"line_number":59,"context_line":"        \"\"\""},{"line_number":60,"context_line":"        command \u003d self.command.lssyscfg(\u0027-r %s --filter \"lpar_names\u003d%s\"\u0027"},{"line_number":61,"context_line":"                                        % (resource_type, instance_name))"},{"line_number":62,"context_line":"        output \u003d self.run_command(command)"},{"line_number":63,"context_line":"        if not output:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6Ck%3D","line":60,"updated":"2012-07-17 16:26:19.000000000","message":"It seems like bad form to be constantly using a local variable that\u0027s the same name as the instance variable, leads to some confusion.","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f9e827c13807f230181aa15900266e38f221a661","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        if not found_vg:"},{"line_number":188,"context_line":"            raise exception.PowerVMNoSpaceLeftOnVolumeGroup()"},{"line_number":189,"context_line":"            LOG.exception(_(\u0027Could not create logical volume. \u0027"},{"line_number":190,"context_line":"                            \u0027No space left on any volume group.\u0027))"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        command \u003d self.command.mklv(\u0027%s %sB\u0027 % (found_vg, size / 512))"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6f4%3D","line":189,"updated":"2012-07-16 21:43:03.000000000","message":"Ooops. Need to log the error before you raise it :)","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"1b9c28410114f7cd3812b1be338f84e3c5c8cf96","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        :param source_path: path to input source file"},{"line_number":208,"context_line":"        :param device: output device name"},{"line_number":209,"context_line":"        \"\"\""},{"line_number":210,"context_line":"        command \u003d \u0027dd if\u003d%s of\u003d/dev/%s bs\u003d1024k\u0027 % (source_path, device)"},{"line_number":211,"context_line":"        self.run_command_as_root(command)"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    def copy_image_file(self, source_path, remote_path):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAKX%2F%2F6C8%3D","line":210,"updated":"2012-07-17 16:26:19.000000000","message":"I\u0027m concerned about the security implications here, as with some tricky manipulation of variables you could over write just about anything in the fs. Is there sanitation somewhere?","commit_id":"4ffca6a3df87ff06bf4de8ffb5e1df6646805f30"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c3bd1862df8194e267d4da2161a310827c7ab285","unresolved":false,"context_lines":[{"line_number":77,"context_line":"                \u0027num_cpu\u0027: lpar_instance[\u0027max_procs\u0027],"},{"line_number":78,"context_line":"                \u0027cpu_time\u0027: lpar_instance[\u0027uptime\u0027]}"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def instance_exists(self, instance_name, raise_exception\u003dFalse):"},{"line_number":81,"context_line":"        \"\"\"Check whether or not the instance exists.\"\"\""},{"line_number":82,"context_line":"        lpar_instance \u003d self._operator.get_lpar(instance_name)"},{"line_number":83,"context_line":"        if lpar_instance is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Tw%3D","line":80,"updated":"2012-08-03 17:44:03.000000000","message":"This really seems like it should return a boolean. Otherwise it should be called get_instance(), right?","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"98ecf285a2479115baecf993824257a8a335b0b2","unresolved":false,"context_lines":[{"line_number":77,"context_line":"                \u0027num_cpu\u0027: lpar_instance[\u0027max_procs\u0027],"},{"line_number":78,"context_line":"                \u0027cpu_time\u0027: lpar_instance[\u0027uptime\u0027]}"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def instance_exists(self, instance_name, raise_exception\u003dFalse):"},{"line_number":81,"context_line":"        \"\"\"Check whether or not the instance exists.\"\"\""},{"line_number":82,"context_line":"        lpar_instance \u003d self._operator.get_lpar(instance_name)"},{"line_number":83,"context_line":"        if lpar_instance is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Jk%3D","line":80,"in_reply_to":"AAAALH%2F%2F5Tw%3D","updated":"2012-08-03 20:59:26.000000000","message":"Since this is not the instance_exists on PowerVMDriver... It is returning the lpar_instance by design to save another self._operator.get_lpar call.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c3bd1862df8194e267d4da2161a310827c7ab285","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        if lpar_instance is not None:"},{"line_number":84,"context_line":"            return lpar_instance"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        if raise_exception:"},{"line_number":87,"context_line":"            LOG.exception(_(\"LPAR instance \u0027%s\u0027 not found\") % instance_name)"},{"line_number":88,"context_line":"            raise exception.PowerVMLPARInstanceNotFound("},{"line_number":89,"context_line":"                                                instance_name\u003dinstance_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5To%3D","line":86,"updated":"2012-08-03 17:44:03.000000000","message":"Again, if this was called get_instance(), then I\u0027d expect it to return an instance, or raise an exception. If it\u0027s called instance_exists(), I\u0027d expect it to return either True or False.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"98ecf285a2479115baecf993824257a8a335b0b2","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        if lpar_instance is not None:"},{"line_number":84,"context_line":"            return lpar_instance"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        if raise_exception:"},{"line_number":87,"context_line":"            LOG.exception(_(\"LPAR instance \u0027%s\u0027 not found\") % instance_name)"},{"line_number":88,"context_line":"            raise exception.PowerVMLPARInstanceNotFound("},{"line_number":89,"context_line":"                                                instance_name\u003dinstance_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Jg%3D","line":86,"in_reply_to":"AAAALH%2F%2F5To%3D","updated":"2012-08-03 20:59:26.000000000","message":"Agreed. Better to rename the method.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c3bd1862df8194e267d4da2161a310827c7ab285","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        lpar_instances \u003d self._operator.list_lpar_instances()"},{"line_number":98,"context_line":"        instances \u003d [instance for instance in lpar_instances"},{"line_number":99,"context_line":"                     if re.search(r\u0027^instance-[0-9]{8}$\u0027, instance)]"},{"line_number":100,"context_line":"        return instances"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def get_host_stats(self, refresh\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Tg%3D","line":99,"updated":"2012-08-03 17:44:03.000000000","message":"Is this to filter out LPARs that may have been created via a different mechanism (other than openstack)? If so, this seems like it could be fragile, if I\u0027m a direct user of the IVM and I created one called instance-12345678 by chance...","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"98ecf285a2479115baecf993824257a8a335b0b2","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        lpar_instances \u003d self._operator.list_lpar_instances()"},{"line_number":98,"context_line":"        instances \u003d [instance for instance in lpar_instances"},{"line_number":99,"context_line":"                     if re.search(r\u0027^instance-[0-9]{8}$\u0027, instance)]"},{"line_number":100,"context_line":"        return instances"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def get_host_stats(self, refresh\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5I8%3D","line":99,"in_reply_to":"AAAALH%2F%2F5Tg%3D","updated":"2012-08-03 20:59:26.000000000","message":"I agree. However I can\u0027t think a better way to handle that for now... maybe we can write a comment here explaining the assumption.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        return self._host_stats"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def _update_host_stats(self):"},{"line_number":109,"context_line":"        total_mem, avail_mem \u003d self._operator.get_memory_info()"},{"line_number":110,"context_line":"        total_procs, avail_procs \u003d self._operator.get_cpu_info()"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        # Note: disk avail information is not accurate. The value"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5PE%3D","line":109,"updated":"2012-08-03 18:36:57.000000000","message":"this is a personal preference thing, but it seems like these should come back as dicts instead of tuples, so their can\u0027t be a possitional error. So get back a dict with mem[\u0027total\u0027] and mem[\u0027avail\u0027] assigned.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            mem_min \u003d mem"},{"line_number":152,"context_line":"        mem_max \u003d mem + constants.POWERVM_MAX_MEM"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        # CPU"},{"line_number":155,"context_line":"        cpus \u003d instance[\u0027vcpus\u0027]"},{"line_number":156,"context_line":"        cpus_min \u003d cpus - 1"},{"line_number":157,"context_line":"        if cpus_min \u003c 1:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Ow%3D","line":154,"updated":"2012-08-03 18:36:57.000000000","message":"I don\u0027t understand what\u0027s going on here, or why this manipulation is happening. I think this needs some explanation about why if vcpus \u003d 3 comes in min is 2 and max is 4.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        disk_name \u003d self._operator.get_disk_name_by_vhost(vhost)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        LOG.debug(_(\"Shutting down the instance \u0027%s\u0027\") % instance_name)"},{"line_number":223,"context_line":"        self._operator.stop_lpar(instance_name)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        if disk_name:"},{"line_number":226,"context_line":"            LOG.debug(_(\"Removing the logical volume \u0027%s\u0027\") % disk_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Oo%3D","line":223,"updated":"2012-08-03 18:36:57.000000000","message":"is there a failure case?","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        if disk_name:"},{"line_number":226,"context_line":"            LOG.debug(_(\"Removing the logical volume \u0027%s\u0027\") % disk_name)"},{"line_number":227,"context_line":"            self._operator.remove_logical_volume(disk_name)"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"        LOG.debug(_(\"Deleting the LPAR instance \u0027%s\u0027\") % instance_name)"},{"line_number":230,"context_line":"        self._operator.remove_lpar(instance_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Ok%3D","line":227,"updated":"2012-08-03 18:36:57.000000000","message":"are we sure the lpar is stopped before this is pulled?","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c3bd1862df8194e267d4da2161a310827c7ab285","unresolved":false,"context_lines":[{"line_number":379,"context_line":"        if (size % 1048576) !\u003d 0:"},{"line_number":380,"context_line":"            megabyteSize \u003d int(size / 1048576) + 1"},{"line_number":381,"context_line":"        else:"},{"line_number":382,"context_line":"            megabyteSize \u003d size / 1048576"},{"line_number":383,"context_line":""},{"line_number":384,"context_line":"        # Search for a volume group with enough free space for"},{"line_number":385,"context_line":"        # the new logical volume."}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5TI%3D","line":382,"updated":"2012-08-03 17:44:03.000000000","message":"Hmm, I think this is deserving of a comment :)","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"06686920a0170887425f2579482466d236d8816a","unresolved":false,"context_lines":[{"line_number":379,"context_line":"        if (size % 1048576) !\u003d 0:"},{"line_number":380,"context_line":"            megabyteSize \u003d int(size / 1048576) + 1"},{"line_number":381,"context_line":"        else:"},{"line_number":382,"context_line":"            megabyteSize \u003d size / 1048576"},{"line_number":383,"context_line":""},{"line_number":384,"context_line":"        # Search for a volume group with enough free space for"},{"line_number":385,"context_line":"        # the new logical volume."}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F5Oc%3D","line":382,"updated":"2012-08-03 18:36:57.000000000","message":"agreed, I\u0027m confused by the logic here. Also is there not a unit conversion in stdlib for it","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":5174,"name":"Mauro Sergio Martins Rodrigues","email":"maurosr@linux.vnet.ibm.com","username":"maurosr"},"change_message_id":"836a3c9aeab4ba191118cbdeb38dbbb1ba6af36a","unresolved":false,"context_lines":[{"line_number":555,"context_line":"        :param command: String with the command to run."},{"line_number":556,"context_line":"        \"\"\""},{"line_number":557,"context_line":"        stdout, stderr \u003d utils.ssh_execute(self._connection, cmd,"},{"line_number":558,"context_line":"                                           check_exit_code\u003dcheck_exit_code)"},{"line_number":559,"context_line":"        return stdout.strip().splitlines()"},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"    def run_command_as_root(self, command, check_exit_code\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F4Ew%3D","line":558,"updated":"2012-08-06 13:48:57.000000000","message":"What if the command failed to execute? Shouldn\u0027t that be raised and/or put into log file? \nI\u0027m not that much familiarized with ivms, but let\u0027s consider a situation where you created a disk, and someone removed your disk (why someone would do that I really don\u0027t know), then you tried to remove your disk by remove_disk, it will fail.\n\nMaybe we just need to check exit code and raise this error? Maybe the commands themselves should check their own success/fail (this look a better solution to me)","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":4601,"name":"Tiago Mello","email":"tmello@linux.vnet.ibm.com","username":"tmello"},"change_message_id":"e2658a589c16a67b78bfb6f3c91ef9967aa7c9a0","unresolved":false,"context_lines":[{"line_number":555,"context_line":"        :param command: String with the command to run."},{"line_number":556,"context_line":"        \"\"\""},{"line_number":557,"context_line":"        stdout, stderr \u003d utils.ssh_execute(self._connection, cmd,"},{"line_number":558,"context_line":"                                           check_exit_code\u003dcheck_exit_code)"},{"line_number":559,"context_line":"        return stdout.strip().splitlines()"},{"line_number":560,"context_line":""},{"line_number":561,"context_line":"    def run_command_as_root(self, command, check_exit_code\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAALH%2F%2F3OA%3D","line":558,"in_reply_to":"AAAALH%2F%2F4Ew%3D","updated":"2012-08-07 19:25:03.000000000","message":"Mauro, utils.ssh_execute already does the check in case the command fails.","commit_id":"db05440a23b13b1d10750400163fa8e8c46447b7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"15250afe9b69f890b1573a2ece593c56f53ea5ae","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                cpus_min \u003d cpus"},{"line_number":171,"context_line":"            cpus_max \u003d cpus + constants.POWERVM_MAX_CPUS"},{"line_number":172,"context_line":"            cpus_units_min \u003d decimal.Decimal(cpus_min) / decimal.Decimal(10)"},{"line_number":173,"context_line":"            cpus_units \u003d decimal.Decimal(cpus) / decimal.Decimal(10)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"            try:"},{"line_number":176,"context_line":"                # Network"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAALH%2F%2Fxpk%3D","line":173,"updated":"2012-08-13 16:34:12.000000000","message":"Did you see my previous comments about using min() here to clean things up? See the comment on line 181 here: https://review.openstack.org/#/c/9666/5/nova/virt/powervm/driver.py","commit_id":"953a0435cf56a0fcd798bd189a32e6b60b1d7d08"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"15250afe9b69f890b1573a2ece593c56f53ea5ae","unresolved":false,"context_lines":[{"line_number":273,"context_line":""},{"line_number":274,"context_line":"            LOG.debug(_(\"Deleting the LPAR instance \u0027%s\u0027\") % instance_name)"},{"line_number":275,"context_line":"            self._operator.remove_lpar(instance_name)"},{"line_number":276,"context_line":"        except Exception:"},{"line_number":277,"context_line":"            LOG.exception(_(\"PowerVM instance cleanup failed\"))"},{"line_number":278,"context_line":"            raise exception.PowerVMLPARInstanceCleanupFailed("},{"line_number":279,"context_line":"                                                  instance_name\u003dinstance_name)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAALH%2F%2Fxpc%3D","line":276,"updated":"2012-08-13 16:34:12.000000000","message":"We are throwing away potentially valuable information here about what failed during cleanup. Perhaps it is better to log the exception information?","commit_id":"953a0435cf56a0fcd798bd189a32e6b60b1d7d08"},{"author":{"_account_id":67,"name":"Vish Ishaya","email":"vishvananda@gmail.com","username":"vishvananda"},"change_message_id":"f023b672fd127930921cbd9f186c1a5c5b0bd921","unresolved":false,"context_lines":[{"line_number":340,"context_line":""},{"line_number":341,"context_line":"        :param instance_name: LPAR instance name"},{"line_number":342,"context_line":"        \"\"\""},{"line_number":343,"context_line":"        cmd \u003d \\"},{"line_number":344,"context_line":"            self.command.chsysstate(\u0027-r lpar -o shutdown --immed -n %s\u0027"},{"line_number":345,"context_line":"                                    % instance_name)"},{"line_number":346,"context_line":"        self.run_command(cmd)"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAALH%2F%2FwYo%3D","line":343,"updated":"2012-08-14 16:17:59.000000000","message":"avoid \\","commit_id":"53958e6f676e67e3a6deac0a54d8e92db6b5ffdb"},{"author":{"_account_id":67,"name":"Vish Ishaya","email":"vishvananda@gmail.com","username":"vishvananda"},"change_message_id":"f023b672fd127930921cbd9f186c1a5c5b0bd921","unresolved":false,"context_lines":[{"line_number":498,"context_line":"                                   remote_path)"},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"            # Verify image file checksums match"},{"line_number":501,"context_line":"            cmd \u003d \"/usr/bin/csum -h MD5 %s|/usr/bin/awk \u0027{print $1}\u0027\" \\"},{"line_number":502,"context_line":"                % comp_path"},{"line_number":503,"context_line":"            output \u003d self.run_command_as_root(cmd)"},{"line_number":504,"context_line":"            if not len(output):"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAALH%2F%2FwY0%3D","line":501,"updated":"2012-08-14 16:17:59.000000000","message":"avoid \\","commit_id":"53958e6f676e67e3a6deac0a54d8e92db6b5ffdb"},{"author":{"_account_id":67,"name":"Vish Ishaya","email":"vishvananda@gmail.com","username":"vishvananda"},"change_message_id":"f023b672fd127930921cbd9f186c1a5c5b0bd921","unresolved":false,"context_lines":[{"line_number":616,"context_line":""},{"line_number":617,"context_line":"        :param command: List of commands."},{"line_number":618,"context_line":"        \"\"\""},{"line_number":619,"context_line":"        stdout, stderr \u003d \\"},{"line_number":620,"context_line":"            common.ssh_command_as_root(self._connection,"},{"line_number":621,"context_line":"                                       command,"},{"line_number":622,"context_line":"                                       check_exit_code\u003dcheck_exit_code)"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAALH%2F%2FwYs%3D","line":619,"updated":"2012-08-14 16:17:59.000000000","message":"avoid \\","commit_id":"53958e6f676e67e3a6deac0a54d8e92db6b5ffdb"}]}