)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6e39620a44485d4fda400023112cc564f1f7e7a6","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     David Bingham \u003cdbingham@godaddy.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-05-30 19:08:44 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow cleanup completion when serial ports already released"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Driver cleanup changed with try-except to be tolerant of InstanceNotFound"},{"line_number":10,"context_line":"exceptions when attempting to fetch missing ports (possibly removed on"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7a016987_e6094832","line":7,"updated":"2015-06-01 11:19:20.000000000","message":"Please prefix with libvirt","commit_id":"b8372af8ad7256a1a39d020652884951f2b273a8"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"03936858f50e71086adbc3cc0c2099734ae00fe7","unresolved":false,"context_lines":[{"line_number":10918,"context_line":"    def test_cleanup_serial_console_domain_gone("},{"line_number":10919,"context_line":"            self, undefine, get_ports, block_device_info_get_mapping):"},{"line_number":10920,"context_line":"        self.flags(enabled\u003d\"True\", group\u003d\u0027serial_console\u0027)"},{"line_number":10921,"context_line":"        instance \u003d {\u0027name\u0027: \u0027i1\u0027}"},{"line_number":10922,"context_line":"        network_info \u003d {}"},{"line_number":10923,"context_line":"        bdm_info \u003d {}"},{"line_number":10924,"context_line":"        firewall_driver \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_73552dc0","line":10921,"updated":"2015-07-08 13:38:42.000000000","message":"We should use instance objects in tests when possible, so you\u0027d use fake_instance.fake_instance_obj(self.test_instance) here.\n\nI see this test is basically copying the previous test though except it raises the InstanceNotFound exception.  At least in this test instance is a dict rather than just a string in the previous test, which is blatantly wrong.","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4b7f0c88ac3ebdbd396b0ed093ff9585e48dd6c1","unresolved":false,"context_lines":[{"line_number":10921,"context_line":"        instance \u003d {\u0027name\u0027: \u0027i1\u0027}"},{"line_number":10922,"context_line":"        network_info \u003d {}"},{"line_number":10923,"context_line":"        bdm_info \u003d {}"},{"line_number":10924,"context_line":"        firewall_driver \u003d mock.MagicMock()"},{"line_number":10925,"context_line":""},{"line_number":10926,"context_line":"        block_device_info_get_mapping.return_value \u003d ()"},{"line_number":10927,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_d191c89a","line":10924,"updated":"2015-06-08 23:05:52.000000000","message":"I think this should be, for example, a mock.Mock(spec\u003dbase_firewall.FirewallDriver) instead of a mock.MagicMock. Otherwise, any method calls (even invalid ones that don\u0027t exist) will succeed on a MagicMock.","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"6e594a270ba1ff6b43b176c434aaeb5b15a56a73","unresolved":false,"context_lines":[{"line_number":10921,"context_line":"        instance \u003d {\u0027name\u0027: \u0027i1\u0027}"},{"line_number":10922,"context_line":"        network_info \u003d {}"},{"line_number":10923,"context_line":"        bdm_info \u003d {}"},{"line_number":10924,"context_line":"        firewall_driver \u003d mock.MagicMock()"},{"line_number":10925,"context_line":""},{"line_number":10926,"context_line":"        block_device_info_get_mapping.return_value \u003d ()"},{"line_number":10927,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_c7ed6cb8","line":10924,"in_reply_to":"3afb71cf_d191c89a","updated":"2015-06-17 01:02:23.000000000","message":"Yeah Melanie, this test is a pretty targeted test. Yes mock.MagicMock() does bypass anything and succeed \"by design\" for this test. This test MUST run through all of the code within the method up to and including the try-except block. At that point the get_ports.side_effect below actually forces us into the \u0027except\u0027 block and proves exception.InstanceNotFound does not stop the method from completing (which fixes the whole reason this bug and patch exists).","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"03936858f50e71086adbc3cc0c2099734ae00fe7","unresolved":false,"context_lines":[{"line_number":10921,"context_line":"        instance \u003d {\u0027name\u0027: \u0027i1\u0027}"},{"line_number":10922,"context_line":"        network_info \u003d {}"},{"line_number":10923,"context_line":"        bdm_info \u003d {}"},{"line_number":10924,"context_line":"        firewall_driver \u003d mock.MagicMock()"},{"line_number":10925,"context_line":""},{"line_number":10926,"context_line":"        block_device_info_get_mapping.return_value \u003d ()"},{"line_number":10927,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_93ad519d","line":10924,"in_reply_to":"ba3cc151_170ced92","updated":"2015-07-08 13:38:42.000000000","message":"Plus this is a copy of the previous test...","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0c39bca49a5f6fd1efbd7bc604bf865763c6bdfe","unresolved":false,"context_lines":[{"line_number":10921,"context_line":"        instance \u003d {\u0027name\u0027: \u0027i1\u0027}"},{"line_number":10922,"context_line":"        network_info \u003d {}"},{"line_number":10923,"context_line":"        bdm_info \u003d {}"},{"line_number":10924,"context_line":"        firewall_driver \u003d mock.MagicMock()"},{"line_number":10925,"context_line":""},{"line_number":10926,"context_line":"        block_device_info_get_mapping.return_value \u003d ()"},{"line_number":10927,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_170ced92","line":10924,"in_reply_to":"fa32b979_c7ed6cb8","updated":"2015-07-07 22:39:36.000000000","message":"Yeah, this is fine with me. We\u0027re just using it to make sure we don\u0027t call something deeper. Even checking that it was called is kinda out of scope, IMHO.","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"2a5994d2b7b5408b632d0f0fd43dae99e194a6d4","unresolved":false,"context_lines":[{"line_number":10923,"context_line":"        bdm_info \u003d {}"},{"line_number":10924,"context_line":"        firewall_driver \u003d mock.MagicMock()"},{"line_number":10925,"context_line":""},{"line_number":10926,"context_line":"        block_device_info_get_mapping.return_value \u003d ()"},{"line_number":10927,"context_line":""},{"line_number":10928,"context_line":"        # Ensure _get_serial_ports_from_instance raises same exception"},{"line_number":10929,"context_line":"        # that would have occurred if domain was gone."}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_2c87fe62","line":10926,"updated":"2015-06-05 12:49:18.000000000","message":"Same style nit applies as noted in driver.py","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"03936858f50e71086adbc3cc0c2099734ae00fe7","unresolved":false,"context_lines":[{"line_number":10927,"context_line":""},{"line_number":10928,"context_line":"        # Ensure _get_serial_ports_from_instance raises same exception"},{"line_number":10929,"context_line":"        # that would have occurred if domain was gone."},{"line_number":10930,"context_line":"        get_ports.side_effect \u003d exception.InstanceNotFound(\"domain undefined\")"},{"line_number":10931,"context_line":""},{"line_number":10932,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())"},{"line_number":10933,"context_line":"        drvr.firewall_driver \u003d firewall_driver"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_b3de550f","line":10930,"updated":"2015-07-08 13:38:42.000000000","message":"You should be passing an instance_id in here:\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/exception.py#n1342\n\nI point this out because I think if CONF.fatal_exception_format_errors\u003dTrue then this would result in a KeyError.","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"b87e8eb99276ac285390cf4147fad9657b569245","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1aca2d91_57f3d00a","side":"PARENT","updated":"2015-05-06 16:04:30.000000000","message":"recheck","commit_id":"1a44d4989970d8e52504e4dc732c5179695ad360"},{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"b87e8eb99276ac285390cf4147fad9657b569245","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1aca2d91_f76f7c90","updated":"2015-05-06 16:04:30.000000000","message":"recheck","commit_id":"916b4561224a59b76967890604e53572912b8687"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b2db932eb72de55da75451f5b578c125d231f5a0","unresolved":false,"context_lines":[{"line_number":863,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":864,"context_line":"                for hostname, port in serials:"},{"line_number":865,"context_line":"                    serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":"            except exception.InstanceNotFound:"},{"line_number":867,"context_line":"                pass  # Serial ports already gone. Nothing to release."},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"        self._undefine_domain(instance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_ad4525b4","line":866,"updated":"2015-05-26 15:57:24.000000000","message":"you should catch the exception before the for loop and then set serials to None so it would make clear that we\u0027re awaiting an InstanceNotFound only in _get_serial_ports_from_instance()","commit_id":"7051b586f1255929d46c0628552c2694beb8d3fd"},{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"1b4a00f755ca048a56f52853a94b589ca6887dbb","unresolved":false,"context_lines":[{"line_number":863,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":864,"context_line":"                for hostname, port in serials:"},{"line_number":865,"context_line":"                    serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":"            except exception.InstanceNotFound:"},{"line_number":867,"context_line":"                pass  # Serial ports already gone. Nothing to release."},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"        self._undefine_domain(instance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_d28203c8","line":866,"in_reply_to":"7a016987_ad4525b4","updated":"2015-05-27 21:47:31.000000000","message":"Going to set serials to an empty tuple rather than None: serials \u003d (). This makes it so we don\u0027t have to check for None before looping.","commit_id":"7051b586f1255929d46c0628552c2694beb8d3fd"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b2db932eb72de55da75451f5b578c125d231f5a0","unresolved":false,"context_lines":[{"line_number":864,"context_line":"                for hostname, port in serials:"},{"line_number":865,"context_line":"                    serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":"            except exception.InstanceNotFound:"},{"line_number":867,"context_line":"                pass  # Serial ports already gone. Nothing to release."},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"        self._undefine_domain(instance)"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_ad1c05c2","line":867,"updated":"2015-05-26 15:57:24.000000000","message":"please provide the comment not on the same line but rather just on top of the line you want to comment","commit_id":"7051b586f1255929d46c0628552c2694beb8d3fd"},{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"1b4a00f755ca048a56f52853a94b589ca6887dbb","unresolved":false,"context_lines":[{"line_number":864,"context_line":"                for hostname, port in serials:"},{"line_number":865,"context_line":"                    serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":"            except exception.InstanceNotFound:"},{"line_number":867,"context_line":"                pass  # Serial ports already gone. Nothing to release."},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"        self._undefine_domain(instance)"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_b23d47df","line":867,"in_reply_to":"7a016987_ad1c05c2","updated":"2015-05-27 21:47:31.000000000","message":"Done","commit_id":"7051b586f1255929d46c0628552c2694beb8d3fd"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6d0584e54537b24ff2833a0c2b892a180da24a55","unresolved":false,"context_lines":[{"line_number":863,"context_line":"            serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":864,"context_line":"            for hostname, port in serials:"},{"line_number":865,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"        self._undefine_domain(instance)"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    def _detach_encrypted_volumes(self, instance):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_11718b76","side":"PARENT","line":866,"updated":"2015-05-28 21:11:03.000000000","message":"sorry, you would think that\u0027s nitpicking but we really need to only change what\u0027s needed","commit_id":"f40619b2b4531bf4f06a22124edae35438bf2cc3"},{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"aecece5d81218cd67bb9766ffa808c0ab0ca4d76","unresolved":false,"context_lines":[{"line_number":863,"context_line":"            serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":864,"context_line":"            for hostname, port in serials:"},{"line_number":865,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"        self._undefine_domain(instance)"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    def _detach_encrypted_volumes(self, instance):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_476a9eb4","side":"PARENT","line":866,"in_reply_to":"7a016987_11718b76","updated":"2015-05-28 23:52:52.000000000","message":"I am guessing \"only change what is needed\" means \"do what I asked the first review\". Quote: \"you should catch the exception before the for loop and then set serials to None so it would make clear that we\u0027re awaiting an InstanceNotFound only in _get_serial_ports_from_instance()\".\n\n\nBut, if I set the variable \u0027serials\u0027 to None as you asked then the for-loop results in: TypeError: \u0027NoneType\u0027 object is not iterable (I tested this)\n\nSo, either \n1) I will need to add a test for None before the for-loop or 2) Set serials to a empty tuple (same type as _get_serial_ports_from_instance returns). This sets \u0027serials\u0027 to something iterable so the for-loop is happy, but does not actually call release_port.\n\nMy setting this to a empty tuple was my attempt to make serials consistent with the result, but not introduce more code to test for None.\n\nLet me know how you want me to proceed.","commit_id":"f40619b2b4531bf4f06a22124edae35438bf2cc3"},{"author":{"_account_id":9060,"name":"Mike Dorman","email":"dorm@dorm.org","username":"gd-mdorman"},"change_message_id":"f47bd98bf23909c885464238a34db1509676935e","unresolved":false,"context_lines":[{"line_number":863,"context_line":"            serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":864,"context_line":"            for hostname, port in serials:"},{"line_number":865,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"        self._undefine_domain(instance)"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    def _detach_encrypted_volumes(self, instance):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_a2894328","side":"PARENT","line":866,"in_reply_to":"7a016987_11718b76","updated":"2015-05-28 23:00:05.000000000","message":"won\u0027t we get a NPE at line 864 if serials is set to None?","commit_id":"f40619b2b4531bf4f06a22124edae35438bf2cc3"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6e39620a44485d4fda400023112cc564f1f7e7a6","unresolved":false,"context_lines":[{"line_number":863,"context_line":"            serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":864,"context_line":"            for hostname, port in serials:"},{"line_number":865,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"        self._undefine_domain(instance)"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    def _detach_encrypted_volumes(self, instance):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_21218a9e","side":"PARENT","line":866,"updated":"2015-06-01 11:19:20.000000000","message":"no need for this change :)","commit_id":"e8b4eaa2a6c6d56dabe2908d105abfe97e289185"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"ea9807e312c7e2365536f0d5cb324c46f0668660","unresolved":false,"context_lines":[{"line_number":863,"context_line":"            serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":864,"context_line":"            for hostname, port in serials:"},{"line_number":865,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"        self._undefine_domain(instance)"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    def _detach_encrypted_volumes(self, instance):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_36de6fdd","side":"PARENT","line":866,"in_reply_to":"7a016987_21218a9e","updated":"2015-06-01 12:52:58.000000000","message":"+1 :)","commit_id":"e8b4eaa2a6c6d56dabe2908d105abfe97e289185"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9e47617f2270b3b5be3893ae9d3baf241c928db0","unresolved":false,"context_lines":[{"line_number":863,"context_line":"            serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":864,"context_line":"            for hostname, port in serials:"},{"line_number":865,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"        self._undefine_domain(instance)"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"    def _detach_encrypted_volumes(self, instance):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_e7e37fa2","side":"PARENT","line":866,"in_reply_to":"7a016987_36de6fdd","updated":"2015-06-01 14:01:10.000000000","message":"Yeah, sorry I was unclear with my previous comment on PS4, I was just meaning that the removal of this empty line doesn\u0027t have to be done.","commit_id":"e8b4eaa2a6c6d56dabe2908d105abfe97e289185"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6e39620a44485d4fda400023112cc564f1f7e7a6","unresolved":false,"context_lines":[{"line_number":857,"context_line":"            instance.system_metadata[\u0027clean_attempts\u0027] \u003d str(attempts + 1)"},{"line_number":858,"context_line":"            if success:"},{"line_number":859,"context_line":"                instance.cleaned \u003d True"},{"line_number":860,"context_line":"            instance.save()"},{"line_number":861,"context_line":""},{"line_number":862,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":863,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_e674a8cf","line":860,"updated":"2015-06-01 11:19:20.000000000","message":"would this not raise an exception if the instance was already gone?","commit_id":"b8372af8ad7256a1a39d020652884951f2b273a8"},{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"cafcff3fc6924bc52acf1e679923064f34d98b60","unresolved":false,"context_lines":[{"line_number":857,"context_line":"            instance.system_metadata[\u0027clean_attempts\u0027] \u003d str(attempts + 1)"},{"line_number":858,"context_line":"            if success:"},{"line_number":859,"context_line":"                instance.cleaned \u003d True"},{"line_number":860,"context_line":"            instance.save()"},{"line_number":861,"context_line":""},{"line_number":862,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":863,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5afe65bd_4b1d990e","line":860,"in_reply_to":"7a016987_e674a8cf","updated":"2015-06-01 17:09:28.000000000","message":"Also, the original bug for this related to cleanup failed the first time, but 2nd cleanup threw exception because serial ports were gone from the 1st run.","commit_id":"b8372af8ad7256a1a39d020652884951f2b273a8"},{"author":{"_account_id":9060,"name":"Mike Dorman","email":"dorm@dorm.org","username":"gd-mdorman"},"change_message_id":"6d40b6c6adbceac320cff38c485b744a7b9fb241","unresolved":false,"context_lines":[{"line_number":857,"context_line":"            instance.system_metadata[\u0027clean_attempts\u0027] \u003d str(attempts + 1)"},{"line_number":858,"context_line":"            if success:"},{"line_number":859,"context_line":"                instance.cleaned \u003d True"},{"line_number":860,"context_line":"            instance.save()"},{"line_number":861,"context_line":""},{"line_number":862,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":863,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_c2c18e81","line":860,"in_reply_to":"7a016987_e674a8cf","updated":"2015-06-01 15:13:09.000000000","message":"Perhaps.  But that is outside the scope of this patch.  I\u0027d rather that be dealt with by a separate bug \u0026 review.","commit_id":"b8372af8ad7256a1a39d020652884951f2b273a8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"ea9807e312c7e2365536f0d5cb324c46f0668660","unresolved":false,"context_lines":[{"line_number":861,"context_line":""},{"line_number":862,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":863,"context_line":"            try:"},{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_16525367","line":864,"updated":"2015-06-01 12:52:58.000000000","message":"To avoid initialize variable \"serials\"\n\n  try\n    serials \u003d self._get_serial_port....\n    for hostname, port in serials:\n      serial_console.release....\n  except exception.InstanceNotFound:\n    pass","commit_id":"b8372af8ad7256a1a39d020652884951f2b273a8"},{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"cafcff3fc6924bc52acf1e679923064f34d98b60","unresolved":false,"context_lines":[{"line_number":861,"context_line":""},{"line_number":862,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":863,"context_line":"            try:"},{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"}],"source_content_type":"text/x-python","patch_set":5,"id":"5afe65bd_2bc49d3e","line":864,"in_reply_to":"7a016987_16525367","updated":"2015-06-01 17:09:28.000000000","message":"sahid, this was exactly my first submission. 2nd submission was to bridge the wants of a review without adding extra tests for None.","commit_id":"b8372af8ad7256a1a39d020652884951f2b273a8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9e47617f2270b3b5be3893ae9d3baf241c928db0","unresolved":false,"context_lines":[{"line_number":861,"context_line":""},{"line_number":862,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":863,"context_line":"            try:"},{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_47a01342","line":864,"in_reply_to":"7a016987_16525367","updated":"2015-06-01 14:01:10.000000000","message":"that was the purpose of the previous PS and I specifically asked to change that, since I prefer to have the exception to be catched the closest to where it\u0027s raised.","commit_id":"b8372af8ad7256a1a39d020652884951f2b273a8"},{"author":{"_account_id":9060,"name":"Mike Dorman","email":"dorm@dorm.org","username":"gd-mdorman"},"change_message_id":"6d40b6c6adbceac320cff38c485b744a7b9fb241","unresolved":false,"context_lines":[{"line_number":861,"context_line":""},{"line_number":862,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":863,"context_line":"            try:"},{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a016987_c2486e2f","line":864,"in_reply_to":"7a016987_47a01342","updated":"2015-06-01 15:13:09.000000000","message":"Functionally, both methods are equivalent.  In the grand scheme of things, does it really matter which one we go with?","commit_id":"b8372af8ad7256a1a39d020652884951f2b273a8"},{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"cafcff3fc6924bc52acf1e679923064f34d98b60","unresolved":false,"context_lines":[{"line_number":861,"context_line":""},{"line_number":862,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":863,"context_line":"            try:"},{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"}],"source_content_type":"text/x-python","patch_set":5,"id":"5afe65bd_19e49b39","line":864,"in_reply_to":"7a016987_c2486e2f","updated":"2015-06-01 17:09:28.000000000","message":"I\u0027m going to submit a new patch\n1) returning the blank line I removed\n2) Changing the commit message to include libvirt\n3) Going to leave the serials \u003d () so the for-loop gets skipped over","commit_id":"b8372af8ad7256a1a39d020652884951f2b273a8"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"2a5994d2b7b5408b632d0f0fd43dae99e194a6d4","unresolved":false,"context_lines":[{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"},{"line_number":868,"context_line":"            for hostname, port in serials:"},{"line_number":869,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_ac652ee2","line":867,"updated":"2015-06-05 12:49:18.000000000","message":"Minor style nit: this is one area where Python is too ambiguous for its own good. When specifying an empty tuple, it is clearer to write:\n\n    serials \u003d tuple()","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0c39bca49a5f6fd1efbd7bc604bf865763c6bdfe","unresolved":false,"context_lines":[{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"},{"line_number":868,"context_line":"            for hostname, port in serials:"},{"line_number":869,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_17fe2d6b","line":867,"in_reply_to":"3afb71cf_7fb2ac95","updated":"2015-07-07 22:39:36.000000000","message":"Yeah, [] would be better, IMHO, but () is fine with me.","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":10962,"name":"David Bingham","email":"dbingham@godaddy.com","username":"wwriverrat"},"change_message_id":"93e84951c7c9b85f28706b632a61d09437d8b043","unresolved":false,"context_lines":[{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"},{"line_number":868,"context_line":"            for hostname, port in serials:"},{"line_number":869,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_d1f11acf","line":867,"in_reply_to":"3afb71cf_ac652ee2","updated":"2015-06-05 15:19:24.000000000","message":"Yeah Ed, after I submitted the last patch I thought about tuple().  Technically, any iterable would have worked.  My intention was to return something of the exact same type as _get_serial_ports_from_instance returns to be consistent.  My original patch with the for loop embedded in the try body would have eliminated the need to set serials to anything. It also would have avoided the construction of a empty tuple as well as the for-loop\u0027s internal next check. A previous review had me change the style so try-except only surrounded the code that could throw it. Both styles have +\u0027s/-\u0027s.\n\nAt this point we\u0027ve beat this little patch to death.  This will fix the bug it intended. There likely could be a lot more debate about style, but in the end it needs to move on so those who are experiencing the issue can get relief.","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"5bb87cc7778c5cf03d7b14d428ad2f67b395abe4","unresolved":false,"context_lines":[{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"},{"line_number":868,"context_line":"            for hostname, port in serials:"},{"line_number":869,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_6beef923","line":867,"in_reply_to":"3afb71cf_ac652ee2","updated":"2015-06-05 14:20:40.000000000","message":"[] is fine, why not ()?","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"cad1f923f71a6c51af87a36413ac9a7780bca004","unresolved":false,"context_lines":[{"line_number":864,"context_line":"                serials \u003d self._get_serial_ports_from_instance(instance)"},{"line_number":865,"context_line":"            except exception.InstanceNotFound:"},{"line_number":866,"context_line":"                # Serial ports already gone. Nothing to release."},{"line_number":867,"context_line":"                serials \u003d ()"},{"line_number":868,"context_line":"            for hostname, port in serials:"},{"line_number":869,"context_line":"                serial_console.release_port(host\u003dhostname, port\u003dport)"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3afb71cf_7fb2ac95","line":867,"in_reply_to":"3afb71cf_d1f11acf","updated":"2015-06-09 20:45:30.000000000","message":"Well, I did say it was a minor nit, and still +1\u0027d the patch. Please don\u0027t respin just for that.\n\nAlexis: because parentheses have other meanings in Python, such as calling, grouping, etc. [] and {} have no other meanings. That\u0027s why it is ambiguous from a readability standpoint. The code will run perfectly fine.","commit_id":"2e72e8f374972f5487660b66758c4d3d1eb87791"}]}