)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"47614c636839601fcc77d95c1dfee1908bae49a9","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Support serial console access"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Use shellinaboxd + ipmitool to get serial console access."},{"line_number":10,"context_line":"blueprint serial-console-access"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I89a21e4d6969b91386896d1abc7035f9ba0232d9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAUX%2F%2Bv2M%3D","line":10,"updated":"2014-02-19 10:41:13.000000000","message":"we should have a space line between the description and blueprint/bug reference line.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"72165dceafe47b06af0f2852a546be67bdd5e4b1","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Support serial console access"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Use shellinaboxd + ipmitool to get serial console access."},{"line_number":10,"context_line":"blueprint serial-console-access"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I89a21e4d6969b91386896d1abc7035f9ba0232d9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAUX%2F%2BnqA%3D","line":10,"in_reply_to":"AAAAUX%2F%2Bv2M%3D","updated":"2014-02-20 06:46:59.000000000","message":"Done","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"711e4f29d14c9b35f8dd18172824074d26def10a","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Support serial console access"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Use shellinaboxd + ipmitool to get serial console access."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Co-Authored-By: Ling Gao \u003clinggao@us.ibm.com\u003e"},{"line_number":12,"context_line":"Co-Authored-By: Matt Wagner \u003cmatt.wagner@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"AAAAXX%2F%2F06E%3D","line":9,"updated":"2014-04-26 00:01:08.000000000","message":"Please add more detail to the commit message, including \n- descriptions of the changes drivers and modules\n- the new config options\n- why you\u0027re using subprocess.Popen instead of processutils\n- any known issues or limitations","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cadda45d71f40a6880532bdd49de95ff6c6d9803","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Support serial console access"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Use shellinaboxd + ipmitool to get serial console access."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Co-Authored-By: Ling Gao \u003clinggao@us.ibm.com\u003e"},{"line_number":12,"context_line":"Co-Authored-By: Matt Wagner \u003cmatt.wagner@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"bad4c104_b5a52806","line":9,"in_reply_to":"AAAAXX%2F%2F06E%3D","updated":"2014-04-29 15:17:04.000000000","message":"Done","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"}],"ironic/common/exception.py":[{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"f7dbdc27803b47c88dcbc12a09de755e4112ca23","unresolved":false,"context_lines":[{"line_number":361,"context_line":"    message \u003d _(\"Could not find config at %(path)s\")"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"class NoConsolePid(NotFound):"},{"line_number":365,"context_line":"    message \u003d _(\"Could not find pid in pid file %(pid_path)s\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FGZU%3D","line":364,"updated":"2014-01-17 14:41:30.000000000","message":"Maybe PidNotFound? Just an opinion.","commit_id":"6cb11260fc856f983d6dac41fb10bffd53f38dcd"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"4a9eccaff37a899d9242e9424a0ac298579ea0bc","unresolved":false,"context_lines":[{"line_number":364,"context_line":""},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"class ConsoleSubprocessFaild(IronicException):"},{"line_number":367,"context_line":"    message \u003d _(\"Console subprocess failed to start.\\n%(error)s\")"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2Fdko%3D","line":367,"updated":"2014-04-14 13:24:50.000000000","message":"Why newline there is?","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cb8ddcd38cd5e8d7031a01f51ee90693ead589f1","unresolved":false,"context_lines":[{"line_number":364,"context_line":""},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"class ConsoleSubprocessFaild(IronicException):"},{"line_number":367,"context_line":"    message \u003d _(\"Console subprocess failed to start.\\n%(error)s\")"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2FdAY%3D","line":367,"in_reply_to":"AAAAW3%2F%2Fdko%3D","updated":"2014-04-14 15:13:54.000000000","message":"Fixed.","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7411,"name":"Matt Wagner","email":"matt.wagner@redhat.com","username":"matty_dubs"},"change_message_id":"2c3bba58821dd4ad22420eeb6f24572a7710098a","unresolved":false,"context_lines":[{"line_number":363,"context_line":"    message \u003d _(\"Could not find pid in pid file %(pid_path)s\")"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"class ConsoleSubprocessFaild(IronicException):"},{"line_number":367,"context_line":"    message \u003d _(\"Console subprocess failed to start. %(error)s\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAW3%2F%2Fb%2FE%3D","line":366,"updated":"2014-04-14 15:40:43.000000000","message":"Could you s/Faild/Failed/ to match the conventional spelling? My eyes keep seeing this as a typo/bug in the code.","commit_id":"92e30f0926469dd6649ed592641a84c496b6deac"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"1f2d13e9a64058b801be15e9b64101ccd85e83c7","unresolved":false,"context_lines":[{"line_number":363,"context_line":"    message \u003d _(\"Could not find pid in pid file %(pid_path)s\")"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"class ConsoleSubprocessFaild(IronicException):"},{"line_number":367,"context_line":"    message \u003d _(\"Console subprocess failed to start. %(error)s\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAW3%2F%2FbjY%3D","line":366,"in_reply_to":"AAAAW3%2F%2Fb%2FE%3D","updated":"2014-04-14 18:55:48.000000000","message":"Oops. Thanks for the catch. Done.","commit_id":"92e30f0926469dd6649ed592641a84c496b6deac"}],"ironic/drivers/fake.py":[{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def __init__(self):"},{"line_number":42,"context_line":"        self.power \u003d ipmitool.IPMIPower()"},{"line_number":43,"context_line":"        self.console \u003d ipmitool.IPMIConsole()"},{"line_number":44,"context_line":"        self.deploy \u003d fake.FakeDeploy()"},{"line_number":45,"context_line":"        self.vendor \u003d self.power"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0NU%3D","line":43,"updated":"2014-01-08 06:50:40.000000000","message":"PXEAndIPMIToolDriver in ironic/drivers/pxe.sh need this console implement also, so can you help to add.","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"1da42fef891698bea1b21086daefa12d705d2092","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def __init__(self):"},{"line_number":42,"context_line":"        self.power \u003d ipmitool.IPMIPower()"},{"line_number":43,"context_line":"        self.console \u003d ipmitool.IPMIConsole()"},{"line_number":44,"context_line":"        self.deploy \u003d fake.FakeDeploy()"},{"line_number":45,"context_line":"        self.vendor \u003d self.power"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2FOfo%3D","line":43,"in_reply_to":"AAAAUH%2F%2F0NU%3D","updated":"2014-01-16 06:45:22.000000000","message":"Done","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"47614c636839601fcc77d95c1dfee1908bae49a9","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def __init__(self):"},{"line_number":45,"context_line":"        self.power \u003d ipmitool.IPMIPower()"},{"line_number":46,"context_line":"        self.console \u003d ipmitool.IPMIConsole()"},{"line_number":47,"context_line":"        self.deploy \u003d fake.FakeDeploy()"},{"line_number":48,"context_line":"        self.vendor \u003d ipmitool.VendorPassthru()"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2BwiU%3D","line":46,"updated":"2014-02-19 10:41:13.000000000","message":"How about a FackConsole is required here?","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"72165dceafe47b06af0f2852a546be67bdd5e4b1","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def __init__(self):"},{"line_number":45,"context_line":"        self.power \u003d ipmitool.IPMIPower()"},{"line_number":46,"context_line":"        self.console \u003d ipmitool.IPMIConsole()"},{"line_number":47,"context_line":"        self.deploy \u003d fake.FakeDeploy()"},{"line_number":48,"context_line":"        self.vendor \u003d ipmitool.VendorPassthru()"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bo9g%3D","line":46,"in_reply_to":"AAAAUX%2F%2BwiU%3D","updated":"2014-02-20 06:46:59.000000000","message":"This is used by the unit test cases.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"}],"ironic/drivers/modules/console_utils.py":[{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"711e4f29d14c9b35f8dd18172824074d26def10a","unresolved":false,"context_lines":[{"line_number":16,"context_line":"#    under the License."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\"\"\""},{"line_number":19,"context_line":"Ironic console utilities."},{"line_number":20,"context_line":"\"\"\""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"import os"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2F1BU%3D","line":19,"updated":"2014-04-26 00:01:08.000000000","message":"There are several methods in this module -- possibly all of them -- which are not private methods (they\u0027re called from other modules) but the method names are prefixed with \"_\". Based on the review comments, it looks like these were recently moved, and this change was overlooked. Still, please remove the leading underscore from public methods.","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cadda45d71f40a6880532bdd49de95ff6c6d9803","unresolved":false,"context_lines":[{"line_number":16,"context_line":"#    under the License."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\"\"\""},{"line_number":19,"context_line":"Ironic console utilities."},{"line_number":20,"context_line":"\"\"\""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"import os"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2Fs1E%3D","line":19,"in_reply_to":"AAAAXX%2F%2F1BU%3D","updated":"2014-04-29 15:17:04.000000000","message":"Done. \nThanks for the catch. Yes, I forgot to remove the \"-\" after moving the files. :-).","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"711e4f29d14c9b35f8dd18172824074d26def10a","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        utils.unlink_without_raise(pid_file)"},{"line_number":122,"context_line":"        utils.execute(\u0027kill\u0027, str(console_pid),"},{"line_number":123,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":124,"context_line":"    except Exception:"},{"line_number":125,"context_line":"        pass"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    # put together the command and arguments for invoking the console"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2F1Fk%3D","line":124,"updated":"2014-04-26 00:01:08.000000000","message":"One should not squelch all exceptions like this.\n\nInstead, please catch the expected exceptions. In this case, I see three exceptions likely to be raised, and I would suggest doing something like this:\n\n  try:\n    console_pid \u003d ..\n    utils.unlink...\n    utils.execute...\n  except exception.NoConsolePid:\n    pass\n  except (exception.ProcessExecutionError, exception.UnknownArgumentError):\n    LOG.warning(_(...))","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cadda45d71f40a6880532bdd49de95ff6c6d9803","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        utils.unlink_without_raise(pid_file)"},{"line_number":122,"context_line":"        utils.execute(\u0027kill\u0027, str(console_pid),"},{"line_number":123,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":124,"context_line":"    except Exception:"},{"line_number":125,"context_line":"        pass"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    # put together the command and arguments for invoking the console"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2Fsdc%3D","line":124,"in_reply_to":"AAAAXX%2F%2F1Fk%3D","updated":"2014-04-29 15:17:04.000000000","message":"Done","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"711e4f29d14c9b35f8dd18172824074d26def10a","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    # put together the command and arguments for invoking the console"},{"line_number":128,"context_line":"    args \u003d []"},{"line_number":129,"context_line":"    args.append(CONF.console.terminal)"},{"line_number":130,"context_line":"    if CONF.console.terminal_cert_dir:"},{"line_number":131,"context_line":"        args.append(\"-c\")"},{"line_number":132,"context_line":"        args.append(CONF.console.terminal_cert_dir)"},{"line_number":133,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2F1DI%3D","line":130,"updated":"2014-04-26 00:01:08.000000000","message":"this would read slightly better if compound options were on a single line, eg,\n\n  if CONF.console.terminal_cert_dir:\n    args.append(\"-c %s\" % CONF.console...)\n  else:\n    args.append(\"-t\")\n  args.append(\"-p %s\" % str(port))\n  ...","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cadda45d71f40a6880532bdd49de95ff6c6d9803","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    # put together the command and arguments for invoking the console"},{"line_number":128,"context_line":"    args \u003d []"},{"line_number":129,"context_line":"    args.append(CONF.console.terminal)"},{"line_number":130,"context_line":"    if CONF.console.terminal_cert_dir:"},{"line_number":131,"context_line":"        args.append(\"-c\")"},{"line_number":132,"context_line":"        args.append(CONF.console.terminal_cert_dir)"},{"line_number":133,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2Fsck%3D","line":130,"in_reply_to":"AAAAXX%2F%2F1DI%3D","updated":"2014-04-29 15:17:04.000000000","message":"Popen gave an error when I put the them in one line. The command parsing failed to recognize the option.","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"711e4f29d14c9b35f8dd18172824074d26def10a","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        # check if the console pid is created."},{"line_number":151,"context_line":"        # if it is, then the shellinaboxd is invoked successfully as a daemon."},{"line_number":152,"context_line":"        # otherwise check the error."},{"line_number":153,"context_line":"        retry \u003d 10"},{"line_number":154,"context_line":"        _returncode \u003d obj.poll()"},{"line_number":155,"context_line":"        while not _returncode and not os.path.exists(pid_file) and retry \u003e 0:"},{"line_number":156,"context_line":"            time.sleep(.5)"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2F1CU%3D","line":153,"updated":"2014-04-26 00:01:08.000000000","message":"please avoid hard-coded timeouts like this. Instead, a config option would be best.\n\nAlso, there are common utilities in ironic.openstack.common.loopingcall which provide this functionality. See ironic.drivers.modules.ipmitool for an example.","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cadda45d71f40a6880532bdd49de95ff6c6d9803","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        # check if the console pid is created."},{"line_number":151,"context_line":"        # if it is, then the shellinaboxd is invoked successfully as a daemon."},{"line_number":152,"context_line":"        # otherwise check the error."},{"line_number":153,"context_line":"        retry \u003d 10"},{"line_number":154,"context_line":"        _returncode \u003d obj.poll()"},{"line_number":155,"context_line":"        while not _returncode and not os.path.exists(pid_file) and retry \u003e 0:"},{"line_number":156,"context_line":"            time.sleep(.5)"}],"source_content_type":"text/x-python","patch_set":26,"id":"bad4c104_f567b0e8","line":153,"in_reply_to":"AAAAXX%2F%2F1CU%3D","updated":"2014-04-29 15:17:04.000000000","message":"Done.","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"711e4f29d14c9b35f8dd18172824074d26def10a","unresolved":false,"context_lines":[{"line_number":183,"context_line":"    try:"},{"line_number":184,"context_line":"        console_pid \u003d _get_console_pid(node_uuid)"},{"line_number":185,"context_line":"    except exception.NoConsolePid:"},{"line_number":186,"context_line":"        LOG.warning(_(\"No console pid found for node %s.\")"},{"line_number":187,"context_line":"            % node_uuid)"},{"line_number":188,"context_line":"    else:"},{"line_number":189,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2F1B8%3D","line":186,"updated":"2014-04-26 00:01:08.000000000","message":"this warning message does not indicate why it is being logged. It would be better to say:\n\n  LOG.warning(_(\"No console pid found for node %s \"\n    \" while trying to stop shellinaboxd console.\") % node_uuid)","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cadda45d71f40a6880532bdd49de95ff6c6d9803","unresolved":false,"context_lines":[{"line_number":183,"context_line":"    try:"},{"line_number":184,"context_line":"        console_pid \u003d _get_console_pid(node_uuid)"},{"line_number":185,"context_line":"    except exception.NoConsolePid:"},{"line_number":186,"context_line":"        LOG.warning(_(\"No console pid found for node %s.\")"},{"line_number":187,"context_line":"            % node_uuid)"},{"line_number":188,"context_line":"    else:"},{"line_number":189,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2FsX8%3D","line":186,"in_reply_to":"AAAAXX%2F%2F1B8%3D","updated":"2014-04-29 15:17:04.000000000","message":"Done","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"fea3eaf23317d2e95e28fca35b196862dd6d1ded","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        # if it is, then the shellinaboxd is invoked successfully as a daemon."},{"line_number":176,"context_line":"        # otherwise check the error."},{"line_number":177,"context_line":"        if locals[\u0027returncode\u0027] is not None:"},{"line_number":178,"context_line":"            if not locals[\u0027returncode\u0027] and os.path.exists(pid_file):"},{"line_number":179,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":180,"context_line":"            else:"},{"line_number":181,"context_line":"                (stdout, stderr) \u003d popen_obj.communicate()"}],"source_content_type":"text/x-python","patch_set":27,"id":"bad4c104_663778b1","line":178,"updated":"2014-04-29 16:11:12.000000000","message":"I think if locals[\u0027returncode\u0027] \u003d\u003d 0 ... more readable here.","commit_id":"f5213d8930eb6d2649c435b705d2eda325b78f2f"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"ff875f393f52f7e3b3b8adb764c443b15eb304bd","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        # if it is, then the shellinaboxd is invoked successfully as a daemon."},{"line_number":176,"context_line":"        # otherwise check the error."},{"line_number":177,"context_line":"        if locals[\u0027returncode\u0027] is not None:"},{"line_number":178,"context_line":"            if not locals[\u0027returncode\u0027] and os.path.exists(pid_file):"},{"line_number":179,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":180,"context_line":"            else:"},{"line_number":181,"context_line":"                (stdout, stderr) \u003d popen_obj.communicate()"}],"source_content_type":"text/x-python","patch_set":27,"id":"bad4c104_fc81544b","line":178,"in_reply_to":"bad4c104_663778b1","updated":"2014-04-29 20:40:05.000000000","message":"Done","commit_id":"f5213d8930eb6d2649c435b705d2eda325b78f2f"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"fea3eaf23317d2e95e28fca35b196862dd6d1ded","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":180,"context_line":"            else:"},{"line_number":181,"context_line":"                (stdout, stderr) \u003d popen_obj.communicate()"},{"line_number":182,"context_line":"                locals[\u0027errstr\u0027] \u003d _(\"Command: %(command)s.\\n\""},{"line_number":183,"context_line":"                        \"Exit code: %(return_code)s.\\n\""},{"line_number":184,"context_line":"                        \"Stdout: %(stdout)r\\n\""},{"line_number":185,"context_line":"                        \"Stderr: %(stderr)r\") % {\u0027command\u0027: \u0027 \u0027.join(args),"}],"source_content_type":"text/x-python","patch_set":27,"id":"bad4c104_66c77813","line":182,"updated":"2014-04-29 16:11:12.000000000","message":"This debug string will be stored in node.last_error after l205. I think this message only for debug, better to have another one for error.","commit_id":"f5213d8930eb6d2649c435b705d2eda325b78f2f"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"ff875f393f52f7e3b3b8adb764c443b15eb304bd","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":180,"context_line":"            else:"},{"line_number":181,"context_line":"                (stdout, stderr) \u003d popen_obj.communicate()"},{"line_number":182,"context_line":"                locals[\u0027errstr\u0027] \u003d _(\"Command: %(command)s.\\n\""},{"line_number":183,"context_line":"                        \"Exit code: %(return_code)s.\\n\""},{"line_number":184,"context_line":"                        \"Stdout: %(stdout)r\\n\""},{"line_number":185,"context_line":"                        \"Stderr: %(stderr)r\") % {\u0027command\u0027: \u0027 \u0027.join(args),"}],"source_content_type":"text/x-python","patch_set":27,"id":"bad4c104_3c93bca4","line":182,"in_reply_to":"bad4c104_66c77813","updated":"2014-04-29 20:40:05.000000000","message":"I tried to come up with an error string, but sometimes the error string does not make sense if we do not show the command and the stderr.  So I\u0027ll keep it as is for now.","commit_id":"f5213d8930eb6d2649c435b705d2eda325b78f2f"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"fea3eaf23317d2e95e28fca35b196862dd6d1ded","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        if (timeutils.utcnow() \u003e expiration):"},{"line_number":193,"context_line":"            locals[\u0027errstr\u0027] \u003d _(\"Timeout while waiting for\""},{"line_number":194,"context_line":"                    \" console subprocess to finish for node %s.\") % node_uuid"},{"line_number":195,"context_line":"            LOG.warning(locals[\u0027errstr\u0027])"},{"line_number":196,"context_line":"            raise loopingcall.LoopingCallDone()"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    locals \u003d {\u0027returncode\u0027: None, \u0027errstr\u0027: \u0027\u0027}"}],"source_content_type":"text/x-python","patch_set":27,"id":"bad4c104_a6b0a0de","line":195,"updated":"2014-04-29 16:11:12.000000000","message":"We have LOG.debug l189 and LOG.warning in error cases here.","commit_id":"f5213d8930eb6d2649c435b705d2eda325b78f2f"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"ff875f393f52f7e3b3b8adb764c443b15eb304bd","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        if (timeutils.utcnow() \u003e expiration):"},{"line_number":193,"context_line":"            locals[\u0027errstr\u0027] \u003d _(\"Timeout while waiting for\""},{"line_number":194,"context_line":"                    \" console subprocess to finish for node %s.\") % node_uuid"},{"line_number":195,"context_line":"            LOG.warning(locals[\u0027errstr\u0027])"},{"line_number":196,"context_line":"            raise loopingcall.LoopingCallDone()"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    locals \u003d {\u0027returncode\u0027: None, \u0027errstr\u0027: \u0027\u0027}"}],"source_content_type":"text/x-python","patch_set":27,"id":"bad4c104_1c0c3842","line":195,"in_reply_to":"bad4c104_a6b0a0de","updated":"2014-04-29 20:40:05.000000000","message":"Done. Changed both to LOG.warning.","commit_id":"f5213d8930eb6d2649c435b705d2eda325b78f2f"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"fea3eaf23317d2e95e28fca35b196862dd6d1ded","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            raise loopingcall.LoopingCallDone()"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    locals \u003d {\u0027returncode\u0027: None, \u0027errstr\u0027: \u0027\u0027}"},{"line_number":199,"context_line":"    expiration \u003d timeutils.utcnow() + datetime.timedelta("},{"line_number":200,"context_line":"            seconds\u003dCONF.console.subprocess_timeout)"},{"line_number":201,"context_line":"    timer \u003d loopingcall.FixedIntervalLoopingCall(_wait, node_uuid, obj)"},{"line_number":202,"context_line":"    timer.start(interval\u003dCONF.console.subprocess_checking_interval).wait()"}],"source_content_type":"text/x-python","patch_set":27,"id":"bad4c104_c673e4af","line":199,"updated":"2014-04-29 16:11:12.000000000","message":"Why simple time.time() not used here?","commit_id":"f5213d8930eb6d2649c435b705d2eda325b78f2f"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"ff875f393f52f7e3b3b8adb764c443b15eb304bd","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            raise loopingcall.LoopingCallDone()"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    locals \u003d {\u0027returncode\u0027: None, \u0027errstr\u0027: \u0027\u0027}"},{"line_number":199,"context_line":"    expiration \u003d timeutils.utcnow() + datetime.timedelta("},{"line_number":200,"context_line":"            seconds\u003dCONF.console.subprocess_timeout)"},{"line_number":201,"context_line":"    timer \u003d loopingcall.FixedIntervalLoopingCall(_wait, node_uuid, obj)"},{"line_number":202,"context_line":"    timer.start(interval\u003dCONF.console.subprocess_checking_interval).wait()"}],"source_content_type":"text/x-python","patch_set":27,"id":"bad4c104_72b64b3f","line":199,"in_reply_to":"bad4c104_c673e4af","updated":"2014-04-29 20:40:05.000000000","message":"I got the code from somewhere else in nova. Do not understand the logic behind using utcnow() vs. time.time(). I saw both used in nova.  Anyway, I switched back to time.time() because it is simpler. Thanks for making me think twice here.","commit_id":"f5213d8930eb6d2649c435b705d2eda325b78f2f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f09462a0ccb3a9fa94851d00f607637a2eb7e5c5","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"opts \u003d ["},{"line_number":36,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":37,"context_line":"               default\u003d\u0027/usr/bin/shellinaboxd\u0027,"},{"line_number":38,"context_line":"               help\u003d\u0027Absolute path for shellinaboxd program\u0027),"},{"line_number":39,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"},{"line_number":40,"context_line":"               help\u003d\u0027Directory containing the terminal SSL cert(PEM) for \u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad9bd40_578f8723","line":37,"updated":"2014-04-30 10:08:22.000000000","message":"I\u0027m a bit worried about absolute paths, do it really need to use absolute path here? It\u0027s very distro specific, in fedora for e.g this default path won\u0027t work, default path is /usr/sbin/shellinaboxd[1].\n\nNova doesn\u0027t use absolute path for it[2] and other tools that we required (e.g parted, mkfs, mkswap, dd, etc...) are not using absolute path as well.\n\n[1] http://paste.openstack.org/show/77939/\n\n[2] https://github.com/openstack/nova/blob/master/nova/virt/baremetal/ipmi.py#L40-L42","commit_id":"1917f2ddfea6024be24d9b494f52cc09604a7390"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"94f33771d5555c6f180b2affe09c14a59ba86c74","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"opts \u003d ["},{"line_number":36,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":37,"context_line":"               default\u003d\u0027/usr/bin/shellinaboxd\u0027,"},{"line_number":38,"context_line":"               help\u003d\u0027Absolute path for shellinaboxd program\u0027),"},{"line_number":39,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"},{"line_number":40,"context_line":"               help\u003d\u0027Directory containing the terminal SSL cert(PEM) for \u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad9bd40_f776d6d7","line":37,"in_reply_to":"9ad9bd40_578f8723","updated":"2014-04-30 19:46:17.000000000","message":"Done. good catch. thanks.","commit_id":"1917f2ddfea6024be24d9b494f52cc09604a7390"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"a47898135cd92488620c1907c14770cb7292befe","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        with open(path, \u0027wb\u0027) as file:"},{"line_number":96,"context_line":"            file.write(password)"},{"line_number":97,"context_line":"        return path"},{"line_number":98,"context_line":"    except Exception:"},{"line_number":99,"context_line":"        with excutils.save_and_reraise_exception():"},{"line_number":100,"context_line":"            utils.delete_if_exists(path)"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad9bd40_1caa8419","line":98,"updated":"2014-04-30 09:53:56.000000000","message":"I don\u0027t think it\u0027s a good idea to catch all exceptions here. Better to catch specific ones and cover them by a specific one. Otherwise the generic exception will go to node\u0027s last error and will make debugging the problem quite hard","commit_id":"1917f2ddfea6024be24d9b494f52cc09604a7390"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"94f33771d5555c6f180b2affe09c14a59ba86c74","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        with open(path, \u0027wb\u0027) as file:"},{"line_number":96,"context_line":"            file.write(password)"},{"line_number":97,"context_line":"        return path"},{"line_number":98,"context_line":"    except Exception:"},{"line_number":99,"context_line":"        with excutils.save_and_reraise_exception():"},{"line_number":100,"context_line":"            utils.delete_if_exists(path)"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"9ad9bd40_175b3a55","line":98,"in_reply_to":"9ad9bd40_1caa8419","updated":"2014-04-30 19:46:17.000000000","message":"Done. Thanks for the catch.\nI have created a PasswordFileFailedToCreate to cover it. This way the last_error will indicate that the error is from creating the password file. But I have to catch all the exceptions because the file needs to be deleted when any error occurs.\nI have also added 2 new test cases to test this function.","commit_id":"1917f2ddfea6024be24d9b494f52cc09604a7390"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"cdc395d923d9d7b366c4a8cf44184b8084602f45","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        return path"},{"line_number":97,"context_line":"    except Exception as e:"},{"line_number":98,"context_line":"        utils.delete_if_exists(path)"},{"line_number":99,"context_line":"        raise exception.PasswordFileFailedToCreate(error\u003dstr(e))"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"def get_shellinabox_console_url(port):"}],"source_content_type":"text/x-python","patch_set":29,"id":"9ad9bd40_dba00ff7","line":99,"updated":"2014-04-30 22:02:47.000000000","message":"As an improvement here I can suggest adding a log record here. This might be important in the case if some other driver re-uses this method and suppresses the exception.\n\nRight now it\u0027s not critical at all because the exception is not handled and will be added to the log in the upstream, so don\u0027t create a separate patch set _only_ because of this problem.","commit_id":"c1dbca0f0a445bfe2d42954464b3951f3dfd4514"}],"ironic/drivers/modules/ipmitool.py":[{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e729e9a513d72f71ab064bc8c99d5a0934091645","unresolved":false,"context_lines":[{"line_number":40,"context_line":"opts \u003d ["},{"line_number":41,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":42,"context_line":"            default\u003d\u0027shellinaboxd\u0027,"},{"line_number":43,"context_line":"            help\u003d\u0027The terminal program for serial console access\u0027),"},{"line_number":44,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"},{"line_number":45,"context_line":"            help\u003d\u0027The terminal SSL cert(PEM) for serial console access\u0027),"},{"line_number":46,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F9gE%3D","line":43,"updated":"2013-12-26 09:09:47.000000000","message":"Jing, can we list the terminals we support here for user to select one of them, or we just support one single terminal shellinaboxd now?","commit_id":"aac76332395c53c02e19e274337be516e5cc8131"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e729e9a513d72f71ab064bc8c99d5a0934091645","unresolved":false,"context_lines":[{"line_number":42,"context_line":"            default\u003d\u0027shellinaboxd\u0027,"},{"line_number":43,"context_line":"            help\u003d\u0027The terminal program for serial console access\u0027),"},{"line_number":44,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"},{"line_number":45,"context_line":"            help\u003d\u0027The terminal SSL cert(PEM) for serial console access\u0027),"},{"line_number":46,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":47,"context_line":"            default\u003d\u0027$state_path/console\u0027,"},{"line_number":48,"context_line":"            help\u003d\u0027Directory for holding terminal pidfiles\u0027),"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F9gs%3D","line":45,"updated":"2013-12-26 09:09:47.000000000","message":"Jing, should we provide more details to guide how to prepare cert if required?","commit_id":"aac76332395c53c02e19e274337be516e5cc8131"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e729e9a513d72f71ab064bc8c99d5a0934091645","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    ]"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"CONF \u003d cfg.CONF"},{"line_number":52,"context_line":"CONF.register_opts(opts, group\u003d\u0027ipmi\u0027)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F9gY%3D","line":52,"updated":"2013-12-26 09:09:47.000000000","message":"Jing, I think this line is not required, because we have register ipmi section in ipminative.py[1] already.\n\n[1]https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ipminative.py#L40","commit_id":"aac76332395c53c02e19e274337be516e5cc8131"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e729e9a513d72f71ab064bc8c99d5a0934091645","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            # only issue \"power on\" once"},{"line_number":138,"context_line":"            if retries[0] \u003d\u003d 0:"},{"line_number":139,"context_line":"                _exec_ipmitool(driver_info, \"power on\")"},{"line_number":140,"context_line":"                _start_console(driver_info)"},{"line_number":141,"context_line":"            retries[0] +\u003d 1"},{"line_number":142,"context_line":"        except Exception:"},{"line_number":143,"context_line":"            # Log failures but keep trying"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F9hU%3D","line":140,"updated":"2013-12-26 09:09:47.000000000","message":"if there is some exception throws out from this _start_console call, we will log wrong message with next lines - \"IPMI power on failed for node...\", for some time, power on is successfully, but _start_console failed, so it is better that we have to handle for each case.","commit_id":"aac76332395c53c02e19e274337be516e5cc8131"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e729e9a513d72f71ab064bc8c99d5a0934091645","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            # only issue \"power off\" once"},{"line_number":172,"context_line":"            if retries[0] \u003d\u003d 0:"},{"line_number":173,"context_line":"                _exec_ipmitool(driver_info, \"power off\")"},{"line_number":174,"context_line":"                _stop_console(driver_info)"},{"line_number":175,"context_line":"            retries[0] +\u003d 1"},{"line_number":176,"context_line":"        except Exception:"},{"line_number":177,"context_line":"            # Log failures but keep trying"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F9hQ%3D","line":174,"updated":"2013-12-26 09:09:47.000000000","message":"ditto, should handle exception exactly here.","commit_id":"aac76332395c53c02e19e274337be516e5cc8131"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e729e9a513d72f71ab064bc8c99d5a0934091645","unresolved":false,"context_lines":[{"line_number":212,"context_line":"    args.append(\"-s\")"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    try:"},{"line_number":215,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027])"},{"line_number":216,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":217,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":218,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F9g4%3D","line":215,"updated":"2013-12-26 09:09:47.000000000","message":"Jing, our _make_password_file method will delete the password file once the call is ending, so we have to use \u0027with\u0027 to hold password file.","commit_id":"aac76332395c53c02e19e274337be516e5cc8131"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e729e9a513d72f71ab064bc8c99d5a0934091645","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":230,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":231,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":232,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":233,"context_line":"    finally:"},{"line_number":234,"context_line":"        utils.unlink_without_raise(pwfile)"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F9iI%3D","line":232,"updated":"2013-12-26 09:09:47.000000000","message":"Jing, we need to catch exception here and log the error message.","commit_id":"aac76332395c53c02e19e274337be516e5cc8131"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e729e9a513d72f71ab064bc8c99d5a0934091645","unresolved":false,"context_lines":[{"line_number":259,"context_line":""},{"line_number":260,"context_line":"    pid_path \u003d _get_console_pid_file(node_uuid)"},{"line_number":261,"context_line":"    if os.path.exists(pid_path):"},{"line_number":262,"context_line":"        with open(pid_path, \u0027r\u0027) as f:"},{"line_number":263,"context_line":"            pid_str \u003d f.read()"},{"line_number":264,"context_line":"        try:"},{"line_number":265,"context_line":"            return int(pid_str)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAT3%2F%2F9hk%3D","line":262,"updated":"2013-12-26 09:09:47.000000000","message":"Jing, we need to add try catch block and handle the permission or file not find exception for the file opening action to avoid the exceptions are raised to upper level.","commit_id":"aac76332395c53c02e19e274337be516e5cc8131"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"dbbece02b44fe674f5100b85788a52e27faf2b7a","unresolved":false,"context_lines":[{"line_number":38,"context_line":"from ironic.openstack.common import loopingcall"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"opts \u003d ["},{"line_number":41,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":42,"context_line":"            default\u003d\u0027shellinaboxd\u0027,"},{"line_number":43,"context_line":"            help\u003d\u0027The terminal program for serial console access\u0027),"},{"line_number":44,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F9k4%3D","line":41,"updated":"2013-12-26 09:04:44.000000000","message":"AFAIK your code only for shellinaboxd. If user changes this it will not work.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":38,"context_line":"from ironic.openstack.common import loopingcall"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"opts \u003d ["},{"line_number":41,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":42,"context_line":"            default\u003d\u0027shellinaboxd\u0027,"},{"line_number":43,"context_line":"            help\u003d\u0027The terminal program for serial console access\u0027),"},{"line_number":44,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FwtY%3D","line":41,"in_reply_to":"AAAAT3%2F%2F9k4%3D","updated":"2014-01-03 02:54:30.000000000","message":"Done","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"15d3224c5c1aa8eaf9dca9e09f3fb8a49af8abac","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    ]"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"CONF \u003d cfg.CONF"},{"line_number":52,"context_line":"CONF.register_opts(opts, group\u003d\u0027ipmi\u0027)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FsBc%3D","line":52,"updated":"2014-01-03 13:45:01.000000000","message":"As mentioned in other comments, in Ironic we have an independent driver interface for console, so I think these options also should have their own group independent of the ipmi one","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"682968bf245ff7cfa4849ff62bb3c45d867ee4b9","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    ]"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"CONF \u003d cfg.CONF"},{"line_number":52,"context_line":"CONF.register_opts(opts, group\u003d\u0027ipmi\u0027)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUH%2F%2F3nQ%3D","line":52,"in_reply_to":"AAAAT3%2F%2FsBc%3D","updated":"2014-01-07 12:56:55.000000000","message":"yes, I thought about it too, but after I checked the groups defined in ironic.conf.sample, I found these groups are mainly named for functionalities, so I\u0027m thinking the console implementation is based on ipmi (whatever ipmitool or ipminative), maybe it\u0027s ok just put console options in ipmi group?","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"dbbece02b44fe674f5100b85788a52e27faf2b7a","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            # only issue \"power on\" once"},{"line_number":138,"context_line":"            if retries[0] \u003d\u003d 0:"},{"line_number":139,"context_line":"                _exec_ipmitool(driver_info, \"power on\")"},{"line_number":140,"context_line":"                _start_console(driver_info)"},{"line_number":141,"context_line":"            retries[0] +\u003d 1"},{"line_number":142,"context_line":"        except Exception:"},{"line_number":143,"context_line":"            # Log failures but keep trying"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F9mA%3D","line":140,"updated":"2013-12-26 09:04:44.000000000","message":"IMO not needed here, you should implement console interface with start/stop methods\n\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/base.py#L161","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            # only issue \"power on\" once"},{"line_number":138,"context_line":"            if retries[0] \u003d\u003d 0:"},{"line_number":139,"context_line":"                _exec_ipmitool(driver_info, \"power on\")"},{"line_number":140,"context_line":"                _start_console(driver_info)"},{"line_number":141,"context_line":"            retries[0] +\u003d 1"},{"line_number":142,"context_line":"        except Exception:"},{"line_number":143,"context_line":"            # Log failures but keep trying"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F2RY%3D","line":140,"in_reply_to":"AAAAT3%2F%2F7K8%3D","updated":"2014-01-03 02:54:30.000000000","message":"Done","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            # only issue \"power on\" once"},{"line_number":138,"context_line":"            if retries[0] \u003d\u003d 0:"},{"line_number":139,"context_line":"                _exec_ipmitool(driver_info, \"power on\")"},{"line_number":140,"context_line":"                _start_console(driver_info)"},{"line_number":141,"context_line":"            retries[0] +\u003d 1"},{"line_number":142,"context_line":"        except Exception:"},{"line_number":143,"context_line":"            # Log failures but keep trying"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F2Rc%3D","line":140,"in_reply_to":"AAAAT3%2F%2F9mA%3D","updated":"2014-01-03 02:54:30.000000000","message":"Done","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"86676002d7610a243bd5ee699a64d473f20bc567","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            # only issue \"power on\" once"},{"line_number":138,"context_line":"            if retries[0] \u003d\u003d 0:"},{"line_number":139,"context_line":"                _exec_ipmitool(driver_info, \"power on\")"},{"line_number":140,"context_line":"                _start_console(driver_info)"},{"line_number":141,"context_line":"            retries[0] +\u003d 1"},{"line_number":142,"context_line":"        except Exception:"},{"line_number":143,"context_line":"            # Log failures but keep trying"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F7K8%3D","line":140,"in_reply_to":"AAAAT3%2F%2F9mA%3D","updated":"2013-12-27 02:30:00.000000000","message":"I agree --\n\nFirstly, we can\u0027t always start a console when a node is powered on. Some hardware systems have a limited number of concurrent console sessions...\n\nSecond, as Yuriy pointed out, there is a driver interface specifically for console start/stop.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"dbbece02b44fe674f5100b85788a52e27faf2b7a","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            # only issue \"power off\" once"},{"line_number":172,"context_line":"            if retries[0] \u003d\u003d 0:"},{"line_number":173,"context_line":"                _exec_ipmitool(driver_info, \"power off\")"},{"line_number":174,"context_line":"                _stop_console(driver_info)"},{"line_number":175,"context_line":"            retries[0] +\u003d 1"},{"line_number":176,"context_line":"        except Exception:"},{"line_number":177,"context_line":"            # Log failures but keep trying"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F9l0%3D","line":174,"updated":"2013-12-26 09:04:44.000000000","message":"ditto as l140.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            # only issue \"power off\" once"},{"line_number":172,"context_line":"            if retries[0] \u003d\u003d 0:"},{"line_number":173,"context_line":"                _exec_ipmitool(driver_info, \"power off\")"},{"line_number":174,"context_line":"                _stop_console(driver_info)"},{"line_number":175,"context_line":"            retries[0] +\u003d 1"},{"line_number":176,"context_line":"        except Exception:"},{"line_number":177,"context_line":"            # Log failures but keep trying"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FwnY%3D","line":174,"in_reply_to":"AAAAT3%2F%2F9l0%3D","updated":"2014-01-03 02:54:30.000000000","message":"Done","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"dbbece02b44fe674f5100b85788a52e27faf2b7a","unresolved":false,"context_lines":[{"line_number":197,"context_line":"def _start_console(driver_info):"},{"line_number":198,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    if not driver_info[\u0027port\u0027]:"},{"line_number":201,"context_line":"        return"},{"line_number":202,"context_line":"    args \u003d []"},{"line_number":203,"context_line":"    args.append(CONF.ipmi.terminal)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F9lo%3D","line":200,"updated":"2013-12-26 09:04:44.000000000","message":"\u0027validate\u0027 method of console interface needed.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":197,"context_line":"def _start_console(driver_info):"},{"line_number":198,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    if not driver_info[\u0027port\u0027]:"},{"line_number":201,"context_line":"        return"},{"line_number":202,"context_line":"    args \u003d []"},{"line_number":203,"context_line":"    args.append(CONF.ipmi.terminal)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FwnU%3D","line":200,"in_reply_to":"AAAAT3%2F%2F7Ko%3D","updated":"2014-01-03 02:54:30.000000000","message":"Done","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":197,"context_line":"def _start_console(driver_info):"},{"line_number":198,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    if not driver_info[\u0027port\u0027]:"},{"line_number":201,"context_line":"        return"},{"line_number":202,"context_line":"    args \u003d []"},{"line_number":203,"context_line":"    args.append(CONF.ipmi.terminal)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2Fwnc%3D","line":200,"in_reply_to":"AAAAT3%2F%2F9lo%3D","updated":"2014-01-03 02:54:30.000000000","message":"Done","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"86676002d7610a243bd5ee699a64d473f20bc567","unresolved":false,"context_lines":[{"line_number":197,"context_line":"def _start_console(driver_info):"},{"line_number":198,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    if not driver_info[\u0027port\u0027]:"},{"line_number":201,"context_line":"        return"},{"line_number":202,"context_line":"    args \u003d []"},{"line_number":203,"context_line":"    args.append(CONF.ipmi.terminal)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F7Ko%3D","line":200,"in_reply_to":"AAAAT3%2F%2F9lo%3D","updated":"2013-12-27 02:30:00.000000000","message":"Yep. And this should raise an exception, as with other validate methods.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"dbbece02b44fe674f5100b85788a52e27faf2b7a","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        # Run shellinaboxd without pipes. Otherwise utils.execute() waits"},{"line_number":227,"context_line":"        # infinitely since shellinaboxd does not close passed fds."},{"line_number":228,"context_line":"        x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":229,"context_line":"        x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":230,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":231,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":232,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F9iM%3D","line":229,"updated":"2013-12-26 09:04:44.000000000","message":"You use --background parameter. Why lines 229-231 needed?","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        # Run shellinaboxd without pipes. Otherwise utils.execute() waits"},{"line_number":227,"context_line":"        # infinitely since shellinaboxd does not close passed fds."},{"line_number":228,"context_line":"        x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":229,"context_line":"        x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":230,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":231,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":232,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FwiA%3D","line":229,"in_reply_to":"AAAAT3%2F%2F9iM%3D","updated":"2014-01-03 02:54:30.000000000","message":"I saw this from nova baremetal ipmi.py, thought it might mean something even we used -b flag, I kept it to be safe. Maybe Deva knows the reason?","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"dbbece02b44fe674f5100b85788a52e27faf2b7a","unresolved":false,"context_lines":[{"line_number":238,"context_line":"    \"\"\"Close the Serial Console of a node.\"\"\""},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    console_pid \u003d _get_console_pid(driver_info[\u0027uuid\u0027])"},{"line_number":241,"context_line":"    if console_pid:"},{"line_number":242,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":243,"context_line":"        utils.execute(\u0027kill\u0027, \u0027-TERM\u0027, str(console_pid),"},{"line_number":244,"context_line":"                      run_as_root\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F9gc%3D","line":241,"updated":"2013-12-26 09:04:44.000000000","message":"IMO exception should be raised for \u0027else\u0027 case.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":238,"context_line":"    \"\"\"Close the Serial Console of a node.\"\"\""},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    console_pid \u003d _get_console_pid(driver_info[\u0027uuid\u0027])"},{"line_number":241,"context_line":"    if console_pid:"},{"line_number":242,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":243,"context_line":"        utils.execute(\u0027kill\u0027, \u0027-TERM\u0027, str(console_pid),"},{"line_number":244,"context_line":"                      run_as_root\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FthU%3D","line":241,"in_reply_to":"AAAAT3%2F%2F7Hk%3D","updated":"2014-01-03 02:54:30.000000000","message":"yes, I agree, logging a warning in _stop_console if no console_pid.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"86676002d7610a243bd5ee699a64d473f20bc567","unresolved":false,"context_lines":[{"line_number":238,"context_line":"    \"\"\"Close the Serial Console of a node.\"\"\""},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    console_pid \u003d _get_console_pid(driver_info[\u0027uuid\u0027])"},{"line_number":241,"context_line":"    if console_pid:"},{"line_number":242,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":243,"context_line":"        utils.execute(\u0027kill\u0027, \u0027-TERM\u0027, str(console_pid),"},{"line_number":244,"context_line":"                      run_as_root\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F7Hk%3D","line":241,"in_reply_to":"AAAAT3%2F%2F9gc%3D","updated":"2013-12-27 02:30:00.000000000","message":"I don\u0027t think that _stop_console() should raise an exception. However, if you follow my suggestion below (that _get_console_pid should raise an exception) then the code here could be changed to:\n\n  try:\n    console_pid \u003d _get_console_pid()\n  except NoConsolePid:\n    LOG.warn(....)\n  else:\n    utils.execute(...)\n  finally:\n    utils.unlink_without_raise(...)","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":238,"context_line":"    \"\"\"Close the Serial Console of a node.\"\"\""},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    console_pid \u003d _get_console_pid(driver_info[\u0027uuid\u0027])"},{"line_number":241,"context_line":"    if console_pid:"},{"line_number":242,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":243,"context_line":"        utils.execute(\u0027kill\u0027, \u0027-TERM\u0027, str(console_pid),"},{"line_number":244,"context_line":"                      run_as_root\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2Fthc%3D","line":241,"in_reply_to":"AAAAT3%2F%2F9gc%3D","updated":"2014-01-03 02:54:30.000000000","message":"IMO it\u0027s possible that the stop_console will be issued multiple times, because before it\u0027s issued, the user does not know if the console is stopped or not, so I think it\u0027s possible no console_pid when stop_console is issued, then maybe logging a warning is ok?","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"dbbece02b44fe674f5100b85788a52e27faf2b7a","unresolved":false,"context_lines":[{"line_number":240,"context_line":"    console_pid \u003d _get_console_pid(driver_info[\u0027uuid\u0027])"},{"line_number":241,"context_line":"    if console_pid:"},{"line_number":242,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":243,"context_line":"        utils.execute(\u0027kill\u0027, \u0027-TERM\u0027, str(console_pid),"},{"line_number":244,"context_line":"                      run_as_root\u003dTrue,"},{"line_number":245,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":246,"context_line":"    utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F9ho%3D","line":243,"updated":"2013-12-26 09:04:44.000000000","message":"TERM is default.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":240,"context_line":"    console_pid \u003d _get_console_pid(driver_info[\u0027uuid\u0027])"},{"line_number":241,"context_line":"    if console_pid:"},{"line_number":242,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":243,"context_line":"        utils.execute(\u0027kill\u0027, \u0027-TERM\u0027, str(console_pid),"},{"line_number":244,"context_line":"                      run_as_root\u003dTrue,"},{"line_number":245,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":246,"context_line":"    utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FthQ%3D","line":243,"in_reply_to":"AAAAT3%2F%2F9ho%3D","updated":"2014-01-03 02:54:30.000000000","message":"Done","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"dbbece02b44fe674f5100b85788a52e27faf2b7a","unresolved":false,"context_lines":[{"line_number":241,"context_line":"    if console_pid:"},{"line_number":242,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":243,"context_line":"        utils.execute(\u0027kill\u0027, \u0027-TERM\u0027, str(console_pid),"},{"line_number":244,"context_line":"                      run_as_root\u003dTrue,"},{"line_number":245,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":246,"context_line":"    utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F9h8%3D","line":244,"updated":"2013-12-26 09:04:44.000000000","message":"IMO run_as_root not needed here.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":241,"context_line":"    if console_pid:"},{"line_number":242,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":243,"context_line":"        utils.execute(\u0027kill\u0027, \u0027-TERM\u0027, str(console_pid),"},{"line_number":244,"context_line":"                      run_as_root\u003dTrue,"},{"line_number":245,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":246,"context_line":"    utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FthM%3D","line":244,"in_reply_to":"AAAAT3%2F%2F9h8%3D","updated":"2014-01-03 02:54:30.000000000","message":"Done.\nYuri, Thanks for the comments!!","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"86676002d7610a243bd5ee699a64d473f20bc567","unresolved":false,"context_lines":[{"line_number":258,"context_line":"    \"\"\"Get the terminal process id from pid file.\"\"\""},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"    pid_path \u003d _get_console_pid_file(node_uuid)"},{"line_number":261,"context_line":"    if os.path.exists(pid_path):"},{"line_number":262,"context_line":"        with open(pid_path, \u0027r\u0027) as f:"},{"line_number":263,"context_line":"            pid_str \u003d f.read()"},{"line_number":264,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F7IU%3D","line":261,"updated":"2013-12-27 02:30:00.000000000","message":"Instead of LBYL here, you should simply try to read the file. Eg:\n\n  pid_path \u003d ...\n  try:\n    with open(pid_path, \u0027r\u0027) as f:\n      pid \u003d f.readline()\n      return int(pid)\n  except IOError, ValueError:\n    raise ...","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":258,"context_line":"    \"\"\"Get the terminal process id from pid file.\"\"\""},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"    pid_path \u003d _get_console_pid_file(node_uuid)"},{"line_number":261,"context_line":"    if os.path.exists(pid_path):"},{"line_number":262,"context_line":"        with open(pid_path, \u0027r\u0027) as f:"},{"line_number":263,"context_line":"            pid_str \u003d f.read()"},{"line_number":264,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FtgQ%3D","line":261,"in_reply_to":"AAAAT3%2F%2F7IU%3D","updated":"2014-01-03 02:54:30.000000000","message":"yes, I agree, raising a exception in _get_console_pid().\nThanks, Deva :)","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"86676002d7610a243bd5ee699a64d473f20bc567","unresolved":false,"context_lines":[{"line_number":260,"context_line":"    pid_path \u003d _get_console_pid_file(node_uuid)"},{"line_number":261,"context_line":"    if os.path.exists(pid_path):"},{"line_number":262,"context_line":"        with open(pid_path, \u0027r\u0027) as f:"},{"line_number":263,"context_line":"            pid_str \u003d f.read()"},{"line_number":264,"context_line":"        try:"},{"line_number":265,"context_line":"            return int(pid_str)"},{"line_number":266,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2F7JQ%3D","line":263,"updated":"2013-12-27 02:30:00.000000000","message":"Please use f.readline() instead of f.read(). There is no need to read more than one line, and an unbounded read is potentially exploitable...","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e8c01f0fc74b35f633a8cd5008af213954e195e4","unresolved":false,"context_lines":[{"line_number":260,"context_line":"    pid_path \u003d _get_console_pid_file(node_uuid)"},{"line_number":261,"context_line":"    if os.path.exists(pid_path):"},{"line_number":262,"context_line":"        with open(pid_path, \u0027r\u0027) as f:"},{"line_number":263,"context_line":"            pid_str \u003d f.read()"},{"line_number":264,"context_line":"        try:"},{"line_number":265,"context_line":"            return int(pid_str)"},{"line_number":266,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2FwcI%3D","line":263,"in_reply_to":"AAAAT3%2F%2F7JQ%3D","updated":"2014-01-03 02:54:30.000000000","message":"Done","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"950557a27e26d22ede9f2572923812f3e06797ee","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"opts \u003d ["},{"line_number":41,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"},{"line_number":42,"context_line":"            help\u003d\u0027The terminal SSL cert(PEM) for serial console access\u0027),"},{"line_number":43,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":44,"context_line":"            default\u003d\u0027$state_path/console\u0027,"},{"line_number":45,"context_line":"            help\u003d\u0027Directory for holding terminal pidfiles\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2Fqtk%3D","line":42,"updated":"2014-01-09 21:57:43.000000000","message":"How about \"Directory containing the terminal ...\"","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"0ea5ef14614a57d237fe65dc4120cc270f46bcb7","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"opts \u003d ["},{"line_number":41,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"},{"line_number":42,"context_line":"            help\u003d\u0027The terminal SSL cert(PEM) for serial console access\u0027),"},{"line_number":43,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":44,"context_line":"            default\u003d\u0027$state_path/console\u0027,"},{"line_number":45,"context_line":"            help\u003d\u0027Directory for holding terminal pidfiles\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2FOfI%3D","line":42,"in_reply_to":"AAAAUH%2F%2Fqtk%3D","updated":"2014-01-16 07:14:39.000000000","message":"Done","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    args \u003d []"},{"line_number":196,"context_line":"    args.append(\"shellinaboxd\")"},{"line_number":197,"context_line":"    if CONF.ipmi.terminal_cert_dir:"},{"line_number":198,"context_line":"        args.append(\"-c\")"},{"line_number":199,"context_line":"        args.append(CONF.ipmi.terminal_cert_dir)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0Fo%3D","line":196,"updated":"2014-01-08 06:50:40.000000000","message":"We need absolute path to call command to avoid missing path issue, suggest to use default path:\n\n    /usr/local/bin/shellinaboxd\n\nStill has concern about \u0027shellinaboxd\u0027 command, it is not shipped with Linux by default, so should we add it to our documents or requirement files to make sure it is clear that we depends on this 3rd tool.\nIf no such information, user have no idea until she/he check the log files in details and found the message about \"shellinaboxd\" command call.","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"0ea5ef14614a57d237fe65dc4120cc270f46bcb7","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    args \u003d []"},{"line_number":196,"context_line":"    args.append(\"shellinaboxd\")"},{"line_number":197,"context_line":"    if CONF.ipmi.terminal_cert_dir:"},{"line_number":198,"context_line":"        args.append(\"-c\")"},{"line_number":199,"context_line":"        args.append(CONF.ipmi.terminal_cert_dir)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2FOfA%3D","line":196,"in_reply_to":"AAAAUH%2F%2F0Fo%3D","updated":"2014-01-16 07:14:39.000000000","message":"Done","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        args.append(CONF.ipmi.terminal_cert_dir)"},{"line_number":200,"context_line":"    else:"},{"line_number":201,"context_line":"        args.append(\"-t\")"},{"line_number":202,"context_line":"    args.append(\"-p\")"},{"line_number":203,"context_line":"    args.append(str(driver_info[\u0027port\u0027]))"},{"line_number":204,"context_line":"    args.append(\"--background\u003d%s\" % _get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":205,"context_line":"    args.append(\"-s\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0E4%3D","line":202,"updated":"2014-01-08 06:50:40.000000000","message":"Looks like this command support \u0027-u\u0027 option to run as a special os user, just have a concern here if we have permission to run this with current openstack process user, and if we need to wrap the command with this file:\n\nhttps://github.com/openstack/ironic/blob/master/etc/ironic/rootwrap.conf","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    else:"},{"line_number":201,"context_line":"        args.append(\"-t\")"},{"line_number":202,"context_line":"    args.append(\"-p\")"},{"line_number":203,"context_line":"    args.append(str(driver_info[\u0027port\u0027]))"},{"line_number":204,"context_line":"    args.append(\"--background\u003d%s\" % _get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":205,"context_line":"    args.append(\"-s\")"},{"line_number":206,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0Fg%3D","line":203,"updated":"2014-01-08 06:50:40.000000000","message":"The \u0027port\u0027 is too generic, not sure what is this port, so can we rename to a special name, ie. \u0027terminal_port\u0027 or \u0027console_port\u0027?\n\nPlease use object attributes as attributes instead of dictionary keys, check below path for more details:\n\nhttps://review.openstack.org/#/c/64108/","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"51bb1f404d1268a3e1970af876832fc7c93f8a85","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    else:"},{"line_number":201,"context_line":"        args.append(\"-t\")"},{"line_number":202,"context_line":"    args.append(\"-p\")"},{"line_number":203,"context_line":"    args.append(str(driver_info[\u0027port\u0027]))"},{"line_number":204,"context_line":"    args.append(\"--background\u003d%s\" % _get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":205,"context_line":"    args.append(\"-s\")"},{"line_number":206,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2Fzxg%3D","line":203,"in_reply_to":"AAAAUH%2F%2F0Fg%3D","updated":"2014-01-08 07:36:36.000000000","message":"driver_info is not object, so ignore here, it applies with object dictionary-like-accessing only.","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":205,"context_line":"    args.append(\"-s\")"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    try:"},{"line_number":208,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027])"},{"line_number":209,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":210,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":211,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0QE%3D","line":208,"updated":"2014-01-08 06:50:40.000000000","message":"_make_password_file does not return file name or others, so no need to associate with a var pwfile, will return None\t\nfor ever.\nIf we need use the file created by _make_password_file, \u0027with\u0027 keyword is required to hold the file with with-block, here is the reference code:\n\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ipmitool.py#L99","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"13d9c2d9903f71e248b06b8252e609b1cafc9cbd","unresolved":false,"context_lines":[{"line_number":205,"context_line":"    args.append(\"-s\")"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    try:"},{"line_number":208,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027])"},{"line_number":209,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":210,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":211,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0A4%3D","line":208,"in_reply_to":"AAAAUH%2F%2F0QE%3D","updated":"2014-01-08 07:00:32.000000000","message":"Ignore my previous comment, and correct myself here, we can get pwfile without \u0027with\u0027 keyword.","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":212,"context_line":"                   \u0027gid\u0027: os.getgid(),"},{"line_number":213,"context_line":"                   \u0027address\u0027: driver_info[\u0027address\u0027],"},{"line_number":214,"context_line":"                   \u0027user\u0027: driver_info[\u0027username\u0027],"},{"line_number":215,"context_line":"                   \u0027pwfile\u0027: pwfile,"},{"line_number":216,"context_line":"                   }"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        args.append(ipmi_args)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0H0%3D","line":215,"updated":"2014-01-08 06:50:40.000000000","message":"As previous comments, pwfile can not be used here, we can  add \u0027with\u0027 block to hold the pwfile during command running.\n\nwith _make_password_file(driver_info[\u0027password\u0027]) as pw_file:\n   #to use pw_file in this current \u0027with\u0027 block","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"13d9c2d9903f71e248b06b8252e609b1cafc9cbd","unresolved":false,"context_lines":[{"line_number":212,"context_line":"                   \u0027gid\u0027: os.getgid(),"},{"line_number":213,"context_line":"                   \u0027address\u0027: driver_info[\u0027address\u0027],"},{"line_number":214,"context_line":"                   \u0027user\u0027: driver_info[\u0027username\u0027],"},{"line_number":215,"context_line":"                   \u0027pwfile\u0027: pwfile,"},{"line_number":216,"context_line":"                   }"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        args.append(ipmi_args)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0Aw%3D","line":215,"in_reply_to":"AAAAUH%2F%2F0H0%3D","updated":"2014-01-08 07:00:32.000000000","message":"Ignore my previous comment, and correct myself here, we can use  pwfile  without \u0027with\u0027 keyword, but it is better we use \u0027with\u0027 block for more convenient:)","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":222,"context_line":"        x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":223,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":224,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":225,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":226,"context_line":"    finally:"},{"line_number":227,"context_line":"        utils.unlink_without_raise(pwfile)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0GQ%3D","line":224,"updated":"2014-01-08 06:50:40.000000000","message":"Some time the stdout/stderr is important for debugging/troubleshooting, so can we take care to redirect to log files? If there is something wrong during launching this command we can get nothing message to root cause without these command stand out and stand err outputs.","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":223,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":224,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":225,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":226,"context_line":"    finally:"},{"line_number":227,"context_line":"        utils.unlink_without_raise(pwfile)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0Gs%3D","line":225,"updated":"2014-01-08 06:50:40.000000000","message":"if shellinaboxd is synchronization command call, I dont think utils.execute is asynchronous method or it has timeout-control mechanism, so we should take care here to avoid our code hang here within the _start_console method call.","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"0ea5ef14614a57d237fe65dc4120cc270f46bcb7","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":223,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":224,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":225,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":226,"context_line":"    finally:"},{"line_number":227,"context_line":"        utils.unlink_without_raise(pwfile)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2FOdg%3D","line":225,"in_reply_to":"AAAAUH%2F%2F0Gs%3D","updated":"2014-01-16 07:14:39.000000000","message":"Haomeng,\n\nThank you for the useful comments!\n\nwe called shellinaboxd with --background flag so that it will run in background, so utils.execute() should not hang here.","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":223,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":224,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":225,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":226,"context_line":"    finally:"},{"line_number":227,"context_line":"        utils.unlink_without_raise(pwfile)"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0Ek%3D","line":226,"updated":"2014-01-08 06:50:40.000000000","message":"we just have finally block in this try block, should expect block is required to handle/log exceptions?","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":225,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":226,"context_line":"    finally:"},{"line_number":227,"context_line":"        utils.unlink_without_raise(pwfile)"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"def _stop_console(driver_info):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0PM%3D","line":227,"updated":"2014-01-08 06:50:40.000000000","message":"No need to delete file here, because after unlink_without_raise called, the password file will be deleted by line 65","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"13d9c2d9903f71e248b06b8252e609b1cafc9cbd","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":225,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":226,"context_line":"    finally:"},{"line_number":227,"context_line":"        utils.unlink_without_raise(pwfile)"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"def _stop_console(driver_info):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0BE%3D","line":227,"in_reply_to":"AAAAUH%2F%2F0PM%3D","updated":"2014-01-08 07:00:32.000000000","message":"We can use \u0027with\u0027 to call _make_password_file, then the pwd file will be removed by _make_password_file internally.","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"950557a27e26d22ede9f2572923812f3e06797ee","unresolved":false,"context_lines":[{"line_number":244,"context_line":""},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"def _get_console_pid_file(node_uuid):"},{"line_number":247,"context_line":"    \"\"\"Generate the pid file to hold the terminal process id.\"\"\""},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    name \u003d \"%s.pid\" % node_uuid"},{"line_number":250,"context_line":"    path \u003d os.path.join(CONF.ipmi.terminal_pid_dir, name)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2Fqro%3D","line":247,"updated":"2014-01-09 21:57:43.000000000","message":"This is getting the filename, not the actual file. Might want to change it to be clearer. eg _get_console_pid_filename()","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":334,"context_line":"class IPMIConsole(base.ConsoleInterface):"},{"line_number":335,"context_line":"    \"\"\"Interface for console-related actions.\"\"\""},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def validate(self, node):"},{"line_number":338,"context_line":"        \"\"\"Validate the Node console info."},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"        :param node: a single Node to validate."}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0Jg%3D","line":337,"updated":"2014-01-08 06:50:40.000000000","message":"We just implement this ConsoleInterface, but no code to call this method to validate/start/stop console, so I think we should launch the console in below method:\n\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ipmitool.py#L199\n\nAnd if we want to enable ConsoleInterface on high level for all drivers that will support launch-console function, we should enable more code in drivers together to call ConsoleInterface methods on driver level:\n\nhttps://github.com/openstack/ironic/blob/master/ironic/conductor/utils.py#L25","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"0ea5ef14614a57d237fe65dc4120cc270f46bcb7","unresolved":false,"context_lines":[{"line_number":334,"context_line":"class IPMIConsole(base.ConsoleInterface):"},{"line_number":335,"context_line":"    \"\"\"Interface for console-related actions.\"\"\""},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def validate(self, node):"},{"line_number":338,"context_line":"        \"\"\"Validate the Node console info."},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"        :param node: a single Node to validate."}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2FOZs%3D","line":337,"in_reply_to":"AAAAUH%2F%2F0Jg%3D","updated":"2014-01-16 07:14:39.000000000","message":"Haomeng,\n\nConsole should be an independent method to power, see the comments from Deva and Yuri listed in previous patches, so I implemented IPMIConsole as a new class.","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"efa1600cd5db460c539848d04fafe8259388d8e6","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        :raises: InvalidParameterValue"},{"line_number":342,"context_line":"        \"\"\""},{"line_number":343,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":344,"context_line":"        if not driver_info[\u0027port\u0027]:"},{"line_number":345,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":346,"context_line":"                \"IPMI ternimal port not supplied to IPMI driver.\"))"},{"line_number":347,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2F0JI%3D","line":344,"updated":"2014-01-08 06:50:40.000000000","message":"The \u0027port\u0027 is too generic, so can we rename to a special name, ie. \u0027terminal_port\u0027 or \u0027console_port\u0027?","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"950557a27e26d22ede9f2572923812f3e06797ee","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def start_console(self, task, node):"},{"line_number":349,"context_line":"        \"\"\"Start a remote console for the node.\"\"\""},{"line_number":350,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":351,"context_line":"        return _start_console(driver_info)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def stop_console(self, task, node):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F%2FqoY%3D","line":350,"updated":"2014-01-09 21:57:43.000000000","message":"If port has to be specified, you might want to call validate(node) instead?","commit_id":"f12f6b75ccf9f4ec3b8d301038df89a99b5ecff2"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"d9f748f9111c180fc6c9f3e0894309e49b6cc505","unresolved":false,"context_lines":[{"line_number":42,"context_line":"            help\u003d\u0027Directory containing the terminal SSL cert(PEM) for \u0027"},{"line_number":43,"context_line":"            \u0027serial console access\u0027),"},{"line_number":44,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":45,"context_line":"            default\u003d\u0027$state_path/console\u0027,"},{"line_number":46,"context_line":"            help\u003d\u0027Directory for holding terminal pidfiles\u0027),"},{"line_number":47,"context_line":"    ]"},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FHIA%3D","line":45,"updated":"2014-01-17 10:40:25.000000000","message":"The option $state_path is loaded by paths.py [1], here $state_path is not working, suggest the following code as reference:\n\n    from ironic.common import paths    \n    \n    default\u003dpaths.basedir_def(\u0027console\u0027),\n\n[1] https://github.com/openstack/ironic/blob/master/ironic/common/paths.py#L32","commit_id":"6cb11260fc856f983d6dac41fb10bffd53f38dcd"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"0ae159adaf1c34ccedf57d3c3bae1698e95140a8","unresolved":false,"context_lines":[{"line_number":42,"context_line":"            help\u003d\u0027Directory containing the terminal SSL cert(PEM) for \u0027"},{"line_number":43,"context_line":"            \u0027serial console access\u0027),"},{"line_number":44,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":45,"context_line":"            default\u003d\u0027$state_path/console\u0027,"},{"line_number":46,"context_line":"            help\u003d\u0027Directory for holding terminal pidfiles\u0027),"},{"line_number":47,"context_line":"    ]"},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2B4M8%3D","line":45,"in_reply_to":"AAAAUH%2F%2FHIA%3D","updated":"2014-01-21 06:37:09.000000000","message":"haomeng,\n\nyes, you are correct, I have updated it.","commit_id":"6cb11260fc856f983d6dac41fb10bffd53f38dcd"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"f7dbdc27803b47c88dcbc12a09de755e4112ca23","unresolved":false,"context_lines":[{"line_number":193,"context_line":"def _start_console(driver_info):"},{"line_number":194,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    args \u003d []"},{"line_number":197,"context_line":"    args.append(\"/usr/local/bin/shellinaboxd\")"},{"line_number":198,"context_line":"    if CONF.ipmi.terminal_cert_dir:"},{"line_number":199,"context_line":"        args.append(\"-c\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FGHw%3D","line":196,"updated":"2014-01-17 14:41:30.000000000","message":"I\u0027d recommend to specify all the arguments which are not conditional in-plase, i.e.: \n\n    args \u003d [path_to_the_shellinaboxd,\n            \u0027-p\u0027, # Description of the parameter\n            \u0027-s\u0027, # Description of the parameter 2\n            ....]","commit_id":"6cb11260fc856f983d6dac41fb10bffd53f38dcd"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"f7dbdc27803b47c88dcbc12a09de755e4112ca23","unresolved":false,"context_lines":[{"line_number":194,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    args \u003d []"},{"line_number":197,"context_line":"    args.append(\"/usr/local/bin/shellinaboxd\")"},{"line_number":198,"context_line":"    if CONF.ipmi.terminal_cert_dir:"},{"line_number":199,"context_line":"        args.append(\"-c\")"},{"line_number":200,"context_line":"        args.append(CONF.ipmi.terminal_cert_dir)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FGNw%3D","line":197,"updated":"2014-01-17 14:41:30.000000000","message":"On different distributions this path might be different.\nAlso due to various reasons users might want to use custom build/installation of shellinaboxd. That\u0027s why think it\u0027s better to move this path to the confi.","commit_id":"6cb11260fc856f983d6dac41fb10bffd53f38dcd"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"0ae159adaf1c34ccedf57d3c3bae1698e95140a8","unresolved":false,"context_lines":[{"line_number":194,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    args \u003d []"},{"line_number":197,"context_line":"    args.append(\"/usr/local/bin/shellinaboxd\")"},{"line_number":198,"context_line":"    if CONF.ipmi.terminal_cert_dir:"},{"line_number":199,"context_line":"        args.append(\"-c\")"},{"line_number":200,"context_line":"        args.append(CONF.ipmi.terminal_cert_dir)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2B4KY%3D","line":197,"in_reply_to":"AAAAUH%2F%2FGNw%3D","updated":"2014-01-21 06:37:09.000000000","message":"Roman,\n\nThanks for the comments!\nThis makes sense to me, I have updated the code.","commit_id":"6cb11260fc856f983d6dac41fb10bffd53f38dcd"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"f7dbdc27803b47c88dcbc12a09de755e4112ca23","unresolved":false,"context_lines":[{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    try:"},{"line_number":209,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027])"},{"line_number":210,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":211,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":212,"context_line":"                % {\u0027uid\u0027: os.getuid(),"},{"line_number":213,"context_line":"                   \u0027gid\u0027: os.getgid(),"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FF7w%3D","line":210,"updated":"2014-01-17 14:41:30.000000000","message":"For improving readability of the code I can suggest defining the arguments as an array and then joining it.","commit_id":"6cb11260fc856f983d6dac41fb10bffd53f38dcd"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"bec464c5406f8ddf884a1da54cb06299e0514363","unresolved":false,"context_lines":[{"line_number":47,"context_line":"            help\u003d\u0027Directory for holding terminal pidfiles\u0027),"},{"line_number":48,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":49,"context_line":"            default\u003d\u0027/usr/local/bin/shellinaboxd\u0027,"},{"line_number":50,"context_line":"            help\u003d\u0027Absolute path for shellinaboxd program\u0027),"},{"line_number":51,"context_line":"    ]"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FtAM%3D","line":50,"updated":"2014-02-11 11:14:04.000000000","message":"Let\u0027s not use the absolute path here, it makes it hard to support multiple distros. Nova also doesn\u0027t use it[1]\n\n[1] https://github.com/openstack/nova/blob/master/nova/virt/baremetal/ipmi.py#L40-L42","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"c80a9b6b7382d7144d9d4208e26c7ee85c591604","unresolved":false,"context_lines":[{"line_number":47,"context_line":"            help\u003d\u0027Directory for holding terminal pidfiles\u0027),"},{"line_number":48,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":49,"context_line":"            default\u003d\u0027/usr/local/bin/shellinaboxd\u0027,"},{"line_number":50,"context_line":"            help\u003d\u0027Absolute path for shellinaboxd program\u0027),"},{"line_number":51,"context_line":"    ]"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FdXk%3D","line":50,"in_reply_to":"AAAAUX%2F%2FtAM%3D","updated":"2014-02-13 02:43:55.000000000","message":"Lucas,\n\nThanks for the comments :)\n\nyes, in my first patch, I did not use the absolute path here, just referenced nova implementation, but got comments from others(sorry, I forgot whose comments it is), they think to list the absolute path in the ironic.conf file is better, if the user is using other distros, then just change the path accordingly. So I re-submitted current patch...","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"3ee0028347fa58b541cd5a9d65cc797dc2fec41d","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    args.append(\"-s\")"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    try:"},{"line_number":213,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027])"},{"line_number":214,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":215,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":216,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2B3RM%3D","line":213,"updated":"2014-01-21 10:44:14.000000000","message":"1) There is a problem with empty password, \n\n_make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027)\n\nneeded as line 111.\n\n2)  _make_password_file is context manager, \u0027try-finally\u0027 block not needed,\n\nwith _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027) as pw_file:\n    ...","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"c80a9b6b7382d7144d9d4208e26c7ee85c591604","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    args.append(\"-s\")"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    try:"},{"line_number":213,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027])"},{"line_number":214,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":215,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":216,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FdW4%3D","line":213,"in_reply_to":"AAAAUH%2F%2B3RM%3D","updated":"2014-02-13 02:43:55.000000000","message":"Done","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"044d60db716e94bb3c9b9f7392696e02a003bead","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    try:"},{"line_number":213,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027])"},{"line_number":214,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":215,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":216,"context_line":"                % {\u0027uid\u0027: os.getuid(),"},{"line_number":217,"context_line":"                   \u0027gid\u0027: os.getgid(),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2B08Q%3D","line":214,"updated":"2014-01-21 17:24:39.000000000","message":"The latest trend is not to use escape symbols for long strings. Could you please replace them with brackets?","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e86416c153d12253bc63af9fcbe3be7e15047461","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":227,"context_line":"        x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":228,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":229,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":230,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":231,"context_line":"    finally:"},{"line_number":232,"context_line":"        utils.unlink_without_raise(pwfile)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2BeeM%3D","line":229,"updated":"2014-01-25 10:46:59.000000000","message":"We redirect both the stdout and stderror to null, we have no way to check these message returned by command, so it is difficult to check root cause if there is something wrong, such as one base case - shellinabox is not installed in the machine or the listen port is conflict with existing shellinabox process for different pxe node.\n\nSo suggest to log these information into files for tracing back the command std/err output.","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"bec464c5406f8ddf884a1da54cb06299e0514363","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    else:"},{"line_number":244,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":245,"context_line":"        utils.execute(\u0027kill\u0027, str(console_pid),"},{"line_number":246,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":247,"context_line":"    finally:"},{"line_number":248,"context_line":"        utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":249,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2Fs9Y%3D","line":246,"updated":"2014-02-11 11:14:04.000000000","message":"Looking at Nova, the kill command might have to run as root[1]. If that\u0027s the case you also would have to add the permission to our rootwrap filters[2]\n\n[1] https://github.com/openstack/nova/blob/master/nova/virt/baremetal/ipmi.py#L301\n[2] https://github.com/openstack/nova/blob/master/etc/nova/rootwrap.d/baremetal-compute-ipmi.filters#L8-L9","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"c80a9b6b7382d7144d9d4208e26c7ee85c591604","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    else:"},{"line_number":244,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":245,"context_line":"        utils.execute(\u0027kill\u0027, str(console_pid),"},{"line_number":246,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":247,"context_line":"    finally:"},{"line_number":248,"context_line":"        utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":249,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FdWg%3D","line":246,"in_reply_to":"AAAAUX%2F%2Fs9Y%3D","updated":"2014-02-13 02:43:55.000000000","message":"Done","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"3ee0028347fa58b541cd5a9d65cc797dc2fec41d","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"def _get_console_pid_file(node_uuid):"},{"line_number":252,"context_line":"    \"\"\"Generate the pid file to hold the terminal process id.\"\"\""},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    name \u003d \"%s.pid\" % node_uuid"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2B3OE%3D","line":251,"updated":"2014-01-21 10:44:14.000000000","message":"This function returns path, not file. Name like _get_console_pid_path will be better.","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"c80a9b6b7382d7144d9d4208e26c7ee85c591604","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"def _get_console_pid_file(node_uuid):"},{"line_number":252,"context_line":"    \"\"\"Generate the pid file to hold the terminal process id.\"\"\""},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    name \u003d \"%s.pid\" % node_uuid"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FdWA%3D","line":251,"in_reply_to":"AAAAUH%2F%2B3OE%3D","updated":"2014-02-13 02:43:55.000000000","message":"it\u0027s a file indeed :), for example, \u003cCONF.ipmi.terminal_pid_dir\u003e/\u003cnode_uuid\u003e.pid, maybe I need to change \"path\" to \"file\", then return file to make it better.","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"3ee0028347fa58b541cd5a9d65cc797dc2fec41d","unresolved":false,"context_lines":[{"line_number":346,"context_line":"        :raises: InvalidParameterValue"},{"line_number":347,"context_line":"        \"\"\""},{"line_number":348,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":349,"context_line":"        if not driver_info[\u0027port\u0027]:"},{"line_number":350,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":351,"context_line":"                \"IPMI ternimal port not supplied to IPMI driver.\"))"},{"line_number":352,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2B3JM%3D","line":349,"updated":"2014-01-21 10:44:14.000000000","message":"If we start many processes of shellinaboxd simultaneously IMO port should be unique.","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"c80a9b6b7382d7144d9d4208e26c7ee85c591604","unresolved":false,"context_lines":[{"line_number":346,"context_line":"        :raises: InvalidParameterValue"},{"line_number":347,"context_line":"        \"\"\""},{"line_number":348,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":349,"context_line":"        if not driver_info[\u0027port\u0027]:"},{"line_number":350,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":351,"context_line":"                \"IPMI ternimal port not supplied to IPMI driver.\"))"},{"line_number":352,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FdVs%3D","line":349,"in_reply_to":"AAAAUH%2F%2B3JM%3D","updated":"2014-02-13 02:43:55.000000000","message":"yes, you are correct. the port should be defined as part of driver_info.","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"e86416c153d12253bc63af9fcbe3be7e15047461","unresolved":false,"context_lines":[{"line_number":350,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":351,"context_line":"                \"IPMI ternimal port not supplied to IPMI driver.\"))"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def start_console(self, task, node):"},{"line_number":354,"context_line":"        \"\"\"Start a remote console for the node.\"\"\""},{"line_number":355,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":356,"context_line":"        return _start_console(driver_info)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2Beec%3D","line":353,"updated":"2014-01-25 10:46:59.000000000","message":"I applied this patch and try to have a function test, looks like this start_console() method is not called by any code at all, and searched existing code, only one start_console can be found in this method define in our base driver class [1],  so this should be issue, we have to call this console interface method in the power on action code in our IPMI driver or high level code.\n\n[1] https://github.com/openstack/ironic/blob/master/ironic/drivers/base.py#L238","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"b99c27bf9346060a24766b699d3ff414ab2d3bcd","unresolved":false,"context_lines":[{"line_number":350,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":351,"context_line":"                \"IPMI ternimal port not supplied to IPMI driver.\"))"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def start_console(self, task, node):"},{"line_number":354,"context_line":"        \"\"\"Start a remote console for the node.\"\"\""},{"line_number":355,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":356,"context_line":"        return _start_console(driver_info)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F9x9A%3D","line":353,"in_reply_to":"AAAAUH%2F%2Beec%3D","updated":"2014-02-04 01:09:03.000000000","message":"Haomeng, that is one way, but that approach means starting a console for every single node that is powered on, which is neither scalable nor possible on some hardware (eg, bridged IPMI with a limited number of shared concurrent SOL sessions).\n\nThe proper solution is that we need to expose the console state in our REST API, just like we have /states/power and /states/provision today.","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"5119b7c73e737fd897e77e08d3f89ad429b11230","unresolved":false,"context_lines":[{"line_number":350,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":351,"context_line":"                \"IPMI ternimal port not supplied to IPMI driver.\"))"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def start_console(self, task, node):"},{"line_number":354,"context_line":"        \"\"\"Start a remote console for the node.\"\"\""},{"line_number":355,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":356,"context_line":"        return _start_console(driver_info)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2B6Q4%3D","line":353,"in_reply_to":"AAAAUH%2F9x9A%3D","updated":"2014-02-18 09:02:17.000000000","message":"Haomeng,\n\nyes, I saw Lucas has implemented the REST API part, please see the details here https://review.openstack.org/#/c/72998/","commit_id":"6dec2dfa20cf69108bf9074fda654ffb32ede552"},{"author":{"_account_id":7411,"name":"Matt Wagner","email":"matt.wagner@redhat.com","username":"matty_dubs"},"change_message_id":"f82bab425e3cb0cfde91e49a4e0c0dfa3e69e2e1","unresolved":false,"context_lines":[{"line_number":265,"context_line":"    try:"},{"line_number":266,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027])"},{"line_number":267,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":268,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":269,"context_line":"                % {\u0027uid\u0027: os.getuid(),"},{"line_number":270,"context_line":"                   \u0027gid\u0027: os.getgid(),"},{"line_number":271,"context_line":"                   \u0027address\u0027: driver_info[\u0027address\u0027],"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2B2N0%3D","line":268,"updated":"2014-02-18 16:54:23.000000000","message":"I had trouble here. It looks like _make_password_file deletes the file before returning, so it can only be used inside a block.\n\nI think we might have to modify _make_password_file to support persisting the password file, because it is started when the user connects to the shellinaboxd session, which might be a while after the console is started.","commit_id":"27b17c748a7e0a3420fbd8482bb6197174c4811e"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"5f762a8c7fa322a0fa1acba84cf6fbff9d2846f8","unresolved":false,"context_lines":[{"line_number":441,"context_line":"    def stop_console(self, task, node):"},{"line_number":442,"context_line":"        \"\"\"Stop the remote console session for the node.\"\"\""},{"line_number":443,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":444,"context_line":"        return _stop_console(driver_info)"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2B6Ns%3D","line":444,"updated":"2014-02-18 09:19:04.000000000","message":"We have new interface method [1] to be implemented.\n\nhttps://github.com/openstack/ironic/blob/master/ironic/drivers/base.py#L255","commit_id":"27b17c748a7e0a3420fbd8482bb6197174c4811e"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"b7b8a206f85e3818d8228db9354304595d570cb1","unresolved":false,"context_lines":[{"line_number":441,"context_line":"    def stop_console(self, task, node):"},{"line_number":442,"context_line":"        \"\"\"Stop the remote console session for the node.\"\"\""},{"line_number":443,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":444,"context_line":"        return _stop_console(driver_info)"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2B594%3D","line":444,"in_reply_to":"AAAAUX%2F%2B6Ns%3D","updated":"2014-02-18 09:32:14.000000000","message":"haomeng,\n\nThanks. I noticed this new methold from jenkins error report too. I will implement it then submit the new patch.","commit_id":"27b17c748a7e0a3420fbd8482bb6197174c4811e"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"47614c636839601fcc77d95c1dfee1908bae49a9","unresolved":false,"context_lines":[{"line_number":263,"context_line":"    args.append(\"-s\")"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    with _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027) as pwfile:"},{"line_number":266,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027)"},{"line_number":267,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":268,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":269,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bwpg%3D","line":266,"updated":"2014-02-19 10:41:13.000000000","message":"Duplicated, this line is not required I think. because we have \u0027with\u0027 block to hold the password file already.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"72165dceafe47b06af0f2852a546be67bdd5e4b1","unresolved":false,"context_lines":[{"line_number":263,"context_line":"    args.append(\"-s\")"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    with _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027) as pwfile:"},{"line_number":266,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027)"},{"line_number":267,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":268,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":269,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bo6Y%3D","line":266,"in_reply_to":"AAAAUX%2F%2BrFs%3D","updated":"2014-02-20 06:46:59.000000000","message":"yes, I think you\u0027re correct, we could make the password file persistent until the console is stopped, but only a little curious about how nova works, I checked nova implementation about this part, it\u0027s the similar, just delete it once utils.execute() done.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7411,"name":"Matt Wagner","email":"matt.wagner@redhat.com","username":"matty_dubs"},"change_message_id":"6ae305bde9534c30ea5022814ecc19db96b282b2","unresolved":false,"context_lines":[{"line_number":263,"context_line":"    args.append(\"-s\")"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    with _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027) as pwfile:"},{"line_number":266,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027)"},{"line_number":267,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":268,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":269,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2BrFs%3D","line":266,"in_reply_to":"AAAAUX%2F%2Bwpg%3D","updated":"2014-02-19 22:00:07.000000000","message":"Agreed -- this line should be removed.\n\nThe password file problem goes deeper, though. By design, _make_password_file will delete it once this block terminates. So by the time someone connects to the shellinaboxd session, the password file is gone, so the console command will fail.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"72165dceafe47b06af0f2852a546be67bdd5e4b1","unresolved":false,"context_lines":[{"line_number":263,"context_line":"    args.append(\"-s\")"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    with _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027) as pwfile:"},{"line_number":266,"context_line":"        pwfile \u003d _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027)"},{"line_number":267,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":268,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":269,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bo8M%3D","line":266,"in_reply_to":"AAAAUX%2F%2Bwpg%3D","updated":"2014-02-20 06:46:59.000000000","message":"sorry, my fault, forgot to delete the old code after the change :)","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"47614c636839601fcc77d95c1dfee1908bae49a9","unresolved":false,"context_lines":[{"line_number":279,"context_line":"        x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":280,"context_line":"        x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":281,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":282,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":283,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bwm8%3D","line":282,"updated":"2014-02-19 10:41:13.000000000","message":"we redirect all the stand  console out and error message to null dev, in case of command failed, how can we get these message to check what happens, such as port conflict?","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"72165dceafe47b06af0f2852a546be67bdd5e4b1","unresolved":false,"context_lines":[{"line_number":279,"context_line":"        x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":280,"context_line":"        x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":281,"context_line":"        x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":282,"context_line":"        x.append(\u00272\u003e\u00261\u0027)"},{"line_number":283,"context_line":"        utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2BnyQ%3D","line":282,"in_reply_to":"AAAAUX%2F%2Bwm8%3D","updated":"2014-02-20 06:46:59.000000000","message":"Done","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"47614c636839601fcc77d95c1dfee1908bae49a9","unresolved":false,"context_lines":[{"line_number":303,"context_line":"def _get_console_url(driver_info):"},{"line_number":304,"context_line":"    \"\"\"Get a url to access the console via shellinaboxd.\"\"\""},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    console_host \u003d CONF.my_ip"},{"line_number":307,"context_line":"    if utils.is_valid_ipv6(console_host):"},{"line_number":308,"context_line":"        console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":309,"context_line":"    console_url \u003d \"http://%s:%s\" % (console_host, driver_info[\u0027port\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bwls%3D","line":306,"updated":"2014-02-19 10:41:13.000000000","message":"Not sure if we can access CONF.my_ip value from our ironic.conf(not default value) without registration in this file, need more reviewers confirmation here.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"72165dceafe47b06af0f2852a546be67bdd5e4b1","unresolved":false,"context_lines":[{"line_number":303,"context_line":"def _get_console_url(driver_info):"},{"line_number":304,"context_line":"    \"\"\"Get a url to access the console via shellinaboxd.\"\"\""},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    console_host \u003d CONF.my_ip"},{"line_number":307,"context_line":"    if utils.is_valid_ipv6(console_host):"},{"line_number":308,"context_line":"        console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":309,"context_line":"    console_url \u003d \"http://%s:%s\" % (console_host, driver_info[\u0027port\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2BnyA%3D","line":306,"in_reply_to":"AAAAUX%2F%2Bwls%3D","updated":"2014-02-20 06:46:59.000000000","message":"Matt,\n\nIf possible, could you help to test it in your environment? to see if we could get the correct URL from REST API? Thx a lot.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"47614c636839601fcc77d95c1dfee1908bae49a9","unresolved":false,"context_lines":[{"line_number":306,"context_line":"    console_host \u003d CONF.my_ip"},{"line_number":307,"context_line":"    if utils.is_valid_ipv6(console_host):"},{"line_number":308,"context_line":"        console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":309,"context_line":"    console_url \u003d \"http://%s:%s\" % (console_host, driver_info[\u0027port\u0027])"},{"line_number":310,"context_line":"    return console_url"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bwjo%3D","line":309,"updated":"2014-02-19 10:41:13.000000000","message":"We will get \u0027None\u0027 as port value in case of missing driver_info[\u0027port\u0027] configuration, so suggest to check first.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"72165dceafe47b06af0f2852a546be67bdd5e4b1","unresolved":false,"context_lines":[{"line_number":306,"context_line":"    console_host \u003d CONF.my_ip"},{"line_number":307,"context_line":"    if utils.is_valid_ipv6(console_host):"},{"line_number":308,"context_line":"        console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":309,"context_line":"    console_url \u003d \"http://%s:%s\" % (console_host, driver_info[\u0027port\u0027])"},{"line_number":310,"context_line":"    return console_url"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2BnxM%3D","line":309,"in_reply_to":"AAAAUX%2F%2Bwjo%3D","updated":"2014-02-20 06:46:59.000000000","message":"I think we can use IPMIConsole.validate() to cover the check, before we call IPMIConsole.get_console(), we should call IPMIConsole.validate() first, so we can check if the terminal port is provided or not.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7411,"name":"Matt Wagner","email":"matt.wagner@redhat.com","username":"matty_dubs"},"change_message_id":"6ae305bde9534c30ea5022814ecc19db96b282b2","unresolved":false,"context_lines":[{"line_number":456,"context_line":"class IPMIConsole(base.ConsoleInterface):"},{"line_number":457,"context_line":"    \"\"\"Interface for console-related actions.\"\"\""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    def validate(self, node):"},{"line_number":460,"context_line":"        \"\"\"Validate the Node console info."},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        :param node: a single Node to validate."}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2BrDg%3D","line":459,"updated":"2014-02-19 22:00:07.000000000","message":"It seems that Lucas\u0027 patch adding API support is calling this with (task, node) as params, which makes validation fail with an exception. It\u0027s not clear to me whether we should expect the extra parameter, or if they should stop sending it. I will check with him tomorrow.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"72165dceafe47b06af0f2852a546be67bdd5e4b1","unresolved":false,"context_lines":[{"line_number":456,"context_line":"class IPMIConsole(base.ConsoleInterface):"},{"line_number":457,"context_line":"    \"\"\"Interface for console-related actions.\"\"\""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    def validate(self, node):"},{"line_number":460,"context_line":"        \"\"\"Validate the Node console info."},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        :param node: a single Node to validate."}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bnwo%3D","line":459,"in_reply_to":"AAAAUX%2F%2BrDg%3D","updated":"2014-02-20 06:46:59.000000000","message":"Matt,\n\nyou are correct, my fault, validate() is defined with (self, task, node)  in base.py, I will update the code in next patch. Thx.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"d3fb84b28f7cc84eec97b9ee356f75e1b565a4f2","unresolved":false,"context_lines":[{"line_number":262,"context_line":"    args.append(\"--background\u003d%s\" % _get_console_pid_file(driver_info[\u0027uuid\u0027]))"},{"line_number":263,"context_line":"    args.append(\"-s\")"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    with _make_password_file(driver_info[\u0027password\u0027] or \u0027\\0\u0027) as pwfile:"},{"line_number":266,"context_line":"        ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":267,"context_line":"                \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":268,"context_line":"                % {\u0027uid\u0027: os.getuid(),"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2Bk7w%3D","line":265,"updated":"2014-02-20 12:20:11.000000000","message":"Maybe better move \n\n driver_info[\u0027password\u0027] or \u0027\\0\u0027\n\ncode into _make_password_file function for avoid duplicate with l135.","commit_id":"9ede479a105652919a5e074f70ae2b0651f6b412"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"d3fb84b28f7cc84eec97b9ee356f75e1b565a4f2","unresolved":false,"context_lines":[{"line_number":462,"context_line":"        :raises: InvalidParameterValue"},{"line_number":463,"context_line":"        \"\"\""},{"line_number":464,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":465,"context_line":"        if not driver_info[\u0027port\u0027]:"},{"line_number":466,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":467,"context_line":"                \"IPMI ternimal port not supplied to IPMI driver.\"))"},{"line_number":468,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2BlAw%3D","line":465,"updated":"2014-02-20 12:20:11.000000000","message":"What about check also that int(driver_info[\u0027port\u0027]) does not raise ValueError?","commit_id":"9ede479a105652919a5e074f70ae2b0651f6b412"},{"author":{"_account_id":7411,"name":"Matt Wagner","email":"matt.wagner@redhat.com","username":"matty_dubs"},"change_message_id":"47da85830c7d7ccbf3aedd0f80e3651fd1761cd7","unresolved":false,"context_lines":[{"line_number":327,"context_line":"def _get_console_url(driver_info):"},{"line_number":328,"context_line":"    \"\"\"Get a url to access the console via shellinaboxd.\"\"\""},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    console_host \u003d CONF.my_ip"},{"line_number":331,"context_line":"    if utils.is_valid_ipv6(console_host):"},{"line_number":332,"context_line":"        console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":333,"context_line":"    console_url \u003d \"http://%s:%s\" % (console_host, driver_info[\u0027port\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUX%2F%2BfE8%3D","line":330,"updated":"2014-02-20 22:47:23.000000000","message":"Haomeng asked about this in revision 9. It does seem to work for me:\n\n$ curl -X GET -H \u0027Content-Type: application/json\u0027 -H \u0027Accept: application/json\u0027 -H \"X-Auth-Token: $TOKEN\"  http://127.0.0.1:6385/v1/nodes/$NODE/states/console\n\n\"http://10.18.57.107:4200\"","commit_id":"388f53e61b6b4bbceb1133d9bd99147e52d6afca"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"61e083933c5c468bebd160f6caabbed3b4c701ae","unresolved":false,"context_lines":[{"line_number":327,"context_line":"def _get_console_url(driver_info):"},{"line_number":328,"context_line":"    \"\"\"Get a url to access the console via shellinaboxd.\"\"\""},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    console_host \u003d CONF.my_ip"},{"line_number":331,"context_line":"    if utils.is_valid_ipv6(console_host):"},{"line_number":332,"context_line":"        console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":333,"context_line":"    console_url \u003d \"http://%s:%s\" % (console_host, driver_info[\u0027port\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUX%2F%2BcZk%3D","line":330,"in_reply_to":"AAAAUX%2F%2BfE8%3D","updated":"2014-02-21 04:48:52.000000000","message":"Matt, Thank you for confirming this! :)","commit_id":"388f53e61b6b4bbceb1133d9bd99147e52d6afca"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"opts \u003d ["},{"line_number":42,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"},{"line_number":43,"context_line":"            help\u003d\u0027Directory containing the terminal SSL cert(PEM) for \u0027"},{"line_number":44,"context_line":"            \u0027serial console access\u0027),"},{"line_number":45,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":46,"context_line":"            default\u003dpaths.basedir_def(\u0027console\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BXgQ%3D","line":43,"updated":"2014-02-21 16:45:29.000000000","message":"This need align to the bracket (pep8\u0027s e128)\n\n    cfg.StrOpt(\u0027terminal_cert_dir\u0027,\n               help\u003d\u0027Directory containing the terminal SSL cert(PEM) for \u0027\n                    \u0027serial console access\u0027),","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"4ee2aaadf33044f96b0a480d9c240b0cf145eac1","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"opts \u003d ["},{"line_number":42,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"},{"line_number":43,"context_line":"            help\u003d\u0027Directory containing the terminal SSL cert(PEM) for \u0027"},{"line_number":44,"context_line":"            \u0027serial console access\u0027),"},{"line_number":45,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":46,"context_line":"            default\u003dpaths.basedir_def(\u0027console\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAVn%2F%2FvWo%3D","line":43,"in_reply_to":"AAAAU3%2F%2F7Us%3D","updated":"2014-03-04 11:19:55.000000000","message":"This PEP\u0027s is ignore in our tests, but we shouldn\u0027t do not stick to them. If you remove from ignore `E12` [1] you will see that you have a bug PEP.\n\n[1] https://github.com/openstack/ironic/blob/master/tox.ini#L34","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"e7f896eaa189c47dd06a41474794028004ff8955","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"opts \u003d ["},{"line_number":42,"context_line":"    cfg.StrOpt(\u0027terminal_cert_dir\u0027,"},{"line_number":43,"context_line":"            help\u003d\u0027Directory containing the terminal SSL cert(PEM) for \u0027"},{"line_number":44,"context_line":"            \u0027serial console access\u0027),"},{"line_number":45,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":46,"context_line":"            default\u003dpaths.basedir_def(\u0027console\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAU3%2F%2F7Us%3D","line":43,"in_reply_to":"AAAAUX%2F%2BXgQ%3D","updated":"2014-02-24 02:07:18.000000000","message":"Thank you Mikhail,\n\nI did not see the pep8 error during my unit test, have I missed any options? I\u0027d like to know it for the next unit test.","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":49,"context_line":"            default\u003d\u0027/usr/local/bin/shellinaboxd\u0027,"},{"line_number":50,"context_line":"            help\u003d\u0027Absolute path for shellinaboxd program\u0027),"},{"line_number":51,"context_line":"    ]"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"CONF \u003d cfg.CONF"},{"line_number":54,"context_line":"CONF.register_opts(opts, group\u003d\u0027ipmi\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BXdY%3D","line":51,"updated":"2014-02-21 16:45:29.000000000","message":"Closing bracket does not match visual indentation (pep8\u0027s e124)\n\n    opts \u003d [\n          ...\n    ]","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f3b14189d9e23293770362e2805a100f790172d5","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        return states.ERROR"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"def _start_console(driver_info):"},{"line_number":287,"context_line":"    \"\"\"Open the Serial Console of a node.\"\"\""},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    args \u003d []"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAU3%2F%2Fy8Y%3D","line":286,"updated":"2014-02-24 18:44:19.000000000","message":"start_console needs to account for two more possibilities:\n\n- shellinaboxd is already running and there is a pid file (eg, ironic-conductor restarted)\n- shellinabxod is not running, but there is a pid file (eg, server restarted)\n\nI would be OK with this done in a separate patch, but there should be a TODO comment inline.","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":299,"context_line":"    args.append(\"-s\")"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    pw_file \u003d _make_persistent_password_file(driver_info[\u0027uuid\u0027],"},{"line_number":302,"context_line":"                                            driver_info[\u0027password\u0027])"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":305,"context_line":"            \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BXGo%3D","line":302,"updated":"2014-02-21 16:45:29.000000000","message":"This need align to the bracket (pep8\u0027s e128)","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":301,"context_line":"    pw_file \u003d _make_persistent_password_file(driver_info[\u0027uuid\u0027],"},{"line_number":302,"context_line":"                                            driver_info[\u0027password\u0027])"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    ipmi_args \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":305,"context_line":"            \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":306,"context_line":"            % {\u0027uid\u0027: os.getuid(),"},{"line_number":307,"context_line":"               \u0027gid\u0027: os.getgid(),"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BXHk%3D","line":304,"updated":"2014-02-21 16:45:29.000000000","message":"we are moving away from \u0027\\\u0027 in favor of \u0027(.....)\u0027","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8301a8e2ee3abec2279119505f7ce560ee2c34e3","unresolved":false,"context_lines":[{"line_number":314,"context_line":"    # Run shellinaboxd without pipes. Otherwise utils.execute() waits"},{"line_number":315,"context_line":"    # infinitely since shellinaboxd does not close passed fds."},{"line_number":316,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":317,"context_line":"    out, err \u003d utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":318,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":319,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAU3%2F%2F0eI%3D","line":317,"updated":"2014-02-24 16:38:20.000000000","message":"Even tho we are using the --background option, this execute will hang. This is not a problem with this patch tho, but a problem with the processutils.execute which doesn\u0027t support running tasks in background because it PIPES the std{in,out,err} interface causing communicate to hang[1].\n\nA quick test:\n\n\nIn [15]: p \u003d subprocess.Popen(\"sleep 10\u0026\", stdout\u003dsubprocess.PIPE, stderr\u003dsubprocess.PIPE, shell\u003dTrue)\nIn [16]: p.communicate()\n\nhangs...\n\nIn [11]: p \u003d subprocess.Popen(\"sleep 10\u0026\", shell\u003dTrue)\nIn [12]: p.communicate()\nOut[12]: (None, None)\n\ndoesn\u0027t hangs...\n\n\nSo I suggest that as a workaround for now we use subprocess.Popen ourselfs instead of utils.execute(), add a FIXME to the code as well explaning the problem.\n\nThe long term would be to enhance the processutils.execute from oslo and make it support running background tasks. \n\n\nThoughts anyone?\n\n\n[1] https://github.com/openstack/ironic/blob/master/ironic/openstack/common/processutils.py#L153-L155","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f3b14189d9e23293770362e2805a100f790172d5","unresolved":false,"context_lines":[{"line_number":314,"context_line":"    # Run shellinaboxd without pipes. Otherwise utils.execute() waits"},{"line_number":315,"context_line":"    # infinitely since shellinaboxd does not close passed fds."},{"line_number":316,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":317,"context_line":"    out, err \u003d utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":318,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":319,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAU3%2F%2FzK0%3D","line":317,"in_reply_to":"AAAAU3%2F%2F0eI%3D","updated":"2014-02-24 18:44:19.000000000","message":"This seems reasonable to me. I would suggest a separate patch that copies ironic/openstack/common/processutils:execute into ironic/common and makes the needed change, with a proposal to olso and a comment inline linking to there.","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":316,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":317,"context_line":"    out, err \u003d utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":318,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":319,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"def _stop_console(driver_info):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BXFk%3D","line":319,"updated":"2014-02-21 16:45:29.000000000","message":"This need align to the bracket (pep8\u0027s e128)","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":326,"context_line":"        console_pid \u003d _get_console_pid(driver_info[\u0027uuid\u0027])"},{"line_number":327,"context_line":"    except exception.NoConsolePid:"},{"line_number":328,"context_line":"        LOG.warning(_(\"No console pid found for node %s.\")"},{"line_number":329,"context_line":"            % driver_info[\u0027uuid\u0027])"},{"line_number":330,"context_line":"    else:"},{"line_number":331,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":332,"context_line":"        utils.execute(\u0027kill\u0027, str(console_pid),"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BXDM%3D","line":329,"updated":"2014-02-21 16:45:29.000000000","message":"This need align to the bracket (pep8\u0027s e128)","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":7411,"name":"Matt Wagner","email":"matt.wagner@redhat.com","username":"matty_dubs"},"change_message_id":"b487331f207bf0a9b6a3b87918ceee091e7adb9f","unresolved":false,"context_lines":[{"line_number":330,"context_line":"    else:"},{"line_number":331,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":332,"context_line":"        utils.execute(\u0027kill\u0027, str(console_pid),"},{"line_number":333,"context_line":"                      run_as_root\u003dTrue,"},{"line_number":334,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":335,"context_line":"    finally:"},{"line_number":336,"context_line":"        utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BU8U%3D","line":333,"updated":"2014-02-21 21:16:14.000000000","message":"Wait a minute... Do we need to run this as root? We don\u0027t (and shouldn\u0027t!) start shellinaboxd as root, so I\u0027m not sure why we need to kill it as root. I was able to change this to False in my local environment. Can we change this to False?","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f3b14189d9e23293770362e2805a100f790172d5","unresolved":false,"context_lines":[{"line_number":330,"context_line":"    else:"},{"line_number":331,"context_line":"        # Allow exitcode 99 (RC_UNAUTHORIZED)"},{"line_number":332,"context_line":"        utils.execute(\u0027kill\u0027, str(console_pid),"},{"line_number":333,"context_line":"                      run_as_root\u003dTrue,"},{"line_number":334,"context_line":"                      check_exit_code\u003d[0, 99])"},{"line_number":335,"context_line":"    finally:"},{"line_number":336,"context_line":"        utils.unlink_without_raise(_get_console_pid_file(driver_info[\u0027uuid\u0027]))"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAU3%2F%2FzPc%3D","line":333,"in_reply_to":"AAAAUX%2F%2BU8U%3D","updated":"2014-02-24 18:44:19.000000000","message":"This *really* shouldn\u0027t use run_as_root\u003dTrue","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"af112b5a253fa369bcd277d76792e479a5c821b1","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"def _console_pwfile_path(uuid):"},{"line_number":64,"context_line":"    return \"/tmp/%(uuid)s.pw\" % {\u0027uuid\u0027: uuid}"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"def _make_persistent_password_file(uuid, password):"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2Fsus%3D","line":64,"updated":"2014-02-25 09:32:45.000000000","message":"I would recommend using tempfile.gettempdir() rather than hardcoding /tmp, e.g:\n\n    file_name \u003d \"%(uuid)s.pw\" % {\u0027uuid\u0027: uuid}\n    path.join(tempfile.gettempdir(), file_name)","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"af112b5a253fa369bcd277d76792e479a5c821b1","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            utils.delete_if_exists(path)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"def _remove_persistent_password_file(address, console_port):"},{"line_number":84,"context_line":"    path \u003d _console_pwfile_path(address, console_port)"},{"line_number":85,"context_line":"    os.remove(path)"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2FsCo%3D","line":83,"updated":"2014-02-25 09:32:45.000000000","message":"I\u0027ve looked throughout this file and found no usage of this method so I recommend to remove it.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d2cc5067ab97b148aae66c3a9e3e7052d75b1b2c","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    # Run shellinaboxd without pipes. Otherwise utils.execute() waits"},{"line_number":321,"context_line":"    # infinitely since shellinaboxd does not close passed fds."},{"line_number":322,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":323,"context_line":"    proc \u003d subprocess.Popen(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":324,"context_line":"    out, err \u003d proc.communicate()"},{"line_number":325,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":326,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2FnLI%3D","line":323,"updated":"2014-02-25 17:14:58.000000000","message":"devananda made a suggestion on the previous review[1] which was to copy the function from oslo to the ironic command, make the necessary changes to support running background tasks and submitted the change to olso as well.\n\nI think it\u0027s a reasonable idea, lemme know what u think.\n\n[1] https://review.openstack.org/#/c/64100/12/ironic/drivers/modules/ipmitool.py","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"d1302d35d931b5523aa447952feb180d098c03d2","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    # Run shellinaboxd without pipes. Otherwise utils.execute() waits"},{"line_number":321,"context_line":"    # infinitely since shellinaboxd does not close passed fds."},{"line_number":322,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":323,"context_line":"    proc \u003d subprocess.Popen(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":324,"context_line":"    out, err \u003d proc.communicate()"},{"line_number":325,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":326,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAVH%2F%2F%2F8k%3D","line":323,"in_reply_to":"AAAAU3%2F%2FiGM%3D","updated":"2014-02-26 22:29:02.000000000","message":"First patch:\n- copy the execute() function from ironic.openstack.common.processutils into ironic.common.processutils\n- modify ironic.common.processutils:execute to do what we need\n\nThen rebase this patchset on to the first patch, and use ironic.common.processutils:execute here.\n\nFile a bug that this needs to be pushed back to Oslo, and we can do that later.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"5a04a0ad6188274cf3ad83dd059310cd43d68514","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    # Run shellinaboxd without pipes. Otherwise utils.execute() waits"},{"line_number":321,"context_line":"    # infinitely since shellinaboxd does not close passed fds."},{"line_number":322,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":323,"context_line":"    proc \u003d subprocess.Popen(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":324,"context_line":"    out, err \u003d proc.communicate()"},{"line_number":325,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":326,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2FiGM%3D","line":323,"in_reply_to":"AAAAU3%2F%2FnLI%3D","updated":"2014-02-26 03:29:36.000000000","message":"Lucas,\n\nyes, I saw the suggestions from you and deva, thank you very much! but I may misunderstood something, I saw deva said \"a separate patch\" to address the fix( to copy the function from oslo to the ironic command, make the necessary changes to support running background tasks and submitted the change to olso as well), so I thought I may need to open a bug for it? or what should I do? please point me to the right way since I\u0027m not familiar with the process...","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"5341c312aba820d338f365254756c40e613f112f","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    # Run shellinaboxd without pipes. Otherwise utils.execute() waits"},{"line_number":321,"context_line":"    # infinitely since shellinaboxd does not close passed fds."},{"line_number":322,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":323,"context_line":"    proc \u003d subprocess.Popen(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":324,"context_line":"    out, err \u003d proc.communicate()"},{"line_number":325,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":326,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAVX%2F%2F7SY%3D","line":323,"in_reply_to":"AAAAVH%2F%2F%2F8k%3D","updated":"2014-02-27 10:35:05.000000000","message":"Deva,\n\nI worked for the first patch today, but did not find a long term fix to make execute support running background tasks, sorry... could someone help here?\n\nOn the other hand, I checked nova code for this part, and found https://github.com/openstack/nova/blob/master/nova/virt/baremetal/ipmi.py#L289-L291 to work around this Oslo bug, if we could not find a formal fix in  ironic.common.processutils:execute in time, could this be an option?\n\nThx.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"d2cc5067ab97b148aae66c3a9e3e7052d75b1b2c","unresolved":false,"context_lines":[{"line_number":321,"context_line":"    # infinitely since shellinaboxd does not close passed fds."},{"line_number":322,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":323,"context_line":"    proc \u003d subprocess.Popen(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":324,"context_line":"    out, err \u003d proc.communicate()"},{"line_number":325,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":326,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2FnOM%3D","line":324,"updated":"2014-02-25 17:14:58.000000000","message":"As there\u0027s the std{err,out,in} are not PIPE\u0027d the communicate() is always going to return None, so I think we don\u0027t need to log it.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"d1302d35d931b5523aa447952feb180d098c03d2","unresolved":false,"context_lines":[{"line_number":321,"context_line":"    # infinitely since shellinaboxd does not close passed fds."},{"line_number":322,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":323,"context_line":"    proc \u003d subprocess.Popen(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":324,"context_line":"    out, err \u003d proc.communicate()"},{"line_number":325,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":326,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAVH%2F%2F%2F%2BA%3D","line":324,"in_reply_to":"AAAAU3%2F%2Fidw%3D","updated":"2014-02-26 22:29:02.000000000","message":"In that case, I would recommend logging an error if the command failed and including the output. I would remove this debug line, though.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"c2376d4a4c2f9ec9f899dff02fd67b72b1348964","unresolved":false,"context_lines":[{"line_number":321,"context_line":"    # infinitely since shellinaboxd does not close passed fds."},{"line_number":322,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":323,"context_line":"    proc \u003d subprocess.Popen(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":324,"context_line":"    out, err \u003d proc.communicate()"},{"line_number":325,"context_line":"    LOG.debug(_(\"shellinaboxd stdout: \u0027%(out)s\u0027, stderr: \u0027%(err)s\u0027\"),"},{"line_number":326,"context_line":"          {\u0027out\u0027: out, \u0027err\u0027: err})"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2Fidw%3D","line":324,"in_reply_to":"AAAAU3%2F%2FnOM%3D","updated":"2014-02-26 02:21:19.000000000","message":"Lucas, yes, but we have concern if we don\u0027t log it, if we encountered the command error, how to track the error message? We have to log the command stand error message, else we have no idea why command call failed.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"af112b5a253fa369bcd277d76792e479a5c821b1","unresolved":false,"context_lines":[{"line_number":349,"context_line":"    console_host \u003d CONF.my_ip"},{"line_number":350,"context_line":"    if utils.is_valid_ipv6(console_host):"},{"line_number":351,"context_line":"        console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":352,"context_line":"    console_url \u003d \"http://%s:%s\" % (console_host, driver_info[\u0027port\u0027])"},{"line_number":353,"context_line":"    return console_url"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2Frz8%3D","line":352,"updated":"2014-02-25 09:32:45.000000000","message":"According to the shellinaboxd\u0027s manual it redirectes all incoming HTTP requests to their equivalent HTTPS URLs by default. This can be disabled by running it with -t | --disable-ssl options. However I would recommend extending this driver\u0027s functionality to support both HTTP and HTTPS. Otherwise this might cause severe security problems since unencrypted console traffic might be captured by third parties.\n\nAlso could you please use keyword interpolation here?","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":5805,"name":"Chris Krelle","email":"nobodycam@gmail.com","username":"nobodycam"},"change_message_id":"f95da7b92e760bd0ce57ae1d290bbd33aad79b4e","unresolved":false,"context_lines":[{"line_number":349,"context_line":"    console_host \u003d CONF.my_ip"},{"line_number":350,"context_line":"    if utils.is_valid_ipv6(console_host):"},{"line_number":351,"context_line":"        console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":352,"context_line":"    console_url \u003d \"http://%s:%s\" % (console_host, driver_info[\u0027port\u0027])"},{"line_number":353,"context_line":"    return console_url"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2FnHQ%3D","line":352,"updated":"2014-02-25 17:23:58.000000000","message":"connection method should be configurable. with default set to https. A operator should have to change the configuration to use http:","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"d1302d35d931b5523aa447952feb180d098c03d2","unresolved":false,"context_lines":[{"line_number":349,"context_line":"    console_host \u003d CONF.my_ip"},{"line_number":350,"context_line":"    if utils.is_valid_ipv6(console_host):"},{"line_number":351,"context_line":"        console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":352,"context_line":"    console_url \u003d \"http://%s:%s\" % (console_host, driver_info[\u0027port\u0027])"},{"line_number":353,"context_line":"    return console_url"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAVH%2F%2F%2F9w%3D","line":352,"in_reply_to":"AAAAU3%2F%2FnHQ%3D","updated":"2014-02-26 22:29:02.000000000","message":"I agree that the default should be HTTPS. However, in the interest of time, I suggest filing a bug and leaving this as-is for now.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"b74d05b9b745c3d81d3030b15ca0d225379eb55a","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _console_pwfile_path(uuid):"},{"line_number":62,"context_line":"    return \"/tmp/%(uuid)s.pw\" % {\u0027uuid\u0027: uuid}"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"def _make_persistent_password_file(uuid, password):"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2FtVo%3D","line":62,"updated":"2014-03-04 18:44:07.000000000","message":"Copying Roman\u0027s comment from rev13\n\nI would recommend using tempfile.gettempdir() rather than hardcoding /tmp, e.g:\n    file_name \u003d \"%(uuid)s.pw\" % {\u0027uuid\u0027: uuid}\n    path.join(tempfile.gettempdir(), file_name)","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"19592f106d67ecf2a653b5d253abe951b121cb43","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _console_pwfile_path(uuid):"},{"line_number":62,"context_line":"    return \"/tmp/%(uuid)s.pw\" % {\u0027uuid\u0027: uuid}"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"def _make_persistent_password_file(uuid, password):"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAWH%2F9Ra4%3D","line":62,"in_reply_to":"AAAAVn%2F%2FtVo%3D","updated":"2014-04-10 18:28:54.000000000","message":"Done","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"b74d05b9b745c3d81d3030b15ca0d225379eb55a","unresolved":false,"context_lines":[{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def _remove_persistent_password_file(address, console_port):"},{"line_number":82,"context_line":"    path \u003d _console_pwfile_path(address, console_port)"},{"line_number":83,"context_line":"    os.remove(path)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2Ftpk%3D","line":82,"updated":"2014-03-04 18:44:07.000000000","message":"this is invoking the _console_pwfile_path method with the wrong args, and the wrong number of args.\n\nUnit tests should have caught this. This method does not appear to be used anywhere.","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":7411,"name":"Matt Wagner","email":"matt.wagner@redhat.com","username":"matty_dubs"},"change_message_id":"807350e852d8d07bb25db80e1296ec9bee1e25ba","unresolved":false,"context_lines":[{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def _remove_persistent_password_file(address, console_port):"},{"line_number":82,"context_line":"    path \u003d _console_pwfile_path(address, console_port)"},{"line_number":83,"context_line":"    os.remove(path)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAWH%2F%2BKTw%3D","line":82,"in_reply_to":"AAAAVn%2F%2Ftpk%3D","updated":"2014-03-28 17:55:54.000000000","message":"As the one that added this method, I can confirm that it is not used and should be removed. The remove happens on line 343 of this file.","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"19592f106d67ecf2a653b5d253abe951b121cb43","unresolved":false,"context_lines":[{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def _remove_persistent_password_file(address, console_port):"},{"line_number":82,"context_line":"    path \u003d _console_pwfile_path(address, console_port)"},{"line_number":83,"context_line":"    os.remove(path)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAWH%2F9Raw%3D","line":82,"in_reply_to":"AAAAWH%2F%2BKTw%3D","updated":"2014-04-10 18:28:54.000000000","message":"Done. Removed the function.","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"5e66761f5edd8bcafee809fa7917b8069c8a8886","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    info \u003d node.get(\u0027driver_info\u0027, {})"},{"line_number":116,"context_line":"    address \u003d info.get(\u0027ipmi_address\u0027)"},{"line_number":117,"context_line":"    username \u003d info.get(\u0027ipmi_username\u0027)"},{"line_number":118,"context_line":"    password \u003d info.get(\u0027ipmi_password\u0027)"},{"line_number":119,"context_line":"    port \u003d info.get(\u0027ipmi_terminal_port\u0027)"},{"line_number":120,"context_line":"    priv_level \u003d info.get(\u0027ipmi_priv_level\u0027, \u0027ADMINISTRATOR\u0027)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAWH%2F9Pm4%3D","line":118,"updated":"2014-04-07 17:43:16.000000000","message":"(as talked in IRC) Not related to your code, but would be nice to check whether the terminal port is an integer or not","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"19592f106d67ecf2a653b5d253abe951b121cb43","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    info \u003d node.get(\u0027driver_info\u0027, {})"},{"line_number":116,"context_line":"    address \u003d info.get(\u0027ipmi_address\u0027)"},{"line_number":117,"context_line":"    username \u003d info.get(\u0027ipmi_username\u0027)"},{"line_number":118,"context_line":"    password \u003d info.get(\u0027ipmi_password\u0027)"},{"line_number":119,"context_line":"    port \u003d info.get(\u0027ipmi_terminal_port\u0027)"},{"line_number":120,"context_line":"    priv_level \u003d info.get(\u0027ipmi_priv_level\u0027, \u0027ADMINISTRATOR\u0027)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAW3%2F%2F4nc%3D","line":118,"in_reply_to":"AAAAWH%2F9Pm4%3D","updated":"2014-04-10 18:28:54.000000000","message":"Done","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"b74d05b9b745c3d81d3030b15ca0d225379eb55a","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    # shellinaboxd does not close passed fds. Once bug 1287486 is"},{"line_number":321,"context_line":"    # fixed, please fix me here."},{"line_number":322,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":323,"context_line":"    x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":324,"context_line":"    x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":325,"context_line":"    x.append(\u00272\u003e\u00261\u0027)"},{"line_number":326,"context_line":"    utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2Fsr8%3D","line":323,"updated":"2014-03-04 18:44:07.000000000","message":"Instead of appending /dev/null as an argument, this should use subprocess.DEVNULL","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"19592f106d67ecf2a653b5d253abe951b121cb43","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    # shellinaboxd does not close passed fds. Once bug 1287486 is"},{"line_number":321,"context_line":"    # fixed, please fix me here."},{"line_number":322,"context_line":"    x \u003d [\"\u0027\" + arg.replace(\"\u0027\", \"\u0027\\\\\u0027\u0027\") + \"\u0027\" for arg in args]"},{"line_number":323,"context_line":"    x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":324,"context_line":"    x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":325,"context_line":"    x.append(\u00272\u003e\u00261\u0027)"},{"line_number":326,"context_line":"    utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAW3%2F%2F4lA%3D","line":323,"in_reply_to":"AAAAVn%2F%2Fsr8%3D","updated":"2014-04-10 18:28:54.000000000","message":"This part is totally removed. Use subprocess.Popen directly instead.","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"b74d05b9b745c3d81d3030b15ca0d225379eb55a","unresolved":false,"context_lines":[{"line_number":323,"context_line":"    x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":324,"context_line":"    x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":325,"context_line":"    x.append(\u00272\u003e\u00261\u0027)"},{"line_number":326,"context_line":"    utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"def _stop_console(driver_info):"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2Fsuo%3D","line":326,"updated":"2014-03-04 18:44:07.000000000","message":"I believe that \"shell\u003dTrue\" is a security risk because the arguments are user-supplied. I should have spotted this earlier.","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c68cd51303f77ab1bb2ed25b43835719078ee5b9","unresolved":false,"context_lines":[{"line_number":323,"context_line":"    x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":324,"context_line":"    x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":325,"context_line":"    x.append(\u00272\u003e\u00261\u0027)"},{"line_number":326,"context_line":"    utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"def _stop_console(driver_info):"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAWH%2F9TGM%3D","line":326,"updated":"2014-04-07 09:43:53.000000000","message":"This was pointed on Patch-Set #12, using utils.execute will cause this function to hang because the processutils.execute method doesn\u0027t support running tasks in background, it uses because it PIPES the std{in,out,err} and try to communicate with the process to get the std,std,err output[1].\n\nSome ways to avoid it would be: Use subprocess.Popen with no pipes here or do the necessary changes in the processutils.execute() to be able to run it without pipes.\nNow as we don\u0027t know the result of the process (since we can\u0027t check it\u0027s stderr) we still need to be able to check whether shellinabox is running or not. I think that in the mid-cycle meet was suggested to create a periodic task that would check time to time if the console is enabled and if so it would check if the pidfile exist and if the pid in the shellinabox is running... Maybe should we bring this topic for the meeting today and discuss how we should do it?\n\n[1] https://github.com/openstack/ironic/blob/master/ironic/openstack/common/processutils.py#L153-L163","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"19592f106d67ecf2a653b5d253abe951b121cb43","unresolved":false,"context_lines":[{"line_number":323,"context_line":"    x.append(\u0027\u003c/dev/null\u0027)"},{"line_number":324,"context_line":"    x.append(\u0027\u003e/dev/null\u0027)"},{"line_number":325,"context_line":"    x.append(\u00272\u003e\u00261\u0027)"},{"line_number":326,"context_line":"    utils.execute(\u0027 \u0027.join(x), shell\u003dTrue)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"def _stop_console(driver_info):"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAW3%2F%2F4jc%3D","line":326,"in_reply_to":"AAAAWH%2F9TGM%3D","updated":"2014-04-10 18:28:54.000000000","message":"Added own subprocess.Popen here. Please review.","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"4a9eccaff37a899d9242e9424a0ac298579ea0bc","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":46,"context_line":"               help\u003d\u0027Directory for holding terminal pidfiles\u0027),"},{"line_number":47,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":48,"context_line":"               default\u003d\u0027/usr/local/bin/shellinaboxd\u0027,"},{"line_number":49,"context_line":"               help\u003d\u0027Absolute path for shellinaboxd program\u0027),"},{"line_number":50,"context_line":"    ]"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2Fdfk%3D","line":48,"updated":"2014-04-14 13:24:50.000000000","message":"Admin will be probably install shellinaboxd from distributive package, default path /usr/bin/shellinaboxd for Ubuntu, maybe the same for RH/Fedora.","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cb8ddcd38cd5e8d7031a01f51ee90693ead589f1","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    cfg.StrOpt(\u0027terminal_pid_dir\u0027,"},{"line_number":46,"context_line":"               help\u003d\u0027Directory for holding terminal pidfiles\u0027),"},{"line_number":47,"context_line":"    cfg.StrOpt(\u0027terminal\u0027,"},{"line_number":48,"context_line":"               default\u003d\u0027/usr/local/bin/shellinaboxd\u0027,"},{"line_number":49,"context_line":"               help\u003d\u0027Absolute path for shellinaboxd program\u0027),"},{"line_number":50,"context_line":"    ]"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2FdCs%3D","line":48,"in_reply_to":"AAAAW3%2F%2Fdfk%3D","updated":"2014-04-14 15:13:54.000000000","message":"I inherited this code from sjing. I quess that she got that path from her env.  I installed on Ubuntu, the path is /usr/bin/shellinaboxd. On RH, the path is /usr/sbin/shellinaboxd. So I\u0027ll change it to /usr/bin/shellinaboxd to make it work at least on Ubuntu by default.","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"4a9eccaff37a899d9242e9424a0ac298579ea0bc","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    port \u003d info.get(\u0027ipmi_terminal_port\u0027)"},{"line_number":116,"context_line":"    priv_level \u003d info.get(\u0027ipmi_priv_level\u0027, \u0027ADMINISTRATOR\u0027)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    if port and not isinstance(port, int):"},{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            port \u003d int(port)"},{"line_number":121,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2Fdc4%3D","line":118,"updated":"2014-04-14 13:24:50.000000000","message":"Should work w/o isinstance check also, this check not needed.","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cb8ddcd38cd5e8d7031a01f51ee90693ead589f1","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    port \u003d info.get(\u0027ipmi_terminal_port\u0027)"},{"line_number":116,"context_line":"    priv_level \u003d info.get(\u0027ipmi_priv_level\u0027, \u0027ADMINISTRATOR\u0027)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    if port and not isinstance(port, int):"},{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            port \u003d int(port)"},{"line_number":121,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2FdB8%3D","line":118,"in_reply_to":"AAAAW3%2F%2Fdc4%3D","updated":"2014-04-14 15:13:54.000000000","message":"Done. Thanks for the catch.","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"4a9eccaff37a899d9242e9424a0ac298579ea0bc","unresolved":false,"context_lines":[{"line_number":417,"context_line":""},{"line_number":418,"context_line":"def _get_console_pid_file(node_uuid):"},{"line_number":419,"context_line":"    \"\"\"Generate the pid file name to hold the terminal process id.\"\"\""},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    # make sure the directory exists"},{"line_number":422,"context_line":"    if CONF.ipmi.terminal_pid_dir:"},{"line_number":423,"context_line":"        pid_dir \u003d CONF.ipmi.terminal_pid_dir"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2FdYQ%3D","line":420,"updated":"2014-04-14 13:24:50.000000000","message":"IMO logic in l422-427 should be documented in config parameter helpstring.","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cb8ddcd38cd5e8d7031a01f51ee90693ead589f1","unresolved":false,"context_lines":[{"line_number":417,"context_line":""},{"line_number":418,"context_line":"def _get_console_pid_file(node_uuid):"},{"line_number":419,"context_line":"    \"\"\"Generate the pid file name to hold the terminal process id.\"\"\""},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    # make sure the directory exists"},{"line_number":422,"context_line":"    if CONF.ipmi.terminal_pid_dir:"},{"line_number":423,"context_line":"        pid_dir \u003d CONF.ipmi.terminal_pid_dir"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2FdBc%3D","line":420,"in_reply_to":"AAAAW3%2F%2FdYQ%3D","updated":"2014-04-14 15:13:54.000000000","message":"Done","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"4a9eccaff37a899d9242e9424a0ac298579ea0bc","unresolved":false,"context_lines":[{"line_number":602,"context_line":"    def get_console(self, task, node):"},{"line_number":603,"context_line":"        \"\"\"Get connection information about the console.\"\"\""},{"line_number":604,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":605,"context_line":"        return _get_console_url(driver_info)"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2Fdhw%3D","line":605,"updated":"2014-04-14 13:24:50.000000000","message":"Found a problem with return value (maybe ironicclient problem?):\n\n ironic node-get-console ...\n \u0027unicode\u0027 object has no attribute \u0027iteritems\u0027\n\n ironic --debug node-get-console ...\n\n DEBUG (http:105) \n HTTP/1.0 200 OK\n date: Mon, 14 Apr 2014 13:03:58 GMT\n content-length: 28\n content-type: application/json; charset\u003dUTF-8\n server: WSGIServer/0.1 Python/2.7.3\n\n \"http://172.18.195.165:2225\"\n\n \u0027unicode\u0027 object has no attribute \u0027iteritems\u0027","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cb8ddcd38cd5e8d7031a01f51ee90693ead589f1","unresolved":false,"context_lines":[{"line_number":602,"context_line":"    def get_console(self, task, node):"},{"line_number":603,"context_line":"        \"\"\"Get connection information about the console.\"\"\""},{"line_number":604,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":605,"context_line":"        return _get_console_url(driver_info)"}],"source_content_type":"text/x-python","patch_set":17,"id":"AAAAW3%2F%2FdAo%3D","line":605,"in_reply_to":"AAAAW3%2F%2Fdhw%3D","updated":"2014-04-14 15:13:54.000000000","message":"Yes, This is an API problem. I have talked to Devananda about it. And I will fix the API and here later in a different patch. (Bug #1307555)","commit_id":"91830305774055b2e1cd084deae46ca30c5ee5fd"},{"author":{"_account_id":7411,"name":"Matt Wagner","email":"matt.wagner@redhat.com","username":"matty_dubs"},"change_message_id":"2c3bba58821dd4ad22420eeb6f24572a7710098a","unresolved":false,"context_lines":[{"line_number":121,"context_line":"            port \u003d int(port)"},{"line_number":122,"context_line":"        except ValueError:"},{"line_number":123,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":124,"context_line":"                \"IPMI terminal port is not an interger.\"))"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    if not address:"},{"line_number":127,"context_line":"        raise exception.InvalidParameterValue(_("}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAW3%2F%2Fb%2Bo%3D","line":124,"updated":"2014-04-14 15:40:43.000000000","message":"Minor nit: Could you s/interger/integer/ if you end up resubmitting?","commit_id":"92e30f0926469dd6649ed592641a84c496b6deac"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"1f2d13e9a64058b801be15e9b64101ccd85e83c7","unresolved":false,"context_lines":[{"line_number":121,"context_line":"            port \u003d int(port)"},{"line_number":122,"context_line":"        except ValueError:"},{"line_number":123,"context_line":"            raise exception.InvalidParameterValue(_("},{"line_number":124,"context_line":"                \"IPMI terminal port is not an interger.\"))"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    if not address:"},{"line_number":127,"context_line":"        raise exception.InvalidParameterValue(_("}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAW3%2F%2FbjQ%3D","line":124,"in_reply_to":"AAAAW3%2F%2Fb%2Bo%3D","updated":"2014-04-14 18:55:48.000000000","message":"Done","commit_id":"92e30f0926469dd6649ed592641a84c496b6deac"},{"author":{"_account_id":7411,"name":"Matt Wagner","email":"matt.wagner@redhat.com","username":"matty_dubs"},"change_message_id":"2c3bba58821dd4ad22420eeb6f24572a7710098a","unresolved":false,"context_lines":[{"line_number":425,"context_line":"    else:"},{"line_number":426,"context_line":"        pid_dir \u003d tempfile.gettempdir()"},{"line_number":427,"context_line":"    if not os.path.exists(pid_dir):"},{"line_number":428,"context_line":"        os.mkdir(pid_dir)"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    name \u003d \"%s.pid\" % node_uuid"},{"line_number":431,"context_line":"    path \u003d os.path.join(pid_dir, name)"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAW3%2F%2Fb%2Bc%3D","line":428,"updated":"2014-04-14 15:40:43.000000000","message":"I think we might want os.makedirs here instead. AIUI, mkdir() will only create a single directory, while makedirs() will recursively create directories as needed. This might not be super-common, but I don\u0027t think it\u0027s too much of an edge case to have the given, directory require creating more than one level, so we should probably support it.","commit_id":"92e30f0926469dd6649ed592641a84c496b6deac"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"1f2d13e9a64058b801be15e9b64101ccd85e83c7","unresolved":false,"context_lines":[{"line_number":425,"context_line":"    else:"},{"line_number":426,"context_line":"        pid_dir \u003d tempfile.gettempdir()"},{"line_number":427,"context_line":"    if not os.path.exists(pid_dir):"},{"line_number":428,"context_line":"        os.mkdir(pid_dir)"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    name \u003d \"%s.pid\" % node_uuid"},{"line_number":431,"context_line":"    path \u003d os.path.join(pid_dir, name)"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAW3%2F%2Fbis%3D","line":428,"in_reply_to":"AAAAW3%2F%2Fb%2Bc%3D","updated":"2014-04-14 18:55:48.000000000","message":"Done. Good point.","commit_id":"92e30f0926469dd6649ed592641a84c496b6deac"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"02c85ed512fc506005cff8ecdf61b29e2c6cee79","unresolved":false,"context_lines":[{"line_number":603,"context_line":"    def get_console(self, task, node):"},{"line_number":604,"context_line":"        \"\"\"Get connection information about the console.\"\"\""},{"line_number":605,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":606,"context_line":"        return _get_console_url(driver_info)"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAXH%2F%2F8Qk%3D","line":606,"updated":"2014-04-16 11:17:14.000000000","message":"This should returns \u0027console_info\u0027 instead of url string.","commit_id":"908c382a83a1e8a68bc3b02112207332fea4640c"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"67c13b472b29ffa40d78464e652725ed6f2ee82d","unresolved":false,"context_lines":[{"line_number":603,"context_line":"    def get_console(self, task, node):"},{"line_number":604,"context_line":"        \"\"\"Get connection information about the console.\"\"\""},{"line_number":605,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":606,"context_line":"        return _get_console_url(driver_info)"}],"source_content_type":"text/x-python","patch_set":21,"id":"AAAAXH%2F%2F7Ig%3D","line":606,"in_reply_to":"AAAAXH%2F%2F8Qk%3D","updated":"2014-04-16 14:10:54.000000000","message":"Yuriy, the console_info framework was added in a separate patch later than this one which this patch is not depend on. I\u0027ll use a separate patch to make the change once this get landed.","commit_id":"908c382a83a1e8a68bc3b02112207332fea4640c"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"7bbb2391cd07622235446a6a197630aeb4e2c3e1","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        pw_file \u003d console_utils._make_persistent_password_file("},{"line_number":429,"context_line":"                driver_info[\u0027uuid\u0027], path, driver_info[\u0027password\u0027])"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        ipmi_cmd \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":432,"context_line":"                   \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":433,"context_line":"                   % {\u0027uid\u0027: os.getuid(),"},{"line_number":434,"context_line":"                      \u0027gid\u0027: os.getgid(),"}],"source_content_type":"text/x-python","patch_set":24,"id":"AAAAXX%2F%2F69g%3D","line":431,"updated":"2014-04-25 09:25:20.000000000","message":"Maybe makes sense if \u0027debug\u0027 config option enabled add \u0027-v\u0027 to ipmi_cmd. W/o \u0027-v\u0027 option ipmitool does not show message about invalid priv. level for \"sol activate\" command in my tests.","commit_id":"e8297c06a8b4b9c1a449151df2300a37c4472abd"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"ac7842c14da87786fed3f6875e8fae438bf91dc6","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        pw_file \u003d console_utils._make_persistent_password_file("},{"line_number":429,"context_line":"                driver_info[\u0027uuid\u0027], path, driver_info[\u0027password\u0027])"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        ipmi_cmd \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"},{"line_number":432,"context_line":"                   \" -I lanplus -U %(user)s -f %(pwfile)s sol activate\" \\"},{"line_number":433,"context_line":"                   % {\u0027uid\u0027: os.getuid(),"},{"line_number":434,"context_line":"                      \u0027gid\u0027: os.getgid(),"}],"source_content_type":"text/x-python","patch_set":24,"id":"AAAAXX%2F%2F2x0%3D","line":431,"in_reply_to":"AAAAXX%2F%2F69g%3D","updated":"2014-04-25 18:40:01.000000000","message":"Yuriy, this is a great idea. Thanks.\nDone.","commit_id":"e8297c06a8b4b9c1a449151df2300a37c4472abd"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"711e4f29d14c9b35f8dd18172824074d26def10a","unresolved":false,"context_lines":[{"line_number":425,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        path \u003d _console_pwfile_path(driver_info[\u0027uuid\u0027])"},{"line_number":428,"context_line":"        pw_file \u003d console_utils._make_persistent_password_file("},{"line_number":429,"context_line":"                driver_info[\u0027uuid\u0027], path, driver_info[\u0027password\u0027])"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        ipmi_cmd \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"}],"source_content_type":"text/x-python","patch_set":26,"id":"AAAAXX%2F%2F0%2Fw%3D","line":428,"updated":"2014-04-26 00:01:08.000000000","message":"why are you calling a private method from another module?\n\nedit: I\u0027ve commented on that file. it\u0027s not a private method, even though its name indicates it as such. Looks like it was recently refactored and the method names weren\u0027t updated, now that they\u0027re public methods.","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"cadda45d71f40a6880532bdd49de95ff6c6d9803","unresolved":false,"context_lines":[{"line_number":425,"context_line":"        driver_info \u003d _parse_driver_info(node)"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        path \u003d _console_pwfile_path(driver_info[\u0027uuid\u0027])"},{"line_number":428,"context_line":"        pw_file \u003d console_utils._make_persistent_password_file("},{"line_number":429,"context_line":"                driver_info[\u0027uuid\u0027], path, driver_info[\u0027password\u0027])"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        ipmi_cmd \u003d \"/:%(uid)s:%(gid)s:HOME:ipmitool -H %(address)s\" \\"}],"source_content_type":"text/x-python","patch_set":26,"id":"bad4c104_924f5a92","line":428,"in_reply_to":"AAAAXX%2F%2F0%2Fw%3D","updated":"2014-04-29 15:17:04.000000000","message":"Done","commit_id":"eca027d970bdf6360d07d7640ad5072de9631fa2"}],"ironic/drivers/pxe.py":[{"author":{"_account_id":5805,"name":"Chris Krelle","email":"nobodycam@gmail.com","username":"nobodycam"},"change_message_id":"f95da7b92e760bd0ce57ae1d290bbd33aad79b4e","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def __init__(self):"},{"line_number":38,"context_line":"        self.power \u003d ipmitool.IPMIPower()"},{"line_number":39,"context_line":"        self.console \u003d ipmitool.IPMIConsole()"},{"line_number":40,"context_line":"        self.deploy \u003d pxe.PXEDeploy()"},{"line_number":41,"context_line":"        self.rescue \u003d self.deploy"},{"line_number":42,"context_line":"        self.vendor \u003d pxe.VendorPassthru()"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2FnD0%3D","line":39,"updated":"2014-02-25 17:23:58.000000000","message":"I would like to see place holders in PXEAndSSHDriver \u0026 PXEAndIPMINativeDriver set to none (ie. self.console \u003d None)","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"d1302d35d931b5523aa447952feb180d098c03d2","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def __init__(self):"},{"line_number":38,"context_line":"        self.power \u003d ipmitool.IPMIPower()"},{"line_number":39,"context_line":"        self.console \u003d ipmitool.IPMIConsole()"},{"line_number":40,"context_line":"        self.deploy \u003d pxe.PXEDeploy()"},{"line_number":41,"context_line":"        self.rescue \u003d self.deploy"},{"line_number":42,"context_line":"        self.vendor \u003d pxe.VendorPassthru()"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAVH%2F%2F%2F8Q%3D","line":39,"in_reply_to":"AAAAU3%2F%2FnD0%3D","updated":"2014-02-26 22:29:02.000000000","message":"That isn\u0027t really necessary -- the default is None.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"}],"ironic/tests/drivers/test_ipmitool.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"15d3224c5c1aa8eaf9dca9e09f3fb8a49af8abac","unresolved":false,"context_lines":[{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        pid \u003d ipmi._get_console_pid(self.info[\u0027uuid\u0027])"},{"line_number":259,"context_line":"        self.assertIsNone(pid)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"class IPMIToolDriverTestCase(db_base.DbTestCase):"},{"line_number":263,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAT3%2F%2Fr%2Fc%3D","line":260,"updated":"2014-01-03 13:45:01.000000000","message":"I\u0027d like to see more tests here on the start/stop methods, basically testing if execute is being called with the expected parameters","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"682968bf245ff7cfa4849ff62bb3c45d867ee4b9","unresolved":false,"context_lines":[{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        pid \u003d ipmi._get_console_pid(self.info[\u0027uuid\u0027])"},{"line_number":259,"context_line":"        self.assertIsNone(pid)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"class IPMIToolDriverTestCase(db_base.DbTestCase):"},{"line_number":263,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUH%2F%2F3nI%3D","line":260,"in_reply_to":"AAAAT3%2F%2Fr%2Fc%3D","updated":"2014-01-07 12:56:55.000000000","message":"Lucas,\n\nThank you for pointing it out, I will update the tests accordingly.","commit_id":"79d9844b5830599b915f034d4bede629ef2f82b1"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"f7dbdc27803b47c88dcbc12a09de755e4112ca23","unresolved":false,"context_lines":[{"line_number":212,"context_line":"            self.assertEqual(mock_exec.call_args_list, expected)"},{"line_number":213,"context_line":"            self.assertEqual(state, states.ERROR)"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def test__get_console_pid_file(self):"},{"line_number":216,"context_line":"        self.config(terminal_pid_dir\u003d\u0027/tmp\u0027, group\u003d\u0027ipmi\u0027)"},{"line_number":217,"context_line":"        path \u003d ipmi._get_console_pid_file(self.info[\u0027uuid\u0027])"},{"line_number":218,"context_line":"        self.assertEqual(path, \u0027/tmp/%s.pid\u0027 % self.info.get(\u0027uuid\u0027))"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FF6c%3D","line":215,"updated":"2014-01-17 14:41:30.000000000","message":"Extra underscore here and on the other tests you added.","commit_id":"6cb11260fc856f983d6dac41fb10bffd53f38dcd"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"72165dceafe47b06af0f2852a546be67bdd5e4b1","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        if utils.is_valid_ipv6(console_host):"},{"line_number":295,"context_line":"            console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":296,"context_line":"        http_url \u003d \"http://%s:%s\" % (console_host, self.info[\u0027port\u0027])"},{"line_number":297,"context_line":"        self.assertEqual(generated_url, http_url)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"class IPMIToolDriverTestCase(db_base.DbTestCase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2BnsA%3D","line":297,"updated":"2014-02-20 06:46:59.000000000","message":"Haomeng,\n\nI checked the other self.assertEqual() calls in test_ipmitool.py, and seems the expected para is put the latter, am I wrong?","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":8106,"name":"Haomeng,Wang","email":"wanghaomeng@gmail.com","username":"whaom"},"change_message_id":"47614c636839601fcc77d95c1dfee1908bae49a9","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        if utils.is_valid_ipv6(console_host):"},{"line_number":295,"context_line":"            console_host \u003d \u0027[%s]\u0027 % console_host"},{"line_number":296,"context_line":"        http_url \u003d \"http://%s:%s\" % (console_host, self.info[\u0027port\u0027])"},{"line_number":297,"context_line":"        self.assertEqual(generated_url, http_url)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"class IPMIToolDriverTestCase(db_base.DbTestCase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2Bwi0%3D","line":297,"updated":"2014-02-19 10:41:13.000000000","message":"assertEqual should put the expected parameter first. This will make it easier to read the error when it fails. ditto for all other assertEqual, too.","commit_id":"90e4bff2aed1fbf7a444a75348a0c54fd36a72c5"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"3979e851560102e740ba47dfe7c33e1b66234ca7","unresolved":false,"context_lines":[{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def test__get_console_pid(self):"},{"line_number":255,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"},{"line_number":256,"context_line":"        tmp_file \u003d tmp_file_handle.name"},{"line_number":257,"context_line":"        with open(tmp_file, \"w\") as f:"},{"line_number":258,"context_line":"            f.write(\"12345\\n\")"},{"line_number":259,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2BfWQ%3D","line":256,"updated":"2014-02-20 22:09:03.000000000","message":"add\n  self.addCleanup(utils.unlink_without_raise, tmp_file)","commit_id":"9ede479a105652919a5e074f70ae2b0651f6b412"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"81fef28279bb34351f6de1df3e0db8bee623d199","unresolved":false,"context_lines":[{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def test__get_console_pid(self):"},{"line_number":255,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"},{"line_number":256,"context_line":"        tmp_file \u003d tmp_file_handle.name"},{"line_number":257,"context_line":"        with open(tmp_file, \"w\") as f:"},{"line_number":258,"context_line":"            f.write(\"12345\\n\")"},{"line_number":259,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2BbQg%3D","line":256,"in_reply_to":"AAAAUX%2F%2BfWQ%3D","updated":"2014-02-21 08:07:16.000000000","message":"Deva,\n\nThank you for the comments!\n\nFixed it in Patch12.","commit_id":"9ede479a105652919a5e074f70ae2b0651f6b412"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"3979e851560102e740ba47dfe7c33e1b66234ca7","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            pid \u003d ipmi._get_console_pid(self.info[\u0027uuid\u0027])"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"            mock_exec.assert_called_once_with(self.info[\u0027uuid\u0027])"},{"line_number":267,"context_line":"            utils.unlink_without_raise(tmp_file)"},{"line_number":268,"context_line":"            self.assertEqual(pid, 12345)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def test__get_console_pid_not_a_num(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2BfXM%3D","line":267,"updated":"2014-02-20 22:09:03.000000000","message":"this should be done via self.addCleanup(). If the test fails before :267, it will leave a dangling file","commit_id":"9ede479a105652919a5e074f70ae2b0651f6b412"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"3979e851560102e740ba47dfe7c33e1b66234ca7","unresolved":false,"context_lines":[{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def test__get_console_pid_not_a_num(self):"},{"line_number":271,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"},{"line_number":272,"context_line":"        tmp_file \u003d tmp_file_handle.name"},{"line_number":273,"context_line":"        with open(tmp_file, \"w\") as f:"},{"line_number":274,"context_line":"            f.write(\"Hello World\\n\")"},{"line_number":275,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUX%2F%2BfVw%3D","line":272,"updated":"2014-02-20 22:09:03.000000000","message":"ditto","commit_id":"9ede479a105652919a5e074f70ae2b0651f6b412"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":265,"context_line":"    def test__get_console_pid_file(self):"},{"line_number":266,"context_line":"        self.config(terminal_pid_dir\u003d\u0027/tmp\u0027, group\u003d\u0027ipmi\u0027)"},{"line_number":267,"context_line":"        path \u003d ipmi._get_console_pid_file(self.info[\u0027uuid\u0027])"},{"line_number":268,"context_line":"        self.assertEqual(path, \u0027/tmp/%s.pid\u0027 % self.info.get(\u0027uuid\u0027))"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def test__get_console_pid(self):"},{"line_number":271,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BW%2Fs%3D","line":268,"updated":"2014-02-21 16:45:29.000000000","message":"Probably need to change the order in assertEqual.\nassertEqual(expected, actual)\n\n    self.assertEqual(\u0027/tmp/%s.pid\u0027 % self.info.get(\u0027uuid\u0027), path)","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":267,"context_line":"        path \u003d ipmi._get_console_pid_file(self.info[\u0027uuid\u0027])"},{"line_number":268,"context_line":"        self.assertEqual(path, \u0027/tmp/%s.pid\u0027 % self.info.get(\u0027uuid\u0027))"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def test__get_console_pid(self):"},{"line_number":271,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"},{"line_number":272,"context_line":"        tmp_file \u003d tmp_file_handle.name"},{"line_number":273,"context_line":"        self.addCleanup(utils.unlink_without_raise, tmp_file)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BW9U%3D","line":270,"updated":"2014-02-21 16:45:29.000000000","message":"It would be better to use decorators for mock\n\n    @mock.patch.object(ipmi, \u0027_get_console_pid_file\u0027, autospec\u003dTrue)\n    def test__get_console_pid(self, mock_exec):\n            ...\n            mock_exec.return_value \u003d tmp_file\n            ...\n\nand be used decorators for other tests","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":281,"context_line":"            pid \u003d ipmi._get_console_pid(self.info[\u0027uuid\u0027])"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"            mock_exec.assert_called_once_with(self.info[\u0027uuid\u0027])"},{"line_number":284,"context_line":"            self.assertEqual(pid, 12345)"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def test__get_console_pid_not_a_num(self):"},{"line_number":287,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BW8I%3D","line":284,"updated":"2014-02-21 16:45:29.000000000","message":"ditto for assertEqual(expected, actual)","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":514,"context_line":"            mock_exec.return_value \u003d None"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"            with task_manager.acquire(self.context,"},{"line_number":517,"context_line":"                                     [self.node[\u0027uuid\u0027]]) as task:"},{"line_number":518,"context_line":"                self.driver.console.start_console(task, self.node)"},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"            mock_exec.assert_called_once_with(self.info)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BW7I%3D","line":517,"updated":"2014-02-21 16:45:29.000000000","message":"This need align to the bracket (pep8\u0027s e128)","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":526,"context_line":"            mock_exec.return_value \u003d None"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            with task_manager.acquire(self.context,"},{"line_number":529,"context_line":"                                     [self.node[\u0027uuid\u0027]]) as task:"},{"line_number":530,"context_line":"                self.driver.console.stop_console(task, self.node)"},{"line_number":531,"context_line":""},{"line_number":532,"context_line":"            mock_exec.assert_called_once_with(self.info)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BW7M%3D","line":529,"updated":"2014-02-21 16:45:29.000000000","message":"This need align to the bracket (pep8\u0027s e128)","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"7d46e8d2c90f1e4822cd8ecf4cdc0051c2762135","unresolved":false,"context_lines":[{"line_number":538,"context_line":"            mock_exec.return_value \u003d None"},{"line_number":539,"context_line":""},{"line_number":540,"context_line":"            with task_manager.acquire(self.context,"},{"line_number":541,"context_line":"                                     [self.node[\u0027uuid\u0027]]) as task:"},{"line_number":542,"context_line":"                self.driver.console.get_console(task, self.node)"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"            mock_exec.assert_called_once_with(self.info)"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2BW7c%3D","line":541,"updated":"2014-02-21 16:45:29.000000000","message":"This need align to the bracket (pep8\u0027s e128)","commit_id":"347f8485c7006a4b08a9cc5422d5ff90a95aeab8"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"af112b5a253fa369bcd277d76792e479a5c821b1","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        with open(tmp_file, \"w\") as f:"},{"line_number":276,"context_line":"            f.write(\"12345\\n\")"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        with mock.patch.object(ipmi, \u0027_get_console_pid_file\u0027,"},{"line_number":279,"context_line":"                               autospec\u003dTrue) as mock_exec:"},{"line_number":280,"context_line":"            mock_exec.return_value \u003d tmp_file"},{"line_number":281,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2Fr4U%3D","line":278,"updated":"2014-02-25 09:32:45.000000000","message":"If you will be uploading a new patch set, please use the decorator here and bellow instead.\n\nThis is not a reason for -1.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"af112b5a253fa369bcd277d76792e479a5c821b1","unresolved":false,"context_lines":[{"line_number":511,"context_line":"                                              \u0027pxe\u0027, False)"},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"    def test_start_console(self):"},{"line_number":514,"context_line":"        with mock.patch.object(ipmi, \u0027_start_console\u0027,"},{"line_number":515,"context_line":"                               autospec\u003dTrue) as mock_exec:"},{"line_number":516,"context_line":"            mock_exec.return_value \u003d None"},{"line_number":517,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAU3%2F%2Frss%3D","line":514,"updated":"2014-02-25 09:32:45.000000000","message":"This mock causes _start_console to be completely untested. Coverage report confirmed that. Lines 526 and 538 cause similar problems.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"d1302d35d931b5523aa447952feb180d098c03d2","unresolved":false,"context_lines":[{"line_number":511,"context_line":"                                              \u0027pxe\u0027, False)"},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"    def test_start_console(self):"},{"line_number":514,"context_line":"        with mock.patch.object(ipmi, \u0027_start_console\u0027,"},{"line_number":515,"context_line":"                               autospec\u003dTrue) as mock_exec:"},{"line_number":516,"context_line":"            mock_exec.return_value \u003d None"},{"line_number":517,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAVH%2F%2F%2F7c%3D","line":514,"in_reply_to":"AAAAU3%2F%2Frss%3D","updated":"2014-02-26 22:29:02.000000000","message":"I agree -- you can\u0027t mock the very thing you\u0027re claiming to test :)","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":7895,"name":"Sun Jing","email":"sjing@cn.ibm.com","username":"sjing"},"change_message_id":"5341c312aba820d338f365254756c40e613f112f","unresolved":false,"context_lines":[{"line_number":511,"context_line":"                                              \u0027pxe\u0027, False)"},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"    def test_start_console(self):"},{"line_number":514,"context_line":"        with mock.patch.object(ipmi, \u0027_start_console\u0027,"},{"line_number":515,"context_line":"                               autospec\u003dTrue) as mock_exec:"},{"line_number":516,"context_line":"            mock_exec.return_value \u003d None"},{"line_number":517,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAVX%2F%2F7OI%3D","line":514,"in_reply_to":"AAAAVH%2F%2F%2F7c%3D","updated":"2014-02-27 10:35:05.000000000","message":"sorry, I thought L522-L523 could ensure _start_console be covered, at least, it could prove it went into _start_console. I copied it from above existing test cases, such as test_set_boot_device_ok(self), etc.. could you give an example for the good case? Thx a lot.","commit_id":"d4a48e5a3d55819a99146c35e1df9ad309d27db5"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"ca5510336e55a6cd736cbb5e3a65bc027726742a","unresolved":false,"context_lines":[{"line_number":267,"context_line":"        path \u003d ipmi._get_console_pid_file(self.info[\u0027uuid\u0027])"},{"line_number":268,"context_line":"        self.assertEqual(path, \u0027/tmp/%s.pid\u0027 % self.info.get(\u0027uuid\u0027))"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def test__get_console_pid(self):"},{"line_number":271,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"},{"line_number":272,"context_line":"        tmp_file \u003d tmp_file_handle.name"},{"line_number":273,"context_line":"        self.addCleanup(utils.unlink_without_raise, tmp_file)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2FvTM%3D","line":270,"updated":"2014-03-04 11:27:32.000000000","message":"Please use decorators like:\n\n    @mock.patch.object(ipmi, \u0027_get_console_pid_file\u0027, autospec\u003dTrue)\n    def test__get_console_pid(self, mock_exec):\n        ...","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"19592f106d67ecf2a653b5d253abe951b121cb43","unresolved":false,"context_lines":[{"line_number":267,"context_line":"        path \u003d ipmi._get_console_pid_file(self.info[\u0027uuid\u0027])"},{"line_number":268,"context_line":"        self.assertEqual(path, \u0027/tmp/%s.pid\u0027 % self.info.get(\u0027uuid\u0027))"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def test__get_console_pid(self):"},{"line_number":271,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"},{"line_number":272,"context_line":"        tmp_file \u003d tmp_file_handle.name"},{"line_number":273,"context_line":"        self.addCleanup(utils.unlink_without_raise, tmp_file)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAW3%2F%2F4oI%3D","line":270,"in_reply_to":"AAAAVn%2F%2FvTM%3D","updated":"2014-04-10 18:28:54.000000000","message":"Done","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"ca5510336e55a6cd736cbb5e3a65bc027726742a","unresolved":false,"context_lines":[{"line_number":283,"context_line":"            mock_exec.assert_called_once_with(self.info[\u0027uuid\u0027])"},{"line_number":284,"context_line":"            self.assertEqual(pid, 12345)"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def test__get_console_pid_not_a_num(self):"},{"line_number":287,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"},{"line_number":288,"context_line":"        tmp_file \u003d tmp_file_handle.name"},{"line_number":289,"context_line":"        self.addCleanup(utils.unlink_without_raise, tmp_file)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAVn%2F%2FvTI%3D","line":286,"updated":"2014-03-04 11:27:32.000000000","message":"ditto","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"},{"author":{"_account_id":7589,"name":"linggao","email":"linggao@us.ibm.com","username":"linggao"},"change_message_id":"19592f106d67ecf2a653b5d253abe951b121cb43","unresolved":false,"context_lines":[{"line_number":283,"context_line":"            mock_exec.assert_called_once_with(self.info[\u0027uuid\u0027])"},{"line_number":284,"context_line":"            self.assertEqual(pid, 12345)"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def test__get_console_pid_not_a_num(self):"},{"line_number":287,"context_line":"        tmp_file_handle \u003d tempfile.NamedTemporaryFile()"},{"line_number":288,"context_line":"        tmp_file \u003d tmp_file_handle.name"},{"line_number":289,"context_line":"        self.addCleanup(utils.unlink_without_raise, tmp_file)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAAW3%2F%2F4n8%3D","line":286,"in_reply_to":"AAAAVn%2F%2FvTI%3D","updated":"2014-04-10 18:28:54.000000000","message":"Done","commit_id":"d099fc64946d6f570462898445e4d69d5c25ae3d"}]}