)]}'
{"novaclient/client.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"ee57b7e63f49ced6603ea9fd1d90c44b3295138c","unresolved":false,"context_lines":[{"line_number":417,"context_line":"                url \u003d self.get_service_url(self.service_type) + url"},{"line_number":418,"context_line":"            else:"},{"line_number":419,"context_line":"                url \u003d self.management_url + url"},{"line_number":420,"context_line":"            self._last_service_type \u003d self.service_type"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"        # Perform the request once. If we get a 401 back then it"},{"line_number":423,"context_line":"        # might be because the auth token expired, so try to"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a80dd14_4cd8f852","line":420,"updated":"2015-03-16 15:42:51.000000000","message":"Hmmm…you don\u0027t update self.management_url here.  That means that the next time through this function, you\u0027ll use the \"management_url + url\" even though the service_type still says \"volume\".  You probably didn\u0027t pick up on this in unit tests because you\u0027re only making one call within the context manager you defined…","commit_id":"29881b45f6e98b0ae24ac1cf79f7bb0d8fbd6be7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4c0eee22e386c18b3bfa4930ab0859d9a2c5eca6","unresolved":false,"context_lines":[{"line_number":417,"context_line":"                url \u003d self.get_service_url(self.service_type) + url"},{"line_number":418,"context_line":"            else:"},{"line_number":419,"context_line":"                url \u003d self.management_url + url"},{"line_number":420,"context_line":"            self._last_service_type \u003d self.service_type"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"        # Perform the request once. If we get a 401 back then it"},{"line_number":423,"context_line":"        # might be because the auth token expired, so try to"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a80dd14_f617ae43","line":420,"in_reply_to":"9a80dd14_4cd8f852","updated":"2015-03-16 18:28:34.000000000","message":"Ugh, logic error trying to save some lookups on the service catalog. Initially I just had it doing get_service_url all the time but thought maybe it\u0027s too heavy, and tried to optimize. But with your idea of a lightweight lookup layer over things, looking up every time would work fine","commit_id":"29881b45f6e98b0ae24ac1cf79f7bb0d8fbd6be7"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"ee57b7e63f49ced6603ea9fd1d90c44b3295138c","unresolved":false,"context_lines":[{"line_number":460,"context_line":"    def delete(self, url, **kwargs):"},{"line_number":461,"context_line":"        return self._cs_request(url, \u0027DELETE\u0027, **kwargs)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    def get_service_url(self, service_type):"},{"line_number":464,"context_line":"        url \u003d self.service_catalog.url_for("},{"line_number":465,"context_line":"            attr\u003d\u0027region\u0027,"},{"line_number":466,"context_line":"            filter_value\u003dself.region_name,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a80dd14_ec736c4b","line":463,"updated":"2015-03-16 15:42:51.000000000","message":"Have you considered a demand-computed cache?  I.e., initialize self.services_url to {}, then here you can do:\n\n    if service_type not in self.services_url:\n        self.services_url[service_type] \u003d self.service_catalog.url_for(…)\n\n    return self.services_url[service_type]\n\nWith that, you should be able to eliminate the self._last_service_type logic and possibly even self.management_url and just use line 417 unconditionally.","commit_id":"29881b45f6e98b0ae24ac1cf79f7bb0d8fbd6be7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4c0eee22e386c18b3bfa4930ab0859d9a2c5eca6","unresolved":false,"context_lines":[{"line_number":460,"context_line":"    def delete(self, url, **kwargs):"},{"line_number":461,"context_line":"        return self._cs_request(url, \u0027DELETE\u0027, **kwargs)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    def get_service_url(self, service_type):"},{"line_number":464,"context_line":"        url \u003d self.service_catalog.url_for("},{"line_number":465,"context_line":"            attr\u003d\u0027region\u0027,"},{"line_number":466,"context_line":"            filter_value\u003dself.region_name,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a80dd14_3b668f2a","line":463,"in_reply_to":"9a80dd14_ec736c4b","updated":"2015-03-16 18:28:34.000000000","message":"I thought about something similar initially, but when I saw that service catalog is already cached, I didn\u0027t think of adding a lightweight lookup on top of it. I like this idea","commit_id":"29881b45f6e98b0ae24ac1cf79f7bb0d8fbd6be7"}],"novaclient/tests/functional/test_volumes_api.py":[{"author":{"_account_id":14027,"name":"Michal Rostecki","email":"mrostecki@suse.de","username":"mrostecki"},"change_message_id":"36acdd8b0653c79a757fa4900fcc302c258db873","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def wait_for_delete(name, thing, get_func):"},{"line_number":23,"context_line":"    thing.delete()"},{"line_number":24,"context_line":"    for x in xrange(60):"},{"line_number":25,"context_line":"        try:"},{"line_number":26,"context_line":"            thing \u003d get_func(thing.id)"},{"line_number":27,"context_line":"        except exceptions.NotFound:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_cb6873f7","line":24,"updated":"2015-03-15 09:29:06.000000000","message":"Shouldn\u0027t we use six.moves.range for Python 3.x compatibility?","commit_id":"229af18a4790789c5e303e71a6b18ea48598eedd"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b17cc856d9d171ae942ca735502241518782c684","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def wait_for_delete(name, thing, get_func):"},{"line_number":23,"context_line":"    thing.delete()"},{"line_number":24,"context_line":"    for x in xrange(60):"},{"line_number":25,"context_line":"        try:"},{"line_number":26,"context_line":"            thing \u003d get_func(thing.id)"},{"line_number":27,"context_line":"        except exceptions.NotFound:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_13d0fdc7","line":24,"in_reply_to":"9a80dd14_cb6873f7","updated":"2015-03-15 20:10:35.000000000","message":"I didn\u0027t see it in existing functional tests but I can add it","commit_id":"229af18a4790789c5e303e71a6b18ea48598eedd"},{"author":{"_account_id":14027,"name":"Michal Rostecki","email":"mrostecki@suse.de","username":"mrostecki"},"change_message_id":"36acdd8b0653c79a757fa4900fcc302c258db873","unresolved":false,"context_lines":[{"line_number":28,"context_line":"            break"},{"line_number":29,"context_line":"        time.sleep(1)"},{"line_number":30,"context_line":"    else:"},{"line_number":31,"context_line":"        self.fail(\u0027%s %s still not deleted after 60s\u0027 % (name, thing.id))"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class TestVolumesAPI(base.ClientTestBase):"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_8b8cbbd8","line":31,"updated":"2015-03-15 09:29:06.000000000","message":"There is no variable called \u0027self\u0027 in this function. Woludn\u0027t this cause NameError?","commit_id":"229af18a4790789c5e303e71a6b18ea48598eedd"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b17cc856d9d171ae942ca735502241518782c684","unresolved":false,"context_lines":[{"line_number":28,"context_line":"            break"},{"line_number":29,"context_line":"        time.sleep(1)"},{"line_number":30,"context_line":"    else:"},{"line_number":31,"context_line":"        self.fail(\u0027%s %s still not deleted after 60s\u0027 % (name, thing.id))"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class TestVolumesAPI(base.ClientTestBase):"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_b34731a2","line":31,"in_reply_to":"9a80dd14_8b8cbbd8","updated":"2015-03-15 20:10:35.000000000","message":"You\u0027re right, thanks","commit_id":"229af18a4790789c5e303e71a6b18ea48598eedd"},{"author":{"_account_id":14027,"name":"Michal Rostecki","email":"mrostecki@suse.de","username":"mrostecki"},"change_message_id":"36acdd8b0653c79a757fa4900fcc302c258db873","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.addCleanup(volume.delete)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # Wait for the volume to become available"},{"line_number":56,"context_line":"        for x in xrange(60):"},{"line_number":57,"context_line":"            volume \u003d self.client.volumes.get(volume.id)"},{"line_number":58,"context_line":"            if volume.status \u003d\u003d \u0027available\u0027:"},{"line_number":59,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_eb6b77ee","line":56,"updated":"2015-03-15 09:29:06.000000000","message":"six.moves.range","commit_id":"229af18a4790789c5e303e71a6b18ea48598eedd"},{"author":{"_account_id":14027,"name":"Michal Rostecki","email":"mrostecki@suse.de","username":"mrostecki"},"change_message_id":"36acdd8b0653c79a757fa4900fcc302c258db873","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                        self.client.volume_snapshots.get)"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        # Wait for the snapshot to become available"},{"line_number":78,"context_line":"        for x in xrange(60):"},{"line_number":79,"context_line":"            snapshot \u003d self.client.volume_snapshots.get(snapshot.id)"},{"line_number":80,"context_line":"            if snapshot.status \u003d\u003d \u0027available\u0027:"},{"line_number":81,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a80dd14_2b790fc8","line":78,"updated":"2015-03-15 09:29:06.000000000","message":"six.moves.range","commit_id":"229af18a4790789c5e303e71a6b18ea48598eedd"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"ee57b7e63f49ced6603ea9fd1d90c44b3295138c","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        self.addCleanup(volume.delete)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        # Wait for the volume to become available"},{"line_number":58,"context_line":"        for x in six.moves.range(60):"},{"line_number":59,"context_line":"            volume \u003d self.client.volumes.get(volume.id)"},{"line_number":60,"context_line":"            if volume.status \u003d\u003d \u0027available\u0027:"},{"line_number":61,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a80dd14_4f046a14","line":58,"updated":"2015-03-16 15:42:51.000000000","message":"I saw the discussion over using six.moves.range, and this is acceptable, but I wanted to make one more comment: the xrange() built-in does not exist in Python 3 at all; in fact, the Python 3 range() built-in returns a generator like Python 2\u0027s xrange() built-in, and there is no built-in in Python 3 that returns a list like Python 2\u0027s range() built-in.  The six.moves.range() function is just defined to be whichever built-in returns a generator.  For a small range, e.g. ~10, just using the straight-up range() built-in, whichever version of Python it is, would be fine.  With something like 60, it is probably best to use the generator form, so that indicates six.moves.range().","commit_id":"29881b45f6e98b0ae24ac1cf79f7bb0d8fbd6be7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4c0eee22e386c18b3bfa4930ab0859d9a2c5eca6","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        self.addCleanup(volume.delete)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        # Wait for the volume to become available"},{"line_number":58,"context_line":"        for x in six.moves.range(60):"},{"line_number":59,"context_line":"            volume \u003d self.client.volumes.get(volume.id)"},{"line_number":60,"context_line":"            if volume.status \u003d\u003d \u0027available\u0027:"},{"line_number":61,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a80dd14_963e72b8","line":58,"in_reply_to":"9a80dd14_4f046a14","updated":"2015-03-16 18:28:34.000000000","message":"Noted, thanks","commit_id":"29881b45f6e98b0ae24ac1cf79f7bb0d8fbd6be7"}]}