)]}'
{"ironic_python_agent/hardware.py":[{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"30bf7b6dd0ed4a6b86a34f19054407694c1d1248","unresolved":false,"context_lines":[{"line_number":906,"context_line":"        ]"},{"line_number":907,"context_line":""},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"class X710HardwareManager(GenericHardwareManager):"},{"line_number":910,"context_line":"    HARDWARE_MANAGER_NAME \u003d \u0027X710HardwareManager\u0027"},{"line_number":911,"context_line":"    HARDWARE_MANAGER_VERSION \u003d \u00271.0\u0027"},{"line_number":912,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_416cc407","line":909,"updated":"2016-11-03 15:25:21.000000000","message":"I am  not sure if specific hardware code should be located in this file. It\u0027s already quite big. It might make sense for future to create a directory for specific HW managers, and store one file per one HW manager class.","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"55786e6c65b5ced1095bd90c7ac9b3955b5308f9","unresolved":false,"context_lines":[{"line_number":906,"context_line":"        ]"},{"line_number":907,"context_line":""},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"class X710HardwareManager(GenericHardwareManager):"},{"line_number":910,"context_line":"    HARDWARE_MANAGER_NAME \u003d \u0027X710HardwareManager\u0027"},{"line_number":911,"context_line":"    HARDWARE_MANAGER_VERSION \u003d \u00271.0\u0027"},{"line_number":912,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_9ea9ab27","line":909,"in_reply_to":"ba5da102_0d1021a8","updated":"2016-11-04 15:22:01.000000000","message":"+1 please put this in a different module","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"cc9cbb507b27e5084803ad1713d653e0fda65406","unresolved":false,"context_lines":[{"line_number":906,"context_line":"        ]"},{"line_number":907,"context_line":""},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"class X710HardwareManager(GenericHardwareManager):"},{"line_number":910,"context_line":"    HARDWARE_MANAGER_NAME \u003d \u0027X710HardwareManager\u0027"},{"line_number":911,"context_line":"    HARDWARE_MANAGER_VERSION \u003d \u00271.0\u0027"},{"line_number":912,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_0d1021a8","line":909,"in_reply_to":"ba5da102_416cc407","updated":"2016-11-04 11:45:33.000000000","message":"I have spoken with Dmitry, he approves the creation of new directory for custom hardware managers, so I will extract it to a separate file.","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"6ea05661dbc43876161fa3b603861a3dbb1093f8","unresolved":false,"context_lines":[{"line_number":906,"context_line":"        ]"},{"line_number":907,"context_line":""},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"class X710HardwareManager(GenericHardwareManager):"},{"line_number":910,"context_line":"    HARDWARE_MANAGER_NAME \u003d \u0027X710HardwareManager\u0027"},{"line_number":911,"context_line":"    HARDWARE_MANAGER_VERSION \u003d \u00271.0\u0027"},{"line_number":912,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_aa3626e5","line":909,"in_reply_to":"ba5da102_9ea9ab27","updated":"2016-11-04 17:59:37.000000000","message":"This should subclass off HardwareManager.","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"55786e6c65b5ced1095bd90c7ac9b3955b5308f9","unresolved":false,"context_lines":[{"line_number":926,"context_line":"    def evaluate_hardware_support(self):"},{"line_number":927,"context_line":"        if self._detect_x710_card():"},{"line_number":928,"context_line":"            LOG.debug(\u0027Found X710 network card, returning SERVICE_PROVIDER\u0027)"},{"line_number":929,"context_line":"            return HardwareSupport.SERVICE_PROVIDER"},{"line_number":930,"context_line":"        else:"},{"line_number":931,"context_line":"            LOG.debug(\u0027No X710 network card found, returning NONE\u0027)"},{"line_number":932,"context_line":"            return HardwareSupport.NONE"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_5e88f3c6","line":929,"updated":"2016-11-04 15:22:01.000000000","message":"This should be MAINLINE. SERVICE_PROVIDER and above are reserved for out-of-tree hardware managers.","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"55786e6c65b5ced1095bd90c7ac9b3955b5308f9","unresolved":false,"context_lines":[{"line_number":940,"context_line":"                \u0027reboot_requested\u0027: False,  # TODO(sborkows): set/unset flag"},{"line_number":941,"context_line":"                \u0027abortable\u0027: True,  # TODO(sborkows): set/unset flag"},{"line_number":942,"context_line":"            },"},{"line_number":943,"context_line":"            {"},{"line_number":944,"context_line":"                \u0027step\u0027: \u0027erase_devices\u0027,"},{"line_number":945,"context_line":"                \u0027priority\u0027: 10,"},{"line_number":946,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_3e489f87","line":943,"updated":"2016-11-04 15:22:01.000000000","message":"Please do not re-define clean steps from other managers. You only need to define steps relevant for this manager.","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"35b434faa3b6ae1b7c1489d631deec0c4f25e2fd","unresolved":false,"context_lines":[{"line_number":940,"context_line":"                \u0027reboot_requested\u0027: False,  # TODO(sborkows): set/unset flag"},{"line_number":941,"context_line":"                \u0027abortable\u0027: True,  # TODO(sborkows): set/unset flag"},{"line_number":942,"context_line":"            },"},{"line_number":943,"context_line":"            {"},{"line_number":944,"context_line":"                \u0027step\u0027: \u0027erase_devices\u0027,"},{"line_number":945,"context_line":"                \u0027priority\u0027: 10,"},{"line_number":946,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_54a44f07","line":943,"in_reply_to":"ba5da102_3e489f87","updated":"2016-11-04 16:58:53.000000000","message":"I re-defined those steps, because I wanted to inherit this manager from GenericManager, in order to keep some methods from the second one. Is this a right approach, or should I avoid this?","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"6ea05661dbc43876161fa3b603861a3dbb1093f8","unresolved":false,"context_lines":[{"line_number":940,"context_line":"                \u0027reboot_requested\u0027: False,  # TODO(sborkows): set/unset flag"},{"line_number":941,"context_line":"                \u0027abortable\u0027: True,  # TODO(sborkows): set/unset flag"},{"line_number":942,"context_line":"            },"},{"line_number":943,"context_line":"            {"},{"line_number":944,"context_line":"                \u0027step\u0027: \u0027erase_devices\u0027,"},{"line_number":945,"context_line":"                \u0027priority\u0027: 10,"},{"line_number":946,"context_line":"                \u0027interface\u0027: \u0027deploy\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_8a67a2e9","line":943,"in_reply_to":"ba5da102_54a44f07","updated":"2016-11-04 17:59:37.000000000","message":"This is not the right approach.\n\nThe correct method is to add this as an additional HardwareManager, subclassed off HardwareManager. IPA will load all hardware managers with hardware support, which in your case would be this one and Generic, merge their clean steps, then attempt to execute clean steps on them in priority order.\n\nIn order to get your code to work as you expect, only define your one new clean step here, fix the exception handling like I commented, and subclass from HardwareManager instead of GenericHardwareManager.\n\nI have some examples here: https://github.com/jayofdoom/ipa-example-hardware-managers  with comments that may make this clearer. Also please review the dev docs for HardwareManagers as it covers how multiple Hardware Managers interact.\n\nGood luck!","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"30bf7b6dd0ed4a6b86a34f19054407694c1d1248","unresolved":false,"context_lines":[{"line_number":964,"context_line":"                with open(command_path, \u0027w\u0027) as command_file:"},{"line_number":965,"context_line":"                    command_file.write(\u0027lldp stop\u0027)"},{"line_number":966,"context_line":""},{"line_number":967,"context_line":"        except (OSError, IOError) as ex:"},{"line_number":968,"context_line":"            LOG.warning(\"Failed to disable the embedded LLDP on X710 network \""},{"line_number":969,"context_line":"                        \"card: %s\", ex)"},{"line_number":970,"context_line":"            raise ex"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_53a657ae","line":967,"updated":"2016-11-03 15:25:21.000000000","message":"I\u0027m not very enthusiastic about this solution. Whole iteration over all the files section has error handler. That means some command_files may be successfully saved, then one files raises exception. What happens next is information about action failure and reraising the exception, with no information about which command_files were successfully saved, and which were not.\n\nOrder is determined just by file name; I\u0027m not sure, but maybe, each file should be handled separately, and after a failure other files should be also changed, and then the function should raise exception based on results gathered for all the files? I realize that it breaks fail-fast, but fail-fast based on alphabet with no rollback is also not a good idea.","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"55786e6c65b5ced1095bd90c7ac9b3955b5308f9","unresolved":false,"context_lines":[{"line_number":967,"context_line":"        except (OSError, IOError) as ex:"},{"line_number":968,"context_line":"            LOG.warning(\"Failed to disable the embedded LLDP on X710 network \""},{"line_number":969,"context_line":"                        \"card: %s\", ex)"},{"line_number":970,"context_line":"            raise ex"},{"line_number":971,"context_line":"        return True"},{"line_number":972,"context_line":""},{"line_number":973,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_390d1938","line":970,"updated":"2016-11-04 15:22:01.000000000","message":"This should return errors.IncompatibleHardwareMethodError if you want this failure to fail cleaning. Otherwise you need to swallow the exception.","commit_id":"818178976f7365783456bc3af6a85bfe3e3d0344"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"5b4c961f2ac0fd517fde329386eac5ad7473b0f2","unresolved":false,"context_lines":[{"line_number":488,"context_line":"            LOG.warning(\u0027No disks detected in %d seconds\u0027,"},{"line_number":489,"context_line":"                        CONF.disk_wait_delay * CONF.disk_wait_attempts)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"    def cache_lldp_data(self, interface_names):"},{"line_number":492,"context_line":"        interface_names \u003d [name for name in interface_names if name !\u003d \u0027lo\u0027]"},{"line_number":493,"context_line":"        lldp_data \u003d {}"},{"line_number":494,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_01e8820f","line":491,"updated":"2016-12-09 15:33:48.000000000","message":"Please add a docstring for this method.","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"4df62a0a29814bb446a86a80223e0188c8b62844","unresolved":false,"context_lines":[{"line_number":488,"context_line":"            LOG.warning(\u0027No disks detected in %d seconds\u0027,"},{"line_number":489,"context_line":"                        CONF.disk_wait_delay * CONF.disk_wait_attempts)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"    def cache_lldp_data(self, interface_names):"},{"line_number":492,"context_line":"        interface_names \u003d [name for name in interface_names if name !\u003d \u0027lo\u0027]"},{"line_number":493,"context_line":"        lldp_data \u003d {}"},{"line_number":494,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_e4b6f1da","line":491,"in_reply_to":"3a71b18c_01e8820f","updated":"2016-12-12 16:14:58.000000000","message":"Done","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"613ad89ec1f247704e16003b01a2e8b39dbb262e","unresolved":false,"context_lines":[{"line_number":510,"context_line":"                    LOG.warning(\u0027An error occurred while processing TLV type \u0027"},{"line_number":511,"context_line":"                                \u0027%s for interface %s: %s\u0027, (typ, ifname, e))"},{"line_number":512,"context_line":"            lldp_data[ifname] \u003d processed_tlvs"},{"line_number":513,"context_line":"        # NOTE(sborkows) In order to pass cache results through"},{"line_number":514,"context_line":"        # \u0027dispatch_to_managers\u0027 we need to return them from this method"},{"line_number":515,"context_line":"        return lldp_data"},{"line_number":516,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_0a226801","line":513,"updated":"2016-12-09 11:16:04.000000000","message":"Looking at the method name, I would expect it to still cache it (set self.lldp_data)","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"41f463c6c36e5af0a793c68436368b15f932f2f4","unresolved":false,"context_lines":[{"line_number":510,"context_line":"                    LOG.warning(\u0027An error occurred while processing TLV type \u0027"},{"line_number":511,"context_line":"                                \u0027%s for interface %s: %s\u0027, (typ, ifname, e))"},{"line_number":512,"context_line":"            lldp_data[ifname] \u003d processed_tlvs"},{"line_number":513,"context_line":"        # NOTE(sborkows) In order to pass cache results through"},{"line_number":514,"context_line":"        # \u0027dispatch_to_managers\u0027 we need to return them from this method"},{"line_number":515,"context_line":"        return lldp_data"},{"line_number":516,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_fbddfc70","line":513,"in_reply_to":"3a71b18c_0a226801","updated":"2016-12-09 12:22:14.000000000","message":"I wonder about 2 solutions: is it better to rename that method appropriately, or leave saving values directly to self.lldp_data along with the return clause?\nAs I wrote in this note, due to calling this method from non-classy \u0027dispatch_to_managers\u0027 we need to pass this LLDP data in some way through it. Is it a better way to do this?","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"5bcfb8c9f04b82d1d55b61bb549216dbd0252888","unresolved":false,"context_lines":[{"line_number":510,"context_line":"                    LOG.warning(\u0027An error occurred while processing TLV type \u0027"},{"line_number":511,"context_line":"                                \u0027%s for interface %s: %s\u0027, (typ, ifname, e))"},{"line_number":512,"context_line":"            lldp_data[ifname] \u003d processed_tlvs"},{"line_number":513,"context_line":"        # NOTE(sborkows) In order to pass cache results through"},{"line_number":514,"context_line":"        # \u0027dispatch_to_managers\u0027 we need to return them from this method"},{"line_number":515,"context_line":"        return lldp_data"},{"line_number":516,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_6ff9df7f","line":513,"in_reply_to":"3a71b18c_61ecd629","updated":"2016-12-12 13:50:55.000000000","message":"I agree with that, but since there is already a method called _get_lldp_data(), I would rather rename it to collect_lldp_data().","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"5b4c961f2ac0fd517fde329386eac5ad7473b0f2","unresolved":false,"context_lines":[{"line_number":510,"context_line":"                    LOG.warning(\u0027An error occurred while processing TLV type \u0027"},{"line_number":511,"context_line":"                                \u0027%s for interface %s: %s\u0027, (typ, ifname, e))"},{"line_number":512,"context_line":"            lldp_data[ifname] \u003d processed_tlvs"},{"line_number":513,"context_line":"        # NOTE(sborkows) In order to pass cache results through"},{"line_number":514,"context_line":"        # \u0027dispatch_to_managers\u0027 we need to return them from this method"},{"line_number":515,"context_line":"        return lldp_data"},{"line_number":516,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_61ecd629","line":513,"in_reply_to":"3a71b18c_72bf4a60","updated":"2016-12-09 15:33:48.000000000","message":"I think renaming this method is a sane fix for understanding purposes.","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"b64464fb399ef95ea333a5b23b52a7afd0090d9c","unresolved":false,"context_lines":[{"line_number":510,"context_line":"                    LOG.warning(\u0027An error occurred while processing TLV type \u0027"},{"line_number":511,"context_line":"                                \u0027%s for interface %s: %s\u0027, (typ, ifname, e))"},{"line_number":512,"context_line":"            lldp_data[ifname] \u003d processed_tlvs"},{"line_number":513,"context_line":"        # NOTE(sborkows) In order to pass cache results through"},{"line_number":514,"context_line":"        # \u0027dispatch_to_managers\u0027 we need to return them from this method"},{"line_number":515,"context_line":"        return lldp_data"},{"line_number":516,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_72bf4a60","line":513,"in_reply_to":"3a71b18c_fbddfc70","updated":"2016-12-09 13:54:47.000000000","message":"If I didn\u0027t miss anything, this method is used only in list_network_interfaces. That means, it shouldn\u0027t be a problem that this function doesn\u0027t save lldp_data value. I think I\u0027d just rename it to e.g. \u0027get_lldp_data\u0027","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"}],"ironic_python_agent/hardware_managers/cna.py":[{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"60df27effcea7a8d0ed558dab1703e0e9f8c362b","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        try:"},{"line_number":54,"context_line":"            command_path \u003d \u0027{}/{}/command\u0027.format(addr_path, inner_dir)"},{"line_number":55,"context_line":"            with open(command_path, \u0027w\u0027) as command_file:"},{"line_number":56,"context_line":"                command_file.write(\u0027lldp stop\u0027)"},{"line_number":57,"context_line":"        except (OSError, IOError):"},{"line_number":58,"context_line":"            failed_dirs.append(inner_dir)"},{"line_number":59,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a74a57a_a5f8057e","line":56,"updated":"2016-11-23 16:46:58.000000000","message":"I\u0027m really not keen on the idea of this happening every time IPA starts; is it always safe? Does it take a long time to execute?\n\nI know for inspection, ordering for stuff like this can be difficult to handle, but is there no better way than changing the config on startup?","commit_id":"5b12256f5cee8c50435e8d4ba04e3105b3a7d610"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"f876727a2276fd2cc45a56711f34c950d3014fc5","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        try:"},{"line_number":54,"context_line":"            command_path \u003d \u0027{}/{}/command\u0027.format(addr_path, inner_dir)"},{"line_number":55,"context_line":"            with open(command_path, \u0027w\u0027) as command_file:"},{"line_number":56,"context_line":"                command_file.write(\u0027lldp stop\u0027)"},{"line_number":57,"context_line":"        except (OSError, IOError):"},{"line_number":58,"context_line":"            failed_dirs.append(inner_dir)"},{"line_number":59,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a74a57a_1d3bbbec","line":56,"in_reply_to":"5a74a57a_a2d265fe","updated":"2016-11-30 17:08:00.000000000","message":"Would it be better to disable it a the time LLDP is being discovered in Inspector? I\u0027m wondering if we should just have a hook in there rather than doing it every time.","commit_id":"5b12256f5cee8c50435e8d4ba04e3105b3a7d610"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"ae7ba22b1422b13225bf629fb87b803e26923b34","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        try:"},{"line_number":54,"context_line":"            command_path \u003d \u0027{}/{}/command\u0027.format(addr_path, inner_dir)"},{"line_number":55,"context_line":"            with open(command_path, \u0027w\u0027) as command_file:"},{"line_number":56,"context_line":"                command_file.write(\u0027lldp stop\u0027)"},{"line_number":57,"context_line":"        except (OSError, IOError):"},{"line_number":58,"context_line":"            failed_dirs.append(inner_dir)"},{"line_number":59,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a74a57a_a2d265fe","line":56,"in_reply_to":"5a74a57a_a5f8057e","updated":"2016-11-25 13:19:13.000000000","message":"Well, it should not take a long time to execute this, because we only write a command to file and do not care about the result of that command on CNA card.\nIt need to happen on every inspection, because during every boot, the card turns its own embedded LLDP agent on, thus we are not able to gather LLDP data from that card.\nI also want to wait for other reviewers before I will change this approach.","commit_id":"5b12256f5cee8c50435e8d4ba04e3105b3a7d610"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"5b4c961f2ac0fd517fde329386eac5ad7473b0f2","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            LOG.debug(\u0027No Intel CNA network card found\u0027)"},{"line_number":76,"context_line":"            return hardware.HardwareSupport.NONE"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def cache_lldp_data(self, interface_names):"},{"line_number":79,"context_line":"        _disable_embedded_lldp_agent_in_cna_card()"},{"line_number":80,"context_line":"        return super(IntelCnaHardwareManager, self).cache_lldp_data("},{"line_number":81,"context_line":"            interface_names)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_e1bf06fa","line":78,"updated":"2016-12-09 15:33:48.000000000","message":"Can you add a doctoring for this method?","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"4df62a0a29814bb446a86a80223e0188c8b62844","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            LOG.debug(\u0027No Intel CNA network card found\u0027)"},{"line_number":76,"context_line":"            return hardware.HardwareSupport.NONE"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def cache_lldp_data(self, interface_names):"},{"line_number":79,"context_line":"        _disable_embedded_lldp_agent_in_cna_card()"},{"line_number":80,"context_line":"        return super(IntelCnaHardwareManager, self).cache_lldp_data("},{"line_number":81,"context_line":"            interface_names)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_c4a735a5","line":78,"in_reply_to":"3a71b18c_e1bf06fa","updated":"2016-12-12 16:14:58.000000000","message":"Done","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"}],"ironic_python_agent/hardware_managers/x710.py":[{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"26920774f1ce1d11282a67e47900debbaf874696","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2013 Rackspace, Inc."},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\");"},{"line_number":4,"context_line":"# you may not use this file except in compliance with the License."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a629dbe_db302804","line":1,"updated":"2016-11-09 15:18:10.000000000","message":"The year in header doesn\u0027t match, the entity too I believe. I am sorry I missed it before.","commit_id":"ab7891a647ca16b46708928c638dd9345ee9761a"}],"ironic_python_agent/tests/unit/hardware_managers/test_cna.py":[{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"613ad89ec1f247704e16003b01a2e8b39dbb262e","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    @mock.patch.object(hardware.GenericHardwareManager, \u0027cache_lldp_data\u0027,"},{"line_number":139,"context_line":"                       autospec\u003dTrue)"},{"line_number":140,"context_line":"    def test_cache_lldp_data(self, mock_super_cache):"},{"line_number":141,"context_line":"        iface_names \u003d [\u0027foo\u0027, \u0027bar\u0027]"},{"line_number":142,"context_line":"        returned_lldp_data \u003d ["},{"line_number":143,"context_line":"            (0, \u0027foo\u0027),"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_5d37c8af","line":140,"updated":"2016-12-09 11:16:04.000000000","message":"I\u0027d add another test that will check that result is empty when netutils.get_lldp_info throws Exception","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"b64464fb399ef95ea333a5b23b52a7afd0090d9c","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    @mock.patch.object(hardware.GenericHardwareManager, \u0027cache_lldp_data\u0027,"},{"line_number":139,"context_line":"                       autospec\u003dTrue)"},{"line_number":140,"context_line":"    def test_cache_lldp_data(self, mock_super_cache):"},{"line_number":141,"context_line":"        iface_names \u003d [\u0027foo\u0027, \u0027bar\u0027]"},{"line_number":142,"context_line":"        returned_lldp_data \u003d ["},{"line_number":143,"context_line":"            (0, \u0027foo\u0027),"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_f2599a5c","line":140,"in_reply_to":"3a71b18c_5b72704a","updated":"2016-12-09 13:54:47.000000000","message":"yes, test_hardware seems a better place. Yet, as the behavior of _cache_lldp_data changed, a unit test would be a good thing","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"41f463c6c36e5af0a793c68436368b15f932f2f4","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    @mock.patch.object(hardware.GenericHardwareManager, \u0027cache_lldp_data\u0027,"},{"line_number":139,"context_line":"                       autospec\u003dTrue)"},{"line_number":140,"context_line":"    def test_cache_lldp_data(self, mock_super_cache):"},{"line_number":141,"context_line":"        iface_names \u003d [\u0027foo\u0027, \u0027bar\u0027]"},{"line_number":142,"context_line":"        returned_lldp_data \u003d ["},{"line_number":143,"context_line":"            (0, \u0027foo\u0027),"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_5b72704a","line":140,"in_reply_to":"3a71b18c_5d37c8af","updated":"2016-12-09 12:22:14.000000000","message":"When adding this test I was focused on checking only whether the super() method was called or not.\nI think that the tests you propose can be added to test_hardware.py (not here) and that these tests, while still necessary, should not be added in this patch, as they are not in scope of fixing the LLDP bug.","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"4df62a0a29814bb446a86a80223e0188c8b62844","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    @mock.patch.object(hardware.GenericHardwareManager, \u0027cache_lldp_data\u0027,"},{"line_number":139,"context_line":"                       autospec\u003dTrue)"},{"line_number":140,"context_line":"    def test_cache_lldp_data(self, mock_super_cache):"},{"line_number":141,"context_line":"        iface_names \u003d [\u0027foo\u0027, \u0027bar\u0027]"},{"line_number":142,"context_line":"        returned_lldp_data \u003d ["},{"line_number":143,"context_line":"            (0, \u0027foo\u0027),"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a6eadb0_56aab21a","line":140,"in_reply_to":"3a71b18c_f2599a5c","updated":"2016-12-12 16:14:58.000000000","message":"Done","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"613ad89ec1f247704e16003b01a2e8b39dbb262e","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        iface_names \u003d [\u0027foo\u0027, \u0027bar\u0027]"},{"line_number":142,"context_line":"        returned_lldp_data \u003d ["},{"line_number":143,"context_line":"            (0, \u0027foo\u0027),"},{"line_number":144,"context_line":"            (1, \u0027bar\u0027),"},{"line_number":145,"context_line":"        ]"},{"line_number":146,"context_line":"        mock_super_cache.return_value \u003d returned_lldp_data"},{"line_number":147,"context_line":"        with mock.patch.object(cna,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_1d0470be","line":144,"updated":"2016-12-09 11:16:04.000000000","message":"Also a test where one of files results in exception raise for binascii.hexlify(data).decode() and result size is 1 would be nice.","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"4df62a0a29814bb446a86a80223e0188c8b62844","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        iface_names \u003d [\u0027foo\u0027, \u0027bar\u0027]"},{"line_number":142,"context_line":"        returned_lldp_data \u003d ["},{"line_number":143,"context_line":"            (0, \u0027foo\u0027),"},{"line_number":144,"context_line":"            (1, \u0027bar\u0027),"},{"line_number":145,"context_line":"        ]"},{"line_number":146,"context_line":"        mock_super_cache.return_value \u003d returned_lldp_data"},{"line_number":147,"context_line":"        with mock.patch.object(cna,"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a6eadb0_76a7ae4f","line":144,"in_reply_to":"3a71b18c_1d0470be","updated":"2016-12-12 16:14:58.000000000","message":"Done","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"}],"ironic_python_agent/tests/unit/test_agent.py":[{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"613ad89ec1f247704e16003b01a2e8b39dbb262e","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                          enforce_type\u003dTrue)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        self.agent \u003d agent.IronicPythonAgent(None,"},{"line_number":271,"context_line":"                                             agent.Host(\u0027203.0.113.1\u0027, 9990),"},{"line_number":272,"context_line":"                                             agent.Host(\u0027192.0.2.1\u0027, 9999),"},{"line_number":273,"context_line":"                                             3,"},{"line_number":274,"context_line":"                                             10,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_bdb7fcf3","line":271,"updated":"2016-12-09 11:16:04.000000000","message":"NIC: maybe it\u0027s time to save these magic addresses and ports in variables on top? :)","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":19213,"name":"Szymon Borkowski","email":"szymon.borkowski@intel.com","username":"sborkows"},"change_message_id":"41f463c6c36e5af0a793c68436368b15f932f2f4","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                          enforce_type\u003dTrue)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        self.agent \u003d agent.IronicPythonAgent(None,"},{"line_number":271,"context_line":"                                             agent.Host(\u0027203.0.113.1\u0027, 9990),"},{"line_number":272,"context_line":"                                             agent.Host(\u0027192.0.2.1\u0027, 9999),"},{"line_number":273,"context_line":"                                             3,"},{"line_number":274,"context_line":"                                             10,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_db5da07a","line":271,"in_reply_to":"3a71b18c_bdb7fcf3","updated":"2016-12-09 12:22:14.000000000","message":"Good idea. However, I think this refactoring is not in the scope of this patch.","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"b64464fb399ef95ea333a5b23b52a7afd0090d9c","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                          enforce_type\u003dTrue)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        self.agent \u003d agent.IronicPythonAgent(None,"},{"line_number":271,"context_line":"                                             agent.Host(\u0027203.0.113.1\u0027, 9990),"},{"line_number":272,"context_line":"                                             agent.Host(\u0027192.0.2.1\u0027, 9999),"},{"line_number":273,"context_line":"                                             3,"},{"line_number":274,"context_line":"                                             10,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a71b18c_b27002de","line":271,"in_reply_to":"3a71b18c_db5da07a","updated":"2016-12-09 13:54:47.000000000","message":"I\u0027m totally fine with that :)","commit_id":"ae32261cb35495cb0fb0be5a1e397fd886e348fe"}],"ironic_python_agent/tests/unit/test_hardware_managers.py":[{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"9ff66179447fbaf9e09b6488ac05d27cf4c663af","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        with mock.patch(\u0027six.moves.builtins.open\u0027, write_mock, create\u003dTrue):"},{"line_number":141,"context_line":"            write_mock.side_effect \u003d IOError(\u0027fake error\u0027)"},{"line_number":142,"context_line":"            result \u003d self.hardware.disable_embedded_lldp_agent_in_x710_card()"},{"line_number":143,"context_line":"            mock_log.warning.assert_called_once()"},{"line_number":144,"context_line":"            self.assertEqual(True, result)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_73d769bf","line":143,"updated":"2016-11-08 21:42:41.000000000","message":"As logging is used as failure indicator, it might be a  good idea to use assert_called_once_with","commit_id":"d4e75359853a0b1cb47a8f41425de4fe15c2a25e"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"7c86c2569e3c3655d196d5fa5e30162c2276324d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2013 Rackspace, Inc."},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\");"},{"line_number":4,"context_line":"# you may not use this file except in compliance with the License."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a629dbe_49a89fdb","line":1,"range":{"start_line":1,"start_character":0,"end_line":1,"end_character":32},"updated":"2016-11-10 14:32:45.000000000","message":"you should update the copyright","commit_id":"ab7891a647ca16b46708928c638dd9345ee9761a"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"7c86c2569e3c3655d196d5fa5e30162c2276324d","unresolved":false,"context_lines":[{"line_number":11,"context_line":"# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied."},{"line_number":12,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":13,"context_line":"# limitations under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import os"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"import mock"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a629dbe_894ab724","line":14,"updated":"2016-11-10 14:32:45.000000000","message":"I think it is better to move this file\nto tests/unit/hardware_managers/test_x710.py so it will compatible with the hardware_managers/x710.py","commit_id":"ab7891a647ca16b46708928c638dd9345ee9761a"}]}