)]}'
{"cinder/db/api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":375,"context_line":""},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"def snapshot_admin_metadata_update(context, snapshot_id, metadata, delete):"},{"line_number":378,"context_line":"    \"\"\"Update admin metadata if it exists, otherwise create it.\"\"\""},{"line_number":379,"context_line":"    return IMPL.snapshot_admin_metadata_update(context, snapshot_id,"},{"line_number":380,"context_line":"                                               metadata, delete)"},{"line_number":381,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_7a47b577","line":378,"updated":"2015-07-30 16:34:31.000000000","message":"Docstring needs improvement, delete argument is not clear on what it does.","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":375,"context_line":""},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"def snapshot_admin_metadata_update(context, snapshot_id, metadata, delete):"},{"line_number":378,"context_line":"    \"\"\"Update admin metadata if it exists, otherwise create it.\"\"\""},{"line_number":379,"context_line":"    return IMPL.snapshot_admin_metadata_update(context, snapshot_id,"},{"line_number":380,"context_line":"                                               metadata, delete)"},{"line_number":381,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_54b83b75","line":378,"in_reply_to":"3a50d1a3_7a47b577","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"591e0b52e940b255880793dd81dcdc5e55398481","unresolved":false,"context_lines":[{"line_number":2000,"context_line":"def _snapshot_get(context, snapshot_id, session\u003dNone):"},{"line_number":2001,"context_line":"    result \u003d model_query(context, models.Snapshot, session\u003dsession,"},{"line_number":2002,"context_line":"                         project_only\u003dTrue).\\"},{"line_number":2003,"context_line":"        options(joinedload(\u0027volume\u0027)).\\"},{"line_number":2004,"context_line":"        options(joinedload(\u0027snapshot_metadata\u0027)).\\"},{"line_number":2005,"context_line":"        filter_by(id\u003dsnapshot_id).\\"},{"line_number":2006,"context_line":"        first()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_057b34f6","line":2003,"updated":"2015-07-22 13:56:38.000000000","message":"Should the admin_metadata be joinedload too?","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"e7fbe67be86287d5cde1c1b023c84875b5affbbe","unresolved":false,"context_lines":[{"line_number":2000,"context_line":"def _snapshot_get(context, snapshot_id, session\u003dNone):"},{"line_number":2001,"context_line":"    result \u003d model_query(context, models.Snapshot, session\u003dsession,"},{"line_number":2002,"context_line":"                         project_only\u003dTrue).\\"},{"line_number":2003,"context_line":"        options(joinedload(\u0027volume\u0027)).\\"},{"line_number":2004,"context_line":"        options(joinedload(\u0027snapshot_metadata\u0027)).\\"},{"line_number":2005,"context_line":"        filter_by(id\u003dsnapshot_id).\\"},{"line_number":2006,"context_line":"        first()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_6f96a03f","line":2003,"in_reply_to":"3a50d1a3_057b34f6","updated":"2015-07-29 07:16:15.000000000","message":"Looking at _volume_get, it seems like we need to load admin_md only if the context has admin priviledges, will follow that model here too.","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"591e0b52e940b255880793dd81dcdc5e55398481","unresolved":false,"context_lines":[{"line_number":2275,"context_line":"            original_metadata \u003d _snapshot_admin_metadata_get(context,"},{"line_number":2276,"context_line":"                                                             snapshot_id,"},{"line_number":2277,"context_line":"                                                             session)"},{"line_number":2278,"context_line":"            for meta_key, meta_value in original_metadata.iteritems():"},{"line_number":2279,"context_line":"                if meta_key not in metadata:"},{"line_number":2280,"context_line":"                    meta_ref \u003d _snapshot_admin_metadata_get_item(context,"},{"line_number":2281,"context_line":"                                                                 snapshot_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_a5c9205b","line":2278,"updated":"2015-07-22 13:56:38.000000000","message":"Should use items() instead of iteritems(), as mentioned in https://wiki.openstack.org/wiki/Python3.","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"e7fbe67be86287d5cde1c1b023c84875b5affbbe","unresolved":false,"context_lines":[{"line_number":2275,"context_line":"            original_metadata \u003d _snapshot_admin_metadata_get(context,"},{"line_number":2276,"context_line":"                                                             snapshot_id,"},{"line_number":2277,"context_line":"                                                             session)"},{"line_number":2278,"context_line":"            for meta_key, meta_value in original_metadata.iteritems():"},{"line_number":2279,"context_line":"                if meta_key not in metadata:"},{"line_number":2280,"context_line":"                    meta_ref \u003d _snapshot_admin_metadata_get_item(context,"},{"line_number":2281,"context_line":"                                                                 snapshot_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_cf8ed42d","line":2278,"in_reply_to":"3a50d1a3_a5c9205b","updated":"2015-07-29 07:16:15.000000000","message":"Done","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"        values[\u0027snapshot_admin_metadata\u0027] \u003d \\"},{"line_number":1972,"context_line":"            _metadata_refs(values.get(\u0027admin_metadata\u0027),"},{"line_number":1973,"context_line":"                           models.SnapshotAdminMetadata)"},{"line_number":1974,"context_line":"    elif values.get(\u0027snapshot_admin_metadata\u0027):"},{"line_number":1975,"context_line":"        del values[\u0027snapshot_admin_metadata\u0027]"},{"line_number":1976,"context_line":""},{"line_number":1977,"context_line":"    if not values.get(\u0027id\u0027):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_ba077d80","line":1974,"updated":"2015-07-30 16:34:31.000000000","message":"The if should be\n\n elif \u0027snapshot_admin_metadata\u0027 in values:\n\nAnd you can actually just do:\n\n else:\n     values.pop(\u0027snapshot_admin_metadata\u0027, None)","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"        values[\u0027snapshot_admin_metadata\u0027] \u003d \\"},{"line_number":1972,"context_line":"            _metadata_refs(values.get(\u0027admin_metadata\u0027),"},{"line_number":1973,"context_line":"                           models.SnapshotAdminMetadata)"},{"line_number":1974,"context_line":"    elif values.get(\u0027snapshot_admin_metadata\u0027):"},{"line_number":1975,"context_line":"        del values[\u0027snapshot_admin_metadata\u0027]"},{"line_number":1976,"context_line":""},{"line_number":1977,"context_line":"    if not values.get(\u0027id\u0027):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_9465631e","line":1974,"in_reply_to":"3a50d1a3_ba077d80","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2004,"context_line":"                    \u0027updated_at\u0027: literal_column(\u0027updated_at\u0027)})"},{"line_number":2005,"context_line":""},{"line_number":2006,"context_line":""},{"line_number":2007,"context_line":"@require_context"},{"line_number":2008,"context_line":"def _snapshot_get_query(context, session\u003dNone, project_only\u003dFalse):"},{"line_number":2009,"context_line":"    if is_admin_context(context):"},{"line_number":2010,"context_line":"        return model_query(context, models.Snapshot, session\u003dsession,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_5a33d962","line":2007,"updated":"2015-07-30 16:34:31.000000000","message":"You don\u0027t need this here","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2004,"context_line":"                    \u0027updated_at\u0027: literal_column(\u0027updated_at\u0027)})"},{"line_number":2005,"context_line":""},{"line_number":2006,"context_line":""},{"line_number":2007,"context_line":"@require_context"},{"line_number":2008,"context_line":"def _snapshot_get_query(context, session\u003dNone, project_only\u003dFalse):"},{"line_number":2009,"context_line":"    if is_admin_context(context):"},{"line_number":2010,"context_line":"        return model_query(context, models.Snapshot, session\u003dsession,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_1472b3e3","line":2007,"in_reply_to":"3a50d1a3_5a33d962","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2006,"context_line":""},{"line_number":2007,"context_line":"@require_context"},{"line_number":2008,"context_line":"def _snapshot_get_query(context, session\u003dNone, project_only\u003dFalse):"},{"line_number":2009,"context_line":"    if is_admin_context(context):"},{"line_number":2010,"context_line":"        return model_query(context, models.Snapshot, session\u003dsession,"},{"line_number":2011,"context_line":"                           project_only\u003dproject_only).\\"},{"line_number":2012,"context_line":"            options(joinedload(\u0027volume\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_2ff49185","line":2009,"updated":"2015-07-30 16:34:31.000000000","message":"This should be changed to something like what I suggest in line 2103","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2006,"context_line":""},{"line_number":2007,"context_line":"@require_context"},{"line_number":2008,"context_line":"def _snapshot_get_query(context, session\u003dNone, project_only\u003dFalse):"},{"line_number":2009,"context_line":"    if is_admin_context(context):"},{"line_number":2010,"context_line":"        return model_query(context, models.Snapshot, session\u003dsession,"},{"line_number":2011,"context_line":"                           project_only\u003dproject_only).\\"},{"line_number":2012,"context_line":"            options(joinedload(\u0027volume\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_c74fbf32","line":2009,"in_reply_to":"3a50d1a3_2ff49185","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2019,"context_line":"            options(joinedload(\u0027snapshot_metadata\u0027))"},{"line_number":2020,"context_line":""},{"line_number":2021,"context_line":""},{"line_number":2022,"context_line":"@require_context"},{"line_number":2023,"context_line":"def _snapshot_get(context, snapshot_id, session\u003dNone):"},{"line_number":2024,"context_line":"    result \u003d _snapshot_get_query(context, session\u003dsession, project_only\u003dTrue).\\"},{"line_number":2025,"context_line":"        filter_by(id\u003dsnapshot_id).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_1a3eb129","line":2022,"updated":"2015-07-30 16:34:31.000000000","message":"You don\u0027t need this here","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2019,"context_line":"            options(joinedload(\u0027snapshot_metadata\u0027))"},{"line_number":2020,"context_line":""},{"line_number":2021,"context_line":""},{"line_number":2022,"context_line":"@require_context"},{"line_number":2023,"context_line":"def _snapshot_get(context, snapshot_id, session\u003dNone):"},{"line_number":2024,"context_line":"    result \u003d _snapshot_get_query(context, session\u003dsession, project_only\u003dTrue).\\"},{"line_number":2025,"context_line":"        filter_by(id\u003dsnapshot_id).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_b4809f8c","line":2022,"in_reply_to":"3a50d1a3_1a3eb129","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2060,"context_line":""},{"line_number":2061,"context_line":"def _snapshot_get_all_for_volume_query(context, read_deleted\u003d\u0027yes\u0027,"},{"line_number":2062,"context_line":"                                       project_only\u003dFalse):"},{"line_number":2063,"context_line":"    if is_admin_context(context):"},{"line_number":2064,"context_line":"        return model_query(context, models.Snapshot, read_deleted\u003dread_deleted,"},{"line_number":2065,"context_line":"                           project_only\u003dproject_only).\\"},{"line_number":2066,"context_line":"            options(joinedload(\u0027snapshot_metadata\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_8f028571","line":2063,"updated":"2015-07-30 16:34:31.000000000","message":"This should be changed to something like what I suggest in line 2103","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2060,"context_line":""},{"line_number":2061,"context_line":"def _snapshot_get_all_for_volume_query(context, read_deleted\u003d\u0027yes\u0027,"},{"line_number":2062,"context_line":"                                       project_only\u003dFalse):"},{"line_number":2063,"context_line":"    if is_admin_context(context):"},{"line_number":2064,"context_line":"        return model_query(context, models.Snapshot, read_deleted\u003dread_deleted,"},{"line_number":2065,"context_line":"                           project_only\u003dproject_only).\\"},{"line_number":2066,"context_line":"            options(joinedload(\u0027snapshot_metadata\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_076367de","line":2063,"in_reply_to":"3a50d1a3_8f028571","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"@require_context"},{"line_number":2075,"context_line":"def snapshot_get_all_for_volume(context, volume_id):"},{"line_number":2076,"context_line":"    return _snapshot_get_all_for_volume_query(context, read_deleted\u003d\u0027no\u0027,"},{"line_number":2077,"context_line":"                                              project_only\u003dTrue).\\"},{"line_number":2078,"context_line":"        filter_by(volume_id\u003dvolume_id).\\"},{"line_number":2079,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_3aaccd34","line":2076,"updated":"2015-07-30 16:34:31.000000000","message":"I don\u0027t see the benefit of the additional method _snapshot_get_all_for_volume_query, we should do it here.","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2073,"context_line":""},{"line_number":2074,"context_line":"@require_context"},{"line_number":2075,"context_line":"def snapshot_get_all_for_volume(context, volume_id):"},{"line_number":2076,"context_line":"    return _snapshot_get_all_for_volume_query(context, read_deleted\u003d\u0027no\u0027,"},{"line_number":2077,"context_line":"                                              project_only\u003dTrue).\\"},{"line_number":2078,"context_line":"        filter_by(volume_id\u003dvolume_id).\\"},{"line_number":2079,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_a76153d5","line":2076,"in_reply_to":"3a50d1a3_3aaccd34","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2086,"context_line":"    if filters:"},{"line_number":2087,"context_line":"        query \u003d query.filter_by(**filters)"},{"line_number":2088,"context_line":""},{"line_number":2089,"context_line":"    if is_admin_context(context):"},{"line_number":2090,"context_line":"        return query.join(models.Snapshot.volume).\\"},{"line_number":2091,"context_line":"            filter(models.Volume.host \u003d\u003d host).\\"},{"line_number":2092,"context_line":"            options(joinedload(\u0027snapshot_metadata\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_9ac5e10b","line":2089,"updated":"2015-07-30 16:34:31.000000000","message":"This should be changed to something like what I suggest in line 2103","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2086,"context_line":"    if filters:"},{"line_number":2087,"context_line":"        query \u003d query.filter_by(**filters)"},{"line_number":2088,"context_line":""},{"line_number":2089,"context_line":"    if is_admin_context(context):"},{"line_number":2090,"context_line":"        return query.join(models.Snapshot.volume).\\"},{"line_number":2091,"context_line":"            filter(models.Volume.host \u003d\u003d host).\\"},{"line_number":2092,"context_line":"            options(joinedload(\u0027snapshot_metadata\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_c7b33f4c","line":2089,"in_reply_to":"3a50d1a3_9ac5e10b","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2100,"context_line":""},{"line_number":2101,"context_line":"def _snapshot_get_all_for_cgsnapshot_query(context, read_deleted\u003d\u0027yes\u0027,"},{"line_number":2102,"context_line":"                                           project_only\u003dFalse):"},{"line_number":2103,"context_line":"    if is_admin_context(context):"},{"line_number":2104,"context_line":"        return model_query(context, models.Snapshot, read_deleted\u003dread_deleted,"},{"line_number":2105,"context_line":"                           project_only\u003dproject_only).\\"},{"line_number":2106,"context_line":"            options(joinedload(\u0027volume\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_cf9ded67","line":2103,"updated":"2015-07-30 16:34:31.000000000","message":"I think this should be simplified:\n\n query \u003d model_query(context, models.Snapshot, read_deleted\u003dread_deleted,  project_only\u003dproject_only).\n            options(joinedload(\u0027volume\u0027)).\n            options(joinedload(\u0027snapshot_metadata\u0027))\n if is_admin_context(context):\n     query \u003d query.options(joinedload(\u0027snapshot_admin_metadata\u0027))\n return query","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2100,"context_line":""},{"line_number":2101,"context_line":"def _snapshot_get_all_for_cgsnapshot_query(context, read_deleted\u003d\u0027yes\u0027,"},{"line_number":2102,"context_line":"                                           project_only\u003dFalse):"},{"line_number":2103,"context_line":"    if is_admin_context(context):"},{"line_number":2104,"context_line":"        return model_query(context, models.Snapshot, read_deleted\u003dread_deleted,"},{"line_number":2105,"context_line":"                           project_only\u003dproject_only).\\"},{"line_number":2106,"context_line":"            options(joinedload(\u0027volume\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_67e82b17","line":2103,"in_reply_to":"3a50d1a3_cf9ded67","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2115,"context_line":""},{"line_number":2116,"context_line":"@require_context"},{"line_number":2117,"context_line":"def snapshot_get_all_for_cgsnapshot(context, cgsnapshot_id):"},{"line_number":2118,"context_line":"    return _snapshot_get_all_for_cgsnapshot_query(context, read_deleted\u003d\u0027no\u0027,"},{"line_number":2119,"context_line":"                                                  project_only\u003dTrue).\\"},{"line_number":2120,"context_line":"        filter_by(cgsnapshot_id\u003dcgsnapshot_id).\\"},{"line_number":2121,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_3afe0d39","line":2118,"updated":"2015-07-30 16:34:31.000000000","message":"I don\u0027t see the benefit of the additional method _snapshot_get_all_for_cgsnapshot_query, we should do it here.","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2115,"context_line":""},{"line_number":2116,"context_line":"@require_context"},{"line_number":2117,"context_line":"def snapshot_get_all_for_cgsnapshot(context, cgsnapshot_id):"},{"line_number":2118,"context_line":"    return _snapshot_get_all_for_cgsnapshot_query(context, read_deleted\u003d\u0027no\u0027,"},{"line_number":2119,"context_line":"                                                  project_only\u003dTrue).\\"},{"line_number":2120,"context_line":"        filter_by(cgsnapshot_id\u003dcgsnapshot_id).\\"},{"line_number":2121,"context_line":"        all()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_87ebf711","line":2118,"in_reply_to":"3a50d1a3_3afe0d39","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2129,"context_line":"    if filters:"},{"line_number":2130,"context_line":"        query \u003d query.filter_by(**filters)"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":"    if is_admin_context(context):"},{"line_number":2133,"context_line":"        return query.filter_by(project_id\u003dproject_id).\\"},{"line_number":2134,"context_line":"            options(joinedload(\u0027snapshot_metadata\u0027)).\\"},{"line_number":2135,"context_line":"            options(joinedload(\u0027snapshot_admin_metadata\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_efe2292f","line":2132,"updated":"2015-07-30 16:34:31.000000000","message":"I think we should update the query with the general query and then add if it\u0027s the admin like we are doing with the filters:\n\n query \u003d query.filter_by(project_id\u003dproject_id).options(joinedload(\u0027snapshot_metadata\u0027))\n if is_admin_context(context):\n     query \u003d query.options(joinedload(\u0027snapshot_admin_metadata\u0027))\n return query.all()","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2129,"context_line":"    if filters:"},{"line_number":2130,"context_line":"        query \u003d query.filter_by(**filters)"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":"    if is_admin_context(context):"},{"line_number":2133,"context_line":"        return query.filter_by(project_id\u003dproject_id).\\"},{"line_number":2134,"context_line":"            options(joinedload(\u0027snapshot_metadata\u0027)).\\"},{"line_number":2135,"context_line":"            options(joinedload(\u0027snapshot_admin_metadata\u0027)).\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_87d2175a","line":2132,"in_reply_to":"3a50d1a3_efe2292f","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2286,"context_line":"        snapshot_id\u003dsnapshot_id)"},{"line_number":2287,"context_line":""},{"line_number":2288,"context_line":""},{"line_number":2289,"context_line":"@require_context"},{"line_number":2290,"context_line":"def _snapshot_admin_metadata_get(context, snapshot_id, session\u003dNone):"},{"line_number":2291,"context_line":"    rows \u003d _snapshot_admin_metadata_get_query(context, snapshot_id,"},{"line_number":2292,"context_line":"                                              session).all()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_e3fec8ad","line":2289,"updated":"2015-07-30 16:34:31.000000000","message":"This is not needed","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2286,"context_line":"        snapshot_id\u003dsnapshot_id)"},{"line_number":2287,"context_line":""},{"line_number":2288,"context_line":""},{"line_number":2289,"context_line":"@require_context"},{"line_number":2290,"context_line":"def _snapshot_admin_metadata_get(context, snapshot_id, session\u003dNone):"},{"line_number":2291,"context_line":"    rows \u003d _snapshot_admin_metadata_get_query(context, snapshot_id,"},{"line_number":2292,"context_line":"                                              session).all()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_944c839e","line":2289,"in_reply_to":"3a50d1a3_e3fec8ad","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2292,"context_line":"                                              session).all()"},{"line_number":2293,"context_line":"    result \u003d {}"},{"line_number":2294,"context_line":"    for row in rows:"},{"line_number":2295,"context_line":"        result[row[\u0027key\u0027]] \u003d row[\u0027value\u0027]"},{"line_number":2296,"context_line":""},{"line_number":2297,"context_line":"    return result"},{"line_number":2298,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_c3c62c68","line":2295,"updated":"2015-07-30 16:34:31.000000000","message":"This should use dict comprehension:\n\n result \u003d {r[\u0027key\u0027]: r[\u0027value\u0027] for r in rows}","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2292,"context_line":"                                              session).all()"},{"line_number":2293,"context_line":"    result \u003d {}"},{"line_number":2294,"context_line":"    for row in rows:"},{"line_number":2295,"context_line":"        result[row[\u0027key\u0027]] \u003d row[\u0027value\u0027]"},{"line_number":2296,"context_line":""},{"line_number":2297,"context_line":"    return result"},{"line_number":2298,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_870777c8","line":2295,"in_reply_to":"3a50d1a3_c3c62c68","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2297,"context_line":"    return result"},{"line_number":2298,"context_line":""},{"line_number":2299,"context_line":""},{"line_number":2300,"context_line":"@require_context"},{"line_number":2301,"context_line":"@require_snapshot_exists"},{"line_number":2302,"context_line":"def snapshot_admin_metadata_get(context, snapshot_id):"},{"line_number":2303,"context_line":"    return _snapshot_admin_metadata_get(context, snapshot_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_87bc57dc","line":2300,"updated":"2015-08-02 07:30:48.000000000","message":"Using require_admin_context","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2306,"context_line":"@require_context"},{"line_number":2307,"context_line":"@require_snapshot_exists"},{"line_number":2308,"context_line":"@_retry_on_deadlock"},{"line_number":2309,"context_line":"def snapshot_admin_metadata_delete(context, snapshot_id, key):"},{"line_number":2310,"context_line":"    _snapshot_admin_metadata_get_query(context, snapshot_id).\\"},{"line_number":2311,"context_line":"        filter_by(key\u003dkey).\\"},{"line_number":2312,"context_line":"        update({\u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_f553d637","line":2309,"updated":"2015-07-30 16:34:31.000000000","message":"You need to check context is admin and raise an exception if it\u0027s not.","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2306,"context_line":"@require_context"},{"line_number":2307,"context_line":"@require_snapshot_exists"},{"line_number":2308,"context_line":"@_retry_on_deadlock"},{"line_number":2309,"context_line":"def snapshot_admin_metadata_delete(context, snapshot_id, key):"},{"line_number":2310,"context_line":"    _snapshot_admin_metadata_get_query(context, snapshot_id).\\"},{"line_number":2311,"context_line":"        filter_by(key\u003dkey).\\"},{"line_number":2312,"context_line":"        update({\u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_27b3030c","line":2309,"in_reply_to":"3a50d1a3_f553d637","updated":"2015-08-02 07:30:48.000000000","message":"Instead Using require_admin_context decorator","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2314,"context_line":"                \u0027updated_at\u0027: literal_column(\u0027updated_at\u0027)})"},{"line_number":2315,"context_line":""},{"line_number":2316,"context_line":""},{"line_number":2317,"context_line":"@require_context"},{"line_number":2318,"context_line":"def _snapshot_admin_metadata_get_item(context, snapshot_id, key, session\u003dNone):"},{"line_number":2319,"context_line":"    result \u003d _snapshot_admin_metadata_get_query("},{"line_number":2320,"context_line":"        context, snapshot_id, session\u003dsession).filter_by(key\u003dkey).first()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_e712fb57","line":2317,"updated":"2015-08-02 07:30:48.000000000","message":"Removed this as its not needed as well","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2317,"context_line":"@require_context"},{"line_number":2318,"context_line":"def _snapshot_admin_metadata_get_item(context, snapshot_id, key, session\u003dNone):"},{"line_number":2319,"context_line":"    result \u003d _snapshot_admin_metadata_get_query("},{"line_number":2320,"context_line":"        context, snapshot_id, session\u003dsession).filter_by(key\u003dkey).first()"},{"line_number":2321,"context_line":""},{"line_number":2322,"context_line":"    if not result:"},{"line_number":2323,"context_line":"        raise exception.SnapshotAdminMetadataNotFound(metadata_key\u003dkey,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_869fd264","line":2320,"updated":"2015-07-30 16:34:31.000000000","message":"Shouldn\u0027t this fail if we get more than 1 result?","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2317,"context_line":"@require_context"},{"line_number":2318,"context_line":"def _snapshot_admin_metadata_get_item(context, snapshot_id, key, session\u003dNone):"},{"line_number":2319,"context_line":"    result \u003d _snapshot_admin_metadata_get_query("},{"line_number":2320,"context_line":"        context, snapshot_id, session\u003dsession).filter_by(key\u003dkey).first()"},{"line_number":2321,"context_line":""},{"line_number":2322,"context_line":"    if not result:"},{"line_number":2323,"context_line":"        raise exception.SnapshotAdminMetadataNotFound(metadata_key\u003dkey,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_c7a49f4c","line":2320,"in_reply_to":"3a50d1a3_869fd264","updated":"2015-08-02 07:30:48.000000000","message":"I thought .first() ensures we get only 1 result","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2325,"context_line":"    return result"},{"line_number":2326,"context_line":""},{"line_number":2327,"context_line":""},{"line_number":2328,"context_line":"@require_context"},{"line_number":2329,"context_line":"@require_snapshot_exists"},{"line_number":2330,"context_line":"@_retry_on_deadlock"},{"line_number":2331,"context_line":"def snapshot_admin_metadata_update(context, snapshot_id, metadata, delete):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_e7cd1b87","line":2328,"updated":"2015-08-02 07:30:48.000000000","message":"Using require_admin_context","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2336,"context_line":"            original_metadata \u003d _snapshot_admin_metadata_get(context,"},{"line_number":2337,"context_line":"                                                             snapshot_id,"},{"line_number":2338,"context_line":"                                                             session)"},{"line_number":2339,"context_line":"            for meta_key, meta_value in original_metadata.items():"},{"line_number":2340,"context_line":"                if meta_key not in metadata:"},{"line_number":2341,"context_line":"                    meta_ref \u003d _snapshot_admin_metadata_get_item(context,"},{"line_number":2342,"context_line":"                                                                 snapshot_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_0f40b534","line":2339,"updated":"2015-07-30 16:34:31.000000000","message":"We don\u0027t need meta_value, so we can do:\n\n for meta_key in original_metadata:","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2336,"context_line":"            original_metadata \u003d _snapshot_admin_metadata_get(context,"},{"line_number":2337,"context_line":"                                                             snapshot_id,"},{"line_number":2338,"context_line":"                                                             session)"},{"line_number":2339,"context_line":"            for meta_key, meta_value in original_metadata.items():"},{"line_number":2340,"context_line":"                if meta_key not in metadata:"},{"line_number":2341,"context_line":"                    meta_ref \u003d _snapshot_admin_metadata_get_item(context,"},{"line_number":2342,"context_line":"                                                                 snapshot_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_87c9b727","line":2339,"in_reply_to":"3a50d1a3_0f40b534","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2342,"context_line":"                                                                 snapshot_id,"},{"line_number":2343,"context_line":"                                                                 meta_key,"},{"line_number":2344,"context_line":"                                                                 session)"},{"line_number":2345,"context_line":"                    meta_ref.update({\u0027deleted\u0027: True,"},{"line_number":2346,"context_line":"                                     \u0027deleted_at\u0027: timeutils.utcnow()})"},{"line_number":2347,"context_line":"                    meta_ref.save(session\u003dsession)"},{"line_number":2348,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_46b46a81","line":2345,"updated":"2015-07-30 16:34:31.000000000","message":"This is not how we should be doing the update.  There is no need to do 2 queries (get current element and then update) where 1 is enough (update using a where clause with snapshot_id, meta_key and deleted\u003d\u003d0)","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2342,"context_line":"                                                                 snapshot_id,"},{"line_number":2343,"context_line":"                                                                 meta_key,"},{"line_number":2344,"context_line":"                                                                 session)"},{"line_number":2345,"context_line":"                    meta_ref.update({\u0027deleted\u0027: True,"},{"line_number":2346,"context_line":"                                     \u0027deleted_at\u0027: timeutils.utcnow()})"},{"line_number":2347,"context_line":"                    meta_ref.save(session\u003dsession)"},{"line_number":2348,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_27d0e3c8","line":2345,"in_reply_to":"3a50d1a3_46b46a81","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":2363,"context_line":"                meta_ref \u003d models.SnapshotAdminMetadata()"},{"line_number":2364,"context_line":"                item.update({\"key\": meta_key, \"snapshot_id\": snapshot_id})"},{"line_number":2365,"context_line":""},{"line_number":2366,"context_line":"            meta_ref.update(item)"},{"line_number":2367,"context_line":"            meta_ref.save(session\u003dsession)"},{"line_number":2368,"context_line":""},{"line_number":2369,"context_line":"    return snapshot_admin_metadata_get(context, snapshot_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_f4ed1cb8","line":2366,"updated":"2015-07-30 16:34:31.000000000","message":"Same here, no need to get the item and then update it.","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":2363,"context_line":"                meta_ref \u003d models.SnapshotAdminMetadata()"},{"line_number":2364,"context_line":"                item.update({\"key\": meta_key, \"snapshot_id\": snapshot_id})"},{"line_number":2365,"context_line":""},{"line_number":2366,"context_line":"            meta_ref.update(item)"},{"line_number":2367,"context_line":"            meta_ref.save(session\u003dsession)"},{"line_number":2368,"context_line":""},{"line_number":2369,"context_line":"    return snapshot_admin_metadata_get(context, snapshot_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_07ab671f","line":2366,"in_reply_to":"3a50d1a3_f4ed1cb8","updated":"2015-08-02 07:30:48.000000000","message":"Since we are handling 2 scenarios here (key present and absent), i don\u0027t think we can entirely do away with 2 queries, esp for the case where key is absent.","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"a3bbe97f1f5e399941659be7a992939ede6657b3","unresolved":false,"context_lines":[{"line_number":2004,"context_line":"                    \u0027updated_at\u0027: literal_column(\u0027updated_at\u0027)})"},{"line_number":2005,"context_line":""},{"line_number":2006,"context_line":""},{"line_number":2007,"context_line":"def _snapshot_get_query(context, session\u003dNone, project_only\u003dFalse):"},{"line_number":2008,"context_line":"    if is_admin_context(context):"},{"line_number":2009,"context_line":"        return model_query(context, models.Snapshot, session\u003dsession,"},{"line_number":2010,"context_line":"                           project_only\u003dproject_only).\\"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_2bcbcea6","line":2007,"updated":"2015-08-05 11:59:16.000000000","message":"Is this method used anywhere?","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"5320e603304381ca307e17162e4a240800926c3c","unresolved":false,"context_lines":[{"line_number":2004,"context_line":"                    \u0027updated_at\u0027: literal_column(\u0027updated_at\u0027)})"},{"line_number":2005,"context_line":""},{"line_number":2006,"context_line":""},{"line_number":2007,"context_line":"def _snapshot_get_query(context, session\u003dNone, project_only\u003dFalse):"},{"line_number":2008,"context_line":"    if is_admin_context(context):"},{"line_number":2009,"context_line":"        return model_query(context, models.Snapshot, session\u003dsession,"},{"line_number":2010,"context_line":"                           project_only\u003dproject_only).\\"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_0f09b5dd","line":2007,"in_reply_to":"1a4dcd0f_2bcbcea6","updated":"2015-08-21 18:15:20.000000000","message":"Doesn\u0027t look like it is","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a54c901aef02facb2992aca3b3413d9c8101242c","unresolved":false,"context_lines":[{"line_number":2008,"context_line":"    if is_admin_context(context):"},{"line_number":2009,"context_line":"        return model_query(context, models.Snapshot, session\u003dsession,"},{"line_number":2010,"context_line":"                           project_only\u003dproject_only).\\"},{"line_number":2011,"context_line":"            options(joinedload(\u0027volume\u0027)).\\"},{"line_number":2012,"context_line":"            options(joinedload(\u0027snapshot_admin_metadata\u0027)).\\"},{"line_number":2013,"context_line":"            options(joinedload(\u0027snapshot_metadata\u0027))"},{"line_number":2014,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_984e2ac0","line":2011,"updated":"2015-08-24 16:17:37.000000000","message":"Can this be done with () instead of \\ ?","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"}],"cinder/db/sqlalchemy/migrate_repo/versions/049_add_snapshot_admin_metadata_table.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"ebb8ae266c3cea6680028ee2a88191ccf8c66e8d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":2,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":3,"context_line":"#    a copy of the License at"},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_8d3d2a90","line":1,"updated":"2015-07-28 14:07:38.000000000","message":"Please, rabase it to the latest master. We already 50 migrations","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"e7fbe67be86287d5cde1c1b023c84875b5affbbe","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":2,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":3,"context_line":"#    a copy of the License at"},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_f96569e0","line":1,"in_reply_to":"3a50d1a3_8d3d2a90","updated":"2015-07-29 07:16:15.000000000","message":"Done","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"ebb8ae266c3cea6680028ee2a88191ccf8c66e8d","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        Column(\u0027updated_at\u0027, DateTime),"},{"line_number":33,"context_line":"        Column(\u0027deleted_at\u0027, DateTime),"},{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_edd86e55","line":35,"updated":"2015-07-28 14:07:38.000000000","message":"I propose to use UUID as primary key to make it more scalable","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"7bc680a94966305cf8e7471743e97cecb81eeb95","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        Column(\u0027updated_at\u0027, DateTime),"},{"line_number":33,"context_line":"        Column(\u0027deleted_at\u0027, DateTime),"},{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_ace55638","line":35,"in_reply_to":"3a50d1a3_16a1f793","updated":"2015-07-29 11:33:22.000000000","message":"Ok, falling back to usign Integer.\nHad other issues when i tried to use UUID for pk, discussed with Ivan over IRC, and he said for now lets go back to Integer","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"4c23ac2f099e341d2c1f61e3257500722f702035","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        Column(\u0027updated_at\u0027, DateTime),"},{"line_number":33,"context_line":"        Column(\u0027deleted_at\u0027, DateTime),"},{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_16a1f793","line":35,"in_reply_to":"3a50d1a3_9ce3f3a3","updated":"2015-07-29 10:46:19.000000000","message":"I won\u0027t block patch for it","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"e7fbe67be86287d5cde1c1b023c84875b5affbbe","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        Column(\u0027updated_at\u0027, DateTime),"},{"line_number":33,"context_line":"        Column(\u0027deleted_at\u0027, DateTime),"},{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_9ce3f3a3","line":35,"in_reply_to":"3a50d1a3_edd86e55","updated":"2015-07-29 07:16:15.000000000","message":"Done","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"0e06c3c25e109425b86cc9b4585a65b5ffbc1047","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        Column(\u0027updated_at\u0027, DateTime),"},{"line_number":33,"context_line":"        Column(\u0027deleted_at\u0027, DateTime),"},{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_bcbd8fb2","line":35,"in_reply_to":"3a50d1a3_edd86e55","updated":"2015-07-29 07:04:38.000000000","message":"What sort of scaling are you concerned with? For scale-out, uuids don\u0027t prevent sync between masters - you need a snowflake style partition for that; for scale-up, less overhead is better (so int pk\u0027s).\n\nThere\u0027s quite a mix of pk types in models.py today.","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"ebb8ae266c3cea6680028ee2a88191ccf8c66e8d","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"},{"line_number":39,"context_line":"        Column(\u0027value\u0027, String(length\u003d255)),"},{"line_number":40,"context_line":"        mysql_engine\u003d\u0027InnoDB\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_adb586ed","line":37,"updated":"2015-07-28 14:07:38.000000000","message":"Should we add index here?","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"e7fbe67be86287d5cde1c1b023c84875b5affbbe","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"},{"line_number":39,"context_line":"        Column(\u0027value\u0027, String(length\u003d255)),"},{"line_number":40,"context_line":"        mysql_engine\u003d\u0027InnoDB\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_dcddfbe0","line":37,"in_reply_to":"3a50d1a3_adb586ed","updated":"2015-07-29 07:16:15.000000000","message":"Done","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"0e06c3c25e109425b86cc9b4585a65b5ffbc1047","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, Integer, primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"},{"line_number":39,"context_line":"        Column(\u0027value\u0027, String(length\u003d255)),"},{"line_number":40,"context_line":"        mysql_engine\u003d\u0027InnoDB\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_dc6f3b29","line":37,"in_reply_to":"3a50d1a3_adb586ed","updated":"2015-07-29 07:04:38.000000000","message":"I agree, lookup by snapshot_id seems likely.","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"ebb8ae266c3cea6680028ee2a88191ccf8c66e8d","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    try:"},{"line_number":44,"context_line":"        snapshot_admin_metadata.create()"},{"line_number":45,"context_line":"    except Exception:"},{"line_number":46,"context_line":"        LOG.error(_LE(\"Table |%s| not created!\"),"},{"line_number":47,"context_line":"                  repr(snapshot_admin_metadata))"},{"line_number":48,"context_line":"        raise"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_4d96424d","line":46,"updated":"2015-07-28 14:07:38.000000000","message":"Please, remove logging statements from the migration script","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"e7fbe67be86287d5cde1c1b023c84875b5affbbe","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    try:"},{"line_number":44,"context_line":"        snapshot_admin_metadata.create()"},{"line_number":45,"context_line":"    except Exception:"},{"line_number":46,"context_line":"        LOG.error(_LE(\"Table |%s| not created!\"),"},{"line_number":47,"context_line":"                  repr(snapshot_admin_metadata))"},{"line_number":48,"context_line":"        raise"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_f99329a1","line":46,"in_reply_to":"3a50d1a3_4d96424d","updated":"2015-07-29 07:16:15.000000000","message":"Done","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"0e06c3c25e109425b86cc9b4585a65b5ffbc1047","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    try:"},{"line_number":44,"context_line":"        snapshot_admin_metadata.create()"},{"line_number":45,"context_line":"    except Exception:"},{"line_number":46,"context_line":"        LOG.error(_LE(\"Table |%s| not created!\"),"},{"line_number":47,"context_line":"                  repr(snapshot_admin_metadata))"},{"line_number":48,"context_line":"        raise"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_9c4c53c9","line":46,"in_reply_to":"3a50d1a3_4d96424d","updated":"2015-07-29 07:04:38.000000000","message":"also remove the try/except/raise - its not needed here.","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"0e06c3c25e109425b86cc9b4585a65b5ffbc1047","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        snapshot_admin_metadata.drop()"},{"line_number":58,"context_line":"    except Exception:"},{"line_number":59,"context_line":"        LOG.error(_LE(\"snapshot_admin_metadata table not dropped.\"))"},{"line_number":60,"context_line":"        raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_bc47cfa1","line":60,"updated":"2015-07-29 07:04:38.000000000","message":"or here.","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"37b370cc818c6375f25260fb0ea960f9db206f0e","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        snapshot_admin_metadata.drop()"},{"line_number":58,"context_line":"    except Exception:"},{"line_number":59,"context_line":"        LOG.error(_LE(\"snapshot_admin_metadata table not dropped.\"))"},{"line_number":60,"context_line":"        raise"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_1ff11501","line":60,"in_reply_to":"3a50d1a3_bc47cfa1","updated":"2015-07-29 07:19:04.000000000","message":"Done","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"}],"cinder/db/sqlalchemy/migrate_repo/versions/051_add_snapshot_admin_metadata_table.py":[{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"3c85a6c2819ef13a45fa047fecfbbbacbe7bda2d","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        Column(\u0027updated_at\u0027, DateTime),"},{"line_number":33,"context_line":"        Column(\u0027deleted_at\u0027, DateTime),"},{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, String(length\u003d36), primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse, index\u003dTrue),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a50d1a3_722dd98d","line":35,"updated":"2015-07-29 14:56:36.000000000","message":"for some reason the change to Integer didn\u0027t happen... will upload proper one","commit_id":"09be456f92867f262d948050a4c3d3bbce0dae1b"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"a66a6477784e28308dca0b569ad0c2427b2cf103","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        Column(\u0027updated_at\u0027, DateTime),"},{"line_number":33,"context_line":"        Column(\u0027deleted_at\u0027, DateTime),"},{"line_number":34,"context_line":"        Column(\u0027deleted\u0027, Boolean),"},{"line_number":35,"context_line":"        Column(\u0027id\u0027, String(length\u003d36), primary_key\u003dTrue, nullable\u003dFalse),"},{"line_number":36,"context_line":"        Column(\u0027snapshot_id\u0027, String(length\u003d36), ForeignKey(\u0027snapshots.id\u0027),"},{"line_number":37,"context_line":"               nullable\u003dFalse, index\u003dTrue),"},{"line_number":38,"context_line":"        Column(\u0027key\u0027, String(length\u003d255)),"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a50d1a3_52e53d35","line":35,"in_reply_to":"3a50d1a3_722dd98d","updated":"2015-07-29 15:00:22.000000000","message":"The patch series from where I am doing `git review` shows me Integer for the \u0027id\u0027 Column, but somehow its not reflecting here, not sure if i am hitting a git review bug ?","commit_id":"09be456f92867f262d948050a4c3d3bbce0dae1b"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a54c901aef02facb2992aca3b3413d9c8101242c","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        mysql_engine\u003d\u0027InnoDB\u0027"},{"line_number":39,"context_line":"    )"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    snapshot_admin_metadata.create()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"def downgrade(migrate_engine):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_d8b0228e","line":41,"updated":"2015-08-24 16:17:37.000000000","message":"Need to have try blocks in here and catch any failures.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a54c901aef02facb2992aca3b3413d9c8101242c","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    meta.bind \u003d migrate_engine"},{"line_number":47,"context_line":"    snapshot_admin_metadata \u003d Table(\u0027snapshot_admin_metadata\u0027,"},{"line_number":48,"context_line":"                                    meta, autoload\u003dTrue)"},{"line_number":49,"context_line":"    snapshot_admin_metadata.drop()"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_7866b62a","line":49,"updated":"2015-08-24 16:17:37.000000000","message":"Same here.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"}],"cinder/exception.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a54c901aef02facb2992aca3b3413d9c8101242c","unresolved":false,"context_lines":[{"line_number":279,"context_line":""},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"class SnapshotAdminMetadataNotFound(NotFound):"},{"line_number":282,"context_line":"    message \u003d _(\"Snapshot %(snapshot_id)s has no administration metadata with \""},{"line_number":283,"context_line":"                \"key %(metadata_key)s.\")"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_f807261b","line":282,"updated":"2015-08-24 16:17:37.000000000","message":"I think this should be just \u0027admin metadata\u0027 .","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a54c901aef02facb2992aca3b3413d9c8101242c","unresolved":false,"context_lines":[{"line_number":988,"context_line":""},{"line_number":989,"context_line":""},{"line_number":990,"context_line":"class AdminMetadataAbsent(CinderException):"},{"line_number":991,"context_line":"    message \u003d _(\"There is no administration metadata in DB object.\")"},{"line_number":992,"context_line":""},{"line_number":993,"context_line":""},{"line_number":994,"context_line":"class NotSupportedOperation(Invalid):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_931dab68","line":991,"updated":"2015-08-24 16:17:37.000000000","message":"Same here.  Don\u0027t see if called administration metadata anywhere else.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"}],"cinder/objects/snapshot.py":[{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"591e0b52e940b255880793dd81dcdc5e55398481","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        \u0027provider_location\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":59,"context_line":"        \u0027provider_id\u0027: fields.UUIDField(nullable\u003dTrue),"},{"line_number":60,"context_line":"        \u0027metadata\u0027: fields.DictOfStringsField(),"},{"line_number":61,"context_line":"        \u0027admin_metadata\u0027: fields.DictOfNullableStringsField(),"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        \u0027volume\u0027: fields.ObjectField(\u0027Volume\u0027, nullable\u003dTrue),"},{"line_number":64,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_8504244a","line":61,"updated":"2015-07-22 13:56:38.000000000","message":"admin_metadata should only be available when the context has the right privileges, so you should make this fields.DictOfNullableStringsField(nullable\u003dTrue).","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"e7fbe67be86287d5cde1c1b023c84875b5affbbe","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        \u0027provider_location\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":59,"context_line":"        \u0027provider_id\u0027: fields.UUIDField(nullable\u003dTrue),"},{"line_number":60,"context_line":"        \u0027metadata\u0027: fields.DictOfStringsField(),"},{"line_number":61,"context_line":"        \u0027admin_metadata\u0027: fields.DictOfNullableStringsField(),"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        \u0027volume\u0027: fields.ObjectField(\u0027Volume\u0027, nullable\u003dTrue),"},{"line_number":64,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_5f9d5dba","line":61,"in_reply_to":"3a50d1a3_8504244a","updated":"2015-07-29 07:16:15.000000000","message":"Done","commit_id":"755660c7cc294c717bdb9d92abc9301cb26499f1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                raise exception.MetadataAbsent()"},{"line_number":132,"context_line":"            snapshot.metadata \u003d {item[\u0027key\u0027]: item[\u0027value\u0027]"},{"line_number":133,"context_line":"                                 for item in metadata}"},{"line_number":134,"context_line":"        if \u0027admin_metadata\u0027 in expected_attrs:"},{"line_number":135,"context_line":"            admin_metadata \u003d db_snapshot.get(\u0027snapshot_admin_metadata\u0027)"},{"line_number":136,"context_line":"            if admin_metadata is None:"},{"line_number":137,"context_line":"                raise exception.AdminMetadataAbsent()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_75d4a6bc","line":134,"updated":"2015-07-30 16:34:31.000000000","message":"You should check that context is admin before populating the admin_metadata","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                raise exception.MetadataAbsent()"},{"line_number":132,"context_line":"            snapshot.metadata \u003d {item[\u0027key\u0027]: item[\u0027value\u0027]"},{"line_number":133,"context_line":"                                 for item in metadata}"},{"line_number":134,"context_line":"        if \u0027admin_metadata\u0027 in expected_attrs:"},{"line_number":135,"context_line":"            admin_metadata \u003d db_snapshot.get(\u0027snapshot_admin_metadata\u0027)"},{"line_number":136,"context_line":"            if admin_metadata is None:"},{"line_number":137,"context_line":"                raise exception.AdminMetadataAbsent()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_d6c9ab5b","line":134,"in_reply_to":"3a50d1a3_75d4a6bc","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                                                            self.id, metadata,"},{"line_number":179,"context_line":"                                                            True)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"            if \u0027admin_metadata\u0027 in updates:"},{"line_number":182,"context_line":"                # Metadata items that are not specified in the"},{"line_number":183,"context_line":"                # self.admin_metadata will be deleted"},{"line_number":184,"context_line":"                admin_metadata \u003d updates.pop(\u0027admin_metadata\u0027, None)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_75eb4679","line":181,"updated":"2015-07-30 16:34:31.000000000","message":"You should not allow saving admin_metadata if context is not admin","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                                                            self.id, metadata,"},{"line_number":179,"context_line":"                                                            True)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"            if \u0027admin_metadata\u0027 in updates:"},{"line_number":182,"context_line":"                # Metadata items that are not specified in the"},{"line_number":183,"context_line":"                # self.admin_metadata will be deleted"},{"line_number":184,"context_line":"                admin_metadata \u003d updates.pop(\u0027admin_metadata\u0027, None)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_96c32338","line":181,"in_reply_to":"3a50d1a3_75eb4679","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        if not md_was_changed:"},{"line_number":219,"context_line":"            self.obj_reset_changes([\u0027metadata\u0027])"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def delete_admin_metadata_key(self, context, key):"},{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(context, self.id, key)"},{"line_number":223,"context_line":"        md_was_changed \u003d \u0027admin_metadata\u0027 in self.obj_what_changed()"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_67ce3db9","line":221,"updated":"2015-07-30 16:34:31.000000000","message":"Why do we need to provide a context when the object already has a context?","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"12815d8a50a6b5b6e3f52544bf01c88c2ba72d0f","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        if not md_was_changed:"},{"line_number":219,"context_line":"            self.obj_reset_changes([\u0027metadata\u0027])"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def delete_admin_metadata_key(self, context, key):"},{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(context, self.id, key)"},{"line_number":223,"context_line":"        md_was_changed \u003d \u0027admin_metadata\u0027 in self.obj_what_changed()"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_5e111851","line":221,"in_reply_to":"3a50d1a3_36e9efb7","updated":"2015-08-03 11:43:32.000000000","message":"You require an admin context here, but the one stashed in the object most likely does not have an admin context (but depends on how it was instantiated).  Hence, you must either provide it as an argument (to avoid a context.elevate()) or put it in a with statement - with self.obj_as_admin().","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        if not md_was_changed:"},{"line_number":219,"context_line":"            self.obj_reset_changes([\u0027metadata\u0027])"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def delete_admin_metadata_key(self, context, key):"},{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(context, self.id, key)"},{"line_number":223,"context_line":"        md_was_changed \u003d \u0027admin_metadata\u0027 in self.obj_what_changed()"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_36e9efb7","line":221,"in_reply_to":"3a50d1a3_67ce3db9","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            self.obj_reset_changes([\u0027metadata\u0027])"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def delete_admin_metadata_key(self, context, key):"},{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(context, self.id, key)"},{"line_number":223,"context_line":"        md_was_changed \u003d \u0027admin_metadata\u0027 in self.obj_what_changed()"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        del self.admin_metadata[key]"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_f5019607","line":222,"updated":"2015-07-30 16:34:31.000000000","message":"This should only be allowed to admins, but once you you add the admin context check in the db api it will be resolved.","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            self.obj_reset_changes([\u0027metadata\u0027])"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def delete_admin_metadata_key(self, context, key):"},{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(context, self.id, key)"},{"line_number":223,"context_line":"        md_was_changed \u003d \u0027admin_metadata\u0027 in self.obj_what_changed()"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        del self.admin_metadata[key]"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_d6d0cbf9","line":222,"in_reply_to":"3a50d1a3_f5019607","updated":"2015-08-02 07:30:48.000000000","message":"db api has admin context decorator for this.","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(context, self.id, key)"},{"line_number":223,"context_line":"        md_was_changed \u003d \u0027admin_metadata\u0027 in self.obj_what_changed()"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        del self.admin_metadata[key]"},{"line_number":226,"context_line":"        self._orig_admin_metadata.pop(key, None)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        if not md_was_changed:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_e7e56d0a","line":225,"updated":"2015-07-30 16:34:31.000000000","message":"We don\u0027t necessarily have the key in admin_metadata, shouldn\u0027t we have checked first that the key existed or use pop here?","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(context, self.id, key)"},{"line_number":223,"context_line":"        md_was_changed \u003d \u0027admin_metadata\u0027 in self.obj_what_changed()"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        del self.admin_metadata[key]"},{"line_number":226,"context_line":"        self._orig_admin_metadata.pop(key, None)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        if not md_was_changed:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_d6252b0a","line":225,"in_reply_to":"3a50d1a3_e7e56d0a","updated":"2015-08-02 07:30:48.000000000","message":"I followed what was being done in delete_metadata_key() above, but I guess you are right too","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        self._orig_admin_metadata.pop(key, None)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        if not md_was_changed:"},{"line_number":229,"context_line":"            self.obj_reset_changes([\u0027admin_metadata\u0027])"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @base.remotable_classmethod"},{"line_number":232,"context_line":"    def snapshot_data_get_for_project(cls, context, project_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_07917952","line":229,"updated":"2015-07-30 16:34:31.000000000","message":"I must be missing something, because I don\u0027t think it\u0027s necessary to reset the changes.  Could you walk me through the case where it is needed please?","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        self._orig_admin_metadata.pop(key, None)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        if not md_was_changed:"},{"line_number":229,"context_line":"            self.obj_reset_changes([\u0027admin_metadata\u0027])"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @base.remotable_classmethod"},{"line_number":232,"context_line":"    def snapshot_data_get_for_project(cls, context, project_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_f62067fa","line":229,"in_reply_to":"3a50d1a3_07917952","updated":"2015-08-02 07:30:48.000000000","message":"I followed what was being done in delete_metadata_key() above, but I guess you are right too","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"88f9431b2beb587d042d8cbfd9257d98c350ece6","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                raise exception.MetadataAbsent()"},{"line_number":132,"context_line":"            snapshot.metadata \u003d {item[\u0027key\u0027]: item[\u0027value\u0027]"},{"line_number":133,"context_line":"                                 for item in metadata}"},{"line_number":134,"context_line":"        if context.is_admin and \u0027admin_metadata\u0027 in expected_attrs:"},{"line_number":135,"context_line":"            admin_metadata \u003d db_snapshot.get(\u0027snapshot_admin_metadata\u0027)"},{"line_number":136,"context_line":"            if admin_metadata is None:"},{"line_number":137,"context_line":"                raise exception.AdminMetadataAbsent()"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_07b87ee4","line":134,"updated":"2015-08-04 14:40:48.000000000","message":"Is it really necessary to check is_admin here?  The db would have already determined if the context is admin or not and done the join.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"7cfa6b5a9ee2f878c7be9f6e7c9db35c4a76701c","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                raise exception.MetadataAbsent()"},{"line_number":132,"context_line":"            snapshot.metadata \u003d {item[\u0027key\u0027]: item[\u0027value\u0027]"},{"line_number":133,"context_line":"                                 for item in metadata}"},{"line_number":134,"context_line":"        if context.is_admin and \u0027admin_metadata\u0027 in expected_attrs:"},{"line_number":135,"context_line":"            admin_metadata \u003d db_snapshot.get(\u0027snapshot_admin_metadata\u0027)"},{"line_number":136,"context_line":"            if admin_metadata is None:"},{"line_number":137,"context_line":"                raise exception.AdminMetadataAbsent()"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_4ce9a7e5","line":134,"in_reply_to":"1a4dcd0f_07b87ee4","updated":"2015-08-05 05:08:16.000000000","message":"Check comment from Gorka in prev patchset, I was asked to check for is_admin here. I think its required bcos if someone does object.Snapshot.get_by_id(context, id) then we need to check if the context passed is havign admin role or not, hence the check.\n\nIf you disagree, can you provide an example of how and why the context will always be admin context for this case ? Not clear to me.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"10478dfc120f45e5d88b2ed9b625e6d5f39c2865","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                raise exception.MetadataAbsent()"},{"line_number":132,"context_line":"            snapshot.metadata \u003d {item[\u0027key\u0027]: item[\u0027value\u0027]"},{"line_number":133,"context_line":"                                 for item in metadata}"},{"line_number":134,"context_line":"        if context.is_admin and \u0027admin_metadata\u0027 in expected_attrs:"},{"line_number":135,"context_line":"            admin_metadata \u003d db_snapshot.get(\u0027snapshot_admin_metadata\u0027)"},{"line_number":136,"context_line":"            if admin_metadata is None:"},{"line_number":137,"context_line":"                raise exception.AdminMetadataAbsent()"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_a8f284d0","line":134,"in_reply_to":"1a4dcd0f_4ce9a7e5","updated":"2015-08-05 11:57:38.000000000","message":"snapshot_get already does a check if the context is_admin and performs the join with snapshot_admin_metadata if it is.  So the fact the we are doing it again here does not make sense to me.  But the code does not hurt anything, it is just redundant.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"88f9431b2beb587d042d8cbfd9257d98c350ece6","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            self.obj_reset_changes([\u0027metadata\u0027])"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def delete_admin_metadata_key(self, key):"},{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(self._context, self.id, key)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        self.admin_metadata.pop(key, None)"},{"line_number":225,"context_line":"        self._orig_admin_metadata.pop(key, None)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_272c9a01","line":222,"updated":"2015-08-04 14:40:48.000000000","message":"As I mentioned in a previous patch, the object would have to be instantiated with an admin context for this method to use self._context, which may not be the case everytime.  You should at least check if self._context.is_admin is True or not before attempting this db operation.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"7cfa6b5a9ee2f878c7be9f6e7c9db35c4a76701c","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            self.obj_reset_changes([\u0027metadata\u0027])"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def delete_admin_metadata_key(self, key):"},{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(self._context, self.id, key)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        self.admin_metadata.pop(key, None)"},{"line_number":225,"context_line":"        self._orig_admin_metadata.pop(key, None)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_2c73fb4e","line":222,"in_reply_to":"1a4dcd0f_272c9a01","updated":"2015-08-05 05:08:16.000000000","message":"The db call db.snapshot_admin_metadata_delete already has the require_admin_context decorator, do we still need  this check ?\n\nIn general, its not clear to me whats the recommended approach ?\nThe decorator way of restrcting admin only access or checking is_admin ?","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"10478dfc120f45e5d88b2ed9b625e6d5f39c2865","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            self.obj_reset_changes([\u0027metadata\u0027])"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    def delete_admin_metadata_key(self, key):"},{"line_number":222,"context_line":"        db.snapshot_admin_metadata_delete(self._context, self.id, key)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        self.admin_metadata.pop(key, None)"},{"line_number":225,"context_line":"        self._orig_admin_metadata.pop(key, None)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_0b37f2e3","line":222,"in_reply_to":"1a4dcd0f_2c73fb4e","updated":"2015-08-05 11:57:38.000000000","message":"It depends on where you want to fail, in the object or db layer.  But this is similar to the argument above, to check for is_admin here (again) or let the db layer handle it.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"}],"cinder/tests/unit/fake_snapshot.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0630e504ea60e55c7f82ca3b8c3fc7849dcc0c3d","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        \u0027display_name\u0027: \u0027fake_name\u0027,"},{"line_number":28,"context_line":"        \u0027display_description\u0027: \u0027fake_description\u0027,"},{"line_number":29,"context_line":"        \u0027metadata\u0027: {},"},{"line_number":30,"context_line":"        \u0027admin_metadata\u0027: {},"},{"line_number":31,"context_line":"        \u0027snapshot_metadata\u0027: [],"},{"line_number":32,"context_line":"        \u0027snapshot_admin_metadata\u0027: [],"},{"line_number":33,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_a7abe574","line":30,"updated":"2015-07-30 16:34:31.000000000","message":"This is not a real field of the DB, it should not be here.","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"5c563446aca4a8bf17631413b10670ded5b47a78","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        \u0027display_name\u0027: \u0027fake_name\u0027,"},{"line_number":28,"context_line":"        \u0027display_description\u0027: \u0027fake_description\u0027,"},{"line_number":29,"context_line":"        \u0027metadata\u0027: {},"},{"line_number":30,"context_line":"        \u0027admin_metadata\u0027: {},"},{"line_number":31,"context_line":"        \u0027snapshot_metadata\u0027: [],"},{"line_number":32,"context_line":"        \u0027snapshot_admin_metadata\u0027: [],"},{"line_number":33,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_f68387a7","line":30,"in_reply_to":"3a50d1a3_a7abe574","updated":"2015-08-02 07:30:48.000000000","message":"Done","commit_id":"4c9263ca6a374cd65e69999c407c5aa14669fef6"}],"cinder/tests/unit/objects/test_snapshot.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a54c901aef02facb2992aca3b3413d9c8101242c","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        self.assertEqual({\u0027key1\u0027: \u0027value1\u0027}, snapshot.metadata)"},{"line_number":147,"context_line":"        snapshot_metadata_delete.assert_called_once_with(self.context, \u00271\u0027,"},{"line_number":148,"context_line":"                                                         \u0027key2\u0027)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    @mock.patch(\u0027cinder.db.snapshot_admin_metadata_delete\u0027)"},{"line_number":151,"context_line":"    def test_delete_admin_metadata_key(self, snapshot_admin_metadata_delete):"},{"line_number":152,"context_line":"        admin_context \u003d self.context.elevated()"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_d3b0038e","line":149,"updated":"2015-08-24 16:17:37.000000000","message":"Need bad path testing here too.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"}],"cinder/volume/api.py":[{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"88f9431b2beb587d042d8cbfd9257d98c350ece6","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"    @wrap_check_policy"},{"line_number":1100,"context_line":"    def get_snapshot_admin_metadata(self, context, snapshot):"},{"line_number":1101,"context_line":"        \"\"\"Get all administration metadata associated with a snapshot.\"\"\""},{"line_number":1102,"context_line":"        rv \u003d self.db.snapshot_admin_metadata_get(context, snapshot[\u0027id\u0027])"},{"line_number":1103,"context_line":"        LOG.info(_LI(\"Get snapshot admin metadata completed successfully.\"),"},{"line_number":1104,"context_line":"                 resource\u003dsnapshot)"},{"line_number":1105,"context_line":"        return dict(rv)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_a76e6a2d","line":1102,"updated":"2015-08-04 14:40:48.000000000","message":"Why are we using db operation here.  You should use objects.","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"7cfa6b5a9ee2f878c7be9f6e7c9db35c4a76701c","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"    @wrap_check_policy"},{"line_number":1100,"context_line":"    def get_snapshot_admin_metadata(self, context, snapshot):"},{"line_number":1101,"context_line":"        \"\"\"Get all administration metadata associated with a snapshot.\"\"\""},{"line_number":1102,"context_line":"        rv \u003d self.db.snapshot_admin_metadata_get(context, snapshot[\u0027id\u0027])"},{"line_number":1103,"context_line":"        LOG.info(_LI(\"Get snapshot admin metadata completed successfully.\"),"},{"line_number":1104,"context_line":"                 resource\u003dsnapshot)"},{"line_number":1105,"context_line":"        return dict(rv)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_ac02ab6d","line":1102,"in_reply_to":"1a4dcd0f_a76e6a2d","updated":"2015-08-05 05:08:16.000000000","message":"will do in next patch","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"88f9431b2beb587d042d8cbfd9257d98c350ece6","unresolved":false,"context_lines":[{"line_number":1107,"context_line":"    @wrap_check_policy"},{"line_number":1108,"context_line":"    def delete_snapshot_admin_metadata(self, context, snapshot, key):"},{"line_number":1109,"context_line":"        \"\"\"Delete the given administration metadata item from a snapshot.\"\"\""},{"line_number":1110,"context_line":"        self.db.snapshot_admin_metadata_delete(context, snapshot[\u0027id\u0027], key)"},{"line_number":1111,"context_line":"        LOG.info(_LI(\"Delete snapshot admin metadata completed successfully.\"),"},{"line_number":1112,"context_line":"                 resource\u003dsnapshot)"},{"line_number":1113,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_c7693627","line":1110,"updated":"2015-08-04 14:40:48.000000000","message":"Ditto","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"},{"author":{"_account_id":10796,"name":"Deepak C Shetty (deepakcs)","email":"dpkshetty@gmail.com","username":"deepakcs"},"change_message_id":"7cfa6b5a9ee2f878c7be9f6e7c9db35c4a76701c","unresolved":false,"context_lines":[{"line_number":1107,"context_line":"    @wrap_check_policy"},{"line_number":1108,"context_line":"    def delete_snapshot_admin_metadata(self, context, snapshot, key):"},{"line_number":1109,"context_line":"        \"\"\"Delete the given administration metadata item from a snapshot.\"\"\""},{"line_number":1110,"context_line":"        self.db.snapshot_admin_metadata_delete(context, snapshot[\u0027id\u0027], key)"},{"line_number":1111,"context_line":"        LOG.info(_LI(\"Delete snapshot admin metadata completed successfully.\"),"},{"line_number":1112,"context_line":"                 resource\u003dsnapshot)"},{"line_number":1113,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_2cef7bb1","line":1110,"in_reply_to":"1a4dcd0f_c7693627","updated":"2015-08-05 05:08:16.000000000","message":"will do in next patch","commit_id":"704454a1f18858f9108ad2721bfc6f03d6e1d7fa"}]}