)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"25f88ffe5d20b3d8476e76f583b76e49d272c32a","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2015-01-02 16:54:58 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Added snapshots list to volume details view"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I1e9cd9de34a039db2f0bfc24ee78864004c2c831"},{"line_number":10,"context_line":"Closes-Bug: #1111839"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a961159_7ceb29a4","line":8,"updated":"2015-01-05 18:21:19.000000000","message":"Can we get a little more detail here?\n\nThanks!","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"efb5dfa17b958aacdd3b5763f86909ee9f8b89c0","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2015-01-02 16:54:58 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Added snapshots list to volume details view"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I1e9cd9de34a039db2f0bfc24ee78864004c2c831"},{"line_number":10,"context_line":"Closes-Bug: #1111839"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a961159_a3f470f1","line":8,"in_reply_to":"3a961159_7ceb29a4","updated":"2015-01-06 15:49:33.000000000","message":"There\u0027s no harm (and probable benefit) in copying the justification of this change from the bug.... makes it easier to work out the \u0027why\u0027 when browsing the git history","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"beb9c77b8f352d882a6207ffa746728479a98d1c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Added snapshots list to volume details view"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I1e9cd9de34a039db2f0bfc24ee78864004c2c831"},{"line_number":10,"context_line":"Closes-Bug: #1111839"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3a961159_d071ee77","line":9,"updated":"2015-01-17 18:41:05.000000000","message":"Been trying to give better detail in commits.  Can you tell more about why we are doing this?  Sample output?   Also should note the changes in assertEqual argument order. \n\nDo we not have a bp for this?","commit_id":"2adf2838c9a097fc4bb5b20f29e6585135b3d3e8"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"e557aa37b1a2158d23e59294f6cf20686e915857","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Added snapshots list to volume details view"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I1e9cd9de34a039db2f0bfc24ee78864004c2c831"},{"line_number":10,"context_line":"Closes-Bug: #1111839"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"3a961159_10e1b69a","line":9,"in_reply_to":"3a961159_d071ee77","updated":"2015-01-17 20:32:37.000000000","message":"OK, I\u0027ll add more detailed commit message.\n\nWe have only bug #1111839","commit_id":"2adf2838c9a097fc4bb5b20f29e6585135b3d3e8"},{"author":{"_account_id":5538,"name":"Rushi Agrawal","email":"rushi.openstack@gmail.com","username":"rushiagr"},"change_message_id":"b30e80efe1ec63785ba3743f0269787b7502f0e0","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Added snapshots list to volume views"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Both volume details and volume list views will contain"},{"line_number":10,"context_line":"list of list of snapshots names as \u0027child_snapshots\u0027."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Argument position of assertEqual method for related tests added."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da86d52c_c8e2d59e","line":10,"updated":"2015-02-02 16:01:50.000000000","message":"Do you really mean \u0027list of list of\u0027, or is it a typo?","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"1daf6c118c3558f32cbb29e84c5555c94b00f794","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Added snapshots list to volume views"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Both volume details and volume list views will contain"},{"line_number":10,"context_line":"list of list of snapshots names as \u0027child_snapshots\u0027."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Argument position of assertEqual method for related tests added."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da86d52c_10ffb368","line":10,"in_reply_to":"da86d52c_c8e2d59e","updated":"2015-02-02 16:53:01.000000000","message":"It\u0027s a typo, thanks!","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"},{"author":{"_account_id":5538,"name":"Rushi Agrawal","email":"rushi.openstack@gmail.com","username":"rushiagr"},"change_message_id":"b30e80efe1ec63785ba3743f0269787b7502f0e0","unresolved":false,"context_lines":[{"line_number":10,"context_line":"list of list of snapshots names as \u0027child_snapshots\u0027."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Argument position of assertEqual method for related tests added."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I1e9cd9de34a039db2f0bfc24ee78864004c2c831"},{"line_number":15,"context_line":"Closes-Bug: #1111839"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da86d52c_6802c93f","line":13,"updated":"2015-02-02 16:01:50.000000000","message":"Requires an APIImpact flag","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"1daf6c118c3558f32cbb29e84c5555c94b00f794","unresolved":false,"context_lines":[{"line_number":10,"context_line":"list of list of snapshots names as \u0027child_snapshots\u0027."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Argument position of assertEqual method for related tests added."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I1e9cd9de34a039db2f0bfc24ee78864004c2c831"},{"line_number":15,"context_line":"Closes-Bug: #1111839"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da86d52c_70ce2f5c","line":13,"in_reply_to":"da86d52c_6802c93f","updated":"2015-02-02 16:53:01.000000000","message":"Done","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"}],"cinder/api/v2/views/volumes.py":[{"author":{"_account_id":5538,"name":"Rushi Agrawal","email":"rushi.openstack@gmail.com","username":"rushiagr"},"change_message_id":"a04a7f18b1d79e218f332cea4b3d8caf0cc122a8","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                \u0027description\u0027: volume.get(\u0027display_description\u0027),"},{"line_number":64,"context_line":"                \u0027volume_type\u0027: self._get_volume_type(volume),"},{"line_number":65,"context_line":"                \u0027snapshot_id\u0027: volume.get(\u0027snapshot_id\u0027),"},{"line_number":66,"context_line":"                \u0027child_snapshots\u0027: volume.get(\u0027child_snapshots\u0027),"},{"line_number":67,"context_line":"                \u0027source_volid\u0027: volume.get(\u0027source_volid\u0027),"},{"line_number":68,"context_line":"                \u0027metadata\u0027: self._get_volume_metadata(volume),"},{"line_number":69,"context_line":"                \u0027links\u0027: self._get_links(request, volume[\u0027id\u0027]),"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_33e465e0","line":66,"updated":"2015-02-11 11:17:50.000000000","message":"As far as I can see, this patch involves changing only the \u0027show\u0027 method, and not the \u0027index\u0027 method. Will \u0027child_snapshots\u0027 attribute be still present in \u0027volume list\u0027 API call response json dict?","commit_id":"21d5372aae238b1ecfdfbebb9e2d936f0b671547"}],"cinder/api/v2/volumes.py":[{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"68ade1f6b5b2ab12df6d1b4f178d6a9098905377","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        volumes \u003d [dict(vol.iteritems()) for vol in volumes]"},{"line_number":244,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        for volume in volumes:"},{"line_number":247,"context_line":"            volume[\u0027snapshots\u0027] \u003d []"},{"line_number":248,"context_line":"            for snapshot in snapshots:"},{"line_number":249,"context_line":"                if snapshot.volume_id \u003d\u003d volume[\u0027id\u0027]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_6243a56c","line":246,"updated":"2015-01-02 17:06:59.000000000","message":"First look at this I was concerned about looping through the volumes and efficiency, but looking again we\u0027re only doing the single DB call for volumes and snaps so this should be just fine.\n\nI am curious about changing the approach slightly; Make it so the list command just shows the number of snapshots, then modify the show command to actually show the snapshot-id\u0027s but that\u0027s probably not a huge deal and it\u0027s more client work than anything else.","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"a1287384e17330f639925246db723316bc7d73d5","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        volumes \u003d [dict(vol.iteritems()) for vol in volumes]"},{"line_number":244,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        for volume in volumes:"},{"line_number":247,"context_line":"            volume[\u0027snapshots\u0027] \u003d []"},{"line_number":248,"context_line":"            for snapshot in snapshots:"},{"line_number":249,"context_line":"                if snapshot.volume_id \u003d\u003d volume[\u0027id\u0027]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_b005b0c3","line":246,"in_reply_to":"3a961159_4523f004","updated":"2015-01-05 20:47:08.000000000","message":"@John, you\u0027re right and I tried to implement such behavior. If I didn\u0027t miss something, \u0027cinder list\u0027 and \u0027cinder show vol_id\u0027 using one API call: \u0027show\u0027 command adds filter to get only one record. \nHere is sample how it is used in \u0027cinder show\u0027 command: https://review.openstack.org/#/c/144740/","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"9dccbc416de542164c97a279ca59d9b239b3bf16","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        volumes \u003d [dict(vol.iteritems()) for vol in volumes]"},{"line_number":244,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        for volume in volumes:"},{"line_number":247,"context_line":"            volume[\u0027snapshots\u0027] \u003d []"},{"line_number":248,"context_line":"            for snapshot in snapshots:"},{"line_number":249,"context_line":"                if snapshot.volume_id \u003d\u003d volume[\u0027id\u0027]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_abb8092b","line":246,"in_reply_to":"3a961159_6243a56c","updated":"2015-01-02 18:45:40.000000000","message":"From my side, additional loop is a good compromise between complexity and performance. I tried to make a single DB call, but I\u0027m not fun of such things except when DB performance and optimization is really needed.\n\nI wouldn\u0027t want to have this logic in client due to the client performance and end-user usability point of view","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a1dfb971ad604af8d748580b6a53212a7ba97f12","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        volumes \u003d [dict(vol.iteritems()) for vol in volumes]"},{"line_number":244,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        for volume in volumes:"},{"line_number":247,"context_line":"            volume[\u0027snapshots\u0027] \u003d []"},{"line_number":248,"context_line":"            for snapshot in snapshots:"},{"line_number":249,"context_line":"                if snapshot.volume_id \u003d\u003d volume[\u0027id\u0027]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_4523f004","line":246,"in_reply_to":"3a961159_abb8092b","updated":"2015-01-05 20:24:14.000000000","message":"@Ivan\nYeah, sorry; I was just making an observation about the loop, I agree that it\u0027s fine.\n\nI would like to think about the second point however and how this info is displayed/returned.  Returning a count of snapshots as opposed to a list of UUID\u0027s might be a bit more reasonable in a list/summary output IMHO and leave details like all of the snapshot ID\u0027s where they belong in the volume-show.","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"45ff708934a779ddd8c73842c1a7fcdf1c38479c","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        volumes \u003d [dict(vol.iteritems()) for vol in volumes]"},{"line_number":244,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        for volume in volumes:"},{"line_number":247,"context_line":"            volume[\u0027snapshots\u0027] \u003d []"},{"line_number":248,"context_line":"            for snapshot in snapshots:"},{"line_number":249,"context_line":"                if snapshot.volume_id \u003d\u003d volume[\u0027id\u0027]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_0719c9d0","line":246,"in_reply_to":"3a961159_b005b0c3","updated":"2015-01-06 20:20:55.000000000","message":"One more reason against returning only snapshots ids: we\u0027ll have different snapshots objects as REST API result for list and show volumes","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"efb5dfa17b958aacdd3b5763f86909ee9f8b89c0","unresolved":false,"context_lines":[{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        for volume in volumes:"},{"line_number":247,"context_line":"            volume[\u0027snapshots\u0027] \u003d []"},{"line_number":248,"context_line":"            for snapshot in snapshots:"},{"line_number":249,"context_line":"                if snapshot.volume_id \u003d\u003d volume[\u0027id\u0027]:"},{"line_number":250,"context_line":"                    volume[\u0027snapshots\u0027].append(dict(snapshot.iteritems()))"},{"line_number":251,"context_line":"            if not volume[\u0027snapshots\u0027]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_83acd489","line":248,"updated":"2015-01-06 15:49:33.000000000","message":"So for every volume in the view (so up to 1k with the default settings) you are looping through a list of every snapshot on the system (potentially 100k+). A quick benchmark suggests that it would be better to convert the snapshots into an in-memory data structure rather than making many loops over the SQL alchemy object.\n\nNot suggesting we block the patch, just wondering if this is easily done?","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"45ff708934a779ddd8c73842c1a7fcdf1c38479c","unresolved":false,"context_lines":[{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        for volume in volumes:"},{"line_number":247,"context_line":"            volume[\u0027snapshots\u0027] \u003d []"},{"line_number":248,"context_line":"            for snapshot in snapshots:"},{"line_number":249,"context_line":"                if snapshot.volume_id \u003d\u003d volume[\u0027id\u0027]:"},{"line_number":250,"context_line":"                    volume[\u0027snapshots\u0027].append(dict(snapshot.iteritems()))"},{"line_number":251,"context_line":"            if not volume[\u0027snapshots\u0027]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_4a0af8d2","line":248,"in_reply_to":"3a961159_46bcba6c","updated":"2015-01-06 20:20:55.000000000","message":"@Duncun, you\u0027re right. This solution is something like O(n^2) :(. \n\nNot sure that I catch up your point with \u0027an in-memory data structure\u0027, but I want to propose something like this:\n\u0027volume_api.get_all\u0027 should return all snapshots or instead of \u0027volume_api.get_all_snapshots\u0027 need to implement something like db_api.snapshot_get_all_for_volume(None)\u0027 to retrieve all snapshots grouped by volumes.\nIn any case, we\u0027ll need to run loop over volumes to convert it to dict but it should be less painful than a proposed solution.\n\nI\u0027ll do it in a next patch","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"502e50da1e21b26025186ed908b7411b56952e4a","unresolved":false,"context_lines":[{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        for volume in volumes:"},{"line_number":247,"context_line":"            volume[\u0027snapshots\u0027] \u003d []"},{"line_number":248,"context_line":"            for snapshot in snapshots:"},{"line_number":249,"context_line":"                if snapshot.volume_id \u003d\u003d volume[\u0027id\u0027]:"},{"line_number":250,"context_line":"                    volume[\u0027snapshots\u0027].append(dict(snapshot.iteritems()))"},{"line_number":251,"context_line":"            if not volume[\u0027snapshots\u0027]:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_46bcba6c","line":248,"in_reply_to":"3a961159_83acd489","updated":"2015-01-06 16:28:46.000000000","message":"+1 to Duncan\u0027s suggestion.","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a076176a80b3a5832db723df93cd48a962fbfc27","unresolved":false,"context_lines":[{"line_number":173,"context_line":"        try:"},{"line_number":174,"context_line":"            vol \u003d self.volume_api.get(context, id, viewable_admin_meta\u003dTrue)"},{"line_number":175,"context_line":"            snapshots \u003d self.volume_api.snapshot_get_all_for_volume(context,"},{"line_number":176,"context_line":"                                                                    vol)"},{"line_number":177,"context_line":"            vol[\u0027child_snapshots\u0027] \u003d snapshots or None"},{"line_number":178,"context_line":"            req.cache_db_volume(vol)"},{"line_number":179,"context_line":"        except exception.NotFound:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_38eeadb5","line":176,"updated":"2015-01-16 17:43:13.000000000","message":"is there any reason to stuff the full db object in here?  All we are using is the UUID, maybe we should just strip that out and use that by itself here?\n\nI\u0027m also not sure about passing the actual db reference anyway, but if we do UUID only it\u0027s irrelevant.","commit_id":"c6285a8023ee3f27b33ef62ecf400de0f44dcaff"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a076176a80b3a5832db723df93cd48a962fbfc27","unresolved":false,"context_lines":[{"line_number":244,"context_line":"                                          viewable_admin_meta\u003dTrue)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        volumes \u003d [dict(vol.iteritems()) for vol in volumes]"},{"line_number":247,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":248,"context_line":"        for volume in volumes:"},{"line_number":249,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d None"},{"line_number":250,"context_line":"            snapshots \u003d volume.pop(\u0027snapshots\u0027, None)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_7812f5b4","line":247,"updated":"2015-01-16 17:43:13.000000000","message":"same note as above here, on a side note sadly our existing snapshot_id column is a bit confusing to begin with and this may or may not help.","commit_id":"c6285a8023ee3f27b33ef62ecf400de0f44dcaff"},{"author":{"_account_id":5538,"name":"Rushi Agrawal","email":"rushi.openstack@gmail.com","username":"rushiagr"},"change_message_id":"b30e80efe1ec63785ba3743f0269787b7502f0e0","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            vol \u003d self.volume_api.get(context, id, viewable_admin_meta\u003dTrue)"},{"line_number":175,"context_line":"            snapshots \u003d self.volume_api.snapshot_get_all_for_volume(context,"},{"line_number":176,"context_line":"                                                                    vol)"},{"line_number":177,"context_line":"            vol[\u0027child_snapshots\u0027] \u003d [s[\u0027id\u0027] for s in snapshots]"},{"line_number":178,"context_line":"            vol[\u0027child_snapshots\u0027] \u003d vol[\u0027child_snapshots\u0027] or None"},{"line_number":179,"context_line":"            req.cache_db_volume(vol)"},{"line_number":180,"context_line":"        except exception.NotFound:"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_bc48865c","line":177,"updated":"2015-02-02 16:01:50.000000000","message":"Micro nit: Two lines can be combined as:\n\n vol[\u0027child_snapshots\u0027] \u003d [s[\u0027id\u0027] for s in snapshots] or None","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"1daf6c118c3558f32cbb29e84c5555c94b00f794","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            vol \u003d self.volume_api.get(context, id, viewable_admin_meta\u003dTrue)"},{"line_number":175,"context_line":"            snapshots \u003d self.volume_api.snapshot_get_all_for_volume(context,"},{"line_number":176,"context_line":"                                                                    vol)"},{"line_number":177,"context_line":"            vol[\u0027child_snapshots\u0027] \u003d [s[\u0027id\u0027] for s in snapshots]"},{"line_number":178,"context_line":"            vol[\u0027child_snapshots\u0027] \u003d vol[\u0027child_snapshots\u0027] or None"},{"line_number":179,"context_line":"            req.cache_db_volume(vol)"},{"line_number":180,"context_line":"        except exception.NotFound:"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_ff3110a3","line":177,"in_reply_to":"da86d52c_bc48865c","updated":"2015-02-02 16:53:01.000000000","message":"Thanks. Done","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"},{"author":{"_account_id":5538,"name":"Rushi Agrawal","email":"rushi.openstack@gmail.com","username":"rushiagr"},"change_message_id":"b30e80efe1ec63785ba3743f0269787b7502f0e0","unresolved":false,"context_lines":[{"line_number":245,"context_line":"                                          viewable_admin_meta\u003dTrue)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        volumes \u003d [dict(vol.iteritems()) for vol in volumes]"},{"line_number":248,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":249,"context_line":"        for volume in volumes:"},{"line_number":250,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d [s[\u0027id\u0027] for s in snapshots]"},{"line_number":251,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d volume[\u0027child_snapshots\u0027] or None"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_fcd83e55","line":248,"updated":"2015-02-02 16:01:50.000000000","message":"So even if the list of volumes returned above (Python variable \u0027volumes\u0027) is small (say, because the specified filters narrows down the search volumes considerably), this will return _all_ the snapshots for that particular tenant, even for the volumes which are filtered out. Can we do better?","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"1daf6c118c3558f32cbb29e84c5555c94b00f794","unresolved":false,"context_lines":[{"line_number":245,"context_line":"                                          viewable_admin_meta\u003dTrue)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        volumes \u003d [dict(vol.iteritems()) for vol in volumes]"},{"line_number":248,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":249,"context_line":"        for volume in volumes:"},{"line_number":250,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d [s[\u0027id\u0027] for s in snapshots]"},{"line_number":251,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d volume[\u0027child_snapshots\u0027] or None"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_df636c18","line":248,"in_reply_to":"da86d52c_fcd83e55","updated":"2015-02-02 16:53:01.000000000","message":"Thanks, Rushi, you\u0027re right.","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"},{"author":{"_account_id":5538,"name":"Rushi Agrawal","email":"rushi.openstack@gmail.com","username":"rushiagr"},"change_message_id":"b30e80efe1ec63785ba3743f0269787b7502f0e0","unresolved":false,"context_lines":[{"line_number":247,"context_line":"        volumes \u003d [dict(vol.iteritems()) for vol in volumes]"},{"line_number":248,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":249,"context_line":"        for volume in volumes:"},{"line_number":250,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d [s[\u0027id\u0027] for s in snapshots]"},{"line_number":251,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d volume[\u0027child_snapshots\u0027] or None"},{"line_number":252,"context_line":"            utils.add_visible_admin_metadata(volume)"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_dc79bafd","line":250,"updated":"2015-02-02 16:01:50.000000000","message":"This doesn\u0027t look right. Will every volume have _all_ snapshots of that same tenant as their child snapshots, if I am understanding this line correctly?","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"},{"author":{"_account_id":5538,"name":"Rushi Agrawal","email":"rushi.openstack@gmail.com","username":"rushiagr"},"change_message_id":"b30e80efe1ec63785ba3743f0269787b7502f0e0","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":249,"context_line":"        for volume in volumes:"},{"line_number":250,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d [s[\u0027id\u0027] for s in snapshots]"},{"line_number":251,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d volume[\u0027child_snapshots\u0027] or None"},{"line_number":252,"context_line":"            utils.add_visible_admin_metadata(volume)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"        limited_list \u003d common.limited(volumes, req)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_1c6fd2c6","line":251,"updated":"2015-02-02 16:01:50.000000000","message":"Micro nit: Two lines can be combined as:\n\n vol[\u0027child_snapshots\u0027] \u003d [s[\u0027id\u0027] for s in snapshots] or None","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"1daf6c118c3558f32cbb29e84c5555c94b00f794","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        snapshots \u003d self.volume_api.get_all_snapshots(context)"},{"line_number":249,"context_line":"        for volume in volumes:"},{"line_number":250,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d [s[\u0027id\u0027] for s in snapshots]"},{"line_number":251,"context_line":"            volume[\u0027child_snapshots\u0027] \u003d volume[\u0027child_snapshots\u0027] or None"},{"line_number":252,"context_line":"            utils.add_visible_admin_metadata(volume)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"        limited_list \u003d common.limited(volumes, req)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_5f6dbc31","line":251,"in_reply_to":"da86d52c_1c6fd2c6","updated":"2015-02-02 16:53:01.000000000","message":"Done","commit_id":"b93c6892d71abe7752fe8e7a01c55b256630e45d"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"148f45ddc87746077abd5d93cd0781e2b540e68f","unresolved":false,"context_lines":[{"line_number":1203,"context_line":"            options(joinedload(\u0027volume_admin_metadata\u0027)).\\"},{"line_number":1204,"context_line":"            options(joinedload(\u0027volume_type\u0027)).\\"},{"line_number":1205,"context_line":"            options(joinedload(\u0027consistencygroup\u0027)).\\"},{"line_number":1206,"context_line":"            options(joinedload(\u0027snapshots\u0027))"},{"line_number":1207,"context_line":"    else:"},{"line_number":1208,"context_line":"        return model_query(context, models.Volume, session\u003dsession,"},{"line_number":1209,"context_line":"                           project_only\u003dproject_only).\\"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_b46de09b","line":1206,"updated":"2015-02-03 05:43:05.000000000","message":"This is turning into a monster.  Do we really need another database join on every single call to get a volume just to return a list of snapshots?","commit_id":"b1a5ae60a2a01a1c7de06b84775336f186b92149"}],"cinder/tests/api/v2/test_volumes.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"25f88ffe5d20b3d8476e76f583b76e49d272c32a","unresolved":false,"context_lines":[{"line_number":1595,"context_line":"        get_all.assert_called_once_with("},{"line_number":1596,"context_line":"            context, None, None, \u0027created_at\u0027, \u0027desc\u0027,"},{"line_number":1597,"context_line":"            {\u0027display_name\u0027: \u0027d-\u0027}, viewable_admin_meta\u003dTrue)"},{"line_number":1598,"context_line":""},{"line_number":1599,"context_line":""},{"line_number":1600,"context_line":"class VolumeSerializerTest(test.TestCase):"},{"line_number":1601,"context_line":"    def _verify_volume_attachment(self, attach, tree):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_3c3c412f","line":1598,"updated":"2015-01-05 18:21:19.000000000","message":"Seems like we should have a test case to make sure that we get a list of snapshots when there are snapshots.","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"570021b69e818eff6a4bffed6aedcf1ed8eec58b","unresolved":false,"context_lines":[{"line_number":1595,"context_line":"        get_all.assert_called_once_with("},{"line_number":1596,"context_line":"            context, None, None, \u0027created_at\u0027, \u0027desc\u0027,"},{"line_number":1597,"context_line":"            {\u0027display_name\u0027: \u0027d-\u0027}, viewable_admin_meta\u003dTrue)"},{"line_number":1598,"context_line":""},{"line_number":1599,"context_line":""},{"line_number":1600,"context_line":"class VolumeSerializerTest(test.TestCase):"},{"line_number":1601,"context_line":"    def _verify_volume_attachment(self, attach, tree):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_df37affc","line":1598,"in_reply_to":"3a961159_3c3c412f","updated":"2015-01-05 19:25:18.000000000","message":"Good catch! I\u0027ll do it, thanks!","commit_id":"9f1e9dca9a4796b5366bfe9d65517d6ec8065e5d"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"beb9c77b8f352d882a6207ffa746728479a98d1c","unresolved":false,"context_lines":[{"line_number":1290,"context_line":"        self.assertEqual(len(resp[\u0027volumes\u0027]), 1)"},{"line_number":1291,"context_line":"        self.assertEqual(resp[\u0027volumes\u0027][0][\u0027name\u0027], \u0027vol3\u0027)"},{"line_number":1292,"context_line":""},{"line_number":1293,"context_line":"    def test_volume_show(self):"},{"line_number":1294,"context_line":"        def stub_volume_get(self, context, volume_id, **kwargs):"},{"line_number":1295,"context_line":"            return stubs.stub_volume(volume_id,"},{"line_number":1296,"context_line":"                                     child_snapshots\u003d[stubs.stub_snapshot(1)])"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_f0450a15","line":1293,"updated":"2015-01-17 18:41:05.000000000","message":"Am i missing something?  I don\u0027t see that this is actually verifying that the new function is working.","commit_id":"2adf2838c9a097fc4bb5b20f29e6585135b3d3e8"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"e557aa37b1a2158d23e59294f6cf20686e915857","unresolved":false,"context_lines":[{"line_number":1290,"context_line":"        self.assertEqual(len(resp[\u0027volumes\u0027]), 1)"},{"line_number":1291,"context_line":"        self.assertEqual(resp[\u0027volumes\u0027][0][\u0027name\u0027], \u0027vol3\u0027)"},{"line_number":1292,"context_line":""},{"line_number":1293,"context_line":"    def test_volume_show(self):"},{"line_number":1294,"context_line":"        def stub_volume_get(self, context, volume_id, **kwargs):"},{"line_number":1295,"context_line":"            return stubs.stub_volume(volume_id,"},{"line_number":1296,"context_line":"                                     child_snapshots\u003d[stubs.stub_snapshot(1)])"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_30dcf2e0","line":1293,"in_reply_to":"3a961159_f0450a15","updated":"2015-01-17 20:32:37.000000000","message":"Good catch, Jay! I\u0027ll add more unit tests in a next patch","commit_id":"2adf2838c9a097fc4bb5b20f29e6585135b3d3e8"}],"cinder/volume/api.py":[{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"148f45ddc87746077abd5d93cd0781e2b540e68f","unresolved":false,"context_lines":[{"line_number":436,"context_line":"        return snapshots"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"    def snapshot_get_all_for_volume(self, context, volume):"},{"line_number":439,"context_line":"        snapshots \u003d self.db.snapshot_get_all_for_volume(context, volume[\u0027id\u0027])"},{"line_number":440,"context_line":"        return snapshots"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    @wrap_check_policy"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_142dcc46","line":439,"updated":"2015-02-03 05:43:05.000000000","message":"Why is this necessary if you have the join?","commit_id":"b1a5ae60a2a01a1c7de06b84775336f186b92149"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"18d7f279e2e5945db484e9687d502f94d8c669f1","unresolved":false,"context_lines":[{"line_number":436,"context_line":"        return snapshots"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"    def snapshot_get_all_for_volume(self, context, volume):"},{"line_number":439,"context_line":"        snapshots \u003d self.db.snapshot_get_all_for_volume(context, volume[\u0027id\u0027])"},{"line_number":440,"context_line":"        return snapshots"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    @wrap_check_policy"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_1503ea73","line":439,"in_reply_to":"da86d52c_142dcc46","updated":"2015-02-03 17:38:21.000000000","message":"Agree, need to remove it from my change-set and from a current master","commit_id":"b1a5ae60a2a01a1c7de06b84775336f186b92149"}]}