)]}'
{"nova/pci/request.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"57def64ebf215e4f4cda110e2dc07f5a2e36ef0d","unresolved":false,"context_lines":[{"line_number":98,"context_line":"_ALIASES \u003d None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def _get_alias_from_config():"},{"line_number":102,"context_line":"    \"\"\"Parse and validate PCI aliases from the nova config.\"\"\""},{"line_number":103,"context_line":"    jaliases \u003d CONF.pci.alias"},{"line_number":104,"context_line":"    global _ALIASES"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_0fc3f757","line":101,"updated":"2017-01-31 13:26:28.000000000","message":"I wonder if you have used a CPU time profiler and found out that this takes a significant portion of time or just doing this for \"do not repeat the same computation\" sake.\n\nHaving benchmark results and/out profiler stats in the commit message would be nice.","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b82cf83256293c158b534777cddb8a63cbca6bb4","unresolved":false,"context_lines":[{"line_number":98,"context_line":"_ALIASES \u003d None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def _get_alias_from_config():"},{"line_number":102,"context_line":"    \"\"\"Parse and validate PCI aliases from the nova config.\"\"\""},{"line_number":103,"context_line":"    jaliases \u003d CONF.pci.alias"},{"line_number":104,"context_line":"    global _ALIASES"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_b2cddb67","line":101,"in_reply_to":"3a461143_0fc3f757","updated":"2017-02-01 08:46:35.000000000","message":"Well, it was second option.\n\nBut here you have the results:\n1) Without memorize, one alias: http://paste.openstack.org/show/597064/\n2) Without memorize, two alias: http://paste.openstack.org/show/597063/\n3) With memorize decorator, one alias (first pass): http://paste.openstack.org/show/597062/\n4) With memorize decorator (second pass, once the result is stored): http://paste.openstack.org/show/597061/\n\n1) 4062 function calls (3743 primitive calls) in 0.004 seconds\n2) 6125 function calls (5700 primitive calls) in 0.005 seconds\n3) 4598 function calls (4281 primitive calls) in 0.004 seconds\n4) 34 function calls in 0.000 seconds\n\nIMHO these results justify this patch","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2e8646b71e0fe9a6f095303909453fa9a1a86207","unresolved":false,"context_lines":[{"line_number":98,"context_line":"_ALIASES \u003d None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def _get_alias_from_config():"},{"line_number":102,"context_line":"    \"\"\"Parse and validate PCI aliases from the nova config.\"\"\""},{"line_number":103,"context_line":"    jaliases \u003d CONF.pci.alias"},{"line_number":104,"context_line":"    global _ALIASES"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_f4a9f790","line":101,"in_reply_to":"3a461143_b2cddb67","updated":"2017-02-01 09:35:39.000000000","message":"I made a mistake. With the new implementation in nova.utils.memoize, the profiler results are:\n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    Traceback (most recent call last):\n      File \"nova/tests/unit/pci/test_request.py\", line 81, in test_good_alias\n        raise Exception(profile.print_stats())\n    Exception\n\n\nCaptured stdout:\n~~~~~~~~~~~~~~~~\n             3 function calls in 0.000 seconds\n\n       Ordered by: standard name\n\n       ncalls  tottime  percall  cumtime  percall filename:lineno(function)\n            1    0.000    0.000    0.000    0.000 utils.py:1506(__call__)\n            1    0.000    0.000    0.000    0.000 {method \u0027disable\u0027 of \u0027_lsprof.Profiler\u0027 objects}\n            1    0.000    0.000    0.000    0.000 {method \u0027iteritems\u0027 of \u0027dict\u0027 objects}","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"57def64ebf215e4f4cda110e2dc07f5a2e36ef0d","unresolved":false,"context_lines":[{"line_number":101,"context_line":"def _get_alias_from_config():"},{"line_number":102,"context_line":"    \"\"\"Parse and validate PCI aliases from the nova config.\"\"\""},{"line_number":103,"context_line":"    jaliases \u003d CONF.pci.alias"},{"line_number":104,"context_line":"    global _ALIASES"},{"line_number":105,"context_line":"    if isinstance(_ALIASES, dict):"},{"line_number":106,"context_line":"        return _ALIASES"},{"line_number":107,"context_line":"    elif isinstance(_ALIASES, Exception):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_cf6baf1c","line":104,"updated":"2017-01-31 13:26:28.000000000","message":"This is (subjectively) ugly. Can we refactor the logic below into a separate function, that we can easily memoize based on the argument value? Something like:\n\n  @memoize\n  def _get_alias(alias):\n       ...\n\n  def _get_alias_from_config():\n      return _get_alias(CONF.pci.alias)\n\nLooks like we have many similar implementations in openstack projects: http://codesearch.openstack.org/?q\u003dmemoize\u0026i\u003dnope\u0026files\u003d\u0026repos\u003d - you could probably add a basic one to oslo.utils, so that it can be reused.","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b82cf83256293c158b534777cddb8a63cbca6bb4","unresolved":false,"context_lines":[{"line_number":101,"context_line":"def _get_alias_from_config():"},{"line_number":102,"context_line":"    \"\"\"Parse and validate PCI aliases from the nova config.\"\"\""},{"line_number":103,"context_line":"    jaliases \u003d CONF.pci.alias"},{"line_number":104,"context_line":"    global _ALIASES"},{"line_number":105,"context_line":"    if isinstance(_ALIASES, dict):"},{"line_number":106,"context_line":"        return _ALIASES"},{"line_number":107,"context_line":"    elif isinstance(_ALIASES, Exception):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_1f3dc1dc","line":104,"in_reply_to":"3a461143_cf6baf1c","updated":"2017-02-01 08:46:35.000000000","message":"Could be ugly but effective.\n\nI the following patch you\u0027ll see in nova.utils a new \"memoize\" function implemented.\n\nIf this patch is accepted, I\u0027ll implement it in oslo.utils\n\nBTW, most of the projects are using horizon.utils.memorized function, which is too complex for this porpouse.","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a2e5767e6f7544daea4d2ca821113052918d1d75","unresolved":false,"context_lines":[{"line_number":104,"context_line":"    global _ALIASES"},{"line_number":105,"context_line":"    if isinstance(_ALIASES, dict):"},{"line_number":106,"context_line":"        return _ALIASES"},{"line_number":107,"context_line":"    elif isinstance(_ALIASES, Exception):"},{"line_number":108,"context_line":"        raise _ALIASES"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    _ALIASES \u003d {}  # map alias name to alias spec list"},{"line_number":111,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_866c2909","line":108,"range":{"start_line":107,"start_character":0,"end_line":108,"end_character":22},"updated":"2017-01-31 16:11:45.000000000","message":"Huh? I don\u0027t see why we\u0027d be storing an exception in this variable. If we fail to parse any of the aliases, I think we should raise an exception (as we do currently) and unset _ALIASES","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b82cf83256293c158b534777cddb8a63cbca6bb4","unresolved":false,"context_lines":[{"line_number":104,"context_line":"    global _ALIASES"},{"line_number":105,"context_line":"    if isinstance(_ALIASES, dict):"},{"line_number":106,"context_line":"        return _ALIASES"},{"line_number":107,"context_line":"    elif isinstance(_ALIASES, Exception):"},{"line_number":108,"context_line":"        raise _ALIASES"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    _ALIASES \u003d {}  # map alias name to alias spec list"},{"line_number":111,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_b1286187","line":108,"range":{"start_line":107,"start_character":0,"end_line":108,"end_character":22},"in_reply_to":"3a461143_866c2909","updated":"2017-02-01 08:46:35.000000000","message":"If the configuration parameter \"pci_alias\" is static, the expect behaviour everytime this function is executed must be the same.\n\nThis function will be call everytime a new instance is required, so the aim is to raise an exeception everytime is called.\n\nNevermind, I\u0027m going to refector this implementation.","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a2e5767e6f7544daea4d2ca821113052918d1d75","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                    _ALIASES[name].append(spec)"},{"line_number":126,"context_line":"                else:"},{"line_number":127,"context_line":"                    reason \u003d _(\"Device type mismatch for alias \u0027%s\u0027\") % name"},{"line_number":128,"context_line":"                    raise exception.PciInvalidAlias(reason\u003dreason)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    except exception.PciInvalidAlias:"},{"line_number":131,"context_line":"        raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_a63da5de","line":128,"updated":"2017-01-31 16:11:45.000000000","message":"If you\u0027re going with the above, shouldn\u0027t you be storing the exception against _ALISES here?","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b82cf83256293c158b534777cddb8a63cbca6bb4","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                    _ALIASES[name].append(spec)"},{"line_number":126,"context_line":"                else:"},{"line_number":127,"context_line":"                    reason \u003d _(\"Device type mismatch for alias \u0027%s\u0027\") % name"},{"line_number":128,"context_line":"                    raise exception.PciInvalidAlias(reason\u003dreason)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    except exception.PciInvalidAlias:"},{"line_number":131,"context_line":"        raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_f15589f9","line":128,"in_reply_to":"3a461143_a63da5de","updated":"2017-02-01 08:46:35.000000000","message":"Ehmmm... yes. I don\u0027t know why I\u0027m not doing this now.","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"}],"nova/tests/unit/pci/test_request.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"57def64ebf215e4f4cda110e2dc07f5a2e36ef0d","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"class AliasTestCase(test.NoDBTestCase):"},{"line_number":67,"context_line":"    def tearDown(self):"},{"line_number":68,"context_line":"        super(AliasTestCase, self).tearDown()"},{"line_number":69,"context_line":"        if request._ALIASES:"},{"line_number":70,"context_line":"            request._ALIASES \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_3a054f10","line":67,"updated":"2017-01-31 13:26:28.000000000","message":"We prefer testtool\u0027s addCleanup() instead of tearDown() - http://testtools.readthedocs.io/en/latest/api.html#testtools.TestCase.addCleanup","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b82cf83256293c158b534777cddb8a63cbca6bb4","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"class AliasTestCase(test.NoDBTestCase):"},{"line_number":67,"context_line":"    def tearDown(self):"},{"line_number":68,"context_line":"        super(AliasTestCase, self).tearDown()"},{"line_number":69,"context_line":"        if request._ALIASES:"},{"line_number":70,"context_line":"            request._ALIASES \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_117055a3","line":67,"in_reply_to":"3a461143_3a054f10","updated":"2017-02-01 08:46:35.000000000","message":"Done!\n\nThanks for the tip","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a2e5767e6f7544daea4d2ca821113052918d1d75","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        self.assertRaises(exception.PciInvalidAlias,"},{"line_number":153,"context_line":"            request._get_alias_from_config)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def test_alias_exception_memento(self):"},{"line_number":156,"context_line":"        request._ALIASES \u003d exception.PciInvalidAlias()"},{"line_number":157,"context_line":"        self.assertRaises(exception.PciInvalidAlias,"},{"line_number":158,"context_line":"            request._get_alias_from_config)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def test_dup_aliase(self):"},{"line_number":161,"context_line":"        self.flags(alias\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_21e27f2f","line":158,"range":{"start_line":155,"start_character":0,"end_line":158,"end_character":43},"updated":"2017-01-31 16:11:45.000000000","message":"This isn\u0027t a great test, assuming my comments on the previous page are correct and the code isn\u0027t correct. If you want to go this direction, I would suggest parsing an invalid alias then checking if the \u0027_ALISES\u0027 variable is set to an exception.","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b82cf83256293c158b534777cddb8a63cbca6bb4","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        self.assertRaises(exception.PciInvalidAlias,"},{"line_number":153,"context_line":"            request._get_alias_from_config)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def test_alias_exception_memento(self):"},{"line_number":156,"context_line":"        request._ALIASES \u003d exception.PciInvalidAlias()"},{"line_number":157,"context_line":"        self.assertRaises(exception.PciInvalidAlias,"},{"line_number":158,"context_line":"            request._get_alias_from_config)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def test_dup_aliase(self):"},{"line_number":161,"context_line":"        self.flags(alias\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_915c451a","line":158,"range":{"start_line":155,"start_character":0,"end_line":158,"end_character":43},"in_reply_to":"3a461143_21e27f2f","updated":"2017-02-01 08:46:35.000000000","message":"The whole point of using _ALIASES is not to parse again. I doesn\u0027t make sense to parse again because I\u0027m trying to avoid it.","commit_id":"7ea5622e3a7c622edce4181961fb882bb94b78c2"}]}