)]}'
{"ironic/drivers/base.py":[{"author":{"_account_id":7774,"name":"Kui Shi","email":"skuicloud@gmail.com","username":"Kui"},"change_message_id":"c11c73814448841c943d56abe35de390f9550935","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        :returns: status of the deploy. One of ironic.common.states."},{"line_number":112,"context_line":"        \"\"\""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    @abc.abstractmethod"},{"line_number":115,"context_line":"    def prepare(self, node):"},{"line_number":116,"context_line":"        \"\"\"Prepare the deployment environment for this node."},{"line_number":117,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BP5c%3D","line":114,"updated":"2013-11-28 10:45:07.000000000","message":"Compared to the method definition in pxe module, the parameter \"task\" is missed.","commit_id":"1029a12edca87917359a85908148b44ac5211ea7"},{"author":{"_account_id":7774,"name":"Kui Shi","email":"skuicloud@gmail.com","username":"Kui"},"change_message_id":"c11c73814448841c943d56abe35de390f9550935","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                     on this Conductor."},{"line_number":126,"context_line":"        \"\"\""},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    @abc.abstractmethod"},{"line_number":129,"context_line":"    def cleanup(self, node):"},{"line_number":130,"context_line":"        \"\"\"Clean up the deployment environment for this node."},{"line_number":131,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BP5o%3D","line":128,"updated":"2013-11-28 10:45:07.000000000","message":"Compared to the method definition in pxe module, the parameter \"task\" is missed.","commit_id":"1029a12edca87917359a85908148b44ac5211ea7"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"bfd354eea8a72787f73062278289b5a1974f0b38","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        \"\"\"Tear down a previous deployment."},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        Given a node that has been previously deployed to,"},{"line_number":107,"context_line":"        do all cleanup and tear down necessary to \"un-deploy\" that node."},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"        :param task: a TaskManager instance."},{"line_number":110,"context_line":"        :param node: the Node to act upon."}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F94UY%3D","side":"PARENT","line":107,"updated":"2013-12-04 23:14:03.000000000","message":"For tear_down, it says \"do all cleanup\", but you\u0027re adding a cleanup(). Is it preferable to do cleanup in cleanup() if the cleanup is idempotent?\n\nI see in the code, that tear_down calls cleanup. So I guess it is true that it \"do all cleanup\". Is it worth noting then that teardown might want to call cleanup?","commit_id":"18db8291ab828e673926a07707bd01149c88bfd4"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"bfd354eea8a72787f73062278289b5a1974f0b38","unresolved":false,"context_lines":[{"line_number":130,"context_line":"    def cleanup(self, task, node):"},{"line_number":131,"context_line":"        \"\"\"Clean up the deployment environment for this node."},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        This method should be called before tear_down, and may optionally"},{"line_number":134,"context_line":"        perform any necessary steps to clean up the deployment environment.  It"},{"line_number":135,"context_line":"        should be idempotent, as it may be called repeatedly, and it should be"},{"line_number":136,"context_line":"        callable on multiple conductors, so it should not in any way affect the"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F94S8%3D","line":133,"updated":"2013-12-04 23:14:03.000000000","message":"I\u0027m a bit unclear about this. cleanup() is optional, right? But if it is called, it should be called before tear_down?\n\nOr do you mean it should be called before tear_down, regardless of whether it does any cleanup (the \u0027may optionally perform\u0027 part).","commit_id":"4dc6396c6743f7cd5b0b6af06bebfe1b4f7af079"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"be8a7e8e8397cce5ab65b9e470a4169d0384e934","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    def prepare(self, task, node):"},{"line_number":116,"context_line":"        \"\"\"Prepare the deployment environment for this node."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method should be called before deploy, and may optionally perform"},{"line_number":119,"context_line":"        any necessary steps to prepare the deployment environment.  It should"},{"line_number":120,"context_line":"        be idempotent, as it may be called repeatedly, and it should be"},{"line_number":121,"context_line":"        callable on multiple conductors, so it should not in any way affect the"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F9zUc%3D","line":118,"updated":"2013-12-05 18:58:52.000000000","message":"If it\u0027s optional, why force drivers to implement it? E.g. drop the abstractmethod.","commit_id":"241d10f5a0aee2d9220539d14f114cdfa73ebd1b"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"1a4576c005c75f1765e87eec05e1518aea0fc690","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    def prepare(self, task, node):"},{"line_number":116,"context_line":"        \"\"\"Prepare the deployment environment for this node."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        This method should be called before deploy, and may optionally perform"},{"line_number":119,"context_line":"        any necessary steps to prepare the deployment environment.  It should"},{"line_number":120,"context_line":"        be idempotent, as it may be called repeatedly, and it should be"},{"line_number":121,"context_line":"        callable on multiple conductors, so it should not in any way affect the"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F9uOk%3D","line":118,"in_reply_to":"AAAATn%2F9zUc%3D","updated":"2013-12-06 18:31:11.000000000","message":"Ah, good point.\n\nI will update the docstring to explain how this method will be called on many conductors, in relationship to hash ring # of replicas, and that its purpose is to prepare the environment ahead of taking over management of a node.","commit_id":"241d10f5a0aee2d9220539d14f114cdfa73ebd1b"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"be8a7e8e8397cce5ab65b9e470a4169d0384e934","unresolved":false,"context_lines":[{"line_number":130,"context_line":"    def cleanup(self, task, node):"},{"line_number":131,"context_line":"        \"\"\"Clean up the deployment environment for this node."},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"        This method should be called before tear_down, and may optionally"},{"line_number":134,"context_line":"        perform any necessary steps to clean up the deployment environment.  It"},{"line_number":135,"context_line":"        should be idempotent, as it may be called repeatedly, and it should be"},{"line_number":136,"context_line":"        callable on multiple conductors, so it should not in any way affect the"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F9zUI%3D","line":133,"updated":"2013-12-05 18:58:52.000000000","message":"Same question.","commit_id":"241d10f5a0aee2d9220539d14f114cdfa73ebd1b"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5ff8bc81a877d260d4c342f021d7134bfbf17b97","unresolved":false,"context_lines":[{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        If implemented, this method must be idempotent. It may be called"},{"line_number":123,"context_line":"        multiple times for the same node on the same conductor, and it may be"},{"line_number":124,"context_line":"        called by multiple conductors in parallel. Therefor, it must not"},{"line_number":125,"context_line":"        require an exclusive lock."},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        This method is called before `deploy`."}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F9lHk%3D","line":124,"updated":"2013-12-10 01:24:38.000000000","message":"small nit. typo:Therefore","commit_id":"4dda6209223834918ceede75065474e5203f73d9"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"36fda3a5716b6b9da6f981886afb3f6ca5bd03c3","unresolved":false,"context_lines":[{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        If implemented, this method must be idempotent. It may be called"},{"line_number":123,"context_line":"        multiple times for the same node on the same conductor, and it may be"},{"line_number":124,"context_line":"        called by multiple conductors in parallel. Therefor, it must not"},{"line_number":125,"context_line":"        require an exclusive lock."},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        This method is called before `deploy`."}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F9k7Y%3D","line":124,"in_reply_to":"AAAATn%2F9lHk%3D","updated":"2013-12-10 02:18:30.000000000","message":"Done","commit_id":"4dda6209223834918ceede75065474e5203f73d9"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5ff8bc81a877d260d4c342f021d7134bfbf17b97","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        called by multiple conductors in parallel. Therefor, it must not"},{"line_number":125,"context_line":"        require an exclusive lock."},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        This method is called before `deploy`."},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        :param task: a TaskManager instance."},{"line_number":130,"context_line":"        :param node: the Node for which to prepare a deployment environment"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F9lDc%3D","line":127,"updated":"2013-12-10 01:24:38.000000000","message":"This is still a bit confusing. In pxe.py, deploy() calls this method. Is it something like \u0027if implemented, deploy() should call it before doing the actual deploying\u0027?\n\nSimilar comments for cleanup()","commit_id":"4dda6209223834918ceede75065474e5203f73d9"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5ff8bc81a877d260d4c342f021d7134bfbf17b97","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        \"\"\""},{"line_number":153,"context_line":"        pass"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def takeover(self, task, node):"},{"line_number":156,"context_line":"        \"\"\"Take over management of this node from a dead conductor."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        If conductors\u0027 hosts maintain a static relationship to nodes, this"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F9lDE%3D","line":155,"updated":"2013-12-10 01:24:38.000000000","message":"this is really picky so I\u0027m not sure if I should mention, but I will anyway :-)\n\nwe have deploy, tear_down, prepare, cleanup, takeover. What do you think about clean_up \u0026 take_over, (or teardown).","commit_id":"4dda6209223834918ceede75065474e5203f73d9"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"36fda3a5716b6b9da6f981886afb3f6ca5bd03c3","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        \"\"\""},{"line_number":153,"context_line":"        pass"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def takeover(self, task, node):"},{"line_number":156,"context_line":"        \"\"\"Take over management of this node from a dead conductor."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        If conductors\u0027 hosts maintain a static relationship to nodes, this"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F9k74%3D","line":155,"in_reply_to":"AAAATn%2F9lDE%3D","updated":"2013-12-10 02:18:30.000000000","message":"Done","commit_id":"4dda6209223834918ceede75065474e5203f73d9"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"855935d96d0cda45cfb180b3af26249fb39d5cd9","unresolved":false,"context_lines":[{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        If implemented, this method must be idempotent. It may be called"},{"line_number":123,"context_line":"        multiple times for the same node on the same conductor, and it may be"},{"line_number":124,"context_line":"        called by multiple conductors in parallel. Therefor, it must not"},{"line_number":125,"context_line":"        require an exclusive lock."},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        This method is called before `deploy`."}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F9cxg%3D","line":124,"updated":"2013-12-11 13:00:42.000000000","message":"s/Therefor/Therefore","commit_id":"40f67fcc4a9c03378b63aebf3b8cd5870b802b71"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"855935d96d0cda45cfb180b3af26249fb39d5cd9","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        called by multiple conductors in parallel. Therefor, it must not"},{"line_number":125,"context_line":"        require an exclusive lock."},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        This method is called before `deploy`."},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        :param task: a TaskManager instance."},{"line_number":130,"context_line":"        :param node: the Node for which to prepare a deployment environment"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F9cw4%3D","line":127,"updated":"2013-12-11 13:00:42.000000000","message":"Maybe it would make sence to give a brief scheme in what order methods are called and what they do (3-4 words) in the doc string for a class, and make in-place explanations shorter?","commit_id":"40f67fcc4a9c03378b63aebf3b8cd5870b802b71"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"855935d96d0cda45cfb180b3af26249fb39d5cd9","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        :param node: the Node for which to prepare a deployment environment"},{"line_number":131,"context_line":"                     on this Conductor."},{"line_number":132,"context_line":"        \"\"\""},{"line_number":133,"context_line":"        pass"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def clean_up(self, task, node):"},{"line_number":136,"context_line":"        \"\"\"Clean up the deployment environment for this node."}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F9cvk%3D","line":133,"updated":"2013-12-11 13:00:42.000000000","message":"There are no pass in the end of validate, deploy, tear_down, I\u0027d like to suggest to unify these methods in either of ways (with pass or not).","commit_id":"40f67fcc4a9c03378b63aebf3b8cd5870b802b71"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"c3e52c9c990d89123482cf54f464e4b8961b026a","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        :param node: the Node for which to prepare a deployment environment"},{"line_number":131,"context_line":"                     on this Conductor."},{"line_number":132,"context_line":"        \"\"\""},{"line_number":133,"context_line":"        pass"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def clean_up(self, task, node):"},{"line_number":136,"context_line":"        \"\"\"Clean up the deployment environment for this node."}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F9RSc%3D","line":133,"in_reply_to":"AAAATn%2F9UOo%3D","updated":"2013-12-13 09:14:15.000000000","message":"Ah I see. You\u0027re right!","commit_id":"40f67fcc4a9c03378b63aebf3b8cd5870b802b71"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"90414184676c9f36875fc91456ceebeff9f38feb","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        :param node: the Node for which to prepare a deployment environment"},{"line_number":131,"context_line":"                     on this Conductor."},{"line_number":132,"context_line":"        \"\"\""},{"line_number":133,"context_line":"        pass"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def clean_up(self, task, node):"},{"line_number":136,"context_line":"        \"\"\"Clean up the deployment environment for this node."}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F9UOo%3D","line":133,"in_reply_to":"AAAATn%2F9cvk%3D","updated":"2013-12-12 18:15:29.000000000","message":"This was based on lifeless\u0027 suggestion on a previous review...\n\nvalidate, deploy, and tear_down are @abstractmethods and must be implemented in each driver, whereas prepare, clean_up, and take_over are not -- and therefor must have a body here.","commit_id":"40f67fcc4a9c03378b63aebf3b8cd5870b802b71"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"4f90f8660cd3a1a2c5b736579a228084cae56318","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        :returns: status of the deploy. One of ironic.common.states."},{"line_number":114,"context_line":"        \"\"\""},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    @abc.abstractmethod"},{"line_number":117,"context_line":"    def prepare(self, task, node):"},{"line_number":118,"context_line":"        \"\"\"Prepare the deployment environment for this node."},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F9RRo%3D","line":116,"updated":"2013-12-13 10:15:36.000000000","message":"Sorry if I missed something, but I thought we decided that these methods are not required to implement and therefore you deleted this decorator and added body in previous patches.","commit_id":"ce7d5bfcf13ba812e94d24aa6b3ae50112af07bd"}],"ironic/drivers/modules/fake.py":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"be8a7e8e8397cce5ab65b9e470a4169d0384e934","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    def tear_down(self, task, node):"},{"line_number":57,"context_line":"        pass"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def prepare(self, task, node):"},{"line_number":60,"context_line":"        pass"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def cleanup(self, task, node):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F9zT8%3D","line":59,"updated":"2013-12-05 18:58:52.000000000","message":"These are symptoms of making the methods abstract.","commit_id":"241d10f5a0aee2d9220539d14f114cdfa73ebd1b"}],"ironic/drivers/modules/pxe.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5ff8bc81a877d260d4c342f021d7134bfbf17b97","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"def _update_neutron(task, node):"},{"line_number":439,"context_line":"    \"\"\"Send the DHCP BOOT options to Neutron for this node.\"\"\""},{"line_number":440,"context_line":"    # FIXME: just a stub for the moment"},{"line_number":441,"context_line":"    pass"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F9lC4%3D","line":440,"updated":"2013-12-10 01:24:38.000000000","message":"(Just curious.) When do you use \u0027FIXME\u0027 vs \u0027TODO\u0027, and when would you put your id next to it?","commit_id":"4dda6209223834918ceede75065474e5203f73d9"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"36fda3a5716b6b9da6f981886afb3f6ca5bd03c3","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"def _update_neutron(task, node):"},{"line_number":439,"context_line":"    \"\"\"Send the DHCP BOOT options to Neutron for this node.\"\"\""},{"line_number":440,"context_line":"    # FIXME: just a stub for the moment"},{"line_number":441,"context_line":"    pass"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F9k5g%3D","line":440,"in_reply_to":"AAAATn%2F9lC4%3D","updated":"2013-12-10 02:18:30.000000000","message":"I could not find any policy in the wiki, but I am loosely basing my preference on this discussion:\nhttp://lists.openstack.org/pipermail/openstack-dev/2013-March/006239.html\n\nI know that Don Kehn is working on the Neutron integration within the PXE driver, and so he will, presumably, implement a method which does this in the near future. I\u0027m refactoring the driver code for the consistent hash ring and just adding a placeholder for him -- it\u0027s not a TODO to myself, and it\u0027s bad form to commit code with a TODO with someone else\u0027s name, but it should still have a note. I could have just left a comment, but I felt the FIXME would draw his attention to it.\n\nTo answer your question more directly, a TODO should always have the committer\u0027s name next to it.","commit_id":"4dda6209223834918ceede75065474e5203f73d9"}]}