)]}'
{"ironic/dhcp/neutron.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f653bd6b29362d03797c5e2ac5b4f7c03aef4040","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            if not port.get(\u0027port\u0027) or not port[\u0027port\u0027].get(\u0027id\u0027):"},{"line_number":321,"context_line":"                self._rollback_cleaning_ports(task)"},{"line_number":322,"context_line":"                msg \u003d (_(\u0027Failed to create cleaning ports for node \u0027"},{"line_number":323,"context_line":"                         \u0027%(node)s\u0027) % {\u0027node\u0027: task.node.uuid})"},{"line_number":324,"context_line":"                LOG.error(msg)"},{"line_number":325,"context_line":"                raise exception.NodeCleaningFailure(msg)"},{"line_number":326,"context_line":"            # Match return value of get_node_vif_ids()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_6493a8fb","line":323,"updated":"2015-09-14 09:05:20.000000000","message":"Actually there\u0027s only one variable to be translated you should use \"%s\" instead of \"%(node)s\"\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-translated-messages","commit_id":"fee3754512b6ee77b69c62d83d1fe9d159b19d61"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"88431e94d1b2d81b6fe8c917173e9c5c6bd6ddb6","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            if not port.get(\u0027port\u0027) or not port[\u0027port\u0027].get(\u0027id\u0027):"},{"line_number":321,"context_line":"                self._rollback_cleaning_ports(task)"},{"line_number":322,"context_line":"                msg \u003d (_(\u0027Failed to create cleaning ports for node \u0027"},{"line_number":323,"context_line":"                         \u0027%(node)s\u0027) % {\u0027node\u0027: task.node.uuid})"},{"line_number":324,"context_line":"                LOG.error(msg)"},{"line_number":325,"context_line":"                raise exception.NodeCleaningFailure(msg)"},{"line_number":326,"context_line":"            # Match return value of get_node_vif_ids()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_5726d115","line":323,"in_reply_to":"ba15a1d1_6493a8fb","updated":"2015-09-16 19:52:26.000000000","message":"for strings to be translated even for single formatting variable I personally prefer the dict notation. translators do not see the code, only the string. seeing the meaning of this field helps them translate better, as in some languages this might have influence on word ordering etc.","commit_id":"fee3754512b6ee77b69c62d83d1fe9d159b19d61"}],"ironic/drivers/modules/agent_base_vendor.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"505dcb1028a422cf8b97a0bccc4590f66bb61797","unresolved":false,"context_lines":[{"line_number":422,"context_line":"        \"\"\""},{"line_number":423,"context_line":"        ports \u003d self._find_ports_by_macs(context, mac_addresses)"},{"line_number":424,"context_line":"        if not ports:"},{"line_number":425,"context_line":"            raise exception.NodeNotFound(_("},{"line_number":426,"context_line":"                \u0027No ports matching the given MAC addresses %s exist in the \u0027"},{"line_number":427,"context_line":"                \u0027database.\u0027) % mac_addresses)"},{"line_number":428,"context_line":"        node_id \u003d self._get_node_id(ports)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_45609401","line":425,"updated":"2015-09-15 17:51:06.000000000","message":"A test case for this would also be nice","commit_id":"c462c8bbd13fd267d859e0c31aa8195745e5607f"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"ad8197c2f03536d3bdf8b6a59da18d47f8985b95","unresolved":false,"context_lines":[{"line_number":422,"context_line":"        \"\"\""},{"line_number":423,"context_line":"        ports \u003d self._find_ports_by_macs(context, mac_addresses)"},{"line_number":424,"context_line":"        if not ports:"},{"line_number":425,"context_line":"            raise exception.NodeNotFound(_("},{"line_number":426,"context_line":"                \u0027No ports matching the given MAC addresses %s exist in the \u0027"},{"line_number":427,"context_line":"                \u0027database.\u0027) % mac_addresses)"},{"line_number":428,"context_line":"        node_id \u003d self._get_node_id(ports)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_f7c28533","line":425,"in_reply_to":"ba15a1d1_45609401","updated":"2015-09-16 19:47:41.000000000","message":"as I generally note, Ironic tests rarely assert the exactness of actual error message content. this here is simply a formatting improvement (adding a space between MAC and \"exists\") I stumbled upon.","commit_id":"c462c8bbd13fd267d859e0c31aa8195745e5607f"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"956aee0a557887cd48f52f93cc63f8c4c722ccd9","unresolved":false,"context_lines":[{"line_number":422,"context_line":"        \"\"\""},{"line_number":423,"context_line":"        ports \u003d self._find_ports_by_macs(context, mac_addresses)"},{"line_number":424,"context_line":"        if not ports:"},{"line_number":425,"context_line":"            raise exception.NodeNotFound(_("},{"line_number":426,"context_line":"                \u0027No ports matching the given MAC addresses %s exist in the \u0027"},{"line_number":427,"context_line":"                \u0027database.\u0027) % mac_addresses)"},{"line_number":428,"context_line":"        node_id \u003d self._get_node_id(ports)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_f5b7dd37","line":425,"in_reply_to":"ba15a1d1_f7c28533","updated":"2015-09-16 20:34:37.000000000","message":"And as can be seen this caused an error because the message content was not tested.\n\nI can\u0027t help to think that we (Ironic) should re-visit this not testing message content.","commit_id":"c462c8bbd13fd267d859e0c31aa8195745e5607f"}],"ironic/tests/dhcp/test_neutron.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"273118132660d54fa12689bcd53b4a3e274ca17e","unresolved":false,"context_lines":[{"line_number":399,"context_line":"                                                rollback_mock):"},{"line_number":400,"context_line":"        \"\"\"Check ports are cleaned up on failure to create them"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        This test checks that cleanup occures when port create call"},{"line_number":403,"context_line":"        was successfull but the port nethertheless failed to create."},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_d53e598b","line":402,"updated":"2015-09-16 20:53:36.000000000","message":"Suggestion:\n\n This test checks that the clean-up occurs when the port create call was successful, but the port in fact was not created.","commit_id":"f00d057d0891c83508f5f5026d3ffba4c757a8b1"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"9b85d2ac552f35d5eb760903d2ad73f9487cb2e1","unresolved":false,"context_lines":[{"line_number":399,"context_line":"                                                rollback_mock):"},{"line_number":400,"context_line":"        \"\"\"Check ports are cleaned up on failure to create them"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        This test checks that cleanup occures when port create call"},{"line_number":403,"context_line":"        was successfull but the port nethertheless failed to create."},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_84418765","line":402,"in_reply_to":"ba15a1d1_d53e598b","updated":"2015-09-17 06:57:37.000000000","message":"Done","commit_id":"f00d057d0891c83508f5f5026d3ffba4c757a8b1"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"273118132660d54fa12689bcd53b4a3e274ca17e","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        create_mock.return_value \u003d mockport"},{"line_number":410,"context_line":"        # fail on second \u0027or\u0027 branch to fool lazy eval"},{"line_number":411,"context_line":"        mockport.get.return_value \u003d True"},{"line_number":412,"context_line":"        mockitem \u003d mock.Mock()"},{"line_number":413,"context_line":"        mockport.__getitem__.return_value \u003d mockitem"},{"line_number":414,"context_line":"        mockitem.get.return_value \u003d None"},{"line_number":415,"context_line":"        api \u003d dhcp_factory.DHCPFactory().provider"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_95aea196","line":412,"updated":"2015-09-16 20:53:36.000000000","message":"Why not MagicMock like above?","commit_id":"f00d057d0891c83508f5f5026d3ffba4c757a8b1"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"9b85d2ac552f35d5eb760903d2ad73f9487cb2e1","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        create_mock.return_value \u003d mockport"},{"line_number":410,"context_line":"        # fail on second \u0027or\u0027 branch to fool lazy eval"},{"line_number":411,"context_line":"        mockport.get.return_value \u003d True"},{"line_number":412,"context_line":"        mockitem \u003d mock.Mock()"},{"line_number":413,"context_line":"        mockport.__getitem__.return_value \u003d mockitem"},{"line_number":414,"context_line":"        mockitem.get.return_value \u003d None"},{"line_number":415,"context_line":"        api \u003d dhcp_factory.DHCPFactory().provider"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_bf593437","line":412,"in_reply_to":"ba15a1d1_95aea196","updated":"2015-09-17 06:57:37.000000000","message":"I prefer not to use MagicMock when not needing its features. In the first case, I need the dictionary interface, so I use MagicMock that already has mocks for most __*__ methods including __getitem__. In the second case, I don\u0027t need such functionality, and it is safer to use Mock as it will complain when those extra magic methods will be called. Let\u0027s mock as less as needed :)\n\n\u003e\u003e\u003e import mock\n\u003e\u003e\u003e mm \u003d mock.MagicMock()\n\u003e\u003e\u003e mm[\u0027test\u0027]\n\u003cMagicMock name\u003d\u0027mock.__getitem__()\u0027 id\u003d\u0027140208810958416\u0027\u003e\n\u003e\u003e\u003e m \u003d mock.Mock()\n\u003e\u003e\u003e m[\u0027test\u0027]\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\nTypeError: \u0027Mock\u0027 object has no attribute \u0027__getitem__\u0027","commit_id":"f00d057d0891c83508f5f5026d3ffba4c757a8b1"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"7d9e716ab00260c2e7bd6ab0c3b339875f87299e","unresolved":false,"context_lines":[{"line_number":393,"context_line":"                \u0027admin_state_up\u0027: True, \u0027mac_address\u0027: self.ports[0].address}})"},{"line_number":394,"context_line":"            rollback_mock.assert_called_once_with(task)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"    @mock.patch.object(neutron.NeutronDHCPApi, \u0027_rollback_cleaning_ports\u0027)"},{"line_number":397,"context_line":"    @mock.patch.object(client.Client, \u0027create_port\u0027)"},{"line_number":398,"context_line":"    def test_create_cleaning_ports_fail_delayed(self, create_mock,"},{"line_number":399,"context_line":"                                                rollback_mock):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_22edaf25","line":396,"updated":"2015-09-19 04:13:03.000000000","message":"autospec\u003dTrue","commit_id":"0967238ad47cabccd1ecc2a4ef341120f410302f"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"7d9e716ab00260c2e7bd6ab0c3b339875f87299e","unresolved":false,"context_lines":[{"line_number":406,"context_line":"        \"\"\""},{"line_number":407,"context_line":"        # NOTE(pas-ha) this is trying to emulate the complex port object"},{"line_number":408,"context_line":"        # with both methods and dictionary access with methods on elements"},{"line_number":409,"context_line":"        mockport \u003d mock.MagicMock()"},{"line_number":410,"context_line":"        create_mock.return_value \u003d mockport"},{"line_number":411,"context_line":"        # fail only on second \u0027or\u0027 branch to fool lazy eval"},{"line_number":412,"context_line":"        # and actually execute both expressions to assert on both mocks"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba15a1d1_a2c0df7d","line":409,"updated":"2015-09-19 04:13:03.000000000","message":"Not sure if we need the many mocks mockport and mockitem.  Why not just\n\n  create_mock.return_value \u003d {}\n\nwhich should inturn give the same effect ?\n\nIf not, better to provide spec for these mocks\n\n  mockport \u003d mock.MagicMock(spec\u003ddict)","commit_id":"0967238ad47cabccd1ecc2a4ef341120f410302f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"9ffb3d6d2b4a637f960ba5667bb581bfd3bf1f14","unresolved":false,"context_lines":[{"line_number":406,"context_line":"        \"\"\""},{"line_number":407,"context_line":"        # NOTE(pas-ha) this is trying to emulate the complex port object"},{"line_number":408,"context_line":"        # with both methods and dictionary access with methods on elements"},{"line_number":409,"context_line":"        mockport \u003d mock.MagicMock()"},{"line_number":410,"context_line":"        create_mock.return_value \u003d mockport"},{"line_number":411,"context_line":"        # fail only on second \u0027or\u0027 branch to fool lazy eval"},{"line_number":412,"context_line":"        # and actually execute both expressions to assert on both mocks"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a1a9d01_bf1d36bf","line":409,"in_reply_to":"ba15a1d1_a2c0df7d","updated":"2015-09-25 11:34:35.000000000","message":"I think it would do it, but the idea was to test both conditionals on that if ? I guess\n\n if not port.get(\u0027port\u0027) or not port[\u0027port\u0027].get(\u0027id\u0027):","commit_id":"0967238ad47cabccd1ecc2a4ef341120f410302f"}],"ironic/tests/matchers.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"505dcb1028a422cf8b97a0bccc4590f66bb61797","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        self.d2_value \u003d d2_value"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def describe(self):"},{"line_number":43,"context_line":"        return (\"Dictionaries do not match at %(key)s.\""},{"line_number":44,"context_line":"                \" d1: %(d1_value)s d2: %(d2_value)s\" %"},{"line_number":45,"context_line":"                {\u0027d1_value\u0027: self.d1_value, \u0027d2_value\u0027: self.d2_value,"},{"line_number":46,"context_line":"                 \u0027key\u0027: self.key})"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_259000ec","line":43,"updated":"2015-09-15 17:51:06.000000000","message":"Sounds like it would be good if there was a test case for this.","commit_id":"c462c8bbd13fd267d859e0c31aa8195745e5607f"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"ad8197c2f03536d3bdf8b6a59da18d47f8985b95","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        self.d2_value \u003d d2_value"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def describe(self):"},{"line_number":43,"context_line":"        return (\"Dictionaries do not match at %(key)s.\""},{"line_number":44,"context_line":"                \" d1: %(d1_value)s d2: %(d2_value)s\" %"},{"line_number":45,"context_line":"                {\u0027d1_value\u0027: self.d1_value, \u0027d2_value\u0027: self.d2_value,"},{"line_number":46,"context_line":"                 \u0027key\u0027: self.key})"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_448ce1d9","line":43,"in_reply_to":"ba15a1d1_259000ec","updated":"2015-09-16 19:47:41.000000000","message":"in fact, as we support py27 only, we can dump all this module completely. it seems on py27 the assertEqual handles nested dicts comparison just fine\n\nif you do not use the assertEqual (that is overriden from unittest[2] by testtools) but assertDictEqual coming from parent classes directly, you even get a nice output highlighting the difference between dicts :)\n\nI\u0027ll make an extra patch for this.","commit_id":"c462c8bbd13fd267d859e0c31aa8195745e5607f"}]}