)]}'
{"tempest/common/resource_configuration.py":[{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03ec62337490b313a97301cc0a7f8601b098d332","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        return self.resources[\u0027resources\u0027].get(resource_type) or []"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def get_resource_types(self):"},{"line_number":60,"context_line":"        return self.resources.keys() or []"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_73ef1b1e","line":60,"updated":"2016-03-10 11:03:24.000000000","message":"Why the \u0027or []\u0027 part ?\n\nkeys() will already return an empty list if there\u0027s no key ?","commit_id":"d3e20a6750252242a67e458c41650575482ee725"}],"tempest/common/resource_provider.py":[{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03ec62337490b313a97301cc0a7f8601b098d332","unresolved":false,"context_lines":[{"line_number":21,"context_line":"        self.resource_config \u003d resource_config"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def get_resource_by_type(self, resource_type, used_resources,"},{"line_number":24,"context_line":"                             fallback_enabled\u003dFalse, **kwargs):"},{"line_number":25,"context_line":"        resource_types \u003d self.resource_config.get_resource_types()"},{"line_number":26,"context_line":"        if resource_type not in resource_types:"},{"line_number":27,"context_line":"            raise exceptions.InvalidConfiguration("}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_d398af6c","line":24,"updated":"2016-03-10 11:03:24.000000000","message":"Could you rename **kwargs to \"filters\u003dNone\" ? It seems the only kwargs you are expecting are filters.","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":97,"name":"Daryl Walleck","email":"daryl.walleck@rackspace.com","username":"dwalleck"},"change_message_id":"2dd54dc33c40eea53e717ff5962b8a0ad8efbf00","unresolved":false,"context_lines":[{"line_number":21,"context_line":"        self.resource_config \u003d resource_config"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def get_resource_by_type(self, resource_type, used_resources,"},{"line_number":24,"context_line":"                             fallback_enabled\u003dFalse, **kwargs):"},{"line_number":25,"context_line":"        resource_types \u003d self.resource_config.get_resource_types()"},{"line_number":26,"context_line":"        if resource_type not in resource_types:"},{"line_number":27,"context_line":"            raise exceptions.InvalidConfiguration("}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_92a4494b","line":24,"in_reply_to":"5aef4532_d398af6c","updated":"2016-03-10 17:54:39.000000000","message":"You\u0027re correct, you could. However, at the mid cycle Andrea specifically asked to use kwargs. I\u0027m not tied to either way","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03ec62337490b313a97301cc0a7f8601b098d332","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def get_resource_by_type(self, resource_type, used_resources,"},{"line_number":24,"context_line":"                             fallback_enabled\u003dFalse, **kwargs):"},{"line_number":25,"context_line":"        resource_types \u003d self.resource_config.get_resource_types()"},{"line_number":26,"context_line":"        if resource_type not in resource_types:"},{"line_number":27,"context_line":"            raise exceptions.InvalidConfiguration("},{"line_number":28,"context_line":"                \u0027Resource type of %s not found. Expected resource \u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_d3b38f08","line":25,"updated":"2016-03-10 11:03:24.000000000","message":"You should try to get the resources and if you get \"[]\" then raise this InvalidConfiguration exception.\n\nIn python, better ask forgiveness than permission.","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":97,"name":"Daryl Walleck","email":"daryl.walleck@rackspace.com","username":"dwalleck"},"change_message_id":"2dd54dc33c40eea53e717ff5962b8a0ad8efbf00","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def get_resource_by_type(self, resource_type, used_resources,"},{"line_number":24,"context_line":"                             fallback_enabled\u003dFalse, **kwargs):"},{"line_number":25,"context_line":"        resource_types \u003d self.resource_config.get_resource_types()"},{"line_number":26,"context_line":"        if resource_type not in resource_types:"},{"line_number":27,"context_line":"            raise exceptions.InvalidConfiguration("},{"line_number":28,"context_line":"                \u0027Resource type of %s not found. Expected resource \u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_35f27f06","line":25,"in_reply_to":"5aef4532_d3b38f08","updated":"2016-03-10 17:54:39.000000000","message":"I would argue that this is pythonic from the \"Explicit is better than implicit.\" standpoint. In my opinion, this implementation of the error handling makes this section of code easier to understand. It also handles the error closer to where the actual issue occurred.","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":1980,"name":"jose-idar","email":"jose.idar@gmail.com","username":"jose-idar"},"change_message_id":"0a01de27e5bffc9c4991b8f2499799fe7229b7e3","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def get_resource_by_type(self, resource_type, used_resources,"},{"line_number":24,"context_line":"                             fallback_enabled\u003dFalse, **kwargs):"},{"line_number":25,"context_line":"        resource_types \u003d self.resource_config.get_resource_types()"},{"line_number":26,"context_line":"        if resource_type not in resource_types:"},{"line_number":27,"context_line":"            raise exceptions.InvalidConfiguration("},{"line_number":28,"context_line":"                \u0027Resource type of %s not found. Expected resource \u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_1546637a","line":25,"in_reply_to":"5aef4532_d3b38f08","updated":"2016-03-10 18:05:03.000000000","message":"You\u0027d still need an \u0027if\u0027 to implement that (\u0027asking permission\u0027), and the code here catches that case anyway, in addition to the case where the config isn\u0027t empty, just misconfigured.","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03ec62337490b313a97301cc0a7f8601b098d332","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                               if resource not in used_resources]"},{"line_number":36,"context_line":"        if available_resources:"},{"line_number":37,"context_line":"            # Apply all provided filters"},{"line_number":38,"context_line":"            filtered_resources \u003d list(available_resources)"},{"line_number":39,"context_line":"            for k, v in kwargs.items():"},{"line_number":40,"context_line":"                filtered_resources \u003d ["},{"line_number":41,"context_line":"                    resource for resource in filtered_resources"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_b3e9a3b9","line":38,"updated":"2016-03-10 11:03:24.000000000","message":"Isn\u0027t `available_resources` a list already ?","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"0ec5901289e2e8ec56c478a699ccdd8b15f9bb8f","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                               if resource not in used_resources]"},{"line_number":36,"context_line":"        if available_resources:"},{"line_number":37,"context_line":"            # Apply all provided filters"},{"line_number":38,"context_line":"            filtered_resources \u003d list(available_resources)"},{"line_number":39,"context_line":"            for k, v in kwargs.items():"},{"line_number":40,"context_line":"                filtered_resources \u003d ["},{"line_number":41,"context_line":"                    resource for resource in filtered_resources"}],"source_content_type":"text/x-python","patch_set":4,"id":"1af94dfe_4bb8e8e5","line":38,"in_reply_to":"5aef4532_35dc3f45","updated":"2016-03-16 14:52:38.000000000","message":"OK. I usually use the \u0027[:]\u0027 syntax to make a copy. And I leave a code comment ;)","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":97,"name":"Daryl Walleck","email":"daryl.walleck@rackspace.com","username":"dwalleck"},"change_message_id":"2dd54dc33c40eea53e717ff5962b8a0ad8efbf00","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                               if resource not in used_resources]"},{"line_number":36,"context_line":"        if available_resources:"},{"line_number":37,"context_line":"            # Apply all provided filters"},{"line_number":38,"context_line":"            filtered_resources \u003d list(available_resources)"},{"line_number":39,"context_line":"            for k, v in kwargs.items():"},{"line_number":40,"context_line":"                filtered_resources \u003d ["},{"line_number":41,"context_line":"                    resource for resource in filtered_resources"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_35dc3f45","line":38,"in_reply_to":"5aef4532_b3e9a3b9","updated":"2016-03-10 17:54:39.000000000","message":"This makes a new copy of the list, which allows me to keep the full and filtered lists independent. The full list is needed in case we hit the fallback case","commit_id":"d3e20a6750252242a67e458c41650575482ee725"}],"tempest/tests/common/test_resource_provider.py":[{"author":{"_account_id":1980,"name":"jose-idar","email":"jose.idar@gmail.com","username":"jose-idar"},"change_message_id":"0a01de27e5bffc9c4991b8f2499799fe7229b7e3","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.provider \u003d resource_provider.ResourceProvider(resource_config)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def test_get_resource_no_filter(self):"},{"line_number":48,"context_line":"        resource \u003d self.provider.get_resource_by_type(\u0027images\u0027, [])"},{"line_number":49,"context_line":"        self.assertEqual(resource.get(\u0027resource_name\u0027), \u0027ubuntu\u0027)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def test_get_resource_with_filter(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_686b8ab7","line":48,"range":{"start_line":48,"start_character":64,"end_line":48,"end_character":67},"updated":"2016-03-10 18:05:03.000000000","message":"This should default to dict in this case, since it seems that this is expected to return a dict.","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03ec62337490b313a97301cc0a7f8601b098d332","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def test_get_resource_no_filter(self):"},{"line_number":48,"context_line":"        resource \u003d self.provider.get_resource_by_type(\u0027images\u0027, [])"},{"line_number":49,"context_line":"        self.assertEqual(resource.get(\u0027resource_name\u0027), \u0027ubuntu\u0027)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def test_get_resource_with_filter(self):"},{"line_number":52,"context_line":"        resource \u003d self.provider.get_resource_by_type("}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_931bc780","line":49,"updated":"2016-03-10 11:03:24.000000000","message":"Why do you access your dict using `get` ? I don\"t see why you do this extra function call. Furthermore, it\u0027s not really pythonic.\n\nI suggest `resources[\u0027resource_name\u0027]`","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"0ec5901289e2e8ec56c478a699ccdd8b15f9bb8f","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def test_get_resource_no_filter(self):"},{"line_number":48,"context_line":"        resource \u003d self.provider.get_resource_by_type(\u0027images\u0027, [])"},{"line_number":49,"context_line":"        self.assertEqual(resource.get(\u0027resource_name\u0027), \u0027ubuntu\u0027)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def test_get_resource_with_filter(self):"},{"line_number":52,"context_line":"        resource \u003d self.provider.get_resource_by_type("}],"source_content_type":"text/x-python","patch_set":4,"id":"1af94dfe_4b10e8a8","line":49,"in_reply_to":"5aef4532_88c25651","updated":"2016-03-16 14:52:38.000000000","message":"I advocate to use `resources[\u0027resource_name\u0027]`\n\n`get` is good if you want to provide a fallback value in case the Key is missing. Otherwise I don\"t see why it is used.","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":1980,"name":"jose-idar","email":"jose.idar@gmail.com","username":"jose-idar"},"change_message_id":"0a01de27e5bffc9c4991b8f2499799fe7229b7e3","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def test_get_resource_no_filter(self):"},{"line_number":48,"context_line":"        resource \u003d self.provider.get_resource_by_type(\u0027images\u0027, [])"},{"line_number":49,"context_line":"        self.assertEqual(resource.get(\u0027resource_name\u0027), \u0027ubuntu\u0027)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def test_get_resource_with_filter(self):"},{"line_number":52,"context_line":"        resource \u003d self.provider.get_resource_by_type("}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_88c25651","line":49,"in_reply_to":"5aef4532_931bc780","updated":"2016-03-10 18:05:03.000000000","message":"Are you advocating that the test should raise a KeyError in the case where the dictionary does not contain \u0027resource_name\u0027 instead of failing with a proper assert message?  \n\nUsing .get() on the dictionary guarantees that you get back an object that the assertEqual method can operate on (in this case \u0027None\u0027), allowing this test to fail appropriately in those cases.","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03ec62337490b313a97301cc0a7f8601b098d332","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    def test_get_resource_with_filter(self):"},{"line_number":52,"context_line":"        resource \u003d self.provider.get_resource_by_type("},{"line_number":53,"context_line":"            \u0027images\u0027, [], boot\u003d\u0027fast\u0027)"},{"line_number":54,"context_line":"        self.assertEqual(resource.get(\u0027resource_name\u0027), \u0027ubuntu-lts\u0027)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def test_get_resource_with_multiple_filters(self):"},{"line_number":57,"context_line":"        resource \u003d self.provider.get_resource_by_type("}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_33011307","line":54,"updated":"2016-03-10 11:03:24.000000000","message":"same","commit_id":"d3e20a6750252242a67e458c41650575482ee725"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"03ec62337490b313a97301cc0a7f8601b098d332","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    def test_get_resource_with_multiple_filters(self):"},{"line_number":57,"context_line":"        resource \u003d self.provider.get_resource_by_type("},{"line_number":58,"context_line":"            \u0027images\u0027, [], boot\u003d\u0027slow\u0027, size\u003d\u00279 GB\u0027)"},{"line_number":59,"context_line":"        self.assertEqual(resource.get(\u0027resource_name\u0027), \u0027windows\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def test_get_resource_with_resources_in_use(self):"},{"line_number":62,"context_line":"        resource \u003d self.provider.get_resource_by_type("}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_7308ba28","line":59,"updated":"2016-03-10 11:03:24.000000000","message":"same","commit_id":"d3e20a6750252242a67e458c41650575482ee725"}]}