)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":11,"context_line":"flags. This is achived by iterating over the"},{"line_number":12,"context_line":"nodedev list and finding the nic element via"},{"line_number":13,"context_line":"its parent deivce."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ibf8dca4bd57b3bddb39955b53cc03564506f5754"},{"line_number":16,"context_line":"Closes-Bub: #1883671"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bf51134e_b5440cb9","line":14,"updated":"2020-07-02 20:55:34.000000000","message":"Would prefer to have a paragraph here in the commit message explaining what problem this solves and how, for example some excerpt from the bug report.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":13,"context_line":"its parent deivce."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ibf8dca4bd57b3bddb39955b53cc03564506f5754"},{"line_number":16,"context_line":"Closes-Bub: #1883671"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bf51134e_d5f2609a","line":16,"range":{"start_line":16,"start_character":7,"end_line":16,"end_character":10},"updated":"2020-07-02 20:55:34.000000000","message":"Bug","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-07-16 17:38:38 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"lookup nic feature by pci address"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In some environments the libvirt nodedev list can"},{"line_number":10,"context_line":"become out of sync with the current MAC address"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf51134e_87bf0d5f","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":1},"updated":"2020-07-22 11:15:19.000000000","message":"L","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-07-16 17:38:38 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"lookup nic feature by pci address"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In some environments the libvirt nodedev list can"},{"line_number":10,"context_line":"become out of sync with the current MAC address"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf51134e_e7b6c98a","line":7,"range":{"start_line":7,"start_character":22,"end_line":7,"end_character":25},"updated":"2020-07-22 11:15:19.000000000","message":"PCI","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":9,"context_line":"In some environments the libvirt nodedev list can"},{"line_number":10,"context_line":"become out of sync with the current MAC address"},{"line_number":11,"context_line":"assigned to a netdev, As a result the nodedev"},{"line_number":12,"context_line":"look up can fail. This resulted in an uncaught"},{"line_number":13,"context_line":"libvirt exception which broke the"},{"line_number":14,"context_line":"update_available_resource function resulting"},{"line_number":15,"context_line":"in an incorrect resource view in the database."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf51134e_47a5f546","line":12,"range":{"start_line":12,"start_character":0,"end_line":12,"end_character":7},"updated":"2020-07-22 11:15:19.000000000","message":"lookup","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":9,"context_line":"In some environments the libvirt nodedev list can"},{"line_number":10,"context_line":"become out of sync with the current MAC address"},{"line_number":11,"context_line":"assigned to a netdev, As a result the nodedev"},{"line_number":12,"context_line":"look up can fail. This resulted in an uncaught"},{"line_number":13,"context_line":"libvirt exception which broke the"},{"line_number":14,"context_line":"update_available_resource function resulting"},{"line_number":15,"context_line":"in an incorrect resource view in the database."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf51134e_87642d23","line":12,"range":{"start_line":12,"start_character":23,"end_line":12,"end_character":31},"updated":"2020-07-22 11:15:19.000000000","message":"results","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":10,"context_line":"become out of sync with the current MAC address"},{"line_number":11,"context_line":"assigned to a netdev, As a result the nodedev"},{"line_number":12,"context_line":"look up can fail. This resulted in an uncaught"},{"line_number":13,"context_line":"libvirt exception which broke the"},{"line_number":14,"context_line":"update_available_resource function resulting"},{"line_number":15,"context_line":"in an incorrect resource view in the database."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf51134e_e76b69ee","line":13,"range":{"start_line":13,"start_character":24,"end_line":13,"end_character":29},"updated":"2020-07-22 11:15:19.000000000","message":"breaks","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":12,"context_line":"look up can fail. This resulted in an uncaught"},{"line_number":13,"context_line":"libvirt exception which broke the"},{"line_number":14,"context_line":"update_available_resource function resulting"},{"line_number":15,"context_line":"in an incorrect resource view in the database."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"e.g. libvirt.libvirtError: Node device not found:"},{"line_number":18,"context_line":"no node device with matching name \u0027net_enp7s0f3v1_ea_60_77_1f_21_50\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf51134e_67aab919","line":15,"updated":"2020-07-22 11:15:19.000000000","message":"What on earth are you wrapping this stuff with? You\u0027ve got 72 characters to burn, you know :-P","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[{"line_number":12,"context_line":"look up can fail. This resulted in an uncaught"},{"line_number":13,"context_line":"libvirt exception which broke the"},{"line_number":14,"context_line":"update_available_resource function resulting"},{"line_number":15,"context_line":"in an incorrect resource view in the database."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"e.g. libvirt.libvirtError: Node device not found:"},{"line_number":18,"context_line":"no node device with matching name \u0027net_enp7s0f3v1_ea_60_77_1f_21_50\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9f560f44_738f44cc","line":15,"in_reply_to":"bf51134e_67aab919","updated":"2020-07-27 23:12:40.000000000","message":"randomly by ey with no column or line numbers.\ni use nano for commit messages and by default it does not show them unless you press Alt-C so i normally am very concentrative  to not get near the limit.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8f0222a4c54e28573b4b91e7881510c930e59244","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This change removes the dependency on the nodedev name when looking up"},{"line_number":19,"context_line":"nic feature flags."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: Ibf8dca4bd57b3bddb39955b53cc03564506f5754"},{"line_number":22,"context_line":"Closes-Bug: #1883671"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9f560f44_763fb7b3","line":20,"updated":"2020-08-04 11:50:29.000000000","message":"I was able to reproduce the problem without this patch and I can confirm that applying this patch fixes it.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7019581573e59d432067163ab13c26d59e490c7e","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This change removes the dependency on the nodedev name when looking up"},{"line_number":19,"context_line":"nic feature flags."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: Ibf8dca4bd57b3bddb39955b53cc03564506f5754"},{"line_number":22,"context_line":"Closes-Bug: #1883671"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9f560f44_b6f82f49","line":20,"in_reply_to":"9f560f44_763fb7b3","updated":"2020-08-04 12:29:12.000000000","message":"the benifits and pains of haveing an sriov capably dev setup have caught up with you :) but glad to hear it also works for you.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"}],"nova/tests/unit/virt/libvirt/fakelibvirt.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d24d087de42c5b9f54971b6c2023420037f86b26","unresolved":false,"context_lines":[{"line_number":815,"context_line":"    def reset(self):"},{"line_number":816,"context_line":"        pass"},{"line_number":817,"context_line":""},{"line_number":818,"context_line":"    def XMLDesc(self, _: int) -\u003e str:"},{"line_number":819,"context_line":"        return self._xml"},{"line_number":820,"context_line":""},{"line_number":821,"context_line":"    def parent(self) -\u003e str:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_e6f37972","line":818,"range":{"start_line":818,"start_character":22,"end_line":818,"end_character":23},"updated":"2020-07-03 09:58:45.000000000","message":"Unless you\u0027re copying this from elsewhere, can we give this a name even though it\u0027s not used?\n\nAlso note: we\u0027re unlikely to ever add type checking to any test files because we rely too heavily on duck typing and mocking here. It\u0027s no harm though.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e384032e3380152e9026c11bee9e7818fcf0feb","unresolved":false,"context_lines":[{"line_number":815,"context_line":"    def reset(self):"},{"line_number":816,"context_line":"        pass"},{"line_number":817,"context_line":""},{"line_number":818,"context_line":"    def XMLDesc(self, _: int) -\u003e str:"},{"line_number":819,"context_line":"        return self._xml"},{"line_number":820,"context_line":""},{"line_number":821,"context_line":"    def parent(self) -\u003e str:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_41984f69","line":818,"range":{"start_line":818,"start_character":22,"end_line":818,"end_character":23},"in_reply_to":"bf51134e_86b0bd8d","updated":"2020-07-03 10:56:07.000000000","message":"its flags and its not used and is always 0 \nhttps://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetXMLDesc","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1667221f5c966dd0ce37c1b50b2c802ce7138204","unresolved":false,"context_lines":[{"line_number":815,"context_line":"    def reset(self):"},{"line_number":816,"context_line":"        pass"},{"line_number":817,"context_line":""},{"line_number":818,"context_line":"    def XMLDesc(self, _: int) -\u003e str:"},{"line_number":819,"context_line":"        return self._xml"},{"line_number":820,"context_line":""},{"line_number":821,"context_line":"    def parent(self) -\u003e str:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_86b0bd8d","line":818,"range":{"start_line":818,"start_character":22,"end_line":818,"end_character":23},"in_reply_to":"bf51134e_e6f37972","updated":"2020-07-03 10:07:06.000000000","message":"ya i would prefer to use Any if we need too.\n\ni use _ to call out it was not used but sure. i just have no idea what the parm is we pass 0 everywher ei think ill see if i can find its name but this class comes form the libvirt bidnigns so ill have to dig into the libvirt docs.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":1732,"context_line":"        pass"},{"line_number":1733,"context_line":""},{"line_number":1734,"context_line":"    def listAllDevices(self, flags):"},{"line_number":1735,"context_line":"        # Note this is incomplete as we do not filter"},{"line_number":1736,"context_line":"        # based on the flags however it is enough for our"},{"line_number":1737,"context_line":"        # current testing."},{"line_number":1738,"context_line":"        return [NodeDevice(self, xml\u003ddev.XMLDesc(0))"},{"line_number":1739,"context_line":"                for dev in self.pci_info.devices.values()]"},{"line_number":1740,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_c76e25ff","line":1737,"range":{"start_line":1735,"start_character":0,"end_line":1737,"end_character":26},"updated":"2020-07-22 11:15:19.000000000","message":"How about adding \n\n  assert flags is None, \u0027Not implemented\u0027\n\n?","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[{"line_number":1732,"context_line":"        pass"},{"line_number":1733,"context_line":""},{"line_number":1734,"context_line":"    def listAllDevices(self, flags):"},{"line_number":1735,"context_line":"        # Note this is incomplete as we do not filter"},{"line_number":1736,"context_line":"        # based on the flags however it is enough for our"},{"line_number":1737,"context_line":"        # current testing."},{"line_number":1738,"context_line":"        return [NodeDevice(self, xml\u003ddev.XMLDesc(0))"},{"line_number":1739,"context_line":"                for dev in self.pci_info.devices.values()]"},{"line_number":1740,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_36381ad2","line":1737,"range":{"start_line":1735,"start_character":0,"end_line":1737,"end_character":26},"in_reply_to":"bf51134e_c76e25ff","updated":"2020-07-27 23:12:40.000000000","message":"it wont be None its actually used but we dont actully need to emulate the filtering for any of the test that use it and i dont really want to add more code to do that filtering since doing it proprly involved decoding a bitmask and then traslating that to a serise of prefixes and skiping any device that does not match the prefixes enabled by the bit mask.\n\nlibvirt will do something similar but until we actully need that for some reason i would prefer not to add the extra complexity.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d24d087de42c5b9f54971b6c2023420037f86b26","unresolved":false,"context_lines":[{"line_number":1,"context_line":""},{"line_number":2,"context_line":"#    Copyright 2010 OpenStack Foundation"},{"line_number":3,"context_line":"#    Copyright 2012 University Of Minho"},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_06ef8d87","line":1,"updated":"2020-07-03 09:58:45.000000000","message":"nit","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1667221f5c966dd0ce37c1b50b2c802ce7138204","unresolved":false,"context_lines":[{"line_number":1,"context_line":""},{"line_number":2,"context_line":"#    Copyright 2010 OpenStack Foundation"},{"line_number":3,"context_line":"#    Copyright 2012 University Of Minho"},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_66ada930","line":1,"in_reply_to":"bf51134e_06ef8d87","updated":"2020-07-03 10:07:06.000000000","message":"already fixed locally:)","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":349,"context_line":"    for name, xml in _fake_NodeDevXml.items()"},{"line_number":350,"context_line":"}"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"_fake_NodeDevXml_childern \u003d defaultdict(list)"},{"line_number":353,"context_line":"for key, val in _fake_NodeDevXml_parents.items():"},{"line_number":354,"context_line":"    _fake_NodeDevXml_childern[val].append(key)"},{"line_number":355,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_755b9453","line":352,"range":{"start_line":352,"start_character":17,"end_line":352,"end_character":25},"updated":"2020-07-02 20:55:34.000000000","message":"children\n\n(repeated throughout file)","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6b6839d651c3838d856578f42c53e1a99c884009","unresolved":false,"context_lines":[{"line_number":17419,"context_line":"            lambda self, name: node_devs.get(name))"},{"line_number":17420,"context_line":""},{"line_number":17421,"context_line":"        actjson \u003d drvr._get_pci_passthrough_devices()"},{"line_number":17422,"context_line":"        mock_list.assert_not_called()"},{"line_number":17423,"context_line":"        expectvfs \u003d ["},{"line_number":17424,"context_line":"            {"},{"line_number":17425,"context_line":"                \"dev_id\": \"pci_0000_04_00_3\","}],"source_content_type":"text/x-python","patch_set":12,"id":"9f560f44_5b59ea32","line":17422,"updated":"2020-08-05 08:37:13.000000000","message":"and I guess list_all_devices() was called instead","commit_id":"efc27ff84c3f38fbcbf75b0dc230963c58d093e4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6b6839d651c3838d856578f42c53e1a99c884009","unresolved":false,"context_lines":[{"line_number":17462,"context_line":"                               \u0027label\u0027, \u0027capabilities\u0027]:"},{"line_number":17463,"context_line":"                    self.assertEqual(expectvfs[dev][key], actualvfs[dev][key])"},{"line_number":17464,"context_line":""},{"line_number":17465,"context_line":"        # The first call for every VF is to determine parent_ifname and"},{"line_number":17466,"context_line":"        # the second call to determine the MAC address."},{"line_number":17467,"context_line":"        mock_get_ifname.assert_has_calls(["},{"line_number":17468,"context_line":"            mock.call(\u00270000:04:10.7\u0027, pf_interface\u003dTrue),"},{"line_number":17469,"context_line":"            mock.call(\u00270000:04:11.7\u0027, pf_interface\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":12,"id":"9f560f44_9b39c2d8","line":17466,"range":{"start_line":17465,"start_character":1,"end_line":17466,"end_character":55},"updated":"2020-08-05 08:37:13.000000000","message":"this is outdated now","commit_id":"efc27ff84c3f38fbcbf75b0dc230963c58d093e4"}],"nova/tests/unit/virt/libvirt/test_host.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":1140,"context_line":"                self.host.get_connection(),"},{"line_number":1141,"context_line":"                \"virConnectListAllNodeDevices\") as mock_list_nodedevs:"},{"line_number":1142,"context_line":"            self.host.list_all_nodedevs(flags\u003d42)"},{"line_number":1143,"context_line":"        mock_list_nodedevs.assert_called_once_with(42)"},{"line_number":1144,"context_line":""},{"line_number":1145,"context_line":"    def test_list_all_nodedevs_raises(self):"},{"line_number":1146,"context_line":"        with mock.patch.object("}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_154b3887","line":1143,"updated":"2020-07-02 20:55:34.000000000","message":"Shouldn\u0027t we also assert the return value?","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1ddc416a66a8e6fe92e4647df21bdedcb576c436","unresolved":false,"context_lines":[{"line_number":1140,"context_line":"                self.host.get_connection(),"},{"line_number":1141,"context_line":"                \"virConnectListAllNodeDevices\") as mock_list_nodedevs:"},{"line_number":1142,"context_line":"            self.host.list_all_nodedevs(flags\u003d42)"},{"line_number":1143,"context_line":"        mock_list_nodedevs.assert_called_once_with(42)"},{"line_number":1144,"context_line":""},{"line_number":1145,"context_line":"    def test_list_all_nodedevs_raises(self):"},{"line_number":1146,"context_line":"        with mock.patch.object("}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_6320fbb1","line":1143,"in_reply_to":"bf51134e_154b3887","updated":"2020-07-03 09:26:27.000000000","message":"sure i can mock some devs and assert they are returned.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":1150,"context_line":"                \"message\")"},{"line_number":1151,"context_line":"            ret \u003d self.host.list_all_nodedevs(flags\u003d42)"},{"line_number":1152,"context_line":"            self.assertEqual([], ret)"},{"line_number":1153,"context_line":"        mock_list_nodedevs.assert_called_once_with(42)"},{"line_number":1154,"context_line":""},{"line_number":1155,"context_line":"    @mock.patch.object(fakelibvirt.virConnect, \"listDevices\")"},{"line_number":1156,"context_line":"    def test_list_devices(self, mock_listDevices):"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_35711ce2","line":1153,"updated":"2020-07-02 20:55:34.000000000","message":"Note: not asserting the log warning was emitted.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1ddc416a66a8e6fe92e4647df21bdedcb576c436","unresolved":false,"context_lines":[{"line_number":1150,"context_line":"                \"message\")"},{"line_number":1151,"context_line":"            ret \u003d self.host.list_all_nodedevs(flags\u003d42)"},{"line_number":1152,"context_line":"            self.assertEqual([], ret)"},{"line_number":1153,"context_line":"        mock_list_nodedevs.assert_called_once_with(42)"},{"line_number":1154,"context_line":""},{"line_number":1155,"context_line":"    @mock.patch.object(fakelibvirt.virConnect, \"listDevices\")"},{"line_number":1156,"context_line":"    def test_list_devices(self, mock_listDevices):"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_03193f87","line":1153,"in_reply_to":"bf51134e_35711ce2","updated":"2020-07-03 09:26:27.000000000","message":"good point ill add that","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":6921,"context_line":"                cfgdev.pci_capability.slot, cfgdev.pci_capability.function)"},{"line_number":6922,"context_line":""},{"line_number":6923,"context_line":"            # check if the pci device is the vf and recored its name"},{"line_number":6924,"context_line":"            parent \u003d dev.name() if address in vf_address else parent"},{"line_number":6925,"context_line":"            # then check if we have already seen it"},{"line_number":6926,"context_line":"            net_dev \u003d net_devs.get(parent)"},{"line_number":6927,"context_line":"            if net_dev:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_75ad5430","line":6924,"updated":"2020-07-02 20:55:34.000000000","message":"If address is not in vf_address, then parent \u003d None, is that intended?\n\nDo we have test coverage for this case?","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1ddc416a66a8e6fe92e4647df21bdedcb576c436","unresolved":false,"context_lines":[{"line_number":6921,"context_line":"                cfgdev.pci_capability.slot, cfgdev.pci_capability.function)"},{"line_number":6922,"context_line":""},{"line_number":6923,"context_line":"            # check if the pci device is the vf and recored its name"},{"line_number":6924,"context_line":"            parent \u003d dev.name() if address in vf_address else parent"},{"line_number":6925,"context_line":"            # then check if we have already seen it"},{"line_number":6926,"context_line":"            net_dev \u003d net_devs.get(parent)"},{"line_number":6927,"context_line":"            if net_dev:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_c69d9567","line":6924,"in_reply_to":"bf51134e_75ad5430","updated":"2020-07-03 09:26:27.000000000","message":"yes that is intended. and no but its trivial to add\ni just need to add an additional pci dev first that is not the VF or a Net nodedev.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":6923,"context_line":"            # check if the pci device is the vf and recored its name"},{"line_number":6924,"context_line":"            parent \u003d dev.name() if address in vf_address else parent"},{"line_number":6925,"context_line":"            # then check if we have already seen it"},{"line_number":6926,"context_line":"            net_dev \u003d net_devs.get(parent)"},{"line_number":6927,"context_line":"            if net_dev:"},{"line_number":6928,"context_line":"                break"},{"line_number":6929,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_d5aa6017","line":6926,"range":{"start_line":6926,"start_character":35,"end_line":6926,"end_character":41},"updated":"2020-07-02 20:55:34.000000000","message":"Would it be valid for parent \u003d None here?","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1ddc416a66a8e6fe92e4647df21bdedcb576c436","unresolved":false,"context_lines":[{"line_number":6923,"context_line":"            # check if the pci device is the vf and recored its name"},{"line_number":6924,"context_line":"            parent \u003d dev.name() if address in vf_address else parent"},{"line_number":6925,"context_line":"            # then check if we have already seen it"},{"line_number":6926,"context_line":"            net_dev \u003d net_devs.get(parent)"},{"line_number":6927,"context_line":"            if net_dev:"},{"line_number":6928,"context_line":"                break"},{"line_number":6929,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_46b505ee","line":6926,"range":{"start_line":6926,"start_character":35,"end_line":6926,"end_character":41},"in_reply_to":"bf51134e_d5aa6017","updated":"2020-07-03 09:26:27.000000000","message":"yes parent can be None here which is why im using get and not []","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"ddca16e1012894a8ff28317c37249b685dae9f6e","unresolved":false,"context_lines":[{"line_number":6929,"context_line":""},{"line_number":6930,"context_line":"        # early out if not found"},{"line_number":6931,"context_line":"        if net_dev is None:"},{"line_number":6932,"context_line":"            return"},{"line_number":6933,"context_line":""},{"line_number":6934,"context_line":"        # other return the capablities"},{"line_number":6935,"context_line":"        xmlstr \u003d net_dev.XMLDesc(0)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_bb507dd6","line":6932,"updated":"2020-07-02 22:32:03.000000000","message":"pep8: error: Return value expected","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1667221f5c966dd0ce37c1b50b2c802ce7138204","unresolved":false,"context_lines":[{"line_number":6929,"context_line":""},{"line_number":6930,"context_line":"        # early out if not found"},{"line_number":6931,"context_line":"        if net_dev is None:"},{"line_number":6932,"context_line":"            return"},{"line_number":6933,"context_line":""},{"line_number":6934,"context_line":"        # other return the capablities"},{"line_number":6935,"context_line":"        xmlstr \u003d net_dev.XMLDesc(0)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_0662ed16","line":6932,"in_reply_to":"bf51134e_a6e06198","updated":"2020-07-03 10:07:06.000000000","message":"yep i was pretty sure that was the issue i also need to make the return type an optional of dict since it can be none","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d24d087de42c5b9f54971b6c2023420037f86b26","unresolved":false,"context_lines":[{"line_number":6929,"context_line":""},{"line_number":6930,"context_line":"        # early out if not found"},{"line_number":6931,"context_line":"        if net_dev is None:"},{"line_number":6932,"context_line":"            return"},{"line_number":6933,"context_line":""},{"line_number":6934,"context_line":"        # other return the capablities"},{"line_number":6935,"context_line":"        xmlstr \u003d net_dev.XMLDesc(0)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_a6e06198","line":6932,"in_reply_to":"bf51134e_bb507dd6","updated":"2020-07-03 09:58:45.000000000","message":"FYI, mypy requires explicit \u0027return None\u0027. Yes, it\u0027s dumb.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"3404ea58307c83079894adf015a927e1d712fd37","unresolved":false,"context_lines":[{"line_number":6924,"context_line":"                cfgdev.pci_capability.slot, cfgdev.pci_capability.function)"},{"line_number":6925,"context_line":""},{"line_number":6926,"context_line":"            # check if the pci device is the vf and recored its name"},{"line_number":6927,"context_line":"            parent \u003d dev.name() if address in vf_address else parent"},{"line_number":6928,"context_line":"            # then check if we have already seen it"},{"line_number":6929,"context_line":"            net_dev \u003d net_devs.get(parent)"},{"line_number":6930,"context_line":"            if net_dev:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf51134e_42ae6a98","line":6927,"range":{"start_line":6927,"start_character":21,"end_line":6927,"end_character":31},"updated":"2020-07-07 09:57:11.000000000","message":"we already know the devname when calling this, at line 6944, right? Then we can just find the network device with that parent name. I could be wrong.","commit_id":"ef79b979aee5571a1c03aa6288575ce45646005f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e21158eb592a51708c858e60e060dcae9e712434","unresolved":false,"context_lines":[{"line_number":6924,"context_line":"                cfgdev.pci_capability.slot, cfgdev.pci_capability.function)"},{"line_number":6925,"context_line":""},{"line_number":6926,"context_line":"            # check if the pci device is the vf and recored its name"},{"line_number":6927,"context_line":"            parent \u003d dev.name() if address in vf_address else parent"},{"line_number":6928,"context_line":"            # then check if we have already seen it"},{"line_number":6929,"context_line":"            net_dev \u003d net_devs.get(parent)"},{"line_number":6930,"context_line":"            if net_dev:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf51134e_9071e8e1","line":6927,"range":{"start_line":6927,"start_character":21,"end_line":6927,"end_character":31},"in_reply_to":"bf51134e_42ae6a98","updated":"2020-07-07 12:37:52.000000000","message":"i taught this was called elsewhere but you are right.\nthis is only called in paths where we have a devname.\n\ni will likely refactor this routine significatly as the corrent worklow is qudtratic.\n\nso im going to modify how the devices are retrived in _get_pci_passthrough_devices and pass the devices down to the other functions.\n\ngood catch.","commit_id":"ef79b979aee5571a1c03aa6288575ce45646005f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":131,"context_line":"from nova.volume import cinder"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"libvirt: ty.Any \u003d None"},{"line_number":134,"context_line":"NODEODEV_TYPE \u003d ty.Any"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"uefi_logged \u003d False"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_aa5e58e6","line":134,"range":{"start_line":134,"start_character":0,"end_line":134,"end_character":13},"updated":"2020-07-22 11:15:19.000000000","message":"Did you mean to call this NODEDEV_TYPE (no 0)?","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e6a0fc0caf3f15650ac20d6738ea1f8e998d0616","unresolved":false,"context_lines":[{"line_number":131,"context_line":"from nova.volume import cinder"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"libvirt: ty.Any \u003d None"},{"line_number":134,"context_line":"NODEODEV_TYPE \u003d ty.Any"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"uefi_logged \u003d False"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_b2271fb7","line":134,"range":{"start_line":134,"start_character":0,"end_line":134,"end_character":13},"in_reply_to":"bf51134e_aa5e58e6","updated":"2020-07-27 13:40:24.000000000","message":"yes","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f79a020631a7e7a31c9f0bbd0f03d387f254d326","unresolved":false,"context_lines":[{"line_number":6891,"context_line":"        cpu_info[\u0027features\u0027] \u003d features"},{"line_number":6892,"context_line":"        return cpu_info"},{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_1849935f","line":6894,"updated":"2020-07-22 14:48:22.000000000","message":"I\u0027m not sure why this needs to be a separate function anymore. Could we fold it into \u0027_get_device_capabilities\u0027 which is already partially unpacking the results of this?","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[{"line_number":6891,"context_line":"        cpu_info[\u0027features\u0027] \u003d features"},{"line_number":6892,"context_line":"        return cpu_info"},{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_16a51605","line":6894,"in_reply_to":"bf51134e_1849935f","updated":"2020-07-27 23:12:40.000000000","message":"i suspect it help with testing.\npulling it into _get_device_capabilities is proably ok i would not merge it further then that better to keep the functions small an easialy testable.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6892,"context_line":"        return cpu_info"},{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""},{"line_number":6898,"context_line":"        net_dev \u003d {dev.parent(): dev for dev in net_devs}.get(dev.name(), None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_caf46cf4","line":6895,"range":{"start_line":6895,"start_character":18,"end_line":6895,"end_character":37},"updated":"2020-07-22 11:15:19.000000000","message":"This is \u0027libvirt.virNodeDevice\u0027","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6892,"context_line":"        return cpu_info"},{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""},{"line_number":6898,"context_line":"        net_dev \u003d {dev.parent(): dev for dev in net_devs}.get(dev.name(), None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_4a2fbc87","line":6895,"range":{"start_line":6895,"start_character":8,"end_line":6895,"end_character":12},"updated":"2020-07-22 11:15:19.000000000","message":"nit","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48a1b3fe0dfe545c1e86c38954aa273890346a2b","unresolved":false,"context_lines":[{"line_number":6892,"context_line":"        return cpu_info"},{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""},{"line_number":6898,"context_line":"        net_dev \u003d {dev.parent(): dev for dev in net_devs}.get(dev.name(), None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_b2d27f33","line":6895,"range":{"start_line":6895,"start_character":8,"end_line":6895,"end_character":12},"in_reply_to":"9f560f44_523203f3","updated":"2020-07-27 13:48:56.000000000","message":"\u003e this is require for the hanning indent style i belive otherwise its\n \u003e a pep8 error but ill check\n\nNah, this is perfectly valid (and readable):\n\n  def hello(\n      self, stuff, here,\n  ):\n      todo_foo()","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0070e3e5a4a1203738c85ff408a58b3ed0f44763","unresolved":false,"context_lines":[{"line_number":6892,"context_line":"        return cpu_info"},{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""},{"line_number":6898,"context_line":"        net_dev \u003d {dev.parent(): dev for dev in net_devs}.get(dev.name(), None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_320dcf3e","line":6895,"range":{"start_line":6895,"start_character":8,"end_line":6895,"end_character":12},"in_reply_to":"9f560f44_b2d27f33","updated":"2020-07-27 14:22:55.000000000","message":"only if you put the closeing bracket on a new line","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e6a0fc0caf3f15650ac20d6738ea1f8e998d0616","unresolved":false,"context_lines":[{"line_number":6892,"context_line":"        return cpu_info"},{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""},{"line_number":6898,"context_line":"        net_dev \u003d {dev.parent(): dev for dev in net_devs}.get(dev.name(), None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_523203f3","line":6895,"range":{"start_line":6895,"start_character":8,"end_line":6895,"end_character":12},"in_reply_to":"bf51134e_4a2fbc87","updated":"2020-07-27 13:40:24.000000000","message":"this is require for the hanning indent style i belive otherwise its a pep8 error but ill check","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e6a0fc0caf3f15650ac20d6738ea1f8e998d0616","unresolved":false,"context_lines":[{"line_number":6892,"context_line":"        return cpu_info"},{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""},{"line_number":6898,"context_line":"        net_dev \u003d {dev.parent(): dev for dev in net_devs}.get(dev.name(), None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_32402f36","line":6895,"range":{"start_line":6895,"start_character":18,"end_line":6895,"end_character":37},"in_reply_to":"bf51134e_caf46cf4","updated":"2020-07-27 13:40:24.000000000","message":"yes i can put that above.\n\ni was trying to find it but the python binding really dont have good docs.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""},{"line_number":6898,"context_line":"        net_dev \u003d {dev.parent(): dev for dev in net_devs}.get(dev.name(), None)"},{"line_number":6899,"context_line":"        if net_dev is None:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_477a15ba","line":6896,"range":{"start_line":6896,"start_character":26,"end_line":6896,"end_character":62},"updated":"2020-07-22 11:15:19.000000000","message":"Could you drop this down to the next line and dedent? It makes things easier to read, IMO","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""},{"line_number":6898,"context_line":"        net_dev \u003d {dev.parent(): dev for dev in net_devs}.get(dev.name(), None)"},{"line_number":6899,"context_line":"        if net_dev is None:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_a7185184","line":6896,"range":{"start_line":6896,"start_character":22,"end_line":6896,"end_character":26},"updated":"2020-07-22 11:15:19.000000000","message":"This isn\u0027t a complete (or even valid?) type. It should read:\n\n  ty.List[\u0027libvirt.virNodeDevice\u0027]","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e6a0fc0caf3f15650ac20d6738ea1f8e998d0616","unresolved":false,"context_lines":[{"line_number":6893,"context_line":""},{"line_number":6894,"context_line":"    def _get_pcinet_info("},{"line_number":6895,"context_line":"            self, dev: NODEODEV_TYPE,"},{"line_number":6896,"context_line":"            net_devs: list) -\u003e ty.Optional[ty.Dict[str, str]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""},{"line_number":6898,"context_line":"        net_dev \u003d {dev.parent(): dev for dev in net_devs}.get(dev.name(), None)"},{"line_number":6899,"context_line":"        if net_dev is None:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_1266cbee","line":6896,"range":{"start_line":6896,"start_character":22,"end_line":6896,"end_character":26},"in_reply_to":"bf51134e_a7185184","updated":"2020-07-27 13:40:24.000000000","message":"list is valid it will match a list of anythig\nbut i just used list becasue i could not fine the actully typename for the nodedev \n\nif its libvirt.virtNodeDevice i can use that","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6902,"context_line":"        cfgdev \u003d vconfig.LibvirtConfigNodeDevice()"},{"line_number":6903,"context_line":"        cfgdev.parse_str(xmlstr)"},{"line_number":6904,"context_line":"        return {\u0027name\u0027: cfgdev.name,"},{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_ea3fb084","line":6905,"range":{"start_line":6905,"start_character":32,"end_line":6905,"end_character":62},"updated":"2020-07-22 11:15:19.000000000","message":"This is a list of string so your return type annotation is incorrect. It should read:\n\n  ty.Dict[str, ty.Union[str, ty.List[str]]]","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e6a0fc0caf3f15650ac20d6738ea1f8e998d0616","unresolved":false,"context_lines":[{"line_number":6902,"context_line":"        cfgdev \u003d vconfig.LibvirtConfigNodeDevice()"},{"line_number":6903,"context_line":"        cfgdev.parse_str(xmlstr)"},{"line_number":6904,"context_line":"        return {\u0027name\u0027: cfgdev.name,"},{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_b29a7fc2","line":6905,"range":{"start_line":6905,"start_character":32,"end_line":6905,"end_character":62},"in_reply_to":"bf51134e_ea3fb084","updated":"2020-07-27 13:40:24.000000000","message":"ya you are right. i was debating jus tmaking it ty.Dict but ill change ti to that.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_4a8b3c67","line":6908,"range":{"start_line":6908,"start_character":66,"end_line":6908,"end_character":76},"updated":"2020-07-22 11:15:19.000000000","message":"Can you drag this down?","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_0a91449b","line":6908,"range":{"start_line":6908,"start_character":71,"end_line":6908,"end_character":75},"updated":"2020-07-22 11:15:19.000000000","message":"This is ty.Dict[str, str]","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_ea7cd073","line":6908,"range":{"start_line":6908,"start_character":37,"end_line":6908,"end_character":50},"updated":"2020-07-22 11:15:19.000000000","message":"This should read\n\n  \u0027libvirt.virNodeDevice\u0027","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_2a96c895","line":6908,"range":{"start_line":6908,"start_character":62,"end_line":6908,"end_character":66},"updated":"2020-07-22 11:15:19.000000000","message":"This should read\n\n  ty.List[\u0027libvirt.virNodeDevice\u0027]","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_aae77846","line":6908,"range":{"start_line":6908,"start_character":8,"end_line":6908,"end_character":12},"updated":"2020-07-22 11:15:19.000000000","message":"nit","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_969e4661","line":6908,"range":{"start_line":6908,"start_character":66,"end_line":6908,"end_character":76},"in_reply_to":"9f560f44_d2ba93dc","updated":"2020-07-27 23:12:40.000000000","message":"as i said before dict is fully sufficent.\n\nmypy and type annotioant dont have to be fully qualified.\n\ndict is correct its not fully concret but its a valid type declaration. you dont actully need to to use any of the types form the typeing module.\n\nhttps://mypy.readthedocs.io/en/stable/kinds_of_types.html#class-types\n\ndict will match any object that is actually a dict\n\nty.Dict and ty.List are aliases for the built-ins dict and list, respectively.\nhttps://mypy.readthedocs.io/en/stable/builtin_types.html\n\nthis is why mypy actully passes on the current patch. i did run it before i pushed after all so i checked that the types passed.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c53e72817f4775f00acf35a58cf8e0d867bf4e40","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_d2ba93dc","line":6908,"range":{"start_line":6908,"start_character":66,"end_line":6908,"end_character":76},"in_reply_to":"9f560f44_f27d176f","updated":"2020-07-27 13:52:32.000000000","message":"\u003e honestly i kind of hate just have -\u003e dict\n \u003e on its own on a new line i find that much less readable\n\nFortunately it won\u0027t just be \u0027dict\u0027 because the type needs more work :)","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e6a0fc0caf3f15650ac20d6738ea1f8e998d0616","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_52b5e330","line":6908,"range":{"start_line":6908,"start_character":71,"end_line":6908,"end_character":75},"in_reply_to":"bf51134e_0a91449b","updated":"2020-07-27 13:40:24.000000000","message":"yes but dict should work too ill make it more specific","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e6a0fc0caf3f15650ac20d6738ea1f8e998d0616","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_f27d176f","line":6908,"range":{"start_line":6908,"start_character":66,"end_line":6908,"end_character":76},"in_reply_to":"bf51134e_4a8b3c67","updated":"2020-07-27 13:40:24.000000000","message":"honestly i kind of hate just have -\u003e dict\non its own on a new line i find that much less readable","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e6a0fc0caf3f15650ac20d6738ea1f8e998d0616","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: NODEODEV_TYPE, net_devs: list) -\u003e dict:"},{"line_number":6909,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6910,"context_line":""},{"line_number":6911,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_d282934f","line":6908,"range":{"start_line":6908,"start_character":8,"end_line":6908,"end_character":12},"in_reply_to":"bf51134e_aae77846","updated":"2020-07-27 13:40:24.000000000","message":"this is require by pep8","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e1de6edd3a008a64c4bc231282941aed4cc6a16e","unresolved":false,"context_lines":[{"line_number":7012,"context_line":"            devices \u003d {dev.name(): dev for dev in"},{"line_number":7013,"context_line":"                       self._host.list_all_nodedevs(flags\u003ddev_flags)}"},{"line_number":7014,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":7015,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":7016,"context_line":"            if error_code \u003d\u003d libvirt.VIR_ERR_NO_SUPPORT:"},{"line_number":7017,"context_line":"                self._list_devices_supported \u003d False"},{"line_number":7018,"context_line":"                LOG.warning(\"URI %(uri)s does not support \""},{"line_number":7019,"context_line":"                            \"listDevices: %(error)s\","},{"line_number":7020,"context_line":"                            {\u0027uri\u0027: self._uri(),"},{"line_number":7021,"context_line":"                             \u0027error\u0027: encodeutils.exception_to_unicode(ex)})"},{"line_number":7022,"context_line":"                return jsonutils.dumps([])"},{"line_number":7023,"context_line":"            else:"},{"line_number":7024,"context_line":"                raise"},{"line_number":7025,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_f8b81f12","line":7022,"range":{"start_line":7015,"start_character":0,"end_line":7022,"end_character":42},"updated":"2020-07-22 14:54:49.000000000","message":"You\u0027re already handling \u0027libvirtError\u0027 in \u0027list_all_nodedevs\u0027 so this will never get called. You should move this into that function","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[{"line_number":7012,"context_line":"            devices \u003d {dev.name(): dev for dev in"},{"line_number":7013,"context_line":"                       self._host.list_all_nodedevs(flags\u003ddev_flags)}"},{"line_number":7014,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":7015,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":7016,"context_line":"            if error_code \u003d\u003d libvirt.VIR_ERR_NO_SUPPORT:"},{"line_number":7017,"context_line":"                self._list_devices_supported \u003d False"},{"line_number":7018,"context_line":"                LOG.warning(\"URI %(uri)s does not support \""},{"line_number":7019,"context_line":"                            \"listDevices: %(error)s\","},{"line_number":7020,"context_line":"                            {\u0027uri\u0027: self._uri(),"},{"line_number":7021,"context_line":"                             \u0027error\u0027: encodeutils.exception_to_unicode(ex)})"},{"line_number":7022,"context_line":"                return jsonutils.dumps([])"},{"line_number":7023,"context_line":"            else:"},{"line_number":7024,"context_line":"                raise"},{"line_number":7025,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_76a8d21e","line":7022,"range":{"start_line":7015,"start_character":0,"end_line":7022,"end_character":42},"in_reply_to":"bf51134e_f8b81f12","updated":"2020-07-27 23:12:40.000000000","message":"ill just remove this its not useful.\ni know that all supported version of libvirt support this api as it from before libvirt 2.0 like 1.10 1.13 so it a really old api so we wont get  libvirt.VIR_ERR_NO_SUPPORT\n\nthis just re raises other errors so there is no point in having the try except at all.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":7023,"context_line":"            else:"},{"line_number":7024,"context_line":"                raise"},{"line_number":7025,"context_line":""},{"line_number":7026,"context_line":"        net_devices \u003d [dev for name, dev in"},{"line_number":7027,"context_line":"                       devices.items() if \"net\" in dev.listCaps()]"},{"line_number":7028,"context_line":"        pci_info \u003d [self._get_pcidev_info(name, dev, net_devices) for name, dev"},{"line_number":7029,"context_line":"                    in devices.items() if \"pci\" in dev.listCaps()]"},{"line_number":7030,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_eaa330ff","line":7027,"range":{"start_line":7026,"start_character":22,"end_line":7027,"end_character":66},"updated":"2020-07-22 11:15:19.000000000","message":"You don\u0027t use/need the name.\n\n  net_devices \u003d [dev for dev in devices.values() if \u0027net\u0027 in dev.listCaps()]","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[{"line_number":7023,"context_line":"            else:"},{"line_number":7024,"context_line":"                raise"},{"line_number":7025,"context_line":""},{"line_number":7026,"context_line":"        net_devices \u003d [dev for name, dev in"},{"line_number":7027,"context_line":"                       devices.items() if \"net\" in dev.listCaps()]"},{"line_number":7028,"context_line":"        pci_info \u003d [self._get_pcidev_info(name, dev, net_devices) for name, dev"},{"line_number":7029,"context_line":"                    in devices.items() if \"pci\" in dev.listCaps()]"},{"line_number":7030,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_3673fae9","line":7027,"range":{"start_line":7026,"start_character":22,"end_line":7027,"end_character":66},"in_reply_to":"bf51134e_eaa330ff","updated":"2020-07-27 23:12:40.000000000","message":"ya, i did at one point as i was doing a dict comprehension instead of a list comprehension but i forgot to remove it when i changed it.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":6891,"context_line":"        return cpu_info"},{"line_number":6892,"context_line":""},{"line_number":6893,"context_line":"    def _get_pcinet_info("},{"line_number":6894,"context_line":"            self, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6895,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6896,"context_line":"    ) -\u003e ty.Optional[ty.Dict[str, ty.Union[str, ty.List[str]]]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_32d01df8","line":6894,"range":{"start_line":6894,"start_character":8,"end_line":6894,"end_character":12},"updated":"2020-07-28 10:55:41.000000000","message":"nit","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":6891,"context_line":"        return cpu_info"},{"line_number":6892,"context_line":""},{"line_number":6893,"context_line":"    def _get_pcinet_info("},{"line_number":6894,"context_line":"            self, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6895,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6896,"context_line":"    ) -\u003e ty.Optional[ty.Dict[str, ty.Union[str, ty.List[str]]]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_52d791ee","line":6894,"range":{"start_line":6894,"start_character":18,"end_line":6894,"end_character":20},"updated":"2020-07-28 10:55:41.000000000","message":"style nit: any chance of putting this on its own line, pretty please? :)","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c77ef3ac9eca31b770ac34cc7d3723cdd021bb32","unresolved":false,"context_lines":[{"line_number":6891,"context_line":"        return cpu_info"},{"line_number":6892,"context_line":""},{"line_number":6893,"context_line":"    def _get_pcinet_info("},{"line_number":6894,"context_line":"            self, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6895,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6896,"context_line":"    ) -\u003e ty.Optional[ty.Dict[str, ty.Union[str, ty.List[str]]]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_75543fcb","line":6894,"range":{"start_line":6894,"start_character":18,"end_line":6894,"end_character":20},"in_reply_to":"9f560f44_52d791ee","updated":"2020-07-28 11:30:15.000000000","message":"if\ni\nmust\ni\ncan\nbut\non\nprinciple\ni\nthink\nthat\nis\nbad \nstyle \nto\ndo.\nplaceing\neach\nparamater\non\nits\nown\nline\nis\nhard\nto\nread\nand\nwastes\nvirtical\nspace\nfor\nno\nreason.\nDo\nyou\nnot\nagree?","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"783801d36cd47688dff8d38b1ac495f042a3e161","unresolved":false,"context_lines":[{"line_number":6891,"context_line":"        return cpu_info"},{"line_number":6892,"context_line":""},{"line_number":6893,"context_line":"    def _get_pcinet_info("},{"line_number":6894,"context_line":"            self, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6895,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6896,"context_line":"    ) -\u003e ty.Optional[ty.Dict[str, ty.Union[str, ty.List[str]]]]:"},{"line_number":6897,"context_line":"        \"\"\"Returns a dict of NET device.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_6edd8941","line":6894,"range":{"start_line":6894,"start_character":18,"end_line":6894,"end_character":20},"in_reply_to":"9f560f44_75543fcb","updated":"2020-07-28 17:12:06.000000000","message":"It\u0027s not just each parameter on its own line though. It\u0027s the parameter plus the type hint for that parameter. This becomes more obvious with more complex your signatures get. As an example, I personally find this:\n\n    def _notify_about_instance_usage(\n        self,\n        context: nova_context.RequestContext,\n        instance: \u0027objects.Instance\u0027,\n        event_suffix: str,\n        network_info: ty.Dict[str, ty.Any] \u003d None,\n        extra_usage_info: ty.Dict[str, ty.Any] \u003d None,\n        fault: Exception \u003d None,\n    ):\n        pass\n\nmuch easier to grok than this:\n\n    def _notify_about_instance_usage(\n        self, context: nova_context.RequestContext, instance: \u0027objects.Instance\u0027,\n        event_suffix: str, network_info: ty.Dict[str, ty.Any] \u003d None,\n        extra_usage_info: ty.Dict[str, ty.Any] \u003d None, fault: Exception \u003d None):\n        pass\n\nI mean, you can parse the first one pretty easily without syntax highlighting. I think it\u0027s just easier say always do things this way rather than trying to guess. You might end up wasting a bit of vertical space every now and then, but it\u0027s simpler and better in the general case.\n\n(and yes, I know you\u0027d suggest indenting the parameters in the second example further, but if you do that then you basically end up having to place everything on a separate line anyway)","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":6905,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6909,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]) -\u003e ty.Dict[str, str]:"},{"line_number":6910,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6911,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_32277d15","line":6908,"range":{"start_line":6908,"start_character":8,"end_line":6908,"end_character":12},"updated":"2020-07-28 10:55:41.000000000","message":"nit","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6909,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]) -\u003e ty.Dict[str, str]:"},{"line_number":6910,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6911,"context_line":""},{"line_number":6912,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_123b5938","line":6909,"range":{"start_line":6909,"start_character":59,"end_line":6909,"end_character":76},"updated":"2020-07-28 10:55:41.000000000","message":"I think this is wrong, since \u0027capabilties\u0027 as returned by \u0027_get_device_capabilities\u0027 is a list, right? Weird that mypy isn\u0027t spotting that","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c77ef3ac9eca31b770ac34cc7d3723cdd021bb32","unresolved":false,"context_lines":[{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _get_pcidev_info("},{"line_number":6908,"context_line":"            self, devname: str, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6909,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]) -\u003e ty.Dict[str, str]:"},{"line_number":6910,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6911,"context_line":""},{"line_number":6912,"context_line":"        def _get_device_type("}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_d5042b7e","line":6909,"range":{"start_line":6909,"start_character":59,"end_line":6909,"end_character":76},"in_reply_to":"9f560f44_123b5938","updated":"2020-07-28 11:30:15.000000000","message":"yes manually tracing it i think you are right\n\nit would be\nty.Dict[str,ty.Union[str, ty.Dict[str, ty.Union[str, ty.List[str], None]]]]\n\nbut at some point i think the types are going to get excessive so maybe its betere to just do\n\nty.Dict[str,ty.Union[str, ty.Dict]]\n\ne.g. a dict of strings to strings or dicts.\n\nsome level of type erasure i think is good.","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":6910,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6911,"context_line":""},{"line_number":6912,"context_line":"        def _get_device_type("},{"line_number":6913,"context_line":"                cfgdev: ty.Any, pci_address: str, device: ty.Any,"},{"line_number":6914,"context_line":"                net_devs: list) -\u003e ty.Dict[str, str]:"},{"line_number":6915,"context_line":"            \"\"\"Get a PCI device\u0027s device type."},{"line_number":6916,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_52f03191","line":6913,"range":{"start_line":6913,"start_character":58,"end_line":6913,"end_character":64},"updated":"2020-07-28 10:55:41.000000000","message":"this is \u0027libvirt.virNodeDevice\u0027","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":6910,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""},{"line_number":6911,"context_line":""},{"line_number":6912,"context_line":"        def _get_device_type("},{"line_number":6913,"context_line":"                cfgdev: ty.Any, pci_address: str, device: ty.Any,"},{"line_number":6914,"context_line":"                net_devs: list) -\u003e ty.Dict[str, str]:"},{"line_number":6915,"context_line":"            \"\"\"Get a PCI device\u0027s device type."},{"line_number":6916,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_72f3b59f","line":6913,"range":{"start_line":6913,"start_character":24,"end_line":6913,"end_character":30},"updated":"2020-07-28 10:55:41.000000000","message":"this is vconfig.LibvirtConfigNodeDevice","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":6911,"context_line":""},{"line_number":6912,"context_line":"        def _get_device_type("},{"line_number":6913,"context_line":"                cfgdev: ty.Any, pci_address: str, device: ty.Any,"},{"line_number":6914,"context_line":"                net_devs: list) -\u003e ty.Dict[str, str]:"},{"line_number":6915,"context_line":"            \"\"\"Get a PCI device\u0027s device type."},{"line_number":6916,"context_line":""},{"line_number":6917,"context_line":"            An assignable PCI device can be a normal PCI device,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_b22a6de4","line":6914,"range":{"start_line":6914,"start_character":26,"end_line":6914,"end_character":30},"updated":"2020-07-28 10:55:41.000000000","message":"This is ty.List[\u0027libvirt.virNodeDevice\u0027]","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":6947,"context_line":"            return {\u0027dev_type\u0027: fields.PciDeviceType.STANDARD}"},{"line_number":6948,"context_line":""},{"line_number":6949,"context_line":"        def _get_device_capabilities("},{"line_number":6950,"context_line":"                device_dict: dict, device: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6951,"context_line":"                net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6952,"context_line":"        ) -\u003e ty.Dict[str, ty.Dict[str, ty.Union[str, ty.List[str], None]]]:"},{"line_number":6953,"context_line":"            \"\"\"Get PCI VF device\u0027s additional capabilities."}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_d2eba1ad","line":6950,"range":{"start_line":6950,"start_character":12,"end_line":6950,"end_character":16},"updated":"2020-07-28 10:55:41.000000000","message":"nit","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8f0222a4c54e28573b4b91e7881510c930e59244","unresolved":false,"context_lines":[{"line_number":6906,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6907,"context_line":""},{"line_number":6908,"context_line":"    def _get_pcidev_info("},{"line_number":6909,"context_line":"        self, devname: str, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6910,"context_line":"        net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6911,"context_line":"    ) -\u003e ty.Dict[str, ty.Union[str, dict]]:"},{"line_number":6912,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_1b1ca476","line":6909,"range":{"start_line":6909,"start_character":27,"end_line":6909,"end_character":56},"updated":"2020-08-04 11:50:29.000000000","message":"nit: I would move this to the next line","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7019581573e59d432067163ab13c26d59e490c7e","unresolved":false,"context_lines":[{"line_number":6906,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6907,"context_line":""},{"line_number":6908,"context_line":"    def _get_pcidev_info("},{"line_number":6909,"context_line":"        self, devname: str, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6910,"context_line":"        net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6911,"context_line":"    ) -\u003e ty.Dict[str, ty.Union[str, dict]]:"},{"line_number":6912,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_f6be27b2","line":6909,"range":{"start_line":6909,"start_character":27,"end_line":6909,"end_character":56},"in_reply_to":"9f560f44_1b1ca476","updated":"2020-08-04 12:29:12.000000000","message":"i guess i could but i really dont like restirng to one paramter per line. not quite to the point of not useing type traits but its close.\n\ni dont think we should enfore one line per paramter when using type traits in general unless you need to wrap because of column limits.\n\nill do it just because you and stephen have both asked for it repeatidly but im not going to make this my default stytle unless we come to an agreement about this and someone writes a hacking check for this as i dont think we shoudl require this style.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"180f904967447fcc2006249467ba62406a123e1b","unresolved":false,"context_lines":[{"line_number":6906,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6907,"context_line":""},{"line_number":6908,"context_line":"    def _get_pcidev_info("},{"line_number":6909,"context_line":"        self, devname: str, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6910,"context_line":"        net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6911,"context_line":"    ) -\u003e ty.Dict[str, ty.Union[str, dict]]:"},{"line_number":6912,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_09e4593b","line":6909,"range":{"start_line":6909,"start_character":27,"end_line":6909,"end_character":56},"in_reply_to":"9f560f44_c604104d","updated":"2020-08-04 17:02:51.000000000","message":"i am changing it but be aware i am prepared to fight over this including rewiring the previous cases that have been merged if needed.\n\nI just dont want to have that fight here since i really need to stop working on this and work on numa in placment or just give up on doing that at all this cycle.\n\nim not even sure i have enough time left to work on it as it is this cycle but.\n\nfor what its worth the fact we dont use a tool to enforece style like autopep8 or black still insane to me.\n\ni don\u0027t like some of the choice made by black but we really don\u0027t need to spend time arguing over these things. but while its manual we will.\n\n\npep 484 which added type hints uses multiple argument per line with type hints\n\nhttps://www.python.org/dev/peps/pep-0484/#user-defined-generic-types\n\npep8 does have a section on function annotation but it does not express an opinion on this.\nhttps://www.python.org/dev/peps/pep-0008/#function-annotations\n\nso while i know its stephens prefered style and his recommendation i have not found peps or style guides that advocate for it \n\nit also does not correspond with the sytyle used in the mypy docs https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html#functions\n\nso as far as im aware the syle i am using is the one that mypy and pep 484 advnace and the 1 parmater per line style is not documented as a recommendation.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fe38237f8fe5bd58c38edc25d1ff5110eb698fd8","unresolved":false,"context_lines":[{"line_number":6906,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6907,"context_line":""},{"line_number":6908,"context_line":"    def _get_pcidev_info("},{"line_number":6909,"context_line":"        self, devname: str, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6910,"context_line":"        net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6911,"context_line":"    ) -\u003e ty.Dict[str, ty.Union[str, dict]]:"},{"line_number":6912,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_c604104d","line":6909,"range":{"start_line":6909,"start_character":27,"end_line":6909,"end_character":56},"in_reply_to":"9f560f44_c78fef72","updated":"2020-08-04 16:39:32.000000000","message":"It is a style recommendation and one I\u0027d asked for but wasn\u0027t willing fight over. If I\u0027m not alone though, can this be changed?","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"41574b5ba748ba5476aab409cd1375918e534d0e","unresolved":false,"context_lines":[{"line_number":6906,"context_line":"                \u0027capabilities\u0027: cfgdev.pci_capability.features}"},{"line_number":6907,"context_line":""},{"line_number":6908,"context_line":"    def _get_pcidev_info("},{"line_number":6909,"context_line":"        self, devname: str, dev: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6910,"context_line":"        net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6911,"context_line":"    ) -\u003e ty.Dict[str, ty.Union[str, dict]]:"},{"line_number":6912,"context_line":"        \"\"\"Returns a dict of PCI device.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_c78fef72","line":6909,"range":{"start_line":6909,"start_character":27,"end_line":6909,"end_character":56},"in_reply_to":"9f560f44_f6be27b2","updated":"2020-08-04 14:27:24.000000000","message":"OK. Don\u0027t worry about it. I saw Stephen doing this one parameter per line when he added the type annotations so I assumed that this is some kind of style recommendation. Since Stephen is +2 I guess I was wrong and it is just a personal style for him.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8f0222a4c54e28573b4b91e7881510c930e59244","unresolved":false,"context_lines":[{"line_number":6913,"context_line":""},{"line_number":6914,"context_line":"        def _get_device_type("},{"line_number":6915,"context_line":"                cfgdev: vconfig.LibvirtConfigNodeDevice,"},{"line_number":6916,"context_line":"                pci_address: str, device: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6917,"context_line":"                net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6918,"context_line":"        ) -\u003e ty.Dict[str, str]:"},{"line_number":6919,"context_line":"            \"\"\"Get a PCI device\u0027s device type."}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_bb29389c","line":6916,"range":{"start_line":6916,"start_character":34,"end_line":6916,"end_character":64},"updated":"2020-08-04 11:50:29.000000000","message":"I would move this to the next line","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fe38237f8fe5bd58c38edc25d1ff5110eb698fd8","unresolved":false,"context_lines":[{"line_number":6913,"context_line":""},{"line_number":6914,"context_line":"        def _get_device_type("},{"line_number":6915,"context_line":"                cfgdev: vconfig.LibvirtConfigNodeDevice,"},{"line_number":6916,"context_line":"                pci_address: str, device: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6917,"context_line":"                net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6918,"context_line":"        ) -\u003e ty.Dict[str, str]:"},{"line_number":6919,"context_line":"            \"\"\"Get a PCI device\u0027s device type."}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_e6a5d448","line":6916,"range":{"start_line":6916,"start_character":34,"end_line":6916,"end_character":64},"in_reply_to":"9f560f44_bb29389c","updated":"2020-08-04 16:39:32.000000000","message":"and dedent things, since the double indent isn\u0027t necessary","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8f0222a4c54e28573b4b91e7881510c930e59244","unresolved":false,"context_lines":[{"line_number":6951,"context_line":"            return {\u0027dev_type\u0027: fields.PciDeviceType.STANDARD}"},{"line_number":6952,"context_line":""},{"line_number":6953,"context_line":"        def _get_device_capabilities("},{"line_number":6954,"context_line":"            device_dict: dict, device: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6955,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6956,"context_line":"        ) -\u003e ty.Dict[str, ty.Dict[str, ty.Union[str, ty.List[str], None]]]:"},{"line_number":6957,"context_line":"            \"\"\"Get PCI VF device\u0027s additional capabilities."}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_7b0cc040","line":6954,"range":{"start_line":6954,"start_character":31,"end_line":6954,"end_character":63},"updated":"2020-08-04 11:50:29.000000000","message":"ditto","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8f0222a4c54e28573b4b91e7881510c930e59244","unresolved":false,"context_lines":[{"line_number":6953,"context_line":"        def _get_device_capabilities("},{"line_number":6954,"context_line":"            device_dict: dict, device: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6955,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6956,"context_line":"        ) -\u003e ty.Dict[str, ty.Dict[str, ty.Union[str, ty.List[str], None]]]:"},{"line_number":6957,"context_line":"            \"\"\"Get PCI VF device\u0027s additional capabilities."},{"line_number":6958,"context_line":""},{"line_number":6959,"context_line":"            If a PCI device is a virtual function, this function reads the PCI"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_b6baef06","line":6956,"range":{"start_line":6956,"start_character":48,"end_line":6956,"end_character":71},"updated":"2020-08-04 11:50:29.000000000","message":"Do we really have all of these types in the inner dict? It feels strange that there can be str or List[str] as well.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6fb49dc576898e430b869e0935d764d6b81f713e","unresolved":false,"context_lines":[{"line_number":6953,"context_line":"        def _get_device_capabilities("},{"line_number":6954,"context_line":"            device_dict: dict, device: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6955,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6956,"context_line":"        ) -\u003e ty.Dict[str, ty.Dict[str, ty.Union[str, ty.List[str], None]]]:"},{"line_number":6957,"context_line":"            \"\"\"Get PCI VF device\u0027s additional capabilities."},{"line_number":6958,"context_line":""},{"line_number":6959,"context_line":"            If a PCI device is a virtual function, this function reads the PCI"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_fdbf1ba7","line":6956,"range":{"start_line":6956,"start_character":48,"end_line":6956,"end_character":71},"in_reply_to":"9f560f44_47f11f01","updated":"2020-08-04 22:25:15.000000000","message":"so appently it cant be removed\n\nerror: Dict entry 0 has incompatible type \"str\": \"Union[str, List[str], None]\"; expected \"s\ntr\": \"Optional[List[str]]\"\n\nthe reason is _get_pcinet_info returns \n\n{\u0027name\u0027: cfgdev.name,\n\u0027capabilities\u0027: cfgdev.pci_capability.features}\n\ncfgdev.name is a sting \ncfgdev.pci_capability.features is a list of strings\n\nwe only ever use the capablities not the name entry but mypy cant understnad that and isnce we coudl be retiving a string or a list of strings it require str in the union\n\nwe only use name in one unit test so im going to revome it.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"41574b5ba748ba5476aab409cd1375918e534d0e","unresolved":false,"context_lines":[{"line_number":6953,"context_line":"        def _get_device_capabilities("},{"line_number":6954,"context_line":"            device_dict: dict, device: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6955,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6956,"context_line":"        ) -\u003e ty.Dict[str, ty.Dict[str, ty.Union[str, ty.List[str], None]]]:"},{"line_number":6957,"context_line":"            \"\"\"Get PCI VF device\u0027s additional capabilities."},{"line_number":6958,"context_line":""},{"line_number":6959,"context_line":"            If a PCI device is a virtual function, this function reads the PCI"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_47f11f01","line":6956,"range":{"start_line":6956,"start_character":48,"end_line":6956,"end_character":71},"in_reply_to":"9f560f44_b61cafc1","updated":"2020-08-04 14:27:24.000000000","message":"Yeah, if str can be removed then the remaining type seems OK to me.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7019581573e59d432067163ab13c26d59e490c7e","unresolved":false,"context_lines":[{"line_number":6953,"context_line":"        def _get_device_capabilities("},{"line_number":6954,"context_line":"            device_dict: dict, device: \u0027libvirt.virNodeDevice\u0027,"},{"line_number":6955,"context_line":"            net_devs: ty.List[\u0027libvirt.virNodeDevice\u0027]"},{"line_number":6956,"context_line":"        ) -\u003e ty.Dict[str, ty.Dict[str, ty.Union[str, ty.List[str], None]]]:"},{"line_number":6957,"context_line":"            \"\"\"Get PCI VF device\u0027s additional capabilities."},{"line_number":6958,"context_line":""},{"line_number":6959,"context_line":"            If a PCI device is a virtual function, this function reads the PCI"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_b61cafc1","line":6956,"range":{"start_line":6956,"start_character":48,"end_line":6956,"end_character":71},"in_reply_to":"9f560f44_b6baef06","updated":"2020-08-04 12:29:12.000000000","message":"actully this is alos left over form a refactor\n\nthis function will return an ty.Dict[str,ty.Union[ty.List[str],None]]]\n\nsince it returns  either\n{\u0027capabilities\u0027:{\u0027network\u0027: pcinet_info.get(\u0027capabilities\u0027)}}\n\npcinet_info.get(\u0027capabilities\u0027) is a list of stings.\n\nif we dont get back pcinet_info we return\n{} so we need to include none for this case\n\nwe do have cases where dicts contain both str-\u003estr and str-\u003elist[str] and can be just an empty dict too but this is not one of them.\n\ni can remove str form the union.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8f0222a4c54e28573b4b91e7881510c930e59244","unresolved":false,"context_lines":[{"line_number":6960,"context_line":"            parent\u0027s network capabilities (must be always a NIC device) and"},{"line_number":6961,"context_line":"            appends this information to the device\u0027s dictionary."},{"line_number":6962,"context_line":"            \"\"\""},{"line_number":6963,"context_line":"            if device_dict.get(\u0027dev_type\u0027) in fields.PciDeviceType.SRIOV_VF:"},{"line_number":6964,"context_line":"                pcinet_info \u003d self._get_pcinet_info(device, net_devs)"},{"line_number":6965,"context_line":"                if pcinet_info:"},{"line_number":6966,"context_line":"                    return {\u0027capabilities\u0027:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_563f1386","line":6963,"range":{"start_line":6963,"start_character":43,"end_line":6963,"end_character":75},"updated":"2020-08-04 11:50:29.000000000","message":"why did you change the equality to containment? fields.PciDeviceType.SRIOV_VF is a string as wel as dev_type.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7019581573e59d432067163ab13c26d59e490c7e","unresolved":false,"context_lines":[{"line_number":6960,"context_line":"            parent\u0027s network capabilities (must be always a NIC device) and"},{"line_number":6961,"context_line":"            appends this information to the device\u0027s dictionary."},{"line_number":6962,"context_line":"            \"\"\""},{"line_number":6963,"context_line":"            if device_dict.get(\u0027dev_type\u0027) in fields.PciDeviceType.SRIOV_VF:"},{"line_number":6964,"context_line":"                pcinet_info \u003d self._get_pcinet_info(device, net_devs)"},{"line_number":6965,"context_line":"                if pcinet_info:"},{"line_number":6966,"context_line":"                    return {\u0027capabilities\u0027:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_36389f3b","line":6963,"range":{"start_line":6963,"start_character":43,"end_line":6963,"end_character":75},"in_reply_to":"9f560f44_563f1386","updated":"2020-08-04 12:29:12.000000000","message":"oh good catch.\ni had change this to consider all sriov devices e.g. also work with PFs but then reverted it but obviously not fully.\n\ni did not want to change the actul behavior in this patch which is why i reverted this. ill change it back to a comparison.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8f0222a4c54e28573b4b91e7881510c930e59244","unresolved":false,"context_lines":[{"line_number":7009,"context_line":"        \"\"\""},{"line_number":7010,"context_line":"        # Bail early if we know we can\u0027t support `listDevices` to avoid"},{"line_number":7011,"context_line":"        # repeated warnings within a periodic task"},{"line_number":7012,"context_line":"        if not getattr(self, \u0027_list_devices_supported\u0027, True):"},{"line_number":7013,"context_line":"            return jsonutils.dumps([])"},{"line_number":7014,"context_line":""},{"line_number":7015,"context_line":"        dev_flags \u003d (libvirt.VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET |"},{"line_number":7016,"context_line":"                     libvirt.VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV)"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_b6178f45","line":7013,"range":{"start_line":7012,"start_character":1,"end_line":7013,"end_character":38},"updated":"2020-08-04 11:50:29.000000000","message":"You have removed the only place where _list_devices_supported was set (L7017 in the original code). So this can also be removed.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7019581573e59d432067163ab13c26d59e490c7e","unresolved":false,"context_lines":[{"line_number":7009,"context_line":"        \"\"\""},{"line_number":7010,"context_line":"        # Bail early if we know we can\u0027t support `listDevices` to avoid"},{"line_number":7011,"context_line":"        # repeated warnings within a periodic task"},{"line_number":7012,"context_line":"        if not getattr(self, \u0027_list_devices_supported\u0027, True):"},{"line_number":7013,"context_line":"            return jsonutils.dumps([])"},{"line_number":7014,"context_line":""},{"line_number":7015,"context_line":"        dev_flags \u003d (libvirt.VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET |"},{"line_number":7016,"context_line":"                     libvirt.VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV)"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_96d1ab26","line":7013,"range":{"start_line":7012,"start_character":1,"end_line":7013,"end_character":38},"in_reply_to":"9f560f44_b6178f45","updated":"2020-08-04 12:29:12.000000000","message":"ah good point.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"}],"nova/virt/libvirt/host.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"            else:"},{"line_number":1179,"context_line":"                raise"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_b5f9ac4e","line":1181,"range":{"start_line":1181,"start_character":32,"end_line":1181,"end_character":64},"updated":"2020-07-02 20:55:34.000000000","message":"Are you planning to backport this change? If so, I think you should leave out python3-specific changes. Otherwise, you\u0027d have to change all of the python3-specific stuff for the backports, which will be messy.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"51201aeeaf4c31e799fde9c69b39148093a5030f","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"            else:"},{"line_number":1179,"context_line":"                raise"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_9ad2c533","line":1181,"range":{"start_line":1181,"start_character":32,"end_line":1181,"end_character":64},"in_reply_to":"bf51134e_26d571ba","updated":"2020-07-05 05:58:50.000000000","message":"\u003e Don\u0027t forget Train is Python 3 only too, so we only need to start\n \u003e worrying about this if we go back further than that...at which\n \u003e point we\u0027ll likely have merge conflicts and their ilk to worry\n \u003e about anyway\n\nI hadn\u0027t thought so ... but I don\u0027t know where to look for documentation. I saw that we still have py27 jobs running on stable/train so that made me think we support it for train. Example change:\n\nhttps://review.opendev.org/738455","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1667221f5c966dd0ce37c1b50b2c802ce7138204","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"            else:"},{"line_number":1179,"context_line":"                raise"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_865efd42","line":1181,"range":{"start_line":1181,"start_character":32,"end_line":1181,"end_character":64},"in_reply_to":"bf51134e_26d571ba","updated":"2020-07-03 10:07:06.000000000","message":"ya im planing to backport to train but im not sure ill backport before that. certenly not all the way back to pike but i dont think this will present on older release. i think this bug is kernel/libvirt/dirver dependent and it has not come up until recently so i dont think it extis on older releases.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"aff0312fb5593f762d0bc117f09583ea7f07d2cd","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"            else:"},{"line_number":1179,"context_line":"                raise"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_d2753ace","line":1181,"range":{"start_line":1181,"start_character":32,"end_line":1181,"end_character":64},"in_reply_to":"bf51134e_763545da","updated":"2020-07-06 13:17:14.000000000","message":"I\u0027m honestly on the same page with mel here. I honestly think the benefits/risks balance is so small that I don\u0027t see *why* we should provide type verifications everytime.\n\nThat being sad, if folks want to do it, okay but then that would mean that it would need a bit more reviews for backports.\n\nAnyway, I\u0027m not THAT opiniated that I\u0027d -1 for this but honestly I don\u0027t like it.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3c7aa3890a782759ffc63245939dcfacac4c4e7c","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"            else:"},{"line_number":1179,"context_line":"                raise"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_763545da","line":1181,"range":{"start_line":1181,"start_character":32,"end_line":1181,"end_character":64},"in_reply_to":"bf51134e_9ad2c533","updated":"2020-07-06 10:08:01.000000000","message":"stephen ment downstream\n\nhttps://opendev.org/openstack/governance/src/branch/master/reference/runtimes/train.rst#user-content-python-runtime-for-train\n\nupstream we droped support in ussuri so train still neeeds to support py2.7","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d24d087de42c5b9f54971b6c2023420037f86b26","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"            else:"},{"line_number":1179,"context_line":"                raise"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_26d571ba","line":1181,"range":{"start_line":1181,"start_character":32,"end_line":1181,"end_character":64},"in_reply_to":"bf51134e_a673414d","updated":"2020-07-03 09:58:45.000000000","message":"Don\u0027t forget Train is Python 3 only too, so we only need to start worrying about this if we go back further than that...at which point we\u0027ll likely have merge conflicts and their ilk to worry about anyway","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1ddc416a66a8e6fe92e4647df21bdedcb576c436","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"            else:"},{"line_number":1179,"context_line":"                raise"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_a673414d","line":1181,"range":{"start_line":1181,"start_character":32,"end_line":1181,"end_character":64},"in_reply_to":"bf51134e_b5f9ac4e","updated":"2020-07-03 09:26:27.000000000","message":"i would prefer to make it our policy that we _do_ make the python3 change when we backport. i dont want us to start a patteren of writing worse code because we are going to backport something. the type hints are tivial to remove.\n\ni can put this in a seperate patch but i honestly think that is the wrong way to do things.","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param"},{"line_number":1185,"context_line":"        :returns: a list of virNodeDevice instance"},{"line_number":1186,"context_line":"        \"\"\""},{"line_number":1187,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_1594d8e7","line":1184,"updated":"2020-07-02 20:55:34.000000000","message":"Fill in the param?","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1ddc416a66a8e6fe92e4647df21bdedcb576c436","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param"},{"line_number":1185,"context_line":"        :returns: a list of virNodeDevice instance"},{"line_number":1186,"context_line":"        \"\"\""},{"line_number":1187,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_867c3d21","line":1184,"in_reply_to":"bf51134e_1594d8e7","updated":"2020-07-03 09:26:27.000000000","message":"ya i must have got distracted.\nalthough with the type traits its a little less needed.\nits still nice to have a description but type info is no longer needed in the commets","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b2f67481e737858955de1548b93fefc2b0abd861","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"        :param flags: a bit mask of flags to filter the retruned devices"},{"line_number":1185,"context_line":"        :returns: a list of virNodeDevice instance"},{"line_number":1186,"context_line":"        \"\"\""},{"line_number":1187,"context_line":"        try:"},{"line_number":1188,"context_line":"            return self.get_connection().virConnectListAllNodeDevices(flags)"},{"line_number":1189,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":1190,"context_line":"            LOG.warning(ex)"},{"line_number":1191,"context_line":"            return []"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_9c2acc8d","line":1188,"range":{"start_line":1187,"start_character":11,"end_line":1188,"end_character":76},"updated":"2020-07-03 13:01:00.000000000","message":"this is failing but im not sure why\n\nAttributeError: \u0027virConnect\u0027 object has no attribute \u0027virConnectListAllNodeDevices\u0027\n...\n\nhttps://github.com/libvirt/libvirt-python/blob/5ee6b3ac262a4bf663b35e2617bbfc51329b59df/sanitytest.py#L293-L299\n\nits renamaed the complete lack of python binding docs sucks.\n\nvirConnectListAllNodeDevices becomes listAllDevices","commit_id":"e27ee25f389fe4acb684d8b459656d4026e5d0b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e1de6edd3a008a64c4bc231282941aed4cc6a16e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"bf51134e_f84dbf3b","updated":"2020-07-22 14:54:49.000000000","message":"You haven\u0027t added this file to list in the \u0027mypy-files.txt\u0027","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c53e72817f4775f00acf35a58cf8e0d867bf4e40","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9f560f44_928e5b2c","in_reply_to":"9f560f44_52e32334","updated":"2020-07-27 13:52:32.000000000","message":"Unfortunately not. While you don\u0027t need to annotate everything (far from it), there are some minimal annotations needed to clear up errors for most files. Also, we have to figure out a way to enable mypy for more magic things like o.vo","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9f560f44_39f64b91","in_reply_to":"9f560f44_928e5b2c","updated":"2020-07-27 23:12:40.000000000","message":"actually there are a bunch of unrelated errors if i enable this file so lets do that in a separate patch.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e6a0fc0caf3f15650ac20d6738ea1f8e998d0616","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9f560f44_52e32334","in_reply_to":"bf51134e_f84dbf3b","updated":"2020-07-27 13:40:24.000000000","message":"ah ok ill do that. is there a way we can do this without needing that file?\ni would prefer if mypy ran on all files but only checked type where set","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e1de6edd3a008a64c4bc231282941aed4cc6a16e","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"            else:"},{"line_number":1179,"context_line":"                raise"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param flags: a bit mask of flags to filter the retruned devices"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_78424f11","line":1181,"range":{"start_line":1181,"start_character":8,"end_line":1181,"end_character":25},"updated":"2020-07-22 14:54:49.000000000","message":"Can we call this \u0027list_all_devices\u0027 to mirror the libvirt name?","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"            else:"},{"line_number":1179,"context_line":"                raise"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param flags: a bit mask of flags to filter the retruned devices"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_ca710cab","line":1181,"range":{"start_line":1181,"start_character":59,"end_line":1181,"end_character":62},"updated":"2020-07-22 11:15:19.000000000","message":"This is wrong. listAllDevices returns a list of \u0027libvirt.virNodeDevice\u0027. It should read:\n\n  ty.List[\u0027libvirt.virNodeDevice\u0027]\n\n(you want the strings to delay interpolation since libvirt isn\u0027t necessarily defined)","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param flags: a bit mask of flags to filter the retruned devices"},{"line_number":1185,"context_line":"        :returns: a list of virNodeDevice xml strings."},{"line_number":1186,"context_line":"        \"\"\""},{"line_number":1187,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_47017506","line":1184,"range":{"start_line":1184,"start_character":24,"end_line":1184,"end_character":32},"updated":"2020-07-22 11:15:19.000000000","message":"bitmask","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param flags: a bit mask of flags to filter the retruned devices"},{"line_number":1185,"context_line":"        :returns: a list of virNodeDevice xml strings."},{"line_number":1186,"context_line":"        \"\"\""},{"line_number":1187,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_8a29d48a","line":1184,"range":{"start_line":1184,"start_character":65,"end_line":1184,"end_character":72},"updated":"2020-07-22 11:15:19.000000000","message":"devices by.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param flags: a bit mask of flags to filter the retruned devices"},{"line_number":1185,"context_line":"        :returns: a list of virNodeDevice xml strings."},{"line_number":1186,"context_line":"        \"\"\""},{"line_number":1187,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_67063923","line":1184,"range":{"start_line":1184,"start_character":56,"end_line":1184,"end_character":64},"updated":"2020-07-22 11:15:19.000000000","message":"returned","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"    def list_all_nodedevs(self, flags: int \u003d 0) -\u003e ty.List[str]:"},{"line_number":1182,"context_line":"        \"\"\"Lookup devices."},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"        :param flags: a bit mask of flags to filter the retruned devices"},{"line_number":1185,"context_line":"        :returns: a list of virNodeDevice xml strings."},{"line_number":1186,"context_line":"        \"\"\""},{"line_number":1187,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_d9bbcfe2","line":1184,"range":{"start_line":1184,"start_character":65,"end_line":1184,"end_character":72},"in_reply_to":"bf51134e_8a29d48a","updated":"2020-07-27 23:12:40.000000000","message":"by is not correct.\n\nthere is a specific term for placeholder words that exist in some lanaguage but provide no additional information. \nby in this context is a padding word and that grammatical constitution is actually coming from irish.\nit makes sense to me but i belive its more correct to not include by.\n\nGrammly flags it as an \"Inappropriate Colloquialisms\"\nsince i dont have premium it wont actully tell me the precise rule.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"        try:"},{"line_number":1188,"context_line":"            return self.get_connection().listAllDevices(flags) or []"},{"line_number":1189,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":1190,"context_line":"            LOG.warning(ex)"},{"line_number":1191,"context_line":"            return []"},{"line_number":1192,"context_line":""},{"line_number":1193,"context_line":"    def compare_cpu(self, xmlDesc, flags\u003d0):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_6a3440df","line":1190,"updated":"2020-07-22 11:15:19.000000000","message":"Can you give a more detailed error message like the function above?","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"        try:"},{"line_number":1188,"context_line":"            return self.get_connection().listAllDevices(flags) or []"},{"line_number":1189,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":1190,"context_line":"            LOG.warning(ex)"},{"line_number":1191,"context_line":"            return []"},{"line_number":1192,"context_line":""},{"line_number":1193,"context_line":"    def compare_cpu(self, xmlDesc, flags\u003d0):"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_79c6036a","line":1190,"in_reply_to":"bf51134e_6a3440df","updated":"2020-07-27 23:12:40.000000000","message":"not really.\n\ni dont actully expect this to raise an excption.\nif it does it will be dupe to libvirt dying or some other issue that we cant predict.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":60,"context_line":"from nova.virt.libvirt import migration as libvirt_migrate"},{"line_number":61,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"libvirt: ty.Any \u003d None"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_d28721d7","line":63,"updated":"2020-07-28 10:55:41.000000000","message":"You can fix this by doing:\n\n  if ty.TYPE_CHECKING:\n      import libvirt\n  else:\n      libvirt \u003d None\n\nAnd that way we get type checking for free when libvirt adds type hints [1]\n\n[1] https://gitlab.com/libvirt/libvirt-python/-/merge_requests/9","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fe38237f8fe5bd58c38edc25d1ff5110eb698fd8","unresolved":false,"context_lines":[{"line_number":60,"context_line":"from nova.virt.libvirt import migration as libvirt_migrate"},{"line_number":61,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"libvirt: ty.Any \u003d None"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_e9164549","line":63,"in_reply_to":"9f560f44_1566a3ee","updated":"2020-08-04 16:39:32.000000000","message":"Fair. I only realized this approach worked when experimenting. I\u0027ll go back and update \u0027driver.py\u0027 at some point.","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c77ef3ac9eca31b770ac34cc7d3723cdd021bb32","unresolved":false,"context_lines":[{"line_number":60,"context_line":"from nova.virt.libvirt import migration as libvirt_migrate"},{"line_number":61,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"libvirt: ty.Any \u003d None"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_1566a3ee","line":63,"in_reply_to":"9f560f44_d28721d7","updated":"2020-07-28 11:30:15.000000000","message":"ya i could why do we not do this in driver.py\ni coppied what we did there for here.","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        self._read_only \u003d read_only"},{"line_number":94,"context_line":"        self._initial_connection \u003d True"},{"line_number":95,"context_line":"        self._conn_event_handler \u003d conn_event_handler"},{"line_number":96,"context_line":"        self._conn_event_handler_queue: ty.Any \u003d six.moves.queue.Queue()"},{"line_number":97,"context_line":"        self._lifecycle_event_handler \u003d lifecycle_event_handler"},{"line_number":98,"context_line":"        self._caps \u003d None"},{"line_number":99,"context_line":"        self._domain_caps \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_92664951","line":96,"range":{"start_line":96,"start_character":40,"end_line":96,"end_character":46},"updated":"2020-07-28 10:55:41.000000000","message":"This is \u0027queue.Queue[ty.Callable]\u0027","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c77ef3ac9eca31b770ac34cc7d3723cdd021bb32","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        self._read_only \u003d read_only"},{"line_number":94,"context_line":"        self._initial_connection \u003d True"},{"line_number":95,"context_line":"        self._conn_event_handler \u003d conn_event_handler"},{"line_number":96,"context_line":"        self._conn_event_handler_queue: ty.Any \u003d six.moves.queue.Queue()"},{"line_number":97,"context_line":"        self._lifecycle_event_handler \u003d lifecycle_event_handler"},{"line_number":98,"context_line":"        self._caps \u003d None"},{"line_number":99,"context_line":"        self._domain_caps \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_75639fdd","line":96,"range":{"start_line":96,"start_character":40,"end_line":96,"end_character":46},"in_reply_to":"9f560f44_92664951","updated":"2020-07-28 11:30:15.000000000","message":"i was really really tempted to remove the six.moves and just make it that. i know that will be the type on python 3 but","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        self._wrapped_conn \u003d None"},{"line_number":103,"context_line":"        self._wrapped_conn_lock \u003d threading.Lock()"},{"line_number":104,"context_line":"        self._event_queue: ty.Any \u003d None"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        self._events_delayed \u003d {}"},{"line_number":107,"context_line":"        # Note(toabctl): During a reboot of a domain, STOPPED and"},{"line_number":108,"context_line":"        #                STARTED events are sent. To prevent shutting"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_12d7d9c1","line":105,"updated":"2020-07-28 10:55:41.000000000","message":"You had to do this because \u0027_queue_event\u0027 was complaining about None type, right? The better solution is to add\n\n  if self._event_queue is None:\n      return\n\nto the top of that function, which we should be doing anyway for safety","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fe38237f8fe5bd58c38edc25d1ff5110eb698fd8","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        self._wrapped_conn \u003d None"},{"line_number":103,"context_line":"        self._wrapped_conn_lock \u003d threading.Lock()"},{"line_number":104,"context_line":"        self._event_queue: ty.Any \u003d None"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        self._events_delayed \u003d {}"},{"line_number":107,"context_line":"        # Note(toabctl): During a reboot of a domain, STOPPED and"},{"line_number":108,"context_line":"        #                STARTED events are sent. To prevent shutting"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_6616e457","line":105,"in_reply_to":"9f560f44_10f9634c","updated":"2020-08-04 16:39:32.000000000","message":"Sorry, I meant \u0027_dispatch_events\u0027. That\u0027s where we I was expecting the check I described to go. That doesn\u0027t have the check at the moment because the caller is expected to call \u0027_init_events_pipe\u0027 first, however, there\u0027s nothing structural in the code that enforces this.","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a33c965c62411606495f823f1fb2ccb5f207c5eb","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        self._wrapped_conn \u003d None"},{"line_number":103,"context_line":"        self._wrapped_conn_lock \u003d threading.Lock()"},{"line_number":104,"context_line":"        self._event_queue: ty.Any \u003d None"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        self._events_delayed \u003d {}"},{"line_number":107,"context_line":"        # Note(toabctl): During a reboot of a domain, STOPPED and"},{"line_number":108,"context_line":"        #                STARTED events are sent. To prevent shutting"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_10f9634c","line":105,"in_reply_to":"9f560f44_12d7d9c1","updated":"2020-08-04 11:20:03.000000000","message":"_queue_event already has this check","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a33c965c62411606495f823f1fb2ccb5f207c5eb","unresolved":false,"context_lines":[{"line_number":292,"context_line":"        green thread. Any use of logging APIs is forbidden."},{"line_number":293,"context_line":"        \"\"\""},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        if self._event_queue is None:"},{"line_number":296,"context_line":"            return"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        # Queue the event..."},{"line_number":299,"context_line":"        self._event_queue.put(event)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_6e7600a7","line":296,"range":{"start_line":295,"start_character":8,"end_line":296,"end_character":18},"updated":"2020-08-04 11:20:03.000000000","message":"see its already doing that check.","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79771721448c26040e40d1415fa8003a999c2344","unresolved":false,"context_lines":[{"line_number":810,"context_line":"        if self._domain_caps:"},{"line_number":811,"context_line":"            return self._domain_caps"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":"        domain_caps: ty.Dict \u003d defaultdict(dict)"},{"line_number":814,"context_line":"        caps \u003d self.get_capabilities()"},{"line_number":815,"context_line":"        virt_type \u003d CONF.libvirt.virt_type"},{"line_number":816,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_d260c135","line":813,"range":{"start_line":813,"start_character":21,"end_line":813,"end_character":28},"updated":"2020-07-28 10:55:41.000000000","message":"Is this valid? If not, this is\n\n  ty.Dict[str, ty.Dict[str, ty.Any]]\n\n(I mean, we could get more detailed but that\u0027s good enough for now)","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c77ef3ac9eca31b770ac34cc7d3723cdd021bb32","unresolved":false,"context_lines":[{"line_number":810,"context_line":"        if self._domain_caps:"},{"line_number":811,"context_line":"            return self._domain_caps"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":"        domain_caps: ty.Dict \u003d defaultdict(dict)"},{"line_number":814,"context_line":"        caps \u003d self.get_capabilities()"},{"line_number":815,"context_line":"        virt_type \u003d CONF.libvirt.virt_type"},{"line_number":816,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_d218c182","line":813,"range":{"start_line":813,"start_character":21,"end_line":813,"end_character":28},"in_reply_to":"9f560f44_d260c135","updated":"2020-07-28 11:30:15.000000000","message":"yes its valid, pythons type checking does not require fully conncreate types and also does duck typeing the same way the actually python types do.\n\ntype.Dict can basically be used for any valid mapping or dict like class. so defauldict or orderdeddict will work with ty.Dict or just dict\n\nthe typeing module in general does not require that classes by subclasses of the type you define just that its matches its interface which is true of defaultdict in the case of ty.Dict.\n\n ty.Dict[str, ty.Dict[str, ty.Any]] might work if we are using sting keys\n\nalthough  ty.Dict[ty.Any, ty.Dict[ty.Any, ty.Any]] is proably more correct.\n\nthough i think you are right that later we use this with sting keys. default dict has the same key constraints as a normal dicts specifically the key just has to be hasable.\n\ni can update this but ty.Dict is the minimal valid type trait well minimal beyond ty.any.","commit_id":"479c467746c15e77ed921067581bd0ec1af1c926"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a33c965c62411606495f823f1fb2ccb5f207c5eb","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        self._read_only \u003d read_only"},{"line_number":98,"context_line":"        self._initial_connection \u003d True"},{"line_number":99,"context_line":"        self._conn_event_handler \u003d conn_event_handler"},{"line_number":100,"context_line":"        queue_type \u003d queue.Queue[ty.Callable]"},{"line_number":101,"context_line":"        self._conn_event_handler_queue: queue_type \u003d queue.Queue()"},{"line_number":102,"context_line":"        self._lifecycle_event_handler \u003d lifecycle_event_handler"},{"line_number":103,"context_line":"        self._caps \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_0f2c9997","line":100,"range":{"start_line":100,"start_character":8,"end_line":100,"end_character":45},"updated":"2020-08-04 11:20:03.000000000","message":"so this type check but is not valid.","commit_id":"f1943d6f892792e62985667c1e67e8085eff76a8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8f0222a4c54e28573b4b91e7881510c930e59244","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        self._wrapped_conn \u003d None"},{"line_number":108,"context_line":"        self._wrapped_conn_lock \u003d threading.Lock()"},{"line_number":109,"context_line":"        self._event_queue: ty.Any \u003d None"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self._events_delayed \u003d {}"},{"line_number":112,"context_line":"        # Note(toabctl): During a reboot of a domain, STOPPED and"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_1678fb8d","line":109,"updated":"2020-08-04 11:50:29.000000000","message":"I think we can make this a more precise type declaration by using ty.Optional[native_Queue.Queue]","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7019581573e59d432067163ab13c26d59e490c7e","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        self._wrapped_conn \u003d None"},{"line_number":108,"context_line":"        self._wrapped_conn_lock \u003d threading.Lock()"},{"line_number":109,"context_line":"        self._event_queue: ty.Any \u003d None"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self._events_delayed \u003d {}"},{"line_number":112,"context_line":"        # Note(toabctl): During a reboot of a domain, STOPPED and"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_b6eb0fda","line":109,"in_reply_to":"9f560f44_1678fb8d","updated":"2020-08-04 12:29:12.000000000","message":"or ty.optional[ty.deqeue]\n\nty.deqeue is the generic trait for double eneded queues\nwhic is what a standard queue is.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6fb49dc576898e430b869e0935d764d6b81f713e","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        self._wrapped_conn \u003d None"},{"line_number":108,"context_line":"        self._wrapped_conn_lock \u003d threading.Lock()"},{"line_number":109,"context_line":"        self._event_queue: ty.Any \u003d None"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self._events_delayed \u003d {}"},{"line_number":112,"context_line":"        # Note(toabctl): During a reboot of a domain, STOPPED and"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_d704b84c","line":109,"in_reply_to":"9f560f44_a939edd7","updated":"2020-08-04 22:25:15.000000000","message":"ty.Optional[ty.dequeue[ty.callable]] is just as useful as it specifies the interface but ill use your version..","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fe38237f8fe5bd58c38edc25d1ff5110eb698fd8","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        self._wrapped_conn \u003d None"},{"line_number":108,"context_line":"        self._wrapped_conn_lock \u003d threading.Lock()"},{"line_number":109,"context_line":"        self._event_queue: ty.Any \u003d None"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        self._events_delayed \u003d {}"},{"line_number":112,"context_line":"        # Note(toabctl): During a reboot of a domain, STOPPED and"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_a939edd7","line":109,"in_reply_to":"9f560f44_b6eb0fda","updated":"2020-08-04 16:39:32.000000000","message":"Or\n\n  ty.Optional[queue.Queue[ty.Callable]]\n\nsince it is specifically an instance of queue.Queue","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"888d097fa144444961a075ff43948f5d635dd090","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"                raise"},{"line_number":1185,"context_line":""},{"line_number":1186,"context_line":"    def list_all_nodedevs("},{"line_number":1187,"context_line":"            self, flags: int \u003d 0) -\u003e ty.List[\u0027libvirt.virNodeDevice\u0027]:"},{"line_number":1188,"context_line":"        \"\"\"Lookup devices."},{"line_number":1189,"context_line":""},{"line_number":1190,"context_line":"        :param flags: a bitmask of flags to filter the returned devices."}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_3b3ec8d9","line":1187,"updated":"2020-08-04 11:15:57.000000000","message":"I really wish this was wrapped like\n\n  def list_all_nodedevs(\n      self, flags: int \u003d 0,\n  ) -\u003e ty.List[\u0027libvirt.virNodeDevice\u0027]:\n\nAlso, this would be better named as \u0027list_all_devices\u0027, since that\u0027s the name of the libvirt API.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a33c965c62411606495f823f1fb2ccb5f207c5eb","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"                raise"},{"line_number":1185,"context_line":""},{"line_number":1186,"context_line":"    def list_all_nodedevs("},{"line_number":1187,"context_line":"            self, flags: int \u003d 0) -\u003e ty.List[\u0027libvirt.virNodeDevice\u0027]:"},{"line_number":1188,"context_line":"        \"\"\"Lookup devices."},{"line_number":1189,"context_line":""},{"line_number":1190,"context_line":"        :param flags: a bitmask of flags to filter the returned devices."}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_56071319","line":1187,"in_reply_to":"9f560f44_3b3ec8d9","updated":"2020-08-04 11:20:03.000000000","message":"i used a differetn name because we have a\n\n_list_devices function above the ignore the flags and only allows you to filter by one type of device.\n\nso i renamed i chose list_all_nodedevs to one signal that it s returning nodedevs or well libvirt.virNodeDevice\u0027\nand too have a different nameing convnetion from teh existing functions that dont actully use flags where as this one really does use it.\n\nwe could always change it in a followup.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fe38237f8fe5bd58c38edc25d1ff5110eb698fd8","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"                raise"},{"line_number":1185,"context_line":""},{"line_number":1186,"context_line":"    def list_all_nodedevs("},{"line_number":1187,"context_line":"            self, flags: int \u003d 0) -\u003e ty.List[\u0027libvirt.virNodeDevice\u0027]:"},{"line_number":1188,"context_line":"        \"\"\"Lookup devices."},{"line_number":1189,"context_line":""},{"line_number":1190,"context_line":"        :param flags: a bitmask of flags to filter the returned devices."}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_c6b25094","line":1187,"in_reply_to":"9f560f44_56071319","updated":"2020-08-04 16:39:32.000000000","message":"\u003e i used a differetn name because we have a\n \u003e \n \u003e _list_devices function above the ignore the flags and only allows\n \u003e you to filter by one type of device.\n\nRight, but that maps to the libvirt API, which is listDevices. If listDevices becomes list_devices, then listAllDevices should become list_all_devices, right?\n\n \u003e so i renamed i chose list_all_nodedevs to one signal that it s\n \u003e returning nodedevs or well libvirt.virNodeDevice\u0027\n \u003e and too have a different nameing convnetion from teh existing\n \u003e functions that dont actully use flags where as this one really does\n \u003e use it.\n \u003e \n \u003e we could always change it in a followup.\n\nIf you\u0027re respinning, could you include this in the next PS?","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6fb49dc576898e430b869e0935d764d6b81f713e","unresolved":false,"context_lines":[{"line_number":1184,"context_line":"                raise"},{"line_number":1185,"context_line":""},{"line_number":1186,"context_line":"    def list_all_nodedevs("},{"line_number":1187,"context_line":"            self, flags: int \u003d 0) -\u003e ty.List[\u0027libvirt.virNodeDevice\u0027]:"},{"line_number":1188,"context_line":"        \"\"\"Lookup devices."},{"line_number":1189,"context_line":""},{"line_number":1190,"context_line":"        :param flags: a bitmask of flags to filter the returned devices."}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_5d7727a1","line":1187,"in_reply_to":"9f560f44_c6b25094","updated":"2020-08-04 22:25:15.000000000","message":"i saw this just as i was about to push this but i guess i can rename it but i was intentally tryign to not have devices in the name since _list_devices does not use flags but has the parmater and list_all_devices will.\n\nthe flags field on most of thes function is useless and is always ignored but that is not the case here.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8f0222a4c54e28573b4b91e7881510c930e59244","unresolved":false,"context_lines":[{"line_number":1193,"context_line":"        try:"},{"line_number":1194,"context_line":"            return self.get_connection().listAllDevices(flags) or []"},{"line_number":1195,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":1196,"context_line":"            LOG.warning(ex)"},{"line_number":1197,"context_line":"            return []"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def compare_cpu(self, xmlDesc, flags\u003d0):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_56a39300","line":1196,"updated":"2020-08-04 11:50:29.000000000","message":"This could become a periodic warning based on the comment in https://review.opendev.org/#/c/739131/10/nova/virt/libvirt/driver.py@7010","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7019581573e59d432067163ab13c26d59e490c7e","unresolved":false,"context_lines":[{"line_number":1193,"context_line":"        try:"},{"line_number":1194,"context_line":"            return self.get_connection().listAllDevices(flags) or []"},{"line_number":1195,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":1196,"context_line":"            LOG.warning(ex)"},{"line_number":1197,"context_line":"            return []"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def compare_cpu(self, xmlDesc, flags\u003d0):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_b672ef47","line":1196,"in_reply_to":"9f560f44_56a39300","updated":"2020-08-04 12:29:12.000000000","message":"right so this would only become a periodic warning if for example the libvirt deamon was dead.\n\nthe api im using is very very old so it meeans our minium requirement even on older releases like queens letalone master so we will only log a waring here if there is some other failure.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"41574b5ba748ba5476aab409cd1375918e534d0e","unresolved":false,"context_lines":[{"line_number":1193,"context_line":"        try:"},{"line_number":1194,"context_line":"            return self.get_connection().listAllDevices(flags) or []"},{"line_number":1195,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":1196,"context_line":"            LOG.warning(ex)"},{"line_number":1197,"context_line":"            return []"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def compare_cpu(self, xmlDesc, flags\u003d0):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_07860767","line":1196,"in_reply_to":"9f560f44_b672ef47","updated":"2020-08-04 14:27:24.000000000","message":"OK. If the only reason is a dead libvirt then I\u0027m OK to keep this as is.","commit_id":"b7dec62b3521b52ce06fec0b951899f446499985"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f34025e6944e8c5d43dd9f4f96519836c1915a58","unresolved":false,"context_lines":[{"line_number":326,"context_line":"        # Process as many events as possible without"},{"line_number":327,"context_line":"        # blocking"},{"line_number":328,"context_line":"        last_close_event \u003d None"},{"line_number":329,"context_line":"        # required for mypy"},{"line_number":330,"context_line":"        if self._event_queue is None:"},{"line_number":331,"context_line":"            return"},{"line_number":332,"context_line":"        while not self._event_queue.empty():"}],"source_content_type":"text/x-python","patch_set":12,"id":"9f560f44_38340193","line":329,"range":{"start_line":329,"start_character":7,"end_line":329,"end_character":27},"updated":"2020-08-04 22:33:16.000000000","message":"this or an assert is require for mypy although the code also technically assume it is not none.","commit_id":"efc27ff84c3f38fbcbf75b0dc230963c58d093e4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"68c349bf13d217c03e275c5d4aaa0e6bf156b9ed","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"                raise"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"    def list_all_devices("},{"line_number":1198,"context_line":"            self, flags: int \u003d 0) -\u003e ty.List[\u0027libvirt.virNodeDevice\u0027]:"},{"line_number":1199,"context_line":"        \"\"\"Lookup devices."},{"line_number":1200,"context_line":""},{"line_number":1201,"context_line":"        :param flags: a bitmask of flags to filter the returned devices."}],"source_content_type":"text/x-python","patch_set":12,"id":"9f560f44_7e4e526f","line":1198,"updated":"2020-08-05 17:32:47.000000000","message":"Just wondering about the tl;dr on the type annotations -- I skimmed the comments on this review and from what I can tell, being that python itself does not enforce type annotations [1], it sounds like we intend these to be enforced instead by mypy. And we run mypy during our pep8 job?\n\nhttps://zuul.opendev.org/t/openstack/build/dbe326158c6847fa8b2e584b60219ad7/log/job-output.txt#822\n\nI see invocation of the mypywrap.sh script but it wasn\u0027t clear to me what is the output from mypy. Is this it? \"Success: no issues found in 6 source files\"\n\n[1] https://docs.python.org/3/library/typing.html","commit_id":"efc27ff84c3f38fbcbf75b0dc230963c58d093e4"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4e905fcc8efc1c790ae1771407d0536163376b9e","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"                raise"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"    def list_all_devices("},{"line_number":1198,"context_line":"            self, flags: int \u003d 0) -\u003e ty.List[\u0027libvirt.virNodeDevice\u0027]:"},{"line_number":1199,"context_line":"        \"\"\"Lookup devices."},{"line_number":1200,"context_line":""},{"line_number":1201,"context_line":"        :param flags: a bitmask of flags to filter the returned devices."}],"source_content_type":"text/x-python","patch_set":12,"id":"9f560f44_92535239","line":1198,"in_reply_to":"9f560f44_7e4e526f","updated":"2020-08-06 08:09:09.000000000","message":"\u003e Just wondering about the tl;dr on the type annotations -- I skimmed\n \u003e the comments on this review and from what I can tell, being that\n \u003e python itself does not enforce type annotations [1], it sounds like\n \u003e we intend these to be enforced instead by mypy. And we run mypy\n \u003e during our pep8 job?\n\nCorrect. It\u0027s run via the \u0027pep8\u0027 target and can also be run separately via the \u0027mypy\u0027 tox target. FWIW, it is possible to get runtime type checking using something like typeguard [1] but we\u0027re nowhere near ready for that, even if we wanted to use it (which I\u0027m not sure we do).\n\n \u003e https://zuul.opendev.org/t/openstack/build/dbe326158c6847fa8b2e584b60219ad7/log/job-output.txt#822\n \u003e \n \u003e I see invocation of the mypywrap.sh script but it wasn\u0027t clear to\n \u003e me what is the output from mypy. Is this it? \"Success: no issues\n \u003e found in 6 source files\"\n \u003e \n \u003e [1] https://docs.python.org/3/library/typing.html\n\nmypy can and is being run in \"incremental\" mode, whereby type hints don\u0027t have to be complete and mypy will attempt to guess the various types (typically anything but the most trivial stuff will just resolve to \u0027ty.Any\u0027). Even with incremental mode enabled though, nova has a lot of mypy-incompatible code that has to be addressed. For example, this could include things changing a variable \u0027foo\u0027 from e.g. a list to a set mid-function. We want to avoid a *huge* refactoring change to fix all those issues, so we\u0027ve decided to introduce things gradually. To that end, the \u0027mypy-files.txt\u0027 file at the root of the repo lists files that we have introduced some level of type hints for (or at least resolved the most obvious incompatibilities for). There\u0027s nothing stopping you adding type hints to any file, but if a file isn\u0027t in that list then the type hints aren\u0027t validated and are essentially comments. For example, Sean has (or should have) added this particular file to that list in this change. However, you\u0027ll also note there are type hints in some of the test files. Those aren\u0027t aren\u0027t listed in the \u0027mypy-files.txt\u0027 file which means they\u0027re not currently checked, and will likely never be checked, because we use both duck typing and mocking _very_ liberally in our tests.\n\n[1] https://github.com/agronholm/typeguard","commit_id":"efc27ff84c3f38fbcbf75b0dc230963c58d093e4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3f6b537e54a36596ad19b2fbb2fc52399ec58ba4","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"                raise"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"    def list_all_devices("},{"line_number":1198,"context_line":"            self, flags: int \u003d 0) -\u003e ty.List[\u0027libvirt.virNodeDevice\u0027]:"},{"line_number":1199,"context_line":"        \"\"\"Lookup devices."},{"line_number":1200,"context_line":""},{"line_number":1201,"context_line":"        :param flags: a bitmask of flags to filter the returned devices."}],"source_content_type":"text/x-python","patch_set":12,"id":"9f560f44_a189d875","line":1198,"in_reply_to":"9f560f44_92535239","updated":"2020-08-06 20:41:50.000000000","message":"Gotcha. Thank you for explaining all of the details!","commit_id":"efc27ff84c3f38fbcbf75b0dc230963c58d093e4"}],"releasenotes/notes/libvirt-nodedev-lookup-d80174ac30bc82f0.yaml":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d24d087de42c5b9f54971b6c2023420037f86b26","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Since the pike release nova has collected nic feature flags via libvirt."},{"line_number":5,"context_line":"    To look up the nic feature flags for a whitelisted pci device the nova"},{"line_number":6,"context_line":"    libvirt driver computed the libvirt nodedev-name by rendering a format"},{"line_number":7,"context_line":"    string using the netdev name associated with the interface and its"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_e698991c","line":4,"range":{"start_line":4,"start_character":14,"end_line":4,"end_character":18},"updated":"2020-07-03 09:58:45.000000000","message":"16.0.0 (Pike)","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d24d087de42c5b9f54971b6c2023420037f86b26","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Since the pike release nova has collected nic feature flags via libvirt."},{"line_number":5,"context_line":"    To look up the nic feature flags for a whitelisted pci device the nova"},{"line_number":6,"context_line":"    libvirt driver computed the libvirt nodedev-name by rendering a format"},{"line_number":7,"context_line":"    string using the netdev name associated with the interface and its"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_469ea507","line":4,"range":{"start_line":4,"start_character":46,"end_line":4,"end_character":49},"updated":"2020-07-03 09:58:45.000000000","message":"NIC (here and elsewhere)","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d24d087de42c5b9f54971b6c2023420037f86b26","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Since the pike release nova has collected nic feature flags via libvirt."},{"line_number":5,"context_line":"    To look up the nic feature flags for a whitelisted pci device the nova"},{"line_number":6,"context_line":"    libvirt driver computed the libvirt nodedev-name by rendering a format"},{"line_number":7,"context_line":"    string using the netdev name associated with the interface and its"},{"line_number":8,"context_line":"    current mac address. In some environments the libvirt nodedev list"},{"line_number":9,"context_line":"    can become out of sycn with the current mac address assigined to a"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_26a3b151","line":6,"range":{"start_line":6,"start_character":47,"end_line":6,"end_character":48},"updated":"2020-07-03 09:58:45.000000000","message":"nit: drop this","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d24d087de42c5b9f54971b6c2023420037f86b26","unresolved":false,"context_lines":[{"line_number":5,"context_line":"    To look up the nic feature flags for a whitelisted pci device the nova"},{"line_number":6,"context_line":"    libvirt driver computed the libvirt nodedev-name by rendering a format"},{"line_number":7,"context_line":"    string using the netdev name associated with the interface and its"},{"line_number":8,"context_line":"    current mac address. In some environments the libvirt nodedev list"},{"line_number":9,"context_line":"    can become out of sycn with the current mac address assigined to a"},{"line_number":10,"context_line":"    netdev as a result the nodedev look up can fail. This change removes"},{"line_number":11,"context_line":"    the depency on the mac addres and instead relys only on the pci address."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_c69115f4","line":8,"range":{"start_line":8,"start_character":12,"end_line":8,"end_character":15},"updated":"2020-07-03 09:58:45.000000000","message":"MAC (here and elsewhere)","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    libvirt driver computed the libvirt nodedev-name by rendering a format"},{"line_number":7,"context_line":"    string using the netdev name associated with the interface and its"},{"line_number":8,"context_line":"    current mac address. In some environments the libvirt nodedev list"},{"line_number":9,"context_line":"    can become out of sycn with the current mac address assigined to a"},{"line_number":10,"context_line":"    netdev as a result the nodedev look up can fail. This change removes"},{"line_number":11,"context_line":"    the depency on the mac addres and instead relys only on the pci address."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_157858f6","line":9,"range":{"start_line":9,"start_character":56,"end_line":9,"end_character":65},"updated":"2020-07-02 20:55:34.000000000","message":"assigned","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    libvirt driver computed the libvirt nodedev-name by rendering a format"},{"line_number":7,"context_line":"    string using the netdev name associated with the interface and its"},{"line_number":8,"context_line":"    current mac address. In some environments the libvirt nodedev list"},{"line_number":9,"context_line":"    can become out of sycn with the current mac address assigined to a"},{"line_number":10,"context_line":"    netdev as a result the nodedev look up can fail. This change removes"},{"line_number":11,"context_line":"    the depency on the mac addres and instead relys only on the pci address."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_b51d2cbf","line":9,"range":{"start_line":9,"start_character":22,"end_line":9,"end_character":26},"updated":"2020-07-02 20:55:34.000000000","message":"sync","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    string using the netdev name associated with the interface and its"},{"line_number":8,"context_line":"    current mac address. In some environments the libvirt nodedev list"},{"line_number":9,"context_line":"    can become out of sycn with the current mac address assigined to a"},{"line_number":10,"context_line":"    netdev as a result the nodedev look up can fail. This change removes"},{"line_number":11,"context_line":"    the depency on the mac addres and instead relys only on the pci address."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_f57cc4ec","line":10,"range":{"start_line":10,"start_character":11,"end_line":10,"end_character":13},"updated":"2020-07-02 20:55:34.000000000","message":"and as","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":8,"context_line":"    current mac address. In some environments the libvirt nodedev list"},{"line_number":9,"context_line":"    can become out of sycn with the current mac address assigined to a"},{"line_number":10,"context_line":"    netdev as a result the nodedev look up can fail. This change removes"},{"line_number":11,"context_line":"    the depency on the mac addres and instead relys only on the pci address."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_7568b420","line":11,"range":{"start_line":11,"start_character":27,"end_line":11,"end_character":33},"updated":"2020-07-02 20:55:34.000000000","message":"address","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":8,"context_line":"    current mac address. In some environments the libvirt nodedev list"},{"line_number":9,"context_line":"    can become out of sycn with the current mac address assigined to a"},{"line_number":10,"context_line":"    netdev as a result the nodedev look up can fail. This change removes"},{"line_number":11,"context_line":"    the depency on the mac addres and instead relys only on the pci address."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_35875cda","line":11,"range":{"start_line":11,"start_character":8,"end_line":11,"end_character":15},"updated":"2020-07-02 20:55:34.000000000","message":"dependency","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a0d09b743227707c2f8b4b80e4437ee576d4202a","unresolved":false,"context_lines":[{"line_number":8,"context_line":"    current mac address. In some environments the libvirt nodedev list"},{"line_number":9,"context_line":"    can become out of sycn with the current mac address assigined to a"},{"line_number":10,"context_line":"    netdev as a result the nodedev look up can fail. This change removes"},{"line_number":11,"context_line":"    the depency on the mac addres and instead relys only on the pci address."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf51134e_956bc81d","line":11,"range":{"start_line":11,"start_character":46,"end_line":11,"end_character":51},"updated":"2020-07-02 20:55:34.000000000","message":"relies","commit_id":"5944fc0cdc1cee683a0e748f012c41437cdc8249"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Since the 16.0.0 (Pike) release nova has collected NIC feature"},{"line_number":5,"context_line":"    flags via libvirt. To look up the NIC feature flags for a whitelisted"},{"line_number":6,"context_line":"    PCI device the nova libvirt driver computed the libvirt nodedev name"},{"line_number":7,"context_line":"    by rendering a format string using the netdev name associated with"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"bf51134e_aafdb809","line":4,"range":{"start_line":4,"start_character":28,"end_line":4,"end_character":35},"updated":"2020-07-22 11:15:19.000000000","message":"release,\n\n(missing comma)","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7dd9a30a70e82882558fc992f154e4be3b89d6e","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    by rendering a format string using the netdev name associated with"},{"line_number":8,"context_line":"    the interface and its current MAC address. In some environments the"},{"line_number":9,"context_line":"    libvirt nodedev list can become out of sync with the current MAC address"},{"line_number":10,"context_line":"    assigned to a netdev and as a result the nodedev look up can fail. This"},{"line_number":11,"context_line":"    change removes the dependency on the MAC address and instead relies"},{"line_number":12,"context_line":"    only on the PCI address."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"bf51134e_2a11a854","line":12,"range":{"start_line":10,"start_character":71,"end_line":12,"end_character":28},"updated":"2020-07-22 11:15:19.000000000","message":"Nova now uses PCI addresses, rather than MAC addresses, to look up these PCI network devices.\n\n(\"This change\" makes no sense from the context of a rendered release notes doc)","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a6e97eb1b60bc5736b8b4f885cfe2850f169b30","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    by rendering a format string using the netdev name associated with"},{"line_number":8,"context_line":"    the interface and its current MAC address. In some environments the"},{"line_number":9,"context_line":"    libvirt nodedev list can become out of sync with the current MAC address"},{"line_number":10,"context_line":"    assigned to a netdev and as a result the nodedev look up can fail. This"},{"line_number":11,"context_line":"    change removes the dependency on the MAC address and instead relies"},{"line_number":12,"context_line":"    only on the PCI address."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"9f560f44_999e7761","line":12,"range":{"start_line":10,"start_character":71,"end_line":12,"end_character":28},"in_reply_to":"bf51134e_2a11a854","updated":"2020-07-27 23:12:40.000000000","message":"fair i normally derive the release note form the commit message.","commit_id":"8d03920f3d668e1f40864643c2c1f678bf90c6e3"}]}