)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ea14af4ede200aad6c1d555d8b2e8122fc4bab3b","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This commit is to support management Interface for HP ProLiant Servers"},{"line_number":10,"context_line":"using iLO client python library."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I2d909c682b0db6d6bc447c433e3448d31d3558a4"},{"line_number":13,"context_line":"Implements: blueprint ilo-management-interface"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"5a890539_ba3d16bd","line":11,"updated":"2014-11-26 14:51:33.000000000","message":"if you do another revision. \n\nsince it isn\u0027t mentioned in the spec, it might be useful to mention here that BIOS and SAFE aren\u0027t (weren\u0027t?) supported and were removed.","commit_id":"f71c36ec10a7ec244de4839f5768699d5eaf022a"}],"ironic/drivers/modules/ilo/common.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"21392c552d31bd37067e1b6b4486070fb44a786d","unresolved":false,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"def _update_ipmi_properties(task):"},{"line_number":194,"context_line":"        \"\"\"Update ipmi properties to node driver_info"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        :param task: a task from TaskManager."},{"line_number":197,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_8041bb0d","line":194,"updated":"2014-11-10 09:31:39.000000000","message":"1. this block is overindented; 2. if this function is intended to be used outside of this module, it should not start with _","commit_id":"58b94c976734cb23ce46b6c5805ef9740dae859d"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"643d49ab3939433614f4a39796b5fdc98d208db0","unresolved":false,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"def _update_ipmi_properties(task):"},{"line_number":194,"context_line":"        \"\"\"Update ipmi properties to node driver_info"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        :param task: a task from TaskManager."},{"line_number":197,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_7ed91df2","line":194,"in_reply_to":"5a890539_8041bb0d","updated":"2014-11-17 05:43:53.000000000","message":"Done","commit_id":"58b94c976734cb23ce46b6c5805ef9740dae859d"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c3c782fa3693366b9c30fbc7e63e1be21da58b14","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    # updating ipmi credentials"},{"line_number":202,"context_line":"    info[\u0027ipmi_address\u0027] \u003d info[\u0027ilo_address\u0027]"},{"line_number":203,"context_line":"    info[\u0027ipmi_username\u0027] \u003d info[\u0027ilo_username\u0027]"},{"line_number":204,"context_line":"    info[\u0027ipmi_password\u0027] \u003d info[\u0027ilo_password\u0027]"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    if \u0027console_port\u0027 in info:"},{"line_number":207,"context_line":"        info[\u0027ipmi_terminal_port\u0027] \u003d info[\u0027console_port\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_c2595936","line":204,"updated":"2014-11-25 02:58:44.000000000","message":"this assumes that ilo_address, _username, and _password exist. What if they don\u0027t?","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"5484c4c9a509097d26c1d733c615416f5ee3e702","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    # updating ipmi credentials"},{"line_number":202,"context_line":"    info[\u0027ipmi_address\u0027] \u003d info[\u0027ilo_address\u0027]"},{"line_number":203,"context_line":"    info[\u0027ipmi_username\u0027] \u003d info[\u0027ilo_username\u0027]"},{"line_number":204,"context_line":"    info[\u0027ipmi_password\u0027] \u003d info[\u0027ilo_password\u0027]"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    if \u0027console_port\u0027 in info:"},{"line_number":207,"context_line":"        info[\u0027ipmi_terminal_port\u0027] \u003d info[\u0027console_port\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_187e5b19","line":204,"in_reply_to":"5a890539_6c6085cf","updated":"2014-11-25 16:05:24.000000000","message":"ruby, where ever possible we may want to remove the dependency of ipmi and so currently being used only by get_sensors_data which will also be replaced eventually (https://review.openstack.org/#/c/127378/).\n\nBut if at all we need to use, replacing with get() sounds reasonable to me.\nwill update accordingly.","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"fbd79a160e7cd1fe3e7fe1698c05bf4a4474aa0c","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    # updating ipmi credentials"},{"line_number":202,"context_line":"    info[\u0027ipmi_address\u0027] \u003d info[\u0027ilo_address\u0027]"},{"line_number":203,"context_line":"    info[\u0027ipmi_username\u0027] \u003d info[\u0027ilo_username\u0027]"},{"line_number":204,"context_line":"    info[\u0027ipmi_password\u0027] \u003d info[\u0027ilo_password\u0027]"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    if \u0027console_port\u0027 in info:"},{"line_number":207,"context_line":"        info[\u0027ipmi_terminal_port\u0027] \u003d info[\u0027console_port\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_6c6085cf","line":204,"in_reply_to":"5a890539_b57ac8b7","updated":"2014-11-25 14:43:10.000000000","message":"\u0027should be taken care of\u0027 is the problem. For now it is ok, but in the future, someone may invoke it w/o any validation happening beforehand. Maybe a comment or add something in the docstring, unless you want to change this to ..\u003d info.get().","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"e992c757b427e2d9a62b73797e1ab06646448ecf","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    # updating ipmi credentials"},{"line_number":202,"context_line":"    info[\u0027ipmi_address\u0027] \u003d info[\u0027ilo_address\u0027]"},{"line_number":203,"context_line":"    info[\u0027ipmi_username\u0027] \u003d info[\u0027ilo_username\u0027]"},{"line_number":204,"context_line":"    info[\u0027ipmi_password\u0027] \u003d info[\u0027ilo_password\u0027]"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    if \u0027console_port\u0027 in info:"},{"line_number":207,"context_line":"        info[\u0027ipmi_terminal_port\u0027] \u003d info[\u0027console_port\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_b57ac8b7","line":204,"in_reply_to":"5a890539_c2595936","updated":"2014-11-25 06:41:10.000000000","message":"In conductor itself we do the validate of parameters, so it should be taken care by the time function is called.","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"}],"ironic/drivers/modules/ilo/management.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"21392c552d31bd37067e1b6b4486070fb44a786d","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        :param task: a task from TaskManager."},{"line_number":53,"context_line":"        :raises: InvalidParameterValue if required iLO parameters"},{"line_number":54,"context_line":"            are missing."},{"line_number":55,"context_line":"        :raises: MissingParameterValue if a required parameter is missing."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_2069e78b","line":54,"updated":"2014-11-10 09:31:39.000000000","message":"s/missing/not valid/","commit_id":"58b94c976734cb23ce46b6c5805ef9740dae859d"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"643d49ab3939433614f4a39796b5fdc98d208db0","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        :param task: a task from TaskManager."},{"line_number":53,"context_line":"        :raises: InvalidParameterValue if required iLO parameters"},{"line_number":54,"context_line":"            are missing."},{"line_number":55,"context_line":"        :raises: MissingParameterValue if a required parameter is missing."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_1e5ad982","line":54,"in_reply_to":"5a890539_2069e78b","updated":"2014-11-17 05:43:53.000000000","message":"Done","commit_id":"58b94c976734cb23ce46b6c5805ef9740dae859d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"21392c552d31bd37067e1b6b4486070fb44a786d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            if not persistent:"},{"line_number":130,"context_line":"                ilo_object.set_one_time_boot(boot_device)"},{"line_number":131,"context_line":"            else:"},{"line_number":132,"context_line":"                ilo_object.update_persistent_boot([boot_device])"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        except KeyError:"},{"line_number":135,"context_line":"            raise exception.InvalidParameterValue(_("}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_60a68fc3","line":132,"updated":"2014-11-10 09:31:39.000000000","message":"please split the try..except block so that only expected exceptions are catched like\n\n try:\n   boot_device \u003d BOOT_DEVICE_MAPPING_TO_ILO[device]\n except KeyError:\n   ...\n try:\n   ilo_object \u003d ilo_common.get_ilo_object(task.node)\n   ...\n except ilo_client.IloError as ilo_exception:\n    ...","commit_id":"58b94c976734cb23ce46b6c5805ef9740dae859d"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"643d49ab3939433614f4a39796b5fdc98d208db0","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            if not persistent:"},{"line_number":130,"context_line":"                ilo_object.set_one_time_boot(boot_device)"},{"line_number":131,"context_line":"            else:"},{"line_number":132,"context_line":"                ilo_object.update_persistent_boot([boot_device])"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        except KeyError:"},{"line_number":135,"context_line":"            raise exception.InvalidParameterValue(_("}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_7e52fd67","line":132,"in_reply_to":"5a890539_60a68fc3","updated":"2014-11-17 05:43:53.000000000","message":"Done","commit_id":"58b94c976734cb23ce46b6c5805ef9740dae859d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"21392c552d31bd37067e1b6b4486070fb44a786d","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common._update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return (super(IloManagement, self).get_sensors_data(task))"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_c094a346","line":160,"updated":"2014-11-10 09:31:39.000000000","message":"nit: redundant brackets around the whole expression","commit_id":"58b94c976734cb23ce46b6c5805ef9740dae859d"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"643d49ab3939433614f4a39796b5fdc98d208db0","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common._update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return (super(IloManagement, self).get_sensors_data(task))"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_deca111a","line":160,"in_reply_to":"5a890539_c094a346","updated":"2014-11-17 05:43:53.000000000","message":"Done","commit_id":"58b94c976734cb23ce46b6c5805ef9740dae859d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"6476e73179d29005d9b9485ce048386df3b25ea5","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                               boot_devices.CDROM: \u0027CDROM\u0027"},{"line_number":36,"context_line":"                             }"},{"line_number":37,"context_line":"BOOT_DEVICE_ILO_TO_GENERIC \u003d dict((v, k)"},{"line_number":38,"context_line":"                             for (k, v) in BOOT_DEVICE_MAPPING_TO_ILO.items())"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"class IloManagement(base.ManagementInterface):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_a7e7cdaa","line":38,"updated":"2014-11-18 10:32:48.000000000","message":"nit: py26 is gone, now we can use dict comprehension \\o/\n\n {v: k for k, v in BOOT_DEVICE_MAPPING_TO_ILO.items()}","commit_id":"f2e81f601219e9ab258e2077131d6d14a21bdc6f"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"a2f361900c73f0a8e9cb726ed2ac6d0805c2a020","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                               boot_devices.CDROM: \u0027CDROM\u0027"},{"line_number":36,"context_line":"                             }"},{"line_number":37,"context_line":"BOOT_DEVICE_ILO_TO_GENERIC \u003d dict((v, k)"},{"line_number":38,"context_line":"                             for (k, v) in BOOT_DEVICE_MAPPING_TO_ILO.items())"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"class IloManagement(base.ManagementInterface):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_eaaa55d1","line":38,"in_reply_to":"5a890539_a7e7cdaa","updated":"2014-11-19 04:36:18.000000000","message":"Done","commit_id":"f2e81f601219e9ab258e2077131d6d14a21bdc6f"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"19eb91720c9dd2b0cc866c1c423c28b72fb20c41","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common.update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return super(IloManagement, self).get_sensors_data(task)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_c207501c","line":160,"updated":"2014-11-18 20:22:24.000000000","message":"The super class here is ironic.drivers.base.ManagementInterface, where this operation is a no-op. Seems like we should be using IPMI to get sensor data, so maybe we should subclass from some IPMI driver? Which one? Should this be configurable by the user?","commit_id":"f2e81f601219e9ab258e2077131d6d14a21bdc6f"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"d513db8068e8800bc32b4f52fcb69ebf1d7623d7","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common.update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return super(IloManagement, self).get_sensors_data(task)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_d63555dc","line":160,"in_reply_to":"5a890539_c207501c","updated":"2014-11-18 20:24:29.000000000","message":"And with a look at the tests... I see this isn\u0027t tested at all. I\u0027d love to test that ironic.drivers.ipmitool.IPMIManagement.get_sensors_data is called (or ipminative).","commit_id":"f2e81f601219e9ab258e2077131d6d14a21bdc6f"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"a2f361900c73f0a8e9cb726ed2ac6d0805c2a020","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common.update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return super(IloManagement, self).get_sensors_data(task)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_8f11e35f","line":160,"in_reply_to":"5a890539_c207501c","updated":"2014-11-19 04:36:18.000000000","message":"right. code is carry over from last release , so haven\u0027t looked at this function. thanks for pointing it out. without sub classing it can be called directly with ipmitool right?","commit_id":"f2e81f601219e9ab258e2077131d6d14a21bdc6f"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"a2f361900c73f0a8e9cb726ed2ac6d0805c2a020","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common.update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return super(IloManagement, self).get_sensors_data(task)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_caad99ec","line":160,"in_reply_to":"5a890539_d63555dc","updated":"2014-11-19 04:36:18.000000000","message":"will update tests","commit_id":"f2e81f601219e9ab258e2077131d6d14a21bdc6f"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"ae0bc6e4ed3affa632bc39aa7063da680157f3ef","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common.update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return ipmitool.IPMIManagement.get_sensors_data(task)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_a054e021","line":160,"updated":"2014-11-19 14:38:47.000000000","message":"Yeah, I don\u0027t think this will work.\n\n1) You need an instance of IPMIManagement, not just a reference to the class.\n2) What if users want to use ipminative?\n3) We aren\u0027t checking here if ipmitool is available on the system, what options it has, etc. Things the ipmitool driver does on startup.","commit_id":"d987438ee350fc0714676d8642bb2c54a78998eb"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"1cb53648f1d4919245f7c717094badebc2681820","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common.update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return ipmitool.IPMIManagement.get_sensors_data(task)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_72cab76f","line":160,"in_reply_to":"5a890539_2b7371b2","updated":"2014-11-20 03:48:41.000000000","message":"iLO does support sensor data. It is being proposed as part of https://review.openstack.org/#/c/127378/ .\n\nIn the meantime, would want the same functionality as per juno release for consistency, which uses ipmitool to get the sensor data.\n\nhow about creating a instance of ipmitool.IPMIManagement and call the get_sensors_data() , this would automatically call __init__ which have the driver validataion?\n\nlet me know your thoughts.","commit_id":"d987438ee350fc0714676d8642bb2c54a78998eb"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c641bc720b663de7beb00af2a7c5c195f5c5f1ff","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common.update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return ipmitool.IPMIManagement.get_sensors_data(task)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_3471f644","line":160,"in_reply_to":"5a890539_72cab76f","updated":"2014-11-20 09:45:48.000000000","message":"Hi Anusha,\n\nOh thanks I understand it\u0027s a temp thing, thanks for pointing that out. Right yeah, I think that instantiating it to run the check whether ipmitool is available or not seems fair enough. I\u0027m good with it!","commit_id":"d987438ee350fc0714676d8642bb2c54a78998eb"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"1edf661eed168221299ab594f26a9d0277b4dff8","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common.update_ipmi_properties(task)"},{"line_number":160,"context_line":"        return ipmitool.IPMIManagement.get_sensors_data(task)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_2b7371b2","line":160,"in_reply_to":"5a890539_a054e021","updated":"2014-11-19 14:48:51.000000000","message":"+1 \n\n1) Yeah, I think it may fail as-is because the \u0027self\u0027 parameter won\u0027t be populated. Unles IPMIManagement has get_sensors_data as a @classmethod (but it shouldn\u0027t)\n\n2) and 3) yeah.\n\nThe iLO library doesn\u0027t support getting the sensor data?\n\nIn this case, we may want to think about refactoring part of ipmi* code and adding to a common place where it can be shared between both drivers. That would include the verifications such as whether ipmitool/pyghmi is available in the system, _exec_ipmitool() (it does have a interval configuration) and the method to get sensor itself","commit_id":"d987438ee350fc0714676d8642bb2c54a78998eb"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"cafb904c5a78d365725cec01458f247e896e3620","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        :returns: returns a dict of sensor data group by sensor type."},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        ilo_common.parse_driver_info(task.node)"},{"line_number":159,"context_line":"        ilo_common.update_ipmi_properties(task)"},{"line_number":160,"context_line":"        ipmi_management \u003d ipmitool.IPMIManagement()"},{"line_number":161,"context_line":"        return ipmi_management.get_sensors_data(task)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_50838e55","line":158,"updated":"2014-11-24 11:17:53.000000000","message":"This line not needed because manager\u0027s code do validate https://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L1382","commit_id":"a506a0d2f87117729ce788f6ff3b05b879aa7765"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c3c782fa3693366b9c30fbc7e63e1be21da58b14","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"BOOT_DEVICE_MAPPING_TO_ILO \u003d {boot_devices.PXE: \u0027NETWORK\u0027,"},{"line_number":34,"context_line":"                               boot_devices.DISK: \u0027HDD\u0027,"},{"line_number":35,"context_line":"                               boot_devices.CDROM: \u0027CDROM\u0027"},{"line_number":36,"context_line":"                             }"},{"line_number":37,"context_line":"BOOT_DEVICE_ILO_TO_GENERIC \u003d {v: k"},{"line_number":38,"context_line":"                              for k, v in BOOT_DEVICE_MAPPING_TO_ILO.items()}"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_a76a9fea","line":35,"updated":"2014-11-25 02:58:44.000000000","message":"what happened to BIOS and SAFE?","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"e992c757b427e2d9a62b73797e1ab06646448ecf","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"BOOT_DEVICE_MAPPING_TO_ILO \u003d {boot_devices.PXE: \u0027NETWORK\u0027,"},{"line_number":34,"context_line":"                               boot_devices.DISK: \u0027HDD\u0027,"},{"line_number":35,"context_line":"                               boot_devices.CDROM: \u0027CDROM\u0027"},{"line_number":36,"context_line":"                             }"},{"line_number":37,"context_line":"BOOT_DEVICE_ILO_TO_GENERIC \u003d {v: k"},{"line_number":38,"context_line":"                              for k, v in BOOT_DEVICE_MAPPING_TO_ILO.items()}"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_f54ff08f","line":35,"in_reply_to":"5a890539_a76a9fea","updated":"2014-11-25 06:41:10.000000000","message":"currently, we don\u0027t support them through iLO.","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"5484c4c9a509097d26c1d733c615416f5ee3e702","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"BOOT_DEVICE_MAPPING_TO_ILO \u003d {boot_devices.PXE: \u0027NETWORK\u0027,"},{"line_number":34,"context_line":"                               boot_devices.DISK: \u0027HDD\u0027,"},{"line_number":35,"context_line":"                               boot_devices.CDROM: \u0027CDROM\u0027"},{"line_number":36,"context_line":"                             }"},{"line_number":37,"context_line":"BOOT_DEVICE_ILO_TO_GENERIC \u003d {v: k"},{"line_number":38,"context_line":"                              for k, v in BOOT_DEVICE_MAPPING_TO_ILO.items()}"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_dde685af","line":35,"in_reply_to":"5a890539_cc87b95f","updated":"2014-11-25 16:05:24.000000000","message":"right , previously we directly used ipmi interface for Management operations, so we had the mapping. will update the docstring","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"fbd79a160e7cd1fe3e7fe1698c05bf4a4474aa0c","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"BOOT_DEVICE_MAPPING_TO_ILO \u003d {boot_devices.PXE: \u0027NETWORK\u0027,"},{"line_number":34,"context_line":"                               boot_devices.DISK: \u0027HDD\u0027,"},{"line_number":35,"context_line":"                               boot_devices.CDROM: \u0027CDROM\u0027"},{"line_number":36,"context_line":"                             }"},{"line_number":37,"context_line":"BOOT_DEVICE_ILO_TO_GENERIC \u003d {v: k"},{"line_number":38,"context_line":"                              for k, v in BOOT_DEVICE_MAPPING_TO_ILO.items()}"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_cc87b95f","line":35,"in_reply_to":"5a890539_f54ff08f","updated":"2014-11-25 14:43:10.000000000","message":"but you had them in your previous version. Maybe add a comment in your commit about removing them?","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c3c782fa3693366b9c30fbc7e63e1be21da58b14","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    def get_boot_device(self, task):"},{"line_number":70,"context_line":"        \"\"\"Get the current boot device for a node."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        Provides the current boot device of the node."},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        :param task: a task from TaskManager."},{"line_number":75,"context_line":"        :raises: MissingParameterValue if a required iLO parameter is missing."}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_2755cf1d","line":72,"updated":"2014-11-25 02:58:44.000000000","message":"nit. I find \u0027Provides\u0027 a bit odd. It makes me think that it is setting (providing) the boot device, not getting it. How about \u0027Returns\u0027 instead?","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"e992c757b427e2d9a62b73797e1ab06646448ecf","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    def get_boot_device(self, task):"},{"line_number":70,"context_line":"        \"\"\"Get the current boot device for a node."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        Provides the current boot device of the node."},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        :param task: a task from TaskManager."},{"line_number":75,"context_line":"        :raises: MissingParameterValue if a required iLO parameter is missing."}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_5560c400","line":72,"in_reply_to":"5a890539_2755cf1d","updated":"2014-11-25 06:41:10.000000000","message":"will update it.","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c3c782fa3693366b9c30fbc7e63e1be21da58b14","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                                              error\u003dilo_exception)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        boot_device \u003d BOOT_DEVICE_ILO_TO_GENERIC.get(next_boot, None)"},{"line_number":105,"context_line":"        return {\u0027boot_device\u0027: boot_device, \u0027persistent\u0027: persistent}"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @task_manager.require_exclusive_lock"},{"line_number":108,"context_line":"    def set_boot_device(self, task, device, persistent\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_6296e5f1","line":105,"updated":"2014-11-25 02:58:44.000000000","message":"the docstring sez that persistent is None if it is unknown, but I don\u0027t see where it ever gets set to None here.","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"16627b32b6843ad4f65dfaa5533feef21983b741","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                                              error\u003dilo_exception)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        boot_device \u003d BOOT_DEVICE_ILO_TO_GENERIC.get(next_boot, None)"},{"line_number":105,"context_line":"        return {\u0027boot_device\u0027: boot_device, \u0027persistent\u0027: persistent}"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @task_manager.require_exclusive_lock"},{"line_number":108,"context_line":"    def set_boot_device(self, task, device, persistent\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_bb360e4f","line":105,"in_reply_to":"5a890539_56650fdf","updated":"2014-11-26 04:59:18.000000000","message":"sounds good :) will update it","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"e992c757b427e2d9a62b73797e1ab06646448ecf","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                                              error\u003dilo_exception)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        boot_device \u003d BOOT_DEVICE_ILO_TO_GENERIC.get(next_boot, None)"},{"line_number":105,"context_line":"        return {\u0027boot_device\u0027: boot_device, \u0027persistent\u0027: persistent}"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @task_manager.require_exclusive_lock"},{"line_number":108,"context_line":"    def set_boot_device(self, task, device, persistent\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_d533d411","line":105,"in_reply_to":"5a890539_6296e5f1","updated":"2014-11-25 06:41:10.000000000","message":"boot_device \u003d BOOT_DEVICE_ILO_TO_GENERIC.get(next_boot, None) , this call actually sets it None if it doesn\u0027t find th match.","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"5484c4c9a509097d26c1d733c615416f5ee3e702","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                                              error\u003dilo_exception)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        boot_device \u003d BOOT_DEVICE_ILO_TO_GENERIC.get(next_boot, None)"},{"line_number":105,"context_line":"        return {\u0027boot_device\u0027: boot_device, \u0027persistent\u0027: persistent}"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @task_manager.require_exclusive_lock"},{"line_number":108,"context_line":"    def set_boot_device(self, task, device, persistent\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_fdac21a1","line":105,"in_reply_to":"5a890539_aceb4d11","updated":"2014-11-25 16:05:24.000000000","message":"right. doesn\u0027t set to None. will be either True or False. will update the docstring","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"fbd79a160e7cd1fe3e7fe1698c05bf4a4474aa0c","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                                              error\u003dilo_exception)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        boot_device \u003d BOOT_DEVICE_ILO_TO_GENERIC.get(next_boot, None)"},{"line_number":105,"context_line":"        return {\u0027boot_device\u0027: boot_device, \u0027persistent\u0027: persistent}"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @task_manager.require_exclusive_lock"},{"line_number":108,"context_line":"    def set_boot_device(self, task, device, persistent\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_aceb4d11","line":105,"in_reply_to":"5a890539_d533d411","updated":"2014-11-25 14:43:10.000000000","message":"That call sets boot_device \u003d None. What sets persistent \u003d None?","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f27690dac3950c82668441a520bc50905ee337da","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                                              error\u003dilo_exception)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        boot_device \u003d BOOT_DEVICE_ILO_TO_GENERIC.get(next_boot, None)"},{"line_number":105,"context_line":"        return {\u0027boot_device\u0027: boot_device, \u0027persistent\u0027: persistent}"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @task_manager.require_exclusive_lock"},{"line_number":108,"context_line":"    def set_boot_device(self, task, device, persistent\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_56650fdf","line":105,"in_reply_to":"5a890539_fdac21a1","updated":"2014-11-25 18:32:39.000000000","message":"Would it make sense for persistent to be None if boot_device \u003d\u003d None?","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"}],"ironic/drivers/pxe.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c3c782fa3693366b9c30fbc7e63e1be21da58b14","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    This driver implements the `core` functionality using"},{"line_number":150,"context_line":"    :class:`ironic.drivers.modules.ilo.power.IloPower` for power management"},{"line_number":151,"context_line":"    :class:`ironic.drivers.modules.ilo.deploy.IloPXEDeploy`"},{"line_number":152,"context_line":"    :class:`ironic.drivers.modules.ilo.deploy.IloManagement` for image"},{"line_number":153,"context_line":"    deployment."},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_879be316","line":152,"updated":"2014-11-25 02:58:44.000000000","message":"you might want to update this. Or delete it since it isn\u0027t core functionality, it\u0027s standard functionality.","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"e992c757b427e2d9a62b73797e1ab06646448ecf","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    This driver implements the `core` functionality using"},{"line_number":150,"context_line":"    :class:`ironic.drivers.modules.ilo.power.IloPower` for power management"},{"line_number":151,"context_line":"    :class:`ironic.drivers.modules.ilo.deploy.IloPXEDeploy`"},{"line_number":152,"context_line":"    :class:`ironic.drivers.modules.ilo.deploy.IloManagement` for image"},{"line_number":153,"context_line":"    deployment."},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_75094030","line":152,"in_reply_to":"5a890539_879be316","updated":"2014-11-25 06:41:10.000000000","message":"Done","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"}],"ironic/tests/drivers/ilo/test_common.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"c3c782fa3693366b9c30fbc7e63e1be21da58b14","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            ilo_common.update_ipmi_properties(task)"},{"line_number":167,"context_line":"            actual_info \u003d task.node.driver_info"},{"line_number":168,"context_line":"            expected_info \u003d dict(INFO_DICT, **ipmi_info)"},{"line_number":169,"context_line":"            self.assertEqual(expected_info, actual_info)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    def test__get_floppy_image_name(self):"},{"line_number":172,"context_line":"        image_name_expected \u003d \u0027image-\u0027 + self.node.uuid"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_a2c7ed96","line":169,"updated":"2014-11-25 02:58:44.000000000","message":"this test works but did we really set the test data for ipmi to be similar to ilo? if someone changes one of the test data, this will fail and honestly, it really should only rely on the ilo test data!\n\nNot something to change in this patch, I\u0027m just commenting.","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"},{"author":{"_account_id":11278,"name":"Anusha Ramineni","email":"anusha.ramineni@india.nec.com","username":"anusha08"},"change_message_id":"e992c757b427e2d9a62b73797e1ab06646448ecf","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            ilo_common.update_ipmi_properties(task)"},{"line_number":167,"context_line":"            actual_info \u003d task.node.driver_info"},{"line_number":168,"context_line":"            expected_info \u003d dict(INFO_DICT, **ipmi_info)"},{"line_number":169,"context_line":"            self.assertEqual(expected_info, actual_info)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    def test__get_floppy_image_name(self):"},{"line_number":172,"context_line":"        image_name_expected \u003d \u0027image-\u0027 + self.node.uuid"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_18567106","line":169,"in_reply_to":"5a890539_a2c7ed96","updated":"2014-11-25 06:41:10.000000000","message":"yes, it fails if someone changes the test_impi_info. Included it to just want to reuse the existing data , instead of writing a new dict of ipmi_info.\n\nI will replace with the ipmi dict defined here.","commit_id":"e7064ff314d945f614d30a292ebe75e2ba652d88"}]}