)]}'
{"ironic/common/neutron.py":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"57c304772595ecb6d7d82fa1cfc782486eed8e61","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            self.client.update_port(port_id, port_req_body)"},{"line_number":90,"context_line":"        except neutron_client_exc.NeutronClientException as e:"},{"line_number":91,"context_line":"            LOG.error(_(\"Neutron error updating port %(port)s. Error: \""},{"line_number":92,"context_line":"                        \"%(error)s.\") % {\u0027port\u0027: port_id, \u0027error\u0027: e})"},{"line_number":93,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort("},{"line_number":94,"context_line":"                port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F93Yc%3D","line":92,"updated":"2014-02-03 05:15:39.000000000","message":"For python logging, don\u0027t interpolate here. Instead do\n\nLOG.error(_(\"...\"), port\u003dport_id, error\u003de)\n\nIn fact, here you probably want\n\nLOG.exception(_(\"...\"), port\u003dport_id) which will log the current exception automatically.","commit_id":"277793ee177dd603952f10070f935ec07cbe6ad7"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"69696c18de0a9295a5448754f4277a0bc1127d1b","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            self.client.update_port(port_id, port_req_body)"},{"line_number":90,"context_line":"        except neutron_client_exc.NeutronClientException as e:"},{"line_number":91,"context_line":"            LOG.error(_(\"Neutron error updating port %(port)s. Error: \""},{"line_number":92,"context_line":"                        \"%(error)s.\") % {\u0027port\u0027: port_id, \u0027error\u0027: e})"},{"line_number":93,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort("},{"line_number":94,"context_line":"                port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F9ykE%3D","line":92,"in_reply_to":"AAAAUH%2F93Yc%3D","updated":"2014-02-03 22:39:35.000000000","message":"Done","commit_id":"277793ee177dd603952f10070f935ec07cbe6ad7"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"db1a9a6138dd94e24429bd72b6ec3b23fa6f1b4d","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            self.client.update_port(port_id, port_req_body)"},{"line_number":90,"context_line":"        except neutron_client_exc.NeutronClientException as e:"},{"line_number":91,"context_line":"            LOG.error(_(\"Neutron error updating port %(port)s. Error: \""},{"line_number":92,"context_line":"                        \"%(error)s.\") % {\u0027port\u0027: port_id, \u0027error\u0027: e})"},{"line_number":93,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort("},{"line_number":94,"context_line":"                port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2FhJA%3D","line":92,"in_reply_to":"AAAAUH%2F9ykE%3D","updated":"2014-02-12 16:36:39.000000000","message":"@lifeless, what was the reason for your comment here? I thought it had something to do with LOG.exception when I revised this, but upon further investigation, it doesn\u0027t.\n\nSo, now the discussion has come up about whether or not to do string interpolation here, or to pass parameters to LOG, and I\u0027ve set the precedent by changing my code from \"%\" to \",\" ...\n\nNova does both, it\u0027s not enforced in hacking, in jog0\u0027s perspective, it\u0027s not worth enforcing. I can understand that there\u0027s a tiny performance impact, but in the scheme of where Ironic is going to bottleneck, string interpolation isn\u0027t it ... So what am I missing?","commit_id":"277793ee177dd603952f10070f935ec07cbe6ad7"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"df4c620684a2158fba202c59fd16b8a8ff74b09f","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        except neutron_client_exc.NeutronClientException:"},{"line_number":91,"context_line":"            LOG.exception(_(\u0027Neutron error updating port: %s\u0027) %"},{"line_number":92,"context_line":"                          port_id)"},{"line_number":93,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort("},{"line_number":94,"context_line":"                port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F9wEk%3D","side":"PARENT","line":93,"updated":"2014-02-04 11:55:51.000000000","message":"Looking at the tests and the code at pxe.py I think this exception is expected to be raised, why was it removed?","commit_id":"1689854543b58feaac0f13b3a6a727882679182f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"7c42feadc14a2e733aec756bce6c6a967b7a27c3","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        except neutron_client_exc.NeutronClientException:"},{"line_number":91,"context_line":"            LOG.exception(_(\u0027Neutron error updating port: %s\u0027) %"},{"line_number":92,"context_line":"                          port_id)"},{"line_number":93,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort("},{"line_number":94,"context_line":"                port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F9p4g%3D","side":"PARENT","line":93,"in_reply_to":"AAAAUH%2F9s6Y%3D","updated":"2014-02-05 10:14:08.000000000","message":"+1","commit_id":"1689854543b58feaac0f13b3a6a727882679182f"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"f10ab4e1bd74a2792ac4d26ebe055ea794ffdc29","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        except neutron_client_exc.NeutronClientException:"},{"line_number":91,"context_line":"            LOG.exception(_(\u0027Neutron error updating port: %s\u0027) %"},{"line_number":92,"context_line":"                          port_id)"},{"line_number":93,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort("},{"line_number":94,"context_line":"                port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F9s6Y%3D","side":"PARENT","line":93,"in_reply_to":"AAAAUH%2F9s9w%3D","updated":"2014-02-04 20:37:39.000000000","message":"Actually, this omissions should have been caught by the unit tests -- it wasn\u0027t due to a bug in the unit tests. I\u0027m fixing in the next revision.","commit_id":"1689854543b58feaac0f13b3a6a727882679182f"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"214bf6811a7fde56548d5d2e5f65958775776d7b","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        except neutron_client_exc.NeutronClientException:"},{"line_number":91,"context_line":"            LOG.exception(_(\u0027Neutron error updating port: %s\u0027) %"},{"line_number":92,"context_line":"                          port_id)"},{"line_number":93,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort("},{"line_number":94,"context_line":"                port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F9s9w%3D","side":"PARENT","line":93,"in_reply_to":"AAAAUH%2F9wEk%3D","updated":"2014-02-04 20:34:28.000000000","message":"Woops! This got cut by accident when I changed LOG.error to LOG.exception.\n\nThanks for catching it!","commit_id":"1689854543b58feaac0f13b3a6a727882679182f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"df4c620684a2158fba202c59fd16b8a8ff74b09f","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                         {\u0027opt_name\u0027: \u0027tftp-server\u0027,"},{"line_number":82,"context_line":"                          \u0027opt_value\u0027: \u0027123.123.123.123\u0027}]"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        :raises: FailedToUpdateDHCPOptOnPort"},{"line_number":85,"context_line":"        \"\"\""},{"line_number":86,"context_line":"        port_req_body \u003d {\u0027port\u0027: {\u0027extra_dhcp_opts\u0027: dhcp_options}}"},{"line_number":87,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F9wFA%3D","line":84,"updated":"2014-02-04 11:55:51.000000000","message":"We need to update the docstring if the eception is not being raised anymore","commit_id":"fa06c9cfadfdacca77ddb72dd52dbf0fe4472b1a"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"df4c620684a2158fba202c59fd16b8a8ff74b09f","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        try:"},{"line_number":88,"context_line":"            self.client.update_port(port_id, port_req_body)"},{"line_number":89,"context_line":"        except neutron_client_exc.NeutronClientException:"},{"line_number":90,"context_line":"            LOG.exception(_(\"Failed to update Neutron port\"), port\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F9wCo%3D","line":90,"updated":"2014-02-04 11:55:51.000000000","message":"Do we need a mask here? e.g \"Failed to update Neutron %(port)s\" or something like that? Is there a magic in place that makes the kwargs to be passed to the formatter?\n\nAnyway, it looks obscure, I think it would be better to explicit evaluate the string here.","commit_id":"fa06c9cfadfdacca77ddb72dd52dbf0fe4472b1a"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"7c42feadc14a2e733aec756bce6c6a967b7a27c3","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        try:"},{"line_number":88,"context_line":"            self.client.update_port(port_id, port_req_body)"},{"line_number":89,"context_line":"        except neutron_client_exc.NeutronClientException:"},{"line_number":90,"context_line":"            LOG.exception(_(\"Failed to update Neutron port\"), port\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F9p4w%3D","line":90,"in_reply_to":"AAAAUH%2F9s94%3D","updated":"2014-02-05 10:14:08.000000000","message":"Ohh right, yea I can see it in Nova, it looks weird tho, obscure. Thanks for explaning","commit_id":"fa06c9cfadfdacca77ddb72dd52dbf0fe4472b1a"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"214bf6811a7fde56548d5d2e5f65958775776d7b","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        try:"},{"line_number":88,"context_line":"            self.client.update_port(port_id, port_req_body)"},{"line_number":89,"context_line":"        except neutron_client_exc.NeutronClientException:"},{"line_number":90,"context_line":"            LOG.exception(_(\"Failed to update Neutron port\"), port\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUH%2F9s94%3D","line":90,"in_reply_to":"AAAAUH%2F9wCo%3D","updated":"2014-02-04 20:34:28.000000000","message":"I looked into how other projects (eg, Nova) use LOG.exception and this is consistent. It dumps all parameters into the log message, along with the current exception context.\n\nFor reference to why this change, see lifeless\u0027 comments on rev2.","commit_id":"fa06c9cfadfdacca77ddb72dd52dbf0fe4472b1a"}],"ironic/drivers/modules/pxe.py":[{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"8ae569f475b0c99dec051972c110f2e1e2917ff8","unresolved":false,"context_lines":[{"line_number":487,"context_line":"        if len(failures) \u003d\u003d len(vifs):"},{"line_number":488,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort(_("},{"line_number":489,"context_line":"                \"Failed to set DHCP BOOT options for node %s.\")"},{"line_number":490,"context_line":"                % node.uuid)"},{"line_number":491,"context_line":"        else:"},{"line_number":492,"context_line":"            LOG.warning(_(\"Some errors were encountered when updating the \""},{"line_number":493,"context_line":"                          \"DHCP BOOT options for node %(node)s on the \""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F910w%3D","line":490,"updated":"2014-02-03 12:46:10.000000000","message":"It would be useful for troubleshooting to have the error message in the logs.","commit_id":"277793ee177dd603952f10070f935ec07cbe6ad7"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"69696c18de0a9295a5448754f4277a0bc1127d1b","unresolved":false,"context_lines":[{"line_number":487,"context_line":"        if len(failures) \u003d\u003d len(vifs):"},{"line_number":488,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort(_("},{"line_number":489,"context_line":"                \"Failed to set DHCP BOOT options for node %s.\")"},{"line_number":490,"context_line":"                % node.uuid)"},{"line_number":491,"context_line":"        else:"},{"line_number":492,"context_line":"            LOG.warning(_(\"Some errors were encountered when updating the \""},{"line_number":493,"context_line":"                          \"DHCP BOOT options for node %(node)s on the \""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F9ykg%3D","line":490,"in_reply_to":"AAAAUH%2F910w%3D","updated":"2014-02-03 22:39:35.000000000","message":"The exception will have been logged within api.update_port_dhcp_opts. We don\u0027t need to log the actual exception twice.","commit_id":"277793ee177dd603952f10070f935ec07cbe6ad7"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"57c304772595ecb6d7d82fa1cfc782486eed8e61","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            LOG.warning(_(\"Some errors were encountered when updating the \""},{"line_number":493,"context_line":"                          \"DHCP BOOT options for node %(node)s on the \""},{"line_number":494,"context_line":"                          \"following ports: %(ports)s.\")"},{"line_number":495,"context_line":"                          % {\u0027node\u0027: node.uuid, \u0027ports\u0027: failures})"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"def _create_pxe_config(task, node, pxe_info):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F93YE%3D","line":495,"updated":"2014-02-03 05:15:39.000000000","message":"Same here about logging.","commit_id":"277793ee177dd603952f10070f935ec07cbe6ad7"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"69696c18de0a9295a5448754f4277a0bc1127d1b","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            LOG.warning(_(\"Some errors were encountered when updating the \""},{"line_number":493,"context_line":"                          \"DHCP BOOT options for node %(node)s on the \""},{"line_number":494,"context_line":"                          \"following ports: %(ports)s.\")"},{"line_number":495,"context_line":"                          % {\u0027node\u0027: node.uuid, \u0027ports\u0027: failures})"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"def _create_pxe_config(task, node, pxe_info):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F9ykI%3D","line":495,"in_reply_to":"AAAAUH%2F93YE%3D","updated":"2014-02-03 22:39:35.000000000","message":"See comment above. All the actual exceptions for individual ports that failed should have been logged already. \n\nReaching this LOC means that the update was successful for some but not all of the ports registered to this node. At the moment, I believe this is not a situation where we need to abort the overall task, so this is trapping the exception and logging a warning.","commit_id":"277793ee177dd603952f10070f935ec07cbe6ad7"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"e3fa784cc7436466380fe3176c11013840576505","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    :param task: a TaskManager instance."},{"line_number":188,"context_line":"    :param node: the Node to act upon."},{"line_number":189,"context_line":"    :returns: A dict of the Node\u0027s port UUIDs and their associated VIFs"},{"line_number":190,"context_line":"    \"\"\""},{"line_number":191,"context_line":"    for r in task.resources:"},{"line_number":192,"context_line":"        if r.node.id \u003d\u003d node[\u0027id\u0027]:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FpME%3D","line":189,"updated":"2014-02-11 19:49:25.000000000","message":"According to PEP257 an empty line is required before \"\"\"","commit_id":"f2452e1dfc24adfecd2931ebe478b0a4fbc8395c"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"e3fa784cc7436466380fe3176c11013840576505","unresolved":false,"context_lines":[{"line_number":189,"context_line":"    :returns: A dict of the Node\u0027s port UUIDs and their associated VIFs"},{"line_number":190,"context_line":"    \"\"\""},{"line_number":191,"context_line":"    for r in task.resources:"},{"line_number":192,"context_line":"        if r.node.id \u003d\u003d node[\u0027id\u0027]:"},{"line_number":193,"context_line":"            result \u003d {}"},{"line_number":194,"context_line":"            for port in r.ports:"},{"line_number":195,"context_line":"                vif \u003d port.extra.get(\u0027vif_port_id\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FpNU%3D","line":192,"updated":"2014-02-11 19:49:25.000000000","message":"There are several patches that replace usage of __getitem__ with usage of attributes. So please use node.id instead of node[\u0027id\u0027]","commit_id":"f2452e1dfc24adfecd2931ebe478b0a4fbc8395c"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"e3fa784cc7436466380fe3176c11013840576505","unresolved":false,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"def _update_neutron(task, node):"},{"line_number":466,"context_line":"    \"\"\"Send or update the DHCP BOOT options to Neutron for this node.\"\"\""},{"line_number":467,"context_line":"    options \u003d _dhcp_options_for_instance()"},{"line_number":468,"context_line":"    vifs \u003d _get_node_vif_ids(task, node)"},{"line_number":469,"context_line":"    if not vifs:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FpIY%3D","line":466,"updated":"2014-02-11 19:49:25.000000000","message":"An empty line is required after the comment according to PEP257","commit_id":"f2452e1dfc24adfecd2931ebe478b0a4fbc8395c"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"e3fa784cc7436466380fe3176c11013840576505","unresolved":false,"context_lines":[{"line_number":468,"context_line":"    vifs \u003d _get_node_vif_ids(task, node)"},{"line_number":469,"context_line":"    if not vifs:"},{"line_number":470,"context_line":"        LOG.warning(_(\"No VIFs found for node %s when attempting to \""},{"line_number":471,"context_line":"                      \"update Neutron DHCP BOOT options.\") % node.uuid)"},{"line_number":472,"context_line":"        return"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    # TODO(deva): decouple instantiation of NeutronAPI from task.context."}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FpH4%3D","line":471,"updated":"2014-02-11 19:49:25.000000000","message":"Same as in the comment on L495","commit_id":"f2452e1dfc24adfecd2931ebe478b0a4fbc8395c"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"e3fa784cc7436466380fe3176c11013840576505","unresolved":false,"context_lines":[{"line_number":487,"context_line":"        if len(failures) \u003d\u003d len(vifs):"},{"line_number":488,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort(_("},{"line_number":489,"context_line":"                \"Failed to set DHCP BOOT options for node %s.\")"},{"line_number":490,"context_line":"                % node.uuid)"},{"line_number":491,"context_line":"        else:"},{"line_number":492,"context_line":"            LOG.warning(_(\"Some errors were encountered when updating the \""},{"line_number":493,"context_line":"                          \"DHCP BOOT options for node %(node)s on the \""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FpHw%3D","line":490,"updated":"2014-02-11 19:49:25.000000000","message":"Same as in the comment on L495","commit_id":"f2452e1dfc24adfecd2931ebe478b0a4fbc8395c"},{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"e3fa784cc7436466380fe3176c11013840576505","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            LOG.warning(_(\"Some errors were encountered when updating the \""},{"line_number":493,"context_line":"                          \"DHCP BOOT options for node %(node)s on the \""},{"line_number":494,"context_line":"                          \"following ports: %(ports)s.\")"},{"line_number":495,"context_line":"                          % {\u0027node\u0027: node.uuid, \u0027ports\u0027: failures})"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"def _create_pxe_config(task, node, pxe_info):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUX%2F%2FpIg%3D","line":495,"updated":"2014-02-11 19:49:25.000000000","message":"Could you please replace interpolation by passing arguments to the warning() method, i.e., LOG.warning(_(\"Message\"), node\u003dnode.uuid, ports\u003dfailures)","commit_id":"f2452e1dfc24adfecd2931ebe478b0a4fbc8395c"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"2cc645819528e2f33bfb4c217eb83538c8741549","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    \"\"\""},{"line_number":192,"context_line":"    for r in task.resources:"},{"line_number":193,"context_line":"        if r.node.id \u003d\u003d node.id:"},{"line_number":194,"context_line":"            result \u003d {}"},{"line_number":195,"context_line":"            for port in r.ports:"},{"line_number":196,"context_line":"                vif \u003d port.extra.get(\u0027vif_port_id\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2FoyM%3D","line":193,"updated":"2014-02-11 20:55:02.000000000","message":"This will fire only once in a loop...","commit_id":"22e050b4e7070cd3ad90b968c364d70d9ac666ae"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3819d2efae70e8a5e19e457e4eadfb74c47feff9","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                vif \u003d port.extra.get(\u0027vif_port_id\u0027)"},{"line_number":197,"context_line":"                if vif:"},{"line_number":198,"context_line":"                    result[port.uuid] \u003d vif"},{"line_number":199,"context_line":"        return result"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"def _get_pxe_mac_path(mac):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2FopA%3D","line":199,"updated":"2014-02-11 20:54:40.000000000","message":"Maybe this needs to be indented one more level, so that it happens if r.node.id \u003d\u003d node.id.","commit_id":"22e050b4e7070cd3ad90b968c364d70d9ac666ae"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"2cc645819528e2f33bfb4c217eb83538c8741549","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                vif \u003d port.extra.get(\u0027vif_port_id\u0027)"},{"line_number":197,"context_line":"                if vif:"},{"line_number":198,"context_line":"                    result[port.uuid] \u003d vif"},{"line_number":199,"context_line":"        return result"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"def _get_pxe_mac_path(mac):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2FowM%3D","line":199,"updated":"2014-02-11 20:55:02.000000000","message":"This return going to break loop on the first iteration - won\u0027t work for multiple resources\n\nHow about:\n\n# Finding node resource with given node_id\nnode_resource \u003d None\nfor resource in task.resources:\n    if resource.node.id \u003d\u003d node_id:\n        node_resource \u003d resource\n        break\nelse:\n    # If we hit this, then there\u0027s a bug somwhere\n    raise Exception(\"Task does not contain specified nodes\")\n\nport_vifs \u003d {}\nfor port in node_resource.ports:\n    vif \u003d port.extra.get(\u0027vif_port_id\u0027)\n    if vif:\n        port_vifs[port.uuid] \u003d vif\nreturn port_vifs\n\n(and change function arg from node to node_id)","commit_id":"22e050b4e7070cd3ad90b968c364d70d9ac666ae"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"7c618b8c519fe7eafe6e04e801ca397333e81e94","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                vif \u003d port.extra.get(\u0027vif_port_id\u0027)"},{"line_number":197,"context_line":"                if vif:"},{"line_number":198,"context_line":"                    result[port.uuid] \u003d vif"},{"line_number":199,"context_line":"        return result"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"def _get_pxe_mac_path(mac):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2FoY8%3D","line":199,"in_reply_to":"AAAAUX%2F%2FowM%3D","updated":"2014-02-11 21:32:13.000000000","message":"For now, let\u0027s assume we don\u0027t support multiple nodes per task. then it will be:\n\nport_vifs \u003d {}\nfor port in task.node.ports:\n    vif \u003d port.extra.get(\u0027vif_port_id\u0027)\n    if vif:\n        port_vifs[port.uuid] \u003d vif\nreturn port_vifs\n\nnode arg is not needed anymore","commit_id":"22e050b4e7070cd3ad90b968c364d70d9ac666ae"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"2cc645819528e2f33bfb4c217eb83538c8741549","unresolved":false,"context_lines":[{"line_number":483,"context_line":"        try:"},{"line_number":484,"context_line":"            api.update_port_dhcp_opts(vifs[port], options)"},{"line_number":485,"context_line":"        except exception.FailedToUpdateDHCPOptOnPort:"},{"line_number":486,"context_line":"            failures.append(port)"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    if len(failures):"},{"line_number":489,"context_line":"        if len(failures) \u003d\u003d len(vifs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2Foq4%3D","line":486,"updated":"2014-02-11 20:55:02.000000000","message":"for port_id, port_vif in vifs.iteritems():\n    try:\n        api.update_port_dhcp_opts(port_vif, options)\n    except ...:\n        failures.append(port)\n\nAnd maybe to store exception messages, or at least log them","commit_id":"22e050b4e7070cd3ad90b968c364d70d9ac666ae"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"2cc645819528e2f33bfb4c217eb83538c8741549","unresolved":false,"context_lines":[{"line_number":485,"context_line":"        except exception.FailedToUpdateDHCPOptOnPort:"},{"line_number":486,"context_line":"            failures.append(port)"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    if len(failures):"},{"line_number":489,"context_line":"        if len(failures) \u003d\u003d len(vifs):"},{"line_number":490,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort(_("},{"line_number":491,"context_line":"                \"Failed to set DHCP BOOT options for node %s.\") %"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2Foqk%3D","line":488,"updated":"2014-02-11 20:55:02.000000000","message":"if failures:","commit_id":"22e050b4e7070cd3ad90b968c364d70d9ac666ae"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"92e9a9d2534e30601be4f477126593a364c83d17","unresolved":false,"context_lines":[{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    if len(failures):"},{"line_number":489,"context_line":"        if len(failures) \u003d\u003d len(vifs):"},{"line_number":490,"context_line":"            raise exception.FailedToUpdateDHCPOptOnPort(_("},{"line_number":491,"context_line":"                \"Failed to set DHCP BOOT options for node %s.\") %"},{"line_number":492,"context_line":"                node.uuid)"},{"line_number":493,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2FoS0%3D","line":490,"updated":"2014-02-11 21:41:59.000000000","message":"How do you think maybe to replace this exception with some other? Something like FailedToUpdateDHCPOptOnNode. Because it\u0027s not for particular port, but for a whole node with all its ports","commit_id":"22e050b4e7070cd3ad90b968c364d70d9ac666ae"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"2cc645819528e2f33bfb4c217eb83538c8741549","unresolved":false,"context_lines":[{"line_number":493,"context_line":"        else:"},{"line_number":494,"context_line":"            LOG.warning(_(\"Some errors were encountered when updating the \""},{"line_number":495,"context_line":"                          \"DHCP BOOT options for node %(node)s on the \""},{"line_number":496,"context_line":"                          \"following ports: %(ports)s.\"),"},{"line_number":497,"context_line":"                          {\u0027node\u0027: node.uuid, \u0027ports\u0027: failures})"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2Fopc%3D","line":496,"updated":"2014-02-11 20:55:02.000000000","message":"Not very informative, but if we log FailedToUpdateDHCPOptOnPort exceptions above, it will be good enough :)","commit_id":"22e050b4e7070cd3ad90b968c364d70d9ac666ae"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"9c24f30ec45065cc814fafce475d54dcaa9f7aeb","unresolved":false,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"def _get_node_vif_ids(task):"},{"line_number":185,"context_line":"    \"\"\"Get all Neutron VIF ids for a node."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    :param task: a TaskManager instance."},{"line_number":188,"context_line":"    :param node: the Node to act upon."}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2FnTA%3D","line":185,"updated":"2014-02-12 00:34:04.000000000","message":"This method assumes that the task is not handling multi-nodes, so some NOTE or something is needed. Otherwise, it just looks wrong. (Not sure if something ought to be added about this in the commit msg too.)","commit_id":"981e9d3590724c572454392df7af8009cc7ef57f"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"9c24f30ec45065cc814fafce475d54dcaa9f7aeb","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    \"\"\"Get all Neutron VIF ids for a node."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    :param task: a TaskManager instance."},{"line_number":188,"context_line":"    :param node: the Node to act upon."},{"line_number":189,"context_line":"    :returns: A dict of the Node\u0027s port UUIDs and their associated VIFs"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAUX%2F%2FnYM%3D","line":188,"updated":"2014-02-12 00:34:04.000000000","message":"No need for this.","commit_id":"981e9d3590724c572454392df7af8009cc7ef57f"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"8a9e75d1a42f67ab77eaef843847622909f0998f","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    \"\"\""},{"line_number":192,"context_line":"    port_vifs \u003d {}"},{"line_number":193,"context_line":"    for port in task.resources[0].ports:"},{"line_number":194,"context_line":"        vif \u003d port.extra.get(\u0027vif_port_id\u0027)"},{"line_number":195,"context_line":"        if vif:"},{"line_number":196,"context_line":"            port_vifs[port.uuid] \u003d vif"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUX%2F%2FjtY%3D","line":193,"updated":"2014-02-12 11:22:27.000000000","message":"I\u0027d suggest to use taks.node property here (task.node.ports). There is a property - if task has only one resource we can use it. https://github.com/openstack/ironic/blob/master/ironic/conductor/task_manager.py#L145\n\nOnce we remove multi node support from task we\u0027ll remove task.resources list, replace task.node property with usual instance member, so the syntax will remain the same and this place will not require update\n\nMakes sense?","commit_id":"edbfc15b230d1e516de69b1b7980544aa5fc5dd3"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"8a9e75d1a42f67ab77eaef843847622909f0998f","unresolved":false,"context_lines":[{"line_number":479,"context_line":"    failures \u003d []"},{"line_number":480,"context_line":"    for port_id, port_vif in vifs.iteritems():"},{"line_number":481,"context_line":"        try:"},{"line_number":482,"context_line":"            api.update_port_dhcp_opts(vifs[port_id], options)"},{"line_number":483,"context_line":"        except exception.FailedToUpdateDHCPOptOnPort:"},{"line_number":484,"context_line":"            failures.append(port_id)"},{"line_number":485,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUX%2F%2FjrA%3D","line":482,"updated":"2014-02-12 11:22:27.000000000","message":"vifs[port_id] not needed, we can use directly port_vif","commit_id":"edbfc15b230d1e516de69b1b7980544aa5fc5dd3"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"8a9e75d1a42f67ab77eaef843847622909f0998f","unresolved":false,"context_lines":[{"line_number":481,"context_line":"        try:"},{"line_number":482,"context_line":"            api.update_port_dhcp_opts(vifs[port_id], options)"},{"line_number":483,"context_line":"        except exception.FailedToUpdateDHCPOptOnPort:"},{"line_number":484,"context_line":"            failures.append(port_id)"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    if failures:"},{"line_number":487,"context_line":"        if len(failures) \u003d\u003d len(vifs):"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUX%2F%2FjsE%3D","line":484,"updated":"2014-02-12 11:22:27.000000000","message":"Sorry, maybe I missed that and you guys already discussed, but there was a suggestion to log the exception here.. Maybe to apply it? It will be handy to check what exactly fails happened if the user find LOG.warning(_(\"Some errors were encountered...","commit_id":"edbfc15b230d1e516de69b1b7980544aa5fc5dd3"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"1dae194f245d507d18a09ecd86503aaef8827ffe","unresolved":false,"context_lines":[{"line_number":181,"context_line":"            return [p.address for p in r.ports]"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"def _get_node_vif_ids(task):"},{"line_number":185,"context_line":"    \"\"\"Get all Neutron VIF ids for a node."},{"line_number":186,"context_line":"       This function dose not handle multi node operations."},{"line_number":187,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2FhQU%3D","line":184,"updated":"2014-02-12 16:28:37.000000000","message":"this method is now inconsistent with the sibling method just above it. I would prefer both method\u0027s signatures be consistent. I\u0027ll post a patch to refactor _get_node_mac_addresses.","commit_id":"8df8e265d450135f93d6586f013f941e2e0f6644"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"1dae194f245d507d18a09ecd86503aaef8827ffe","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"def _get_node_vif_ids(task):"},{"line_number":185,"context_line":"    \"\"\"Get all Neutron VIF ids for a node."},{"line_number":186,"context_line":"       This function dose not handle multi node operations."},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    :param task: a TaskManager instance."},{"line_number":189,"context_line":"    :returns: A dict of the Node\u0027s port UUIDs and their associated VIFs"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2FhS8%3D","line":186,"updated":"2014-02-12 16:28:37.000000000","message":"nit: s/dose/does/","commit_id":"8df8e265d450135f93d6586f013f941e2e0f6644"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"1dae194f245d507d18a09ecd86503aaef8827ffe","unresolved":false,"context_lines":[{"line_number":481,"context_line":"        try:"},{"line_number":482,"context_line":"            api.update_port_dhcp_opts(port_vif, options)"},{"line_number":483,"context_line":"        except exception.FailedToUpdateDHCPOptOnPort as e:"},{"line_number":484,"context_line":"            LOG.warning(e.message, {\u0027port_id\u0027: port_id})"},{"line_number":485,"context_line":"            failures.append(port_id)"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    if failures:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUX%2F%2FhRo%3D","line":484,"updated":"2014-02-12 16:28:37.000000000","message":"@Max, I commented on this on a previous review -- we should not log here because it\u0027s already logged inside ironic.common.neutron\n\nhttp://git.openstack.org/cgit/openstack/ironic/tree/ironic/common/neutron.py#n90","commit_id":"8df8e265d450135f93d6586f013f941e2e0f6644"}],"ironic/tests/drivers/test_pxe.py":[{"author":{"_account_id":6623,"name":"Roman Prykhodchenko","email":"me@romcheg.me","username":"prykhodchenko"},"change_message_id":"8ae569f475b0c99dec051972c110f2e1e2917ff8","unresolved":false,"context_lines":[{"line_number":324,"context_line":"            result \u003d pxe._get_node_vif_ids(task, self.node)"},{"line_number":325,"context_line":"        self.assertEqual(expected, result)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        # test with two ports"},{"line_number":328,"context_line":"        port2 \u003d self._create_test_port(node_id\u003dself.node.id, id\u003d7,"},{"line_number":329,"context_line":"                                       address\u003d\u0027dd:ee:ff\u0027,"},{"line_number":330,"context_line":"                                       uuid\u003dutils.generate_uuid(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F910M%3D","line":327,"updated":"2014-02-03 12:46:10.000000000","message":"Shouldn\u0027t that be a separate test?","commit_id":"277793ee177dd603952f10070f935ec07cbe6ad7"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"69696c18de0a9295a5448754f4277a0bc1127d1b","unresolved":false,"context_lines":[{"line_number":324,"context_line":"            result \u003d pxe._get_node_vif_ids(task, self.node)"},{"line_number":325,"context_line":"        self.assertEqual(expected, result)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        # test with two ports"},{"line_number":328,"context_line":"        port2 \u003d self._create_test_port(node_id\u003dself.node.id, id\u003d7,"},{"line_number":329,"context_line":"                                       address\u003d\u0027dd:ee:ff\u0027,"},{"line_number":330,"context_line":"                                       uuid\u003dutils.generate_uuid(),"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F9yjQ%3D","line":327,"in_reply_to":"AAAAUH%2F910M%3D","updated":"2014-02-03 22:39:35.000000000","message":"Done","commit_id":"277793ee177dd603952f10070f935ec07cbe6ad7"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"deadf3d37ab8be878deba0b1e6f022f3c052675b","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        }"},{"line_number":208,"context_line":"        self.dbapi \u003d dbapi.get_instance()"},{"line_number":209,"context_line":"        self.node \u003d self._create_test_node(**n)"},{"line_number":210,"context_line":"        self.ports \u003d []"},{"line_number":211,"context_line":"        self.context \u003d context.get_admin_context()"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    def _create_test_node(self, **kwargs):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F9pzs%3D","line":210,"updated":"2014-02-05 10:32:52.000000000","message":"nit: this variable is not being used by the tests, it can be removed","commit_id":"2717534e9cda0d0f776f15ec6198927a77231218"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"924607ed894d50918b93a1627f328540c208e1e4","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        }"},{"line_number":208,"context_line":"        self.dbapi \u003d dbapi.get_instance()"},{"line_number":209,"context_line":"        self.node \u003d self._create_test_node(**n)"},{"line_number":210,"context_line":"        self.ports \u003d []"},{"line_number":211,"context_line":"        self.context \u003d context.get_admin_context()"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    def _create_test_node(self, **kwargs):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F9m%2F8%3D","line":210,"in_reply_to":"AAAAUH%2F9pzs%3D","updated":"2014-02-05 17:38:19.000000000","message":"Done","commit_id":"2717534e9cda0d0f776f15ec6198927a77231218"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"dc24d7940bf6153662699f23f8469759277240e4","unresolved":false,"context_lines":[{"line_number":369,"context_line":"                    pxe._update_neutron(task, self.node)"},{"line_number":370,"context_line":"                self.assertEqual(2, mock_updo.call_count)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    def test__update_neutron_failes(self):"},{"line_number":373,"context_line":"        # confirm update is called twice, both fail, and exception is raised"},{"line_number":374,"context_line":"        with mock.patch.object(pxe, \u0027_get_node_vif_ids\u0027) as mock_gnvi:"},{"line_number":375,"context_line":"            mock_gnvi.return_value \u003d {\u0027p1\u0027: \u0027v1\u0027, \u0027p2\u0027: \u0027v2\u0027}"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUX%2F%2FofQ%3D","line":372,"updated":"2014-02-11 21:14:57.000000000","message":"small nit: failes -\u003e fails.","commit_id":"22e050b4e7070cd3ad90b968c364d70d9ac666ae"}]}