)]}'
{"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"76c1f4d53b2800c6bc652c56fe925468dbe17906","unresolved":false,"context_lines":[{"line_number":3018,"context_line":"    return model_query(context, models.BlockDeviceMapping, session\u003dsession)"},{"line_number":3019,"context_line":""},{"line_number":3020,"context_line":""},{"line_number":3021,"context_line":"def _remove_empty_str_volume_size(values):"},{"line_number":3022,"context_line":"    \"\"\"Remove volume_size: \u0027\u0027, if found, from the dict.\"\"\""},{"line_number":3023,"context_line":"    try:"},{"line_number":3024,"context_line":"        if values[\u0027volume_size\u0027] \u003d\u003d \u0027\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOn%2F%2Fdig%3D","line":3021,"updated":"2013-05-08 21:57:08.000000000","message":"is this something that we\u0027re going to see more of? Maybe it makes sense to make this a generic \"scrub empty\" kind of function that takes a list of keys that could be a problem and deletes them.","commit_id":"378a601fecbe3adbb641f97540639b51ec292ad3"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"76c1f4d53b2800c6bc652c56fe925468dbe17906","unresolved":false,"context_lines":[{"line_number":3021,"context_line":"def _remove_empty_str_volume_size(values):"},{"line_number":3022,"context_line":"    \"\"\"Remove volume_size: \u0027\u0027, if found, from the dict.\"\"\""},{"line_number":3023,"context_line":"    try:"},{"line_number":3024,"context_line":"        if values[\u0027volume_size\u0027] \u003d\u003d \u0027\u0027:"},{"line_number":3025,"context_line":"            del values[\u0027volume_size\u0027]"},{"line_number":3026,"context_line":"    except KeyError:"},{"line_number":3027,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOn%2F%2FdiU%3D","line":3024,"updated":"2013-05-08 21:57:08.000000000","message":"instead of try / except here, how about values.get(...) which won\u0027t explode.","commit_id":"378a601fecbe3adbb641f97540639b51ec292ad3"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"1875ecc25f6498e1e2b525549cd0890d089839b1","unresolved":false,"context_lines":[{"line_number":3021,"context_line":"def _remove_empty_str_volume_size(values):"},{"line_number":3022,"context_line":"    \"\"\"Remove volume_size: \u0027\u0027, if found, from the dict.\"\"\""},{"line_number":3023,"context_line":"    try:"},{"line_number":3024,"context_line":"        if values[\u0027volume_size\u0027] \u003d\u003d \u0027\u0027:"},{"line_number":3025,"context_line":"            del values[\u0027volume_size\u0027]"},{"line_number":3026,"context_line":"    except KeyError:"},{"line_number":3027,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOn%2F%2FdhI%3D","line":3024,"in_reply_to":"AAAAOn%2F%2FdiU%3D","updated":"2013-05-08 22:00:34.000000000","message":"Wy we are not able just to do next\n  if \u0027volume_size\u0027 in values and values[\u0027volume_size\u0027] \u003d\u003d \u0027\u0027:\n     del values[\u0027volume_size\u0027]","commit_id":"378a601fecbe3adbb641f97540639b51ec292ad3"},{"author":{"_account_id":5371,"name":"Mathew Odden","email":"locke105@gmail.com","username":"locke105"},"change_message_id":"6b63b6a066bf2e6cfca69a18bd1617c63243db49","unresolved":false,"context_lines":[{"line_number":3021,"context_line":"def _remove_empty_str_volume_size(values):"},{"line_number":3022,"context_line":"    \"\"\"Remove volume_size: \u0027\u0027, if found, from the dict.\"\"\""},{"line_number":3023,"context_line":"    try:"},{"line_number":3024,"context_line":"        if values[\u0027volume_size\u0027] \u003d\u003d \u0027\u0027:"},{"line_number":3025,"context_line":"            del values[\u0027volume_size\u0027]"},{"line_number":3026,"context_line":"    except KeyError:"},{"line_number":3027,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAOn%2F%2FdZk%3D","line":3024,"updated":"2013-05-08 22:52:44.000000000","message":"I think Boris\u0027 approach would probably be more concise here instead of catching a KeyError.\n\nAlso, this could easily be extend to scrub any arbitrary key in the dictionary for empty strings by adding a parameter for the value to scrub. (I think Sean was hinting at something like this as well.)","commit_id":"c732e222eff9f30aa14b103cef1a3d062eec7efb"},{"author":{"_account_id":5371,"name":"Mathew Odden","email":"locke105@gmail.com","username":"locke105"},"change_message_id":"a447816b169677fc7aa3c6e254a574d8a368d9a9","unresolved":false,"context_lines":[{"line_number":3018,"context_line":"    return model_query(context, models.BlockDeviceMapping, session\u003dsession)"},{"line_number":3019,"context_line":""},{"line_number":3020,"context_line":""},{"line_number":3021,"context_line":"def _scrub_empty_str_values(values, keys_to_scrub):"},{"line_number":3022,"context_line":"    \"\"\""},{"line_number":3023,"context_line":"    Remove any keys found in sequence keys_to_scrub from the dict"},{"line_number":3024,"context_line":"    if they have the value \u0027\u0027."}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FdYg%3D","line":3021,"updated":"2013-05-08 23:02:17.000000000","message":"You don\u0027t appear to be using keys_to_scrub...?\n\nAlso, its not an optional parameter so the calls in the functions below will fail.","commit_id":"83abc79d32b1206b263cc5baacd2395017b7eb3b"}],"nova/tests/test_db_api.py":[{"author":{"_account_id":5371,"name":"Mathew Odden","email":"locke105@gmail.com","username":"locke105"},"change_message_id":"6b63b6a066bf2e6cfca69a18bd1617c63243db49","unresolved":false,"context_lines":[{"line_number":3345,"context_line":"            if bdm[\u0027device_name\u0027] \u003d\u003d values[\u0027device_name\u0027]:"},{"line_number":3346,"context_line":"                return bdm"},{"line_number":3347,"context_line":""},{"line_number":3348,"context_line":"    def test_remove_empty_str_volume_size(self):"},{"line_number":3349,"context_line":"        values \u003d {\u0027volume_size\u0027: 5}"},{"line_number":3350,"context_line":"        values2 \u003d copy.copy(values)"},{"line_number":3351,"context_line":"        sqlalchemy_api._remove_empty_str_volume_size(values)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAOn%2F%2FdaQ%3D","line":3348,"updated":"2013-05-08 22:52:44.000000000","message":"so it looks to me that you have 3 test cases in one here and it would be smart and fairly simple to split them into 3 test functions\n\n1. volume size is a number\n\n2. volume size is empty string\n\n3. volume size is empty unicode string","commit_id":"c732e222eff9f30aa14b103cef1a3d062eec7efb"},{"author":{"_account_id":5371,"name":"Mathew Odden","email":"locke105@gmail.com","username":"locke105"},"change_message_id":"6b63b6a066bf2e6cfca69a18bd1617c63243db49","unresolved":false,"context_lines":[{"line_number":3347,"context_line":""},{"line_number":3348,"context_line":"    def test_remove_empty_str_volume_size(self):"},{"line_number":3349,"context_line":"        values \u003d {\u0027volume_size\u0027: 5}"},{"line_number":3350,"context_line":"        values2 \u003d copy.copy(values)"},{"line_number":3351,"context_line":"        sqlalchemy_api._remove_empty_str_volume_size(values)"},{"line_number":3352,"context_line":"        self.assertEqual(values, values2)"},{"line_number":3353,"context_line":"        values \u003d {\u0027volume_size\u0027: \u0027\u0027}"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAOn%2F%2FdaE%3D","line":3350,"updated":"2013-05-08 22:52:44.000000000","message":"nit: values2 would probably be better named \u0027expected_values\u0027 or just \u0027expected\u0027","commit_id":"c732e222eff9f30aa14b103cef1a3d062eec7efb"}]}