)]}'
{"ironic/conf/ipmi.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ce776a755f8ab19a93110c8a8069bd61638e84d1","unresolved":false,"context_lines":[{"line_number":39,"context_line":"                default\u003dTrue,"},{"line_number":40,"context_line":"                help\u003d_(\u0027Kill `ipmitool` process invoked by ironic to read \u0027"},{"line_number":41,"context_line":"                       \u0027node power state if `ipmitool` process does not exit \u0027"},{"line_number":42,"context_line":"                       \u0027after configured timeout expires. Recommended \u0027"},{"line_number":43,"context_line":"                       \u0027setting is True\u0027)),"},{"line_number":44,"context_line":"]"},{"line_number":45,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_000772ca","line":42,"updated":"2018-12-04 11:01:13.000000000","message":"You do not have \"configured timeout\" - see my other comments.","commit_id":"3d36084cc831b20a520d5c6d73b045c02e067662"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"c24227f7e5feecdb930c370d3d5365ae428b2b9e","unresolved":false,"context_lines":[{"line_number":39,"context_line":"                default\u003dTrue,"},{"line_number":40,"context_line":"                help\u003d_(\u0027Kill `ipmitool` process invoked by ironic to read \u0027"},{"line_number":41,"context_line":"                       \u0027node power state if `ipmitool` process does not exit \u0027"},{"line_number":42,"context_line":"                       \u0027after configured timeout expires. Recommended \u0027"},{"line_number":43,"context_line":"                       \u0027setting is True\u0027)),"},{"line_number":44,"context_line":"]"},{"line_number":45,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_03ce7f8d","line":42,"in_reply_to":"3f79a3b5_000772ca","updated":"2018-12-05 08:20:36.000000000","message":"Done","commit_id":"3d36084cc831b20a520d5c6d73b045c02e067662"}],"ironic/drivers/modules/ipmitool.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a11efc9b76f90d355e2bff279a301b0137215f00","unresolved":false,"context_lines":[{"line_number":371,"context_line":"    }"},{"line_number":372,"context_line":""},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"def _exec_ipmitool_wait(popen_obj, timeout\u003d5, driver_info\u003dNone):"},{"line_number":375,"context_line":"    wait_interval \u003d min(timeout, 0.5)"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"    while timeout \u003e 0:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_b23042ec","line":374,"updated":"2018-11-05 11:03:32.000000000","message":"Let us not have a default for timeout here, it\u0027s always overridden (and 5 is probably wrong in any case). Ditto for driver_info, is it ever None?","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"abb704c99381be889c38b4515729ab8ef0c8027e","unresolved":false,"context_lines":[{"line_number":371,"context_line":"    }"},{"line_number":372,"context_line":""},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"def _exec_ipmitool_wait(popen_obj, timeout\u003d5, driver_info\u003dNone):"},{"line_number":375,"context_line":"    wait_interval \u003d min(timeout, 0.5)"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"    while timeout \u003e 0:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_58530f3b","line":374,"in_reply_to":"3f79a3b5_b23042ec","updated":"2018-11-05 13:57:37.000000000","message":"Done","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a11efc9b76f90d355e2bff279a301b0137215f00","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    time.sleep(0.5)"},{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"def _exec_ipmitool(driver_info, command, check_exit_code\u003dNone,"},{"line_number":394,"context_line":"                   kill_on_timeout\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_522f6e88","line":391,"updated":"2018-11-05 11:03:32.000000000","message":"we need to raise something here maybe? to avoid ironic thinking the command was successful","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"abb704c99381be889c38b4515729ab8ef0c8027e","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    time.sleep(0.5)"},{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"def _exec_ipmitool(driver_info, command, check_exit_code\u003dNone,"},{"line_number":394,"context_line":"                   kill_on_timeout\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_fb53f513","line":391,"in_reply_to":"3f79a3b5_522f6e88","updated":"2018-11-05 13:57:37.000000000","message":"I\u0027ve added some logging to note that kill did not work out.\n\nI am not sure about raising an exception - what ironic could possibly do about inability to terminate ipmitool process?","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"e9cd41f562a860406118b4fde9b1f02bfa6cedb6","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    time.sleep(0.5)"},{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"def _exec_ipmitool(driver_info, command, check_exit_code\u003dNone,"},{"line_number":394,"context_line":"                   kill_on_timeout\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_214a9e59","line":391,"in_reply_to":"3f79a3b5_e60a7c76","updated":"2018-11-06 07:36:35.000000000","message":"Ah, this patch does not change anything in that regard. The `__exec_ipmitool_wait` callback can either kill running `ipmitool` process or not. At any rate, `ipmitool` might exit sooner or later (because of the signal or its internal timeout or crash, whatever), then existing code should handle that in exactly the same way as does it now.\n\nDoes it make sense?","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"103f0f5bbb458ad766b5e9a4c1468faac196f6a0","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    time.sleep(0.5)"},{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"def _exec_ipmitool(driver_info, command, check_exit_code\u003dNone,"},{"line_number":394,"context_line":"                   kill_on_timeout\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_e60a7c76","line":391,"in_reply_to":"3f79a3b5_fb53f513","updated":"2018-11-05 14:25:32.000000000","message":"I guess my question is: if the timeout is reached, will get_power_state/set_power_state properly return an error?","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a11efc9b76f90d355e2bff279a301b0137215f00","unresolved":false,"context_lines":[{"line_number":399,"context_line":"    :param check_exit_code: Single bool, int, or list of allowed exit codes."},{"line_number":400,"context_line":"    :param kill_on_timeout: if `True`, kill unresponsive ipmitool on"},{"line_number":401,"context_line":"        `min_command_interval` timeout. Default is `False`. Makes no"},{"line_number":402,"context_line":"        effect on Windows."},{"line_number":403,"context_line":"    :returns: (stdout, stderr) from executing the command."},{"line_number":404,"context_line":"    :raises: PasswordFileFailedToCreate from creating or writing to the"},{"line_number":405,"context_line":"             temporary file."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_722aaa78","line":402,"updated":"2018-11-05 11:03:32.000000000","message":"Windows? Oo","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"abb704c99381be889c38b4515729ab8ef0c8027e","unresolved":false,"context_lines":[{"line_number":399,"context_line":"    :param check_exit_code: Single bool, int, or list of allowed exit codes."},{"line_number":400,"context_line":"    :param kill_on_timeout: if `True`, kill unresponsive ipmitool on"},{"line_number":401,"context_line":"        `min_command_interval` timeout. Default is `False`. Makes no"},{"line_number":402,"context_line":"        effect on Windows."},{"line_number":403,"context_line":"    :returns: (stdout, stderr) from executing the command."},{"line_number":404,"context_line":"    :raises: PasswordFileFailedToCreate from creating or writing to the"},{"line_number":405,"context_line":"             temporary file."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_3be64d4b","line":402,"in_reply_to":"3f79a3b5_722aaa78","updated":"2018-11-05 13:57:37.000000000","message":"Done","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a11efc9b76f90d355e2bff279a301b0137215f00","unresolved":false,"context_lines":[{"line_number":444,"context_line":""},{"line_number":445,"context_line":"    extra_args \u003d {}"},{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    # NOTE(etingof): Windows is not green, it turns blue at times /o\\"},{"line_number":448,"context_line":"    if kill_on_timeout and os.name !\u003d \u0027nt\u0027:"},{"line_number":449,"context_line":"        # NOTE(etingof): We can\u0027t trust ipmitool to terminate in time."},{"line_number":450,"context_line":"        # Therefore we have to kill it if it is running for longer than"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_1247b654","line":447,"updated":"2018-11-05 11:03:32.000000000","message":"Why do we have to account for windows here? :)","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"abb704c99381be889c38b4515729ab8ef0c8027e","unresolved":false,"context_lines":[{"line_number":444,"context_line":""},{"line_number":445,"context_line":"    extra_args \u003d {}"},{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    # NOTE(etingof): Windows is not green, it turns blue at times /o\\"},{"line_number":448,"context_line":"    if kill_on_timeout and os.name !\u003d \u0027nt\u0027:"},{"line_number":449,"context_line":"        # NOTE(etingof): We can\u0027t trust ipmitool to terminate in time."},{"line_number":450,"context_line":"        # Therefore we have to kill it if it is running for longer than"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_9b91a19e","line":447,"in_reply_to":"3f79a3b5_1247b654","updated":"2018-11-05 13:57:37.000000000","message":"Does Microsoft have anything for hardware provisioning in Azure? We got to be ready! ;-)","commit_id":"c0715ed45e4a109e0a89be79a9f4295d4043b571"},{"author":{"_account_id":25547,"name":"jiapei","email":"jiapeish@gmail.com","username":"jeremy.jia"},"change_message_id":"89374d404cf283b3e480a8447c0745533cc98b88","unresolved":false,"context_lines":[{"line_number":372,"context_line":""},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"def _exec_ipmitool_wait(popen_obj, timeout, driver_info):"},{"line_number":375,"context_line":"    wait_interval \u003d min(timeout, 0.5)"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"    while timeout \u003e 0:"},{"line_number":378,"context_line":"        if not popen_obj.poll():"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_39f643d7","line":375,"range":{"start_line":375,"start_character":33,"end_line":375,"end_character":36},"updated":"2018-11-06 07:25:17.000000000","message":"Why 0.5 here?","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"e9cd41f562a860406118b4fde9b1f02bfa6cedb6","unresolved":false,"context_lines":[{"line_number":372,"context_line":""},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"def _exec_ipmitool_wait(popen_obj, timeout, driver_info):"},{"line_number":375,"context_line":"    wait_interval \u003d min(timeout, 0.5)"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"    while timeout \u003e 0:"},{"line_number":378,"context_line":"        if not popen_obj.poll():"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_19bca7b4","line":375,"range":{"start_line":375,"start_character":33,"end_line":375,"end_character":36},"in_reply_to":"3f79a3b5_39f643d7","updated":"2018-11-06 07:36:35.000000000","message":"That\u0027s how frequent we want to check ipmitool process. Just out of UNIX user experience, that feels adequate.\n\nWhat do you think?","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":25547,"name":"jiapei","email":"jiapeish@gmail.com","username":"jeremy.jia"},"change_message_id":"89374d404cf283b3e480a8447c0745533cc98b88","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                {\u0027node\u0027: driver_info[\u0027uuid\u0027], \u0027cmd\u0027: popen_obj.cmd})"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    popen_obj.terminate()"},{"line_number":388,"context_line":"    time.sleep(0.5)"},{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_d9cdcf92","line":388,"range":{"start_line":388,"start_character":15,"end_line":388,"end_character":18},"updated":"2018-11-06 07:25:17.000000000","message":"Can you explain why we set the sleep time as 0.5 and 1?","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"e9cd41f562a860406118b4fde9b1f02bfa6cedb6","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                {\u0027node\u0027: driver_info[\u0027uuid\u0027], \u0027cmd\u0027: popen_obj.cmd})"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    popen_obj.terminate()"},{"line_number":388,"context_line":"    time.sleep(0.5)"},{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_19d107fa","line":388,"range":{"start_line":388,"start_character":15,"end_line":388,"end_character":18},"in_reply_to":"3f79a3b5_d9cdcf92","updated":"2018-11-06 07:36:35.000000000","message":"It might take time for the ipmitool process to shut down. If we would not have any delay here, the next condition (#389) may yield True even if the process is going to shut down momentarily.","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":25547,"name":"jiapei","email":"jiapeish@gmail.com","username":"jeremy.jia"},"change_message_id":"89374d404cf283b3e480a8447c0745533cc98b88","unresolved":false,"context_lines":[{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    time.sleep(1)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    if popen_obj.poll():"},{"line_number":395,"context_line":"        LOG.warning(\u0027Could not kill IPMI process \"%(cmd)s\" for node %(node)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_b9d81351","line":392,"range":{"start_line":392,"start_character":15,"end_line":392,"end_character":16},"updated":"2018-11-06 07:25:17.000000000","message":"It looks like we\u0027re in debugging, there are 3 time sleep here.","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"da515e3f9ae5bdcabd97d5bf0d708702f0d2ff11","unresolved":false,"context_lines":[{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    time.sleep(1)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    if popen_obj.poll():"},{"line_number":395,"context_line":"        LOG.warning(\u0027Could not kill IPMI process \"%(cmd)s\" for node %(node)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_0e0a031d","line":392,"range":{"start_line":392,"start_character":15,"end_line":392,"end_character":16},"in_reply_to":"3f79a3b5_0bfbd547","updated":"2018-11-07 06:40:47.000000000","message":"Right, I thought of that as well, but it seems to be too complicated to explain and choose the proper value. On top of that, even if these delays do not work at some settings, it is still not the end of the world - we basically fall back to the situation we presently have...","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":25547,"name":"jiapei","email":"jiapeish@gmail.com","username":"jeremy.jia"},"change_message_id":"1c2e51e58b2db3458d29f53a0266052032fedba5","unresolved":false,"context_lines":[{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    time.sleep(1)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    if popen_obj.poll():"},{"line_number":395,"context_line":"        LOG.warning(\u0027Could not kill IPMI process \"%(cmd)s\" for node %(node)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_d914ef52","line":392,"range":{"start_line":392,"start_character":15,"end_line":392,"end_character":16},"in_reply_to":"3f79a3b5_39e723da","updated":"2018-11-06 08:01:31.000000000","message":"Since this is nursing the external process, then the external process may depend on external environment, like different IMMs may need different time, how can we make sure this nursed time also work for other environments?","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"e9cd41f562a860406118b4fde9b1f02bfa6cedb6","unresolved":false,"context_lines":[{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    time.sleep(1)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    if popen_obj.poll():"},{"line_number":395,"context_line":"        LOG.warning(\u0027Could not kill IPMI process \"%(cmd)s\" for node %(node)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_39e723da","line":392,"range":{"start_line":392,"start_character":15,"end_line":392,"end_character":16},"in_reply_to":"3f79a3b5_b9d81351","updated":"2018-11-06 07:36:35.000000000","message":"That can look weird, but mind you, we are nursing the external process. It runs independently of ours therefore we might need to account for racing.","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":25547,"name":"jiapei","email":"jiapeish@gmail.com","username":"jeremy.jia"},"change_message_id":"d745b97ffddcaac2988f7679763d5454b36dcf5d","unresolved":false,"context_lines":[{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    time.sleep(1)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    if popen_obj.poll():"},{"line_number":395,"context_line":"        LOG.warning(\u0027Could not kill IPMI process \"%(cmd)s\" for node %(node)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_0bfbd547","line":392,"range":{"start_line":392,"start_character":15,"end_line":392,"end_character":16},"in_reply_to":"3f79a3b5_cec74497","updated":"2018-11-07 06:20:18.000000000","message":"It make sense that \"good enough\" is good enough. I just have an idea that if we can set this time as configurable in CONF file, but this may seem to add complexity. Well, I don\u0027t have a better approach...","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"0e621aecc57e114e459544d48ca9d9f6c7d0ced9","unresolved":false,"context_lines":[{"line_number":389,"context_line":"    if popen_obj.poll():"},{"line_number":390,"context_line":"        popen_obj.kill()"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    time.sleep(1)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    if popen_obj.poll():"},{"line_number":395,"context_line":"        LOG.warning(\u0027Could not kill IPMI process \"%(cmd)s\" for node %(node)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_cec74497","line":392,"range":{"start_line":392,"start_character":15,"end_line":392,"end_character":16},"in_reply_to":"3f79a3b5_d914ef52","updated":"2018-11-06 09:13:00.000000000","message":"I do not think we could realistically ensure the timing would universally work. I think this is the case when \"good enough\" is good enough. ;-)\n\nIf we bump up the delays significantly, then we would hold up periodic tasks for nothing.\n\nEven if all these timings fail miserably in some slow environment, nothing bad would really happen I think.\n\nDoes it make sense? What do you think would be a better approach?","commit_id":"ddfa45fda57bf81f9b7c0593377f4a71ff977419"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"7c1cb7ebf07496ad6ea078035db53323900e7bdf","unresolved":false,"context_lines":[{"line_number":371,"context_line":"    }"},{"line_number":372,"context_line":""},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"def _exec_ipmitool_wait(timeout, driver_info, popen_obj):"},{"line_number":375,"context_line":"    wait_interval \u003d min(timeout, 0.5)"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"    while timeout \u003e 0:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_ab5e44e5","line":374,"updated":"2018-11-26 12:33:11.000000000","message":"I think this kind of makes sense. I do have a concern about this behavior change breaking existing environments. I think it would be good to provide the capability to turn this off.","commit_id":"86caf90449e8b74f66bbeae17f45e5a41529873d"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"baeb37d0e999c836444f5ea26da1676012389bd4","unresolved":false,"context_lines":[{"line_number":371,"context_line":"    }"},{"line_number":372,"context_line":""},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"def _exec_ipmitool_wait(timeout, driver_info, popen_obj):"},{"line_number":375,"context_line":"    wait_interval \u003d min(timeout, 0.5)"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"    while timeout \u003e 0:"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_ec9c1a5d","line":374,"in_reply_to":"3f79a3b5_ab5e44e5","updated":"2018-11-28 17:41:44.000000000","message":"Done by way of [ipmi]kill_on_timeout option","commit_id":"86caf90449e8b74f66bbeae17f45e5a41529873d"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"7c1cb7ebf07496ad6ea078035db53323900e7bdf","unresolved":false,"context_lines":[{"line_number":374,"context_line":"def _exec_ipmitool_wait(timeout, driver_info, popen_obj):"},{"line_number":375,"context_line":"    wait_interval \u003d min(timeout, 0.5)"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"    while timeout \u003e 0:"},{"line_number":378,"context_line":"        if not popen_obj.poll():"},{"line_number":379,"context_line":"            return"},{"line_number":380,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_6b226c55","line":377,"updated":"2018-11-26 12:33:11.000000000","message":"What would happen if the timeout is set to 0 as if someone wanted to disable the timeout?","commit_id":"86caf90449e8b74f66bbeae17f45e5a41529873d"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"baeb37d0e999c836444f5ea26da1676012389bd4","unresolved":false,"context_lines":[{"line_number":374,"context_line":"def _exec_ipmitool_wait(timeout, driver_info, popen_obj):"},{"line_number":375,"context_line":"    wait_interval \u003d min(timeout, 0.5)"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"    while timeout \u003e 0:"},{"line_number":378,"context_line":"        if not popen_obj.poll():"},{"line_number":379,"context_line":"            return"},{"line_number":380,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_2ca8d271","line":377,"in_reply_to":"3f79a3b5_6b226c55","updated":"2018-11-28 17:41:44.000000000","message":"Well, that value comes from `min_command_interval` which should not be set to zero, perhaps. Anyway, I just changed the condition to `\u003e\u003d`.","commit_id":"86caf90449e8b74f66bbeae17f45e5a41529873d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ce776a755f8ab19a93110c8a8069bd61638e84d1","unresolved":false,"context_lines":[{"line_number":451,"context_line":"    extra_args \u003d {}"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    # NOTE(etingof): Windows is not green, it turns blue at times /o\\"},{"line_number":454,"context_line":"    if kill_on_timeout and os.name !\u003d \u0027nt\u0027:"},{"line_number":455,"context_line":"        # NOTE(etingof): We can\u0027t trust ipmitool to terminate in time."},{"line_number":456,"context_line":"        # Therefore we have to kill it if it is running for longer than"},{"line_number":457,"context_line":"        # we asked it to."}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_200aaecf","line":454,"updated":"2018-12-04 11:01:13.000000000","message":"Please remove. We do not support Windows for running ironic.","commit_id":"3d36084cc831b20a520d5c6d73b045c02e067662"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"c24227f7e5feecdb930c370d3d5365ae428b2b9e","unresolved":false,"context_lines":[{"line_number":451,"context_line":"    extra_args \u003d {}"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    # NOTE(etingof): Windows is not green, it turns blue at times /o\\"},{"line_number":454,"context_line":"    if kill_on_timeout and os.name !\u003d \u0027nt\u0027:"},{"line_number":455,"context_line":"        # NOTE(etingof): We can\u0027t trust ipmitool to terminate in time."},{"line_number":456,"context_line":"        # Therefore we have to kill it if it is running for longer than"},{"line_number":457,"context_line":"        # we asked it to."}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_8852ae36","line":454,"in_reply_to":"3f79a3b5_200aaecf","updated":"2018-12-05 08:20:36.000000000","message":"Done","commit_id":"3d36084cc831b20a520d5c6d73b045c02e067662"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ce776a755f8ab19a93110c8a8069bd61638e84d1","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        # before the uncapped `popen.communicate` is called internally."},{"line_number":460,"context_line":"        # That gives us a chance to kill misbehaving `ipmitool` child."},{"line_number":461,"context_line":"        extra_args[\u0027on_execute\u0027] \u003d functools.partial("},{"line_number":462,"context_line":"            _exec_ipmitool_wait, CONF.ipmi.min_command_interval, driver_info)"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    if check_exit_code is not None:"},{"line_number":465,"context_line":"        extra_args[\u0027check_exit_code\u0027] \u003d check_exit_code"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_c0f09ade","line":462,"updated":"2018-12-04 11:01:13.000000000","message":"min_command_interval is just 5 seconds by default, that\u0027s really not enough (esp. compared with 5 minutes you\u0027re talking about in the commit message). What I suggest is instead of the kill_on_timeout boolean option we add kill_timeout int option defaulting to something more conservative (30 seconds?).","commit_id":"3d36084cc831b20a520d5c6d73b045c02e067662"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1ab3be966f1132defacf790e45fc537edce09e7f","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        # before the uncapped `popen.communicate` is called internally."},{"line_number":460,"context_line":"        # That gives us a chance to kill misbehaving `ipmitool` child."},{"line_number":461,"context_line":"        extra_args[\u0027on_execute\u0027] \u003d functools.partial("},{"line_number":462,"context_line":"            _exec_ipmitool_wait, CONF.ipmi.min_command_interval, driver_info)"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    if check_exit_code is not None:"},{"line_number":465,"context_line":"        extra_args[\u0027check_exit_code\u0027] \u003d check_exit_code"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_dd9638ef","line":462,"in_reply_to":"3f79a3b5_c0f09ade","updated":"2018-12-04 17:07:50.000000000","message":"I thought min_command_interval was 60 seconds... We do need to give it a little more time. I know on my personal ipmi machine it can take 15-20 seconds to reply. (older supermicro)","commit_id":"3d36084cc831b20a520d5c6d73b045c02e067662"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"c24227f7e5feecdb930c370d3d5365ae428b2b9e","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        # before the uncapped `popen.communicate` is called internally."},{"line_number":460,"context_line":"        # That gives us a chance to kill misbehaving `ipmitool` child."},{"line_number":461,"context_line":"        extra_args[\u0027on_execute\u0027] \u003d functools.partial("},{"line_number":462,"context_line":"            _exec_ipmitool_wait, CONF.ipmi.min_command_interval, driver_info)"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    if check_exit_code is not None:"},{"line_number":465,"context_line":"        extra_args[\u0027check_exit_code\u0027] \u003d check_exit_code"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_63f093dc","line":462,"in_reply_to":"3f79a3b5_c0f09ade","updated":"2018-12-05 08:20:36.000000000","message":"Right, that\u0027s a bug. /o\\\n\nHowever I am not convinced we should introduce another timeout setting... Would not it make things ever more confusing to the operator? They would really have to rub their brain cells against one another to combine retry timeout, retry interval and kill timeout?\n\nHow about reusing `[ipmi]command_retry_timeout\u003d60` instead of introducing the new `kill_timeout` option? These timeouts seem to have the same semantics , it appears best to keep them in-sync anyway.\n\nWDYT?","commit_id":"3d36084cc831b20a520d5c6d73b045c02e067662"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"c24227f7e5feecdb930c370d3d5365ae428b2b9e","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        # before the uncapped `popen.communicate` is called internally."},{"line_number":460,"context_line":"        # That gives us a chance to kill misbehaving `ipmitool` child."},{"line_number":461,"context_line":"        extra_args[\u0027on_execute\u0027] \u003d functools.partial("},{"line_number":462,"context_line":"            _exec_ipmitool_wait, CONF.ipmi.min_command_interval, driver_info)"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    if check_exit_code is not None:"},{"line_number":465,"context_line":"        extra_args[\u0027check_exit_code\u0027] \u003d check_exit_code"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_23fa9bba","line":462,"in_reply_to":"3f79a3b5_dd9638ef","updated":"2018-12-05 08:20:36.000000000","message":"Done","commit_id":"3d36084cc831b20a520d5c6d73b045c02e067662"},{"author":{"_account_id":29209,"name":"Dhanuka Warusadura","email":"csx@tuta.io","username":"dnuka"},"change_message_id":"65bd2e67a7620f8ac545b61d799fcec8f6f29624","unresolved":false,"context_lines":[{"line_number":1084,"context_line":"        \"\"\""},{"line_number":1085,"context_line":"        driver_info \u003d _parse_driver_info(task.node)"},{"line_number":1086,"context_line":"        # with \u0027-v\u0027 option, we can get the entire sensor data including the"},{"line_number":1087,"context_line":"        # extended sensor informations"},{"line_number":1088,"context_line":"        cmd \u003d \"sdr -v\""},{"line_number":1089,"context_line":"        try:"},{"line_number":1090,"context_line":"            out, err \u003d _exec_ipmitool("}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_3e84f056","line":1087,"range":{"start_line":1087,"start_character":26,"end_line":1087,"end_character":38},"updated":"2018-12-05 08:55:17.000000000","message":"information","commit_id":"9efb9e313d684489aaa3fbb17564fe2de96877b1"},{"author":{"_account_id":26340,"name":"Ilya Etingof","email":"etingof@gmail.com","username":"etingof"},"change_message_id":"232fb51eb76dd2045821b28526401613e42a4d99","unresolved":false,"context_lines":[{"line_number":1084,"context_line":"        \"\"\""},{"line_number":1085,"context_line":"        driver_info \u003d _parse_driver_info(task.node)"},{"line_number":1086,"context_line":"        # with \u0027-v\u0027 option, we can get the entire sensor data including the"},{"line_number":1087,"context_line":"        # extended sensor informations"},{"line_number":1088,"context_line":"        cmd \u003d \"sdr -v\""},{"line_number":1089,"context_line":"        try:"},{"line_number":1090,"context_line":"            out, err \u003d _exec_ipmitool("}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_e45fca71","line":1087,"range":{"start_line":1087,"start_character":26,"end_line":1087,"end_character":38},"in_reply_to":"3f79a3b5_3e84f056","updated":"2018-12-05 09:56:09.000000000","message":"That\u0027s irrelevant to this patch... Perhaps we need to spellcheck the whole code base from time to time.. ;-)","commit_id":"9efb9e313d684489aaa3fbb17564fe2de96877b1"},{"author":{"_account_id":29209,"name":"Dhanuka Warusadura","email":"csx@tuta.io","username":"dnuka"},"change_message_id":"b04c410071a7c8a1149c419858a562c8c49445a5","unresolved":false,"context_lines":[{"line_number":1084,"context_line":"        \"\"\""},{"line_number":1085,"context_line":"        driver_info \u003d _parse_driver_info(task.node)"},{"line_number":1086,"context_line":"        # with \u0027-v\u0027 option, we can get the entire sensor data including the"},{"line_number":1087,"context_line":"        # extended sensor informations"},{"line_number":1088,"context_line":"        cmd \u003d \"sdr -v\""},{"line_number":1089,"context_line":"        try:"},{"line_number":1090,"context_line":"            out, err \u003d _exec_ipmitool("}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_e7982ccc","line":1087,"range":{"start_line":1087,"start_character":26,"end_line":1087,"end_character":38},"in_reply_to":"3f79a3b5_e45fca71","updated":"2018-12-05 10:16:10.000000000","message":"Oh! Sorry about this Ilya :) I just read the full source code to get some scene of what\u0027s going on and saw the typo :)","commit_id":"9efb9e313d684489aaa3fbb17564fe2de96877b1"}]}