)]}'
{"ironic/drivers/modules/pxe.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"97db31f67af2368a7f91c9a68686478ba4a89a25","unresolved":false,"context_lines":[{"line_number":400,"context_line":"            pxe_info \u003d _get_image_info(node, task.context)"},{"line_number":401,"context_line":"        except exception.MissingParameterValue as e:"},{"line_number":402,"context_line":"            LOG.warning(_LW(\u0027Could not get image info to clean up images \u0027"},{"line_number":403,"context_line":"                            \u0027for node %s: %s\u0027), node.uuid, e)"},{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            for label in pxe_info:"},{"line_number":406,"context_line":"                path \u003d pxe_info[label][1]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_ff50762d","line":403,"updated":"2014-09-16 07:23:49.000000000","message":"nit: I think we prefer using named substitutions, when we have more than one substitution","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"43eb338d4ab247386510a83b321c7432cbc1aab0","unresolved":false,"context_lines":[{"line_number":400,"context_line":"            pxe_info \u003d _get_image_info(node, task.context)"},{"line_number":401,"context_line":"        except exception.MissingParameterValue as e:"},{"line_number":402,"context_line":"            LOG.warning(_LW(\u0027Could not get image info to clean up images \u0027"},{"line_number":403,"context_line":"                            \u0027for node %s: %s\u0027), node.uuid, e)"},{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            for label in pxe_info:"},{"line_number":406,"context_line":"                path \u003d pxe_info[label][1]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_a6a09e92","line":403,"in_reply_to":"fa98f980_907837c5","updated":"2014-09-16 12:59:04.000000000","message":"Will do","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"97b2113930dd914374e282e1d3a83cee0c67c004","unresolved":false,"context_lines":[{"line_number":400,"context_line":"            pxe_info \u003d _get_image_info(node, task.context)"},{"line_number":401,"context_line":"        except exception.MissingParameterValue as e:"},{"line_number":402,"context_line":"            LOG.warning(_LW(\u0027Could not get image info to clean up images \u0027"},{"line_number":403,"context_line":"                            \u0027for node %s: %s\u0027), node.uuid, e)"},{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            for label in pxe_info:"},{"line_number":406,"context_line":"                path \u003d pxe_info[label][1]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_907837c5","line":403,"in_reply_to":"fa98f980_ff50762d","updated":"2014-09-16 09:32:46.000000000","message":"Actually we should be more strict about it. Messages with more than one argument should always use named interpolation because it helps with the translation of the string, allowing ppl to move the arguments around to form a new grammar etc...","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"97b2113930dd914374e282e1d3a83cee0c67c004","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        except exception.MissingParameterValue as e:"},{"line_number":402,"context_line":"            LOG.warning(_LW(\u0027Could not get image info to clean up images \u0027"},{"line_number":403,"context_line":"                            \u0027for node %s: %s\u0027), node.uuid, e)"},{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            for label in pxe_info:"},{"line_number":406,"context_line":"                path \u003d pxe_info[label][1]"},{"line_number":407,"context_line":"                utils.unlink_without_raise(path)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_b008db2d","line":404,"updated":"2014-09-16 09:32:46.000000000","message":"super nit/personal opinion: I know it\u0027s a valid syntax and if the exception is not raised this else is going to be executed... But I really never saw any benefits on using it try...else in python, IMO it just makes the code more confusing for people who are just starting with python.\n\nI would removed this else and simple execute the code after the try..except block\n\n(Please correct me if I\u0027m wrong telling me the use of this else, if there\u0027s one. Even after reading the documentation[1] I\u0027m still not convinced about any benefits it may have)\n\n[1] https://docs.python.org/2/tutorial/errors.html#handling-exceptions","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"7c4dd5a0dc1b7f8794580e11478f9ad14ed53f3d","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        except exception.MissingParameterValue as e:"},{"line_number":402,"context_line":"            LOG.warning(_LW(\u0027Could not get image info to clean up images \u0027"},{"line_number":403,"context_line":"                            \u0027for node %s: %s\u0027), node.uuid, e)"},{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            for label in pxe_info:"},{"line_number":406,"context_line":"                path \u003d pxe_info[label][1]"},{"line_number":407,"context_line":"                utils.unlink_without_raise(path)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_a6d89ef1","line":404,"in_reply_to":"fa98f980_26dbce22","updated":"2014-09-16 13:00:28.000000000","message":"oh gotcha, yeah makes sense","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"43eb338d4ab247386510a83b321c7432cbc1aab0","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        except exception.MissingParameterValue as e:"},{"line_number":402,"context_line":"            LOG.warning(_LW(\u0027Could not get image info to clean up images \u0027"},{"line_number":403,"context_line":"                            \u0027for node %s: %s\u0027), node.uuid, e)"},{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            for label in pxe_info:"},{"line_number":406,"context_line":"                path \u003d pxe_info[label][1]"},{"line_number":407,"context_line":"                utils.unlink_without_raise(path)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_26dbce22","line":404,"in_reply_to":"fa98f980_b008db2d","updated":"2014-09-16 12:59:04.000000000","message":"This code can only be executed if an exception is not raised.\n\nThe other way I could do it is in the except block, set pxe_info \u003d [], and always run the \u0027for label in pxe_info\u0027 block.","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"}],"ironic/tests/drivers/test_pxe.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f4bc45f58697d349043338f3b642c48871255b74","unresolved":false,"context_lines":[{"line_number":708,"context_line":"            def raise_mpv(_arg1, _arg2):"},{"line_number":709,"context_line":"                # this method fights mock side_effect weirdness"},{"line_number":710,"context_line":"                raise exception.MissingParameterValue(\u0027image_source\u0027)"},{"line_number":711,"context_line":"            get_image_info_mock.side_effect \u003d raise_mpv"},{"line_number":712,"context_line":"        else:"},{"line_number":713,"context_line":"            get_image_info_mock.return_value \u003d image_info"},{"line_number":714,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa98f980_19e0fd24","line":711,"updated":"2014-09-16 00:57:24.000000000","message":"I\u0027m curious to know what sort of weirdness. Why doesn\u0027t \"get_image_info_mock.side_effect \u003d exception.MissingParameterValue(\u0027image_source\u0027)\" work?","commit_id":"02da3fc8b45753db32f6fde4be134abecd06e6a3"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"8ea4e98cc29ae94d40a62f06e0cf52d33033f571","unresolved":false,"context_lines":[{"line_number":708,"context_line":"            def raise_mpv(_arg1, _arg2):"},{"line_number":709,"context_line":"                # this method fights mock side_effect weirdness"},{"line_number":710,"context_line":"                raise exception.MissingParameterValue(\u0027image_source\u0027)"},{"line_number":711,"context_line":"            get_image_info_mock.side_effect \u003d raise_mpv"},{"line_number":712,"context_line":"        else:"},{"line_number":713,"context_line":"            get_image_info_mock.return_value \u003d image_info"},{"line_number":714,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa98f980_eba7dc4d","line":711,"in_reply_to":"fa98f980_19e0fd24","updated":"2014-09-16 02:44:39.000000000","message":"Oh, didn\u0027t think of that, was doing just `get_image_info_mock.return_value \u003d exception.MissingParameterValue`\n\nThanks! :)","commit_id":"02da3fc8b45753db32f6fde4be134abecd06e6a3"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"97db31f67af2368a7f91c9a68686478ba4a89a25","unresolved":false,"context_lines":[{"line_number":752,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":753,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":754,"context_line":"            task.driver.deploy.clean_up(task)"},{"line_number":755,"context_line":"            get_image_info_mock.called_once_with(task.node)"},{"line_number":756,"context_line":"        assert_false_path \u003d [config_path, deploy_kernel_path, image_path,"},{"line_number":757,"context_line":"                             pxe_mac_path, image_dir, instance_dir,"},{"line_number":758,"context_line":"                             token_path]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_5a1ef05f","line":755,"updated":"2014-09-16 07:23:49.000000000","message":"quoting:\n\n pxe_info \u003d _get_image_info(node, task.context)\n\nhow can this test pass? O_O","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"97b2113930dd914374e282e1d3a83cee0c67c004","unresolved":false,"context_lines":[{"line_number":752,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":753,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":754,"context_line":"            task.driver.deploy.clean_up(task)"},{"line_number":755,"context_line":"            get_image_info_mock.called_once_with(task.node)"},{"line_number":756,"context_line":"        assert_false_path \u003d [config_path, deploy_kernel_path, image_path,"},{"line_number":757,"context_line":"                             pxe_mac_path, image_dir, instance_dir,"},{"line_number":758,"context_line":"                             token_path]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_92375ce4","line":755,"in_reply_to":"fa98f980_5a1ef05f","updated":"2014-09-16 09:32:46.000000000","message":"Ohhh because the correct method name in the mock object is \"assert_called_once_with\" and not \"called_once_with\"\n\nNice catch","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"43eb338d4ab247386510a83b321c7432cbc1aab0","unresolved":false,"context_lines":[{"line_number":752,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":753,"context_line":"                                  shared\u003dTrue) as task:"},{"line_number":754,"context_line":"            task.driver.deploy.clean_up(task)"},{"line_number":755,"context_line":"            get_image_info_mock.called_once_with(task.node)"},{"line_number":756,"context_line":"        assert_false_path \u003d [config_path, deploy_kernel_path, image_path,"},{"line_number":757,"context_line":"                             pxe_mac_path, image_dir, instance_dir,"},{"line_number":758,"context_line":"                             token_path]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_06d6caf1","line":755,"in_reply_to":"fa98f980_92375ce4","updated":"2014-09-16 12:59:04.000000000","message":"Heh, will fix.","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"97db31f67af2368a7f91c9a68686478ba4a89a25","unresolved":false,"context_lines":[{"line_number":758,"context_line":"                             token_path]"},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"        for path in assert_false_path:"},{"line_number":761,"context_line":"            self.assertFalse(os.path.exists(path))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"    def test_clean_up_fail_get_image_info(self):"},{"line_number":764,"context_line":"        self.clean_up_config(fail_get_image_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_1f1a9a7c","line":761,"updated":"2014-09-16 07:23:49.000000000","message":"hmmm... don\u0027t we expect this test to fail on deploy_kernel_path? I\u0027d like to see test proving that file was not deleted, though clean up succeeded","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"43eb338d4ab247386510a83b321c7432cbc1aab0","unresolved":false,"context_lines":[{"line_number":758,"context_line":"                             token_path]"},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"        for path in assert_false_path:"},{"line_number":761,"context_line":"            self.assertFalse(os.path.exists(path))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"    def test_clean_up_fail_get_image_info(self):"},{"line_number":764,"context_line":"        self.clean_up_config(fail_get_image_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_e64b86af","line":761,"in_reply_to":"fa98f980_1f1a9a7c","updated":"2014-09-16 12:59:04.000000000","message":"Dunno, you would think so... I verified by mucking with the code a bit that the tests follow the expected code path, not sure what\u0027s happening here :/","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"b8f478e0a8a99467051024659755f658e94b3dc7","unresolved":false,"context_lines":[{"line_number":758,"context_line":"                             token_path]"},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"        for path in assert_false_path:"},{"line_number":761,"context_line":"            self.assertFalse(os.path.exists(path))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"    def test_clean_up_fail_get_image_info(self):"},{"line_number":764,"context_line":"        self.clean_up_config(fail_get_image_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_8c5d7b7c","line":761,"in_reply_to":"fa98f980_e64b86af","updated":"2014-09-16 14:00:23.000000000","message":"So I dug into this a bit more... it seems that the path it\u0027s looking for is on the instance... e.g. /tmp/tmpcyq_au/tmpz5TeHZ/1be26c0b-03f2-4d2e-ae87-c02d7f33c123/deploy_kernel\n\nI think the tests are broken here :( will work on it.","commit_id":"e0096a06d1939fcc95e51831bc772980ff9d7942"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"34b5d51d88c2ef55f0396209e41c0637aef6f784","unresolved":false,"context_lines":[{"line_number":700,"context_line":"                    uuid\u003d\u0027bb43dc0b-03f2-4d2e-ae87-c02d7f33cc53\u0027)"},{"line_number":701,"context_line":"         )"},{"line_number":702,"context_line":""},{"line_number":703,"context_line":"        d_kernel_path \u003d os.path.join(tftp_master_dir, \u0027deploy_kernel\u0027)"},{"line_number":704,"context_line":"        image_info \u003d {\u0027deploy_kernel\u0027: (\u0027deploy_kernel_uuid\u0027, d_kernel_path)}"},{"line_number":705,"context_line":""},{"line_number":706,"context_line":"        if fail_get_image_info:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_f93168ee","line":703,"updated":"2014-09-16 14:34:44.000000000","message":"the same as master_deploy_kernel_path. and master copy is never deleted - it\u0027s cached.","commit_id":"985eba60edc339a1444e9206552eedb0e28758b6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"34b5d51d88c2ef55f0396209e41c0637aef6f784","unresolved":false,"context_lines":[{"line_number":744,"context_line":"        else:"},{"line_number":745,"context_line":"            open(deploy_kernel_path, \u0027w\u0027).close()"},{"line_number":746,"context_line":"            open(image_path, \u0027w\u0027).close()"},{"line_number":747,"context_line":"        open(d_kernel_path, \u0027w\u0027).close()"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":"        token_path \u003d self._create_token_file()"},{"line_number":750,"context_line":"        self.config(image_cache_size\u003d0, group\u003d\u0027pxe\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_b9ac40f0","line":747,"updated":"2014-09-16 14:34:44.000000000","message":"it\u0027s deploy_kernel_path, no?","commit_id":"985eba60edc339a1444e9206552eedb0e28758b6"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"34b5d51d88c2ef55f0396209e41c0637aef6f784","unresolved":false,"context_lines":[{"line_number":758,"context_line":"                             pxe_mac_path, image_dir, instance_dir,"},{"line_number":759,"context_line":"                             token_path]"},{"line_number":760,"context_line":"        if not fail_get_image_info:"},{"line_number":761,"context_line":"            assert_false_path.append(d_kernel_path)"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        for path in assert_false_path:"},{"line_number":764,"context_line":"            self.assertFalse(os.path.exists(path))"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_e77e20af","line":761,"updated":"2014-09-16 14:34:44.000000000","message":"I\u0027d even prefer \n\n self.assertIs(os.path.exists(deploy_kernel_path), fail_get_image_info)\n\nthat\u0027s the test that should be passing IMO","commit_id":"985eba60edc339a1444e9206552eedb0e28758b6"}]}