)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"d92f6bf5c6217b8cce94ea111cc004dfb8190578","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Nesenenko \u003cynesenenko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-05-19 18:17:01 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Filter snapshots data on the DB side"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I1c3e47a4f3780de54d7a5920f463e26cc8b7bcc2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a016987_90d31504","line":7,"updated":"2015-05-20 18:23:18.000000000","message":"Is there a Blueprint or a bug associated with this?\n\nCan you please provide more information why this is needed and what it solves?","commit_id":"636bf794adeb96a2276cb40f283597b7f361fb76"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"be7f1b7a89ad47155f924bb2618da845b85e71a0","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Nesenenko \u003cynesenenko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-05-19 18:17:01 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Filter snapshots data on the DB side"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I1c3e47a4f3780de54d7a5920f463e26cc8b7bcc2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a016987_0e15c147","line":7,"in_reply_to":"7a016987_90d31504","updated":"2015-05-20 23:14:11.000000000","message":"Here is a blueprint: https://blueprints.launchpad.net/cinder/+spec/data-filtering-on-the-db-side.\n\nYuriy,\n\nplease add bp link to the commit message","commit_id":"636bf794adeb96a2276cb40f283597b7f361fb76"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e2efd2347a3c6fc9ab4d3af12e1492b22c2936","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Filter snapshots data on the DB side"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is a partial blueprint for snapshots. It\u0027s filtered snapshots data"},{"line_number":10,"context_line":"on the DB side to improve performance. Some tests are removed from"},{"line_number":11,"context_line":"test_snapshots.py because their function implemented on the DB side."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"1af86dd1_21b13e90","line":9,"updated":"2015-06-12 18:01:24.000000000","message":"It should mention that it can also filter on the volume data, right?","commit_id":"64f36046fc5243d39bfb9be8c90be21d02905f1b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"73266e64ed4d846c31abb3dc28020c49af615960","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Filter snapshots data on the DB side"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is a partial blueprint for snapshots. It\u0027s filtered snapshots data"},{"line_number":10,"context_line":"on the DB side to improve performance. Some tests are removed from"},{"line_number":11,"context_line":"test_snapshots.py because their function implemented on the DB side."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"fa32b979_22977f49","line":9,"updated":"2015-06-16 15:02:01.000000000","message":"It should mention that it can also filter on the volume data, right?","commit_id":"4230c0cf0ce124652fe7f9aa962ea3e25c59825e"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"770a116424f06aa09574d4e60e2730d45ad2610c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Filter snapshots data on the DB side"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is a partial blueprint for snapshots. It\u0027s filtered snapshots"},{"line_number":10,"context_line":"data on the DB side to improve performance. Some tests are removed from"},{"line_number":11,"context_line":"test_snapshots.py because their function implemented on the DB side."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"fa32b979_2ec28534","line":9,"updated":"2015-06-25 15:02:44.000000000","message":"\"This is a partial blueprint for snapshots.\" this part is also could be removed if you\u0027ll make one more patch","commit_id":"57a07bc9604f419d6332406d0cb578fcf66bc571"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"a10f14f6269a299a0e24675b221ce691171a11e0","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Filter snapshots data on the DB side"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is a partial blueprint for snapshots. It\u0027s filtered snapshots"},{"line_number":10,"context_line":"data on the DB side to improve performance. Some tests are removed from"},{"line_number":11,"context_line":"test_snapshots.py because their function implemented on the DB side."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"fa32b979_6822c553","line":9,"updated":"2015-06-25 14:18:10.000000000","message":"/It\u0027s filtered/It filters/","commit_id":"57a07bc9604f419d6332406d0cb578fcf66bc571"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"be7f1b7a89ad47155f924bb2618da845b85e71a0","unresolved":false,"context_lines":[{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,"},{"line_number":1979,"context_line":"                        project_only\u003dTrue)"},{"line_number":1980,"context_line":"    if filters:"},{"line_number":1981,"context_line":"        query \u003d query.filter_by(**filters)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_ae046d17","line":1978,"updated":"2015-05-20 23:14:11.000000000","message":"I hope, read_deleted\u003d\u0027no\u0027 is a default option. Could you verify it, please?","commit_id":"636bf794adeb96a2276cb40f283597b7f361fb76"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"978a56e2a6f5376c11390bc1e9db92ec133e4e24","unresolved":false,"context_lines":[{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,"},{"line_number":1979,"context_line":"                        project_only\u003dTrue)"},{"line_number":1980,"context_line":"    if filters:"},{"line_number":1981,"context_line":"        query \u003d query.filter_by(**filters)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_3238dbee","line":1978,"in_reply_to":"7a016987_ae046d17","updated":"2015-05-21 09:21:04.000000000","message":"NameError: global name \u0027read_deleted\u0027 is not defined","commit_id":"636bf794adeb96a2276cb40f283597b7f361fb76"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"84068692af6bac906eecfeb50b846aeef4e2c56d","unresolved":false,"context_lines":[{"line_number":1980,"context_line":"    if filters:"},{"line_number":1981,"context_line":"        query \u003d query.filter_by(**filters)"},{"line_number":1982,"context_line":""},{"line_number":1983,"context_line":"    return query.join(models.Snapshot.volume).\\"},{"line_number":1984,"context_line":"        options(joinedload(\u0027snapshot_metadata\u0027)).all()"},{"line_number":1985,"context_line":""},{"line_number":1986,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa32b979_5eeb7bb0","line":1983,"updated":"2015-06-16 17:24:59.000000000","message":"This join is not needed, right?:\n\n return query.options(joinedload(\u0027snapshot_metadata\u0027)).all()","commit_id":"cd3c10097235cd7014526389ba625c0c17543254"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"84068692af6bac906eecfeb50b846aeef4e2c56d","unresolved":false,"context_lines":[{"line_number":2023,"context_line":"    if filters:"},{"line_number":2024,"context_line":"        query \u003d query.filter_by(**filters)"},{"line_number":2025,"context_line":""},{"line_number":2026,"context_line":"    return query.join(models.Snapshot.volume).\\"},{"line_number":2027,"context_line":"        filter_by(project_id\u003dproject_id).\\"},{"line_number":2028,"context_line":"        options(joinedload(\u0027snapshot_metadata\u0027)).\\"},{"line_number":2029,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa32b979_7e13dfb8","line":2026,"updated":"2015-06-16 17:24:59.000000000","message":"This join is not needed either, right?","commit_id":"cd3c10097235cd7014526389ba625c0c17543254"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9fb8f61e9c247b05a05fbf7a918c8bdb5d21d598","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    # Ensure that the filter value exists on the model"},{"line_number":1979,"context_line":"    if filters:"},{"line_number":1980,"context_line":"        for key in filters.keys():"},{"line_number":1981,"context_line":"            try:"},{"line_number":1982,"context_line":"                getattr(models.Snapshot, key)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa32b979_58159e3d","line":1979,"updated":"2015-06-17 15:31:36.000000000","message":"This should not be here, it should be in cinder.api.v2.volumes and you should use utils.remove_invalid_filter_options and not rise an exception.\n\nYou can look at how it\u0027s done in _get_volumes method of that same file.","commit_id":"db969a58f7b757d033abef681d33fe42d0d08f29"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"7ec5abf09ef51d7f6c9ec9701be85ab15f7ab019","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    # Ensure that the filter value exists on the model"},{"line_number":1979,"context_line":"    if filters:"},{"line_number":1980,"context_line":"        for key in filters.keys():"},{"line_number":1981,"context_line":"            try:"},{"line_number":1982,"context_line":"                getattr(models.Snapshot, key)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa32b979_b8e94ab9","line":1979,"in_reply_to":"fa32b979_58159e3d","updated":"2015-06-17 15:43:56.000000000","message":"You mean filters should be in cinder.api.v2.snapshots?","commit_id":"db969a58f7b757d033abef681d33fe42d0d08f29"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5386dc7912249e0b6af06fc07c140be0a1735aa4","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    # Ensure that the filter value exists on the model"},{"line_number":1979,"context_line":"    if filters:"},{"line_number":1980,"context_line":"        for key in filters.keys():"},{"line_number":1981,"context_line":"            try:"},{"line_number":1982,"context_line":"                getattr(models.Snapshot, key)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa32b979_c23c50ca","line":1979,"in_reply_to":"fa32b979_b8e94ab9","updated":"2015-06-17 18:35:52.000000000","message":"I\u0027m sorry, today I\u0027m explaining myself like a closed book. :(\n\n1- You should filter out invalid options in snapshot api so they are ignored, which you are not doing right now. Check volume filtering api.\n\n2- I think in sqlalchemy api in general when receiving a nonexistent field it is logged in debug level and an empty list is returned.","commit_id":"db969a58f7b757d033abef681d33fe42d0d08f29"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"8d2e65a221c656cbd7dc576de917632d3ecb4113","unresolved":false,"context_lines":[{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    # Ensure that the filter value exists on the model"},{"line_number":1979,"context_line":"    if filters:"},{"line_number":1980,"context_line":"        for key in filters.keys():"},{"line_number":1981,"context_line":"            try:"},{"line_number":1982,"context_line":"                getattr(models.Snapshot, key)"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa32b979_f3ea2fac","line":1979,"in_reply_to":"fa32b979_c23c50ca","updated":"2015-06-19 13:39:12.000000000","message":"It\u0027s done in snapshot api https://github.com/openstack/cinder/blob/master/cinder/api/v2/snapshots.py#L138","commit_id":"db969a58f7b757d033abef681d33fe42d0d08f29"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"92fa32f8de6394c786789b5a1d2e826fcc8c34df","unresolved":false,"context_lines":[{"line_number":1974,"context_line":""},{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,"},{"line_number":1979,"context_line":"                        project_only\u003dTrue)"},{"line_number":1980,"context_line":"    if filters:"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa32b979_9319607c","line":1977,"updated":"2015-06-19 14:21:59.000000000","message":"You shouldn\u0027t remove it completely, since I think e0ne specifically asked for it.\nYou just need not to throw an exception, log the error in debug level and return an empty list like the other methods.","commit_id":"e32c266be3ced17118f67a25b67162666791e939"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4fa908aadf12eb3c12d40289336931aacd3dd868","unresolved":false,"context_lines":[{"line_number":1974,"context_line":""},{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,"},{"line_number":1979,"context_line":"                        project_only\u003dTrue)"},{"line_number":1980,"context_line":"    if filters:"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa32b979_f6dbfa4d","line":1977,"in_reply_to":"fa32b979_931640f0","updated":"2015-06-19 14:47:01.000000000","message":"I would prefer to be consistent with how it is done with the volumes. They also have the API side filter removal but they check here as well in case the call comes from somewhere else.\n\nLook at method _process_volume_filters in this same file.","commit_id":"e32c266be3ced17118f67a25b67162666791e939"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"38a481e728ad1db1ceb20c7d602cb0833ef9fd3b","unresolved":false,"context_lines":[{"line_number":1974,"context_line":""},{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,"},{"line_number":1979,"context_line":"                        project_only\u003dTrue)"},{"line_number":1980,"context_line":"    if filters:"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa32b979_931640f0","line":1977,"in_reply_to":"fa32b979_9319607c","updated":"2015-06-19 14:27:15.000000000","message":"Gorka, we decided to leave it in API side as volumes filtering implemented","commit_id":"e32c266be3ced17118f67a25b67162666791e939"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"2de66cc49c756bfcac93877e894684f82480e323","unresolved":false,"context_lines":[{"line_number":1974,"context_line":""},{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"@require_admin_context"},{"line_number":1977,"context_line":"def snapshot_get_all(context, filters\u003dNone):"},{"line_number":1978,"context_line":"    query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,"},{"line_number":1979,"context_line":"                        project_only\u003dTrue)"},{"line_number":1980,"context_line":"    if filters:"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa32b979_96d70eed","line":1977,"in_reply_to":"fa32b979_f6dbfa4d","updated":"2015-06-19 14:50:18.000000000","message":"Good point, I\u0027m agree with you","commit_id":"e32c266be3ced17118f67a25b67162666791e939"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"d600e8d763a6b2b1055e35568c86fea238e89def","unresolved":false,"context_lines":[{"line_number":1984,"context_line":"                LOG.debug(\"\u0027%s\u0027 filter key is not valid.\", key)"},{"line_number":1985,"context_line":"                return []"},{"line_number":1986,"context_line":""},{"line_number":1987,"context_line":"    query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,"},{"line_number":1988,"context_line":"                        project_only\u003dTrue)"},{"line_number":1989,"context_line":"    if filters:"},{"line_number":1990,"context_line":"        query \u003d query.filter_by(**filters)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa32b979_63800ac0","line":1987,"updated":"2015-06-23 17:38:38.000000000","message":"Why is read_deleted\u003d\u0027no\u0027 introduced here?","commit_id":"8d314923afee04de51fb0a6556b13dc949bee062"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"21d34f1f3a9fb0c0f6b43d73cccefe9c739b4cd4","unresolved":false,"context_lines":[{"line_number":1984,"context_line":"                LOG.debug(\"\u0027%s\u0027 filter key is not valid.\", key)"},{"line_number":1985,"context_line":"                return []"},{"line_number":1986,"context_line":""},{"line_number":1987,"context_line":"    query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,"},{"line_number":1988,"context_line":"                        project_only\u003dTrue)"},{"line_number":1989,"context_line":"    if filters:"},{"line_number":1990,"context_line":"        query \u003d query.filter_by(**filters)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa32b979_ba05d311","line":1987,"in_reply_to":"fa32b979_63800ac0","updated":"2015-06-23 19:24:00.000000000","message":"Good catch, Eric. It should be removed","commit_id":"8d314923afee04de51fb0a6556b13dc949bee062"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"d600e8d763a6b2b1055e35568c86fea238e89def","unresolved":false,"context_lines":[{"line_number":2026,"context_line":"@require_context"},{"line_number":2027,"context_line":"def snapshot_get_all_by_project(context, project_id, filters\u003dNone):"},{"line_number":2028,"context_line":"    authorize_project_context(context, project_id)"},{"line_number":2029,"context_line":"    query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,"},{"line_number":2030,"context_line":"                        project_only\u003dTrue)"},{"line_number":2031,"context_line":"    if filters:"},{"line_number":2032,"context_line":"        query \u003d query.filter_by(**filters)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa32b979_e31dfabe","line":2029,"updated":"2015-06-23 17:38:38.000000000","message":"Why is read_deleted\u003d\u0027no\u0027 introduced here?","commit_id":"8d314923afee04de51fb0a6556b13dc949bee062"}],"cinder/tests/unit/api/v2/test_snapshots.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"be7f1b7a89ad47155f924bb2618da845b85e71a0","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        self.assertEqual(resp_snapshot[\u0027id\u0027], UUID)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"    @mock.patch(\u0027cinder.db.snapshot_metadata_get\u0027, return_value\u003ddict())"},{"line_number":306,"context_line":"    def test_snapshot_list_by_status(self, snapshot_metadata_get):"},{"line_number":307,"context_line":"        def stub_snapshot_get_all_by_project(context, project_id):"},{"line_number":308,"context_line":"            return ["},{"line_number":309,"context_line":"                stubs.stub_snapshot(1, display_name\u003d\u0027backup1\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_3107c019","side":"PARENT","line":306,"updated":"2015-05-20 23:14:11.000000000","message":"Please, add explanation why do you delete this to the commit message","commit_id":"f27c194b738259f95f6030169bf24bd089bdc80e"}],"cinder/tests/unit/objects/test_snapshot.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e2efd2347a3c6fc9ab4d3af12e1492b22c2936","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        snapshots \u003d snapshot_obj.SnapshotList.get_all("},{"line_number":151,"context_line":"            self.context, mock.sentinel.search_opts)"},{"line_number":152,"context_line":"        self.assertEqual(1, len(snapshots))"},{"line_number":153,"context_line":"        TestSnapshot._compare(self, fake_snapshot, snapshots[0])"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    @mock.patch(\u0027cinder.db.snapshot_metadata_get\u0027, return_value\u003d{})"},{"line_number":156,"context_line":"    @mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1af86dd1_1cb0e542","line":153,"updated":"2015-06-12 18:01:24.000000000","message":"This is missing a check that mocked db.snapshot_get_all received the search_opts centinel.","commit_id":"64f36046fc5243d39bfb9be8c90be21d02905f1b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e2efd2347a3c6fc9ab4d3af12e1492b22c2936","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        snapshots \u003d snapshot_obj.SnapshotList.get_all_by_project("},{"line_number":179,"context_line":"            self.context, self.project_id, mock.sentinel.search_opts)"},{"line_number":180,"context_line":"        self.assertEqual(1, len(snapshots))"},{"line_number":181,"context_line":"        TestSnapshot._compare(self, fake_snapshot, snapshots[0])"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    @mock.patch(\u0027cinder.db.snapshot_metadata_get\u0027, return_value\u003d{})"},{"line_number":184,"context_line":"    @mock.patch(\u0027cinder.objects.volume.Volume.get_by_id\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1af86dd1_bcb6713b","line":181,"updated":"2015-06-12 18:01:24.000000000","message":"Same here","commit_id":"64f36046fc5243d39bfb9be8c90be21d02905f1b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"73266e64ed4d846c31abb3dc28020c49af615960","unresolved":false,"context_lines":[{"line_number":152,"context_line":"            self.context, search_opts)"},{"line_number":153,"context_line":"        self.assertEqual(1, len(snapshots))"},{"line_number":154,"context_line":"        TestSnapshot._compare(self, fake_snapshot, snapshots[0])"},{"line_number":155,"context_line":"        snapshot_get_all.assert_called_once_with(self.context, search_opts)"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    @mock.patch(\u0027cinder.db.snapshot_metadata_get\u0027, return_value\u003d{})"},{"line_number":158,"context_line":"    @mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_22fbffc2","line":155,"updated":"2015-06-16 15:02:01.000000000","message":"Thanks for adding this","commit_id":"4230c0cf0ce124652fe7f9aa962ea3e25c59825e"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"73266e64ed4d846c31abb3dc28020c49af615960","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        TestSnapshot._compare(self, fake_snapshot, snapshots[0])"},{"line_number":185,"context_line":"        get_all_by_project.assert_called_once_with(self.context,"},{"line_number":186,"context_line":"                                                   self.project_id,"},{"line_number":187,"context_line":"                                                   search_opts)"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    @mock.patch(\u0027cinder.db.snapshot_metadata_get\u0027, return_value\u003d{})"},{"line_number":190,"context_line":"    @mock.patch(\u0027cinder.objects.volume.Volume.get_by_id\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_c23013a1","line":187,"updated":"2015-06-16 15:02:01.000000000","message":"Thanks for adding this","commit_id":"4230c0cf0ce124652fe7f9aa962ea3e25c59825e"}],"cinder/tests/unit/test_db_api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"71e2efd2347a3c6fc9ab4d3af12e1492b22c2936","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                                        db.snapshot_get_all("},{"line_number":1015,"context_line":"                                            self.ctxt,"},{"line_number":1016,"context_line":"                                            filters),"},{"line_number":1017,"context_line":"                                        ignored_keys\u003d\u0027volume\u0027)"},{"line_number":1018,"context_line":""},{"line_number":1019,"context_line":"    def test_snapshot_get_by_host(self):"},{"line_number":1020,"context_line":"        db.volume_create(self.ctxt, {\u0027id\u0027: 1, \u0027host\u0027: \u0027host1\u0027})"}],"source_content_type":"text/x-python","patch_set":5,"id":"1af86dd1_5c41bd0a","line":1017,"updated":"2015-06-12 18:01:24.000000000","message":"This takes care of checking the filter by snapshot attributes, but we should also have by snapshot metadata and by volume fields like volume_type_id.","commit_id":"64f36046fc5243d39bfb9be8c90be21d02905f1b"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"c6f452508f4882a290f6b7e027899c8303b2e009","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                                        db.snapshot_get_all("},{"line_number":1015,"context_line":"                                            self.ctxt,"},{"line_number":1016,"context_line":"                                            filters),"},{"line_number":1017,"context_line":"                                        ignored_keys\u003d\u0027volume\u0027)"},{"line_number":1018,"context_line":""},{"line_number":1019,"context_line":"    def test_snapshot_get_by_host(self):"},{"line_number":1020,"context_line":"        db.volume_create(self.ctxt, {\u0027id\u0027: 1, \u0027host\u0027: \u0027host1\u0027})"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa32b979_757d67e6","line":1017,"in_reply_to":"1af86dd1_5c41bd0a","updated":"2015-06-15 23:45:00.000000000","message":"Are those use cases that actually occur? It looks like these tests are basically 1:1 with the ones being removed that were testing the higher level filtering. Were the original ones lacking those?","commit_id":"64f36046fc5243d39bfb9be8c90be21d02905f1b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7e9c550a1d2625beb0a0ecb9acd7b59f7c18503f","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                                        db.snapshot_get_all("},{"line_number":1015,"context_line":"                                            self.ctxt,"},{"line_number":1016,"context_line":"                                            filters),"},{"line_number":1017,"context_line":"                                        ignored_keys\u003d\u0027volume\u0027)"},{"line_number":1018,"context_line":""},{"line_number":1019,"context_line":"    def test_snapshot_get_by_host(self):"},{"line_number":1020,"context_line":"        db.volume_create(self.ctxt, {\u0027id\u0027: 1, \u0027host\u0027: \u0027host1\u0027})"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa32b979_2fd78024","line":1017,"in_reply_to":"fa32b979_757d67e6","updated":"2015-06-16 07:27:02.000000000","message":"The original tests didn\u0027t have those test cases because the query was different. \n\nIf you look at the sqlalchemy/api.py changes, you\u0027ll see that now we are joining models.Snapsho.volume that we were not joining before.\n\nThis allows filters to query by volume fields, but those need to be tested as well.","commit_id":"64f36046fc5243d39bfb9be8c90be21d02905f1b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"73266e64ed4d846c31abb3dc28020c49af615960","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                                        db.snapshot_get_all("},{"line_number":1015,"context_line":"                                            self.ctxt,"},{"line_number":1016,"context_line":"                                            filters),"},{"line_number":1017,"context_line":"                                        ignored_keys\u003d[\u0027metadata\u0027, \u0027volume\u0027])"},{"line_number":1018,"context_line":""},{"line_number":1019,"context_line":"    def test_snapshot_get_by_host(self):"},{"line_number":1020,"context_line":"        db.volume_create(self.ctxt, {\u0027id\u0027: 1, \u0027host\u0027: \u0027host1\u0027})"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_02c59b59","line":1017,"updated":"2015-06-16 15:02:01.000000000","message":"We are still missing tests that make use of filtering by volume fields.","commit_id":"4230c0cf0ce124652fe7f9aa962ea3e25c59825e"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"aceac0b5b684bb6329ce8aed77f03db8be843f32","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                                        db.snapshot_get_all("},{"line_number":1015,"context_line":"                                            self.ctxt,"},{"line_number":1016,"context_line":"                                            filters),"},{"line_number":1017,"context_line":"                                        ignored_keys\u003d[\u0027metadata\u0027, \u0027volume\u0027])"},{"line_number":1018,"context_line":""},{"line_number":1019,"context_line":"    def test_snapshot_get_by_host(self):"},{"line_number":1020,"context_line":"        db.volume_create(self.ctxt, {\u0027id\u0027: 1, \u0027host\u0027: \u0027host1\u0027})"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_2486acb9","line":1017,"in_reply_to":"fa32b979_02c59b59","updated":"2015-06-16 15:30:04.000000000","message":"IMO, we don\u0027t need it because it will be tests for tests. I can\u0027t image that search by some field won\u0027t work in the current implementation.\n\nBut it\u0027s a good idea, to have test case with non-existing value","commit_id":"4230c0cf0ce124652fe7f9aa962ea3e25c59825e"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0fe540767c176389543c121ee1f8546f86efa174","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                                        db.snapshot_get_all("},{"line_number":1015,"context_line":"                                            self.ctxt,"},{"line_number":1016,"context_line":"                                            filters),"},{"line_number":1017,"context_line":"                                        ignored_keys\u003d[\u0027metadata\u0027, \u0027volume\u0027])"},{"line_number":1018,"context_line":""},{"line_number":1019,"context_line":"    def test_snapshot_get_by_host(self):"},{"line_number":1020,"context_line":"        db.volume_create(self.ctxt, {\u0027id\u0027: 1, \u0027host\u0027: \u0027host1\u0027})"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa32b979_879c5ac1","line":1017,"in_reply_to":"fa32b979_2486acb9","updated":"2015-06-16 15:41:37.000000000","message":"It\u0027s not just about a different field, it\u0027s about the new join we have done with volume.\n\nBefore this patch we had:\n\n def snapshot_get_all(context):\n     return model_query(context, models.Snapshot).\\\n         options(joinedload(\u0027snapshot_metadata\u0027)).\\\n         all()\n\nBut now we have:\n\n def snapshot_get_all(context, filters\u003dNone):\n     query \u003d model_query(context, models.Snapshot, read_deleted\u003d\u0027no\u0027,\n                         project_only\u003dTrue)\n     if filters:\n         query \u003d query.filter_by(**filters) \n\n     return query.join(models.Snapshot.volume).\\\n         options(joinedload(\u0027snapshot_metadata\u0027)).all()\n\nSo we are adding `query.join(models.Snapshot.volume)` which will allow us to filter by fields from snapshot\u0027s volume. I think this is important enough to be explicitly said in the commit message, which now isn\u0027t, and also to have its own tests.","commit_id":"4230c0cf0ce124652fe7f9aa962ea3e25c59825e"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"c43ce03e5faf8199c5bf60b20a6f3de72d7cbe8f","unresolved":false,"context_lines":[{"line_number":1022,"context_line":"                                            self.ctxt,"},{"line_number":1023,"context_line":"                                            filters),"},{"line_number":1024,"context_line":"                                        ignored_keys\u003d\u0027volume\u0027)"},{"line_number":1025,"context_line":""},{"line_number":1026,"context_line":"    def test_snapshot_get_by_host(self):"},{"line_number":1027,"context_line":"        db.volume_create(self.ctxt, {\u0027id\u0027: 1, \u0027host\u0027: \u0027host1\u0027})"},{"line_number":1028,"context_line":"        db.volume_create(self.ctxt, {\u0027id\u0027: 2, \u0027host\u0027: \u0027host2\u0027})"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_98f27b6e","line":1025,"updated":"2015-06-16 16:44:20.000000000","message":"Please, add tests with non-existing attributes","commit_id":"24775c7ca5272b5dd9b32e1abd394aa1d662552f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4bd87243d2c66a677692f493d8a5b50ef473766e","unresolved":false,"context_lines":[{"line_number":1020,"context_line":"                                        db.snapshot_get_all("},{"line_number":1021,"context_line":"                                            self.ctxt,"},{"line_number":1022,"context_line":"                                            filters),"},{"line_number":1023,"context_line":"                                        ignored_keys\u003d[\u0027metadata\u0027, \u0027volume\u0027])"},{"line_number":1024,"context_line":""},{"line_number":1025,"context_line":"    def test_snapshot_get_by_host(self):"},{"line_number":1026,"context_line":"        db.volume_create(self.ctxt, {\u0027id\u0027: 1, \u0027host\u0027: \u0027host1\u0027})"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_8447d44a","line":1023,"updated":"2015-06-19 15:23:50.000000000","message":"Good idea to keep ignored_keys here for consistency even if it\u0027s not needed in this case.","commit_id":"9c3d3494c6ec1127c0ed7fd9683016a3b35d9903"}]}