)]}'
{"glance/db/sqlalchemy/alembic_migrations/data_migrations/ocata_migrate01_community_images.py":[{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"ca60c4b52b4d63bd3381c13492a3807eb897db24","unresolved":false,"context_lines":[{"line_number":80,"context_line":"    migrated_rows \u003d (images"},{"line_number":81,"context_line":"                     .update().values(visibility\u003d\u0027shared\u0027)"},{"line_number":82,"context_line":"                     .where(images.c.visibility \u003d\u003d \u0027private\u0027)"},{"line_number":83,"context_line":"                     .where(images.c.id.in_("},{"line_number":84,"context_line":"                            select([image_members.c.image_id]).distinct()"},{"line_number":85,"context_line":"                            .where(not_(image_members.c.deleted)))"},{"line_number":86,"context_line":"                            )"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_48e1b599","line":83,"range":{"start_line":83,"start_character":22,"end_line":83,"end_character":27},"updated":"2017-01-29 20:12:15.000000000","message":"I think use of another where might increase the time complexity. Cos, we go through the entire table picking the ones that are private. Then we again go through our entire subset to check for our second condition of an active image member. \nI feel using an \u0027and\u0027 could be better here although I agree \u0027where\u0027 might look a lot cleaner code wise.","commit_id":"50427f9a12b970bc1e7549db805924ec7fc25f02"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"feeede0e6edfd3fb3de6e0f7f6dd3d23ed310abb","unresolved":false,"context_lines":[{"line_number":80,"context_line":"    migrated_rows \u003d (images"},{"line_number":81,"context_line":"                     .update().values(visibility\u003d\u0027shared\u0027)"},{"line_number":82,"context_line":"                     .where(images.c.visibility \u003d\u003d \u0027private\u0027)"},{"line_number":83,"context_line":"                     .where(images.c.id.in_("},{"line_number":84,"context_line":"                            select([image_members.c.image_id]).distinct()"},{"line_number":85,"context_line":"                            .where(not_(image_members.c.deleted)))"},{"line_number":86,"context_line":"                            )"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_c8ebe576","line":83,"range":{"start_line":83,"start_character":22,"end_line":83,"end_character":27},"in_reply_to":"3a461143_48e1b599","updated":"2017-01-29 20:16:50.000000000","message":"Dharini,\nMultiple where clauses is a way to express conjunctions in sqlalchemy.\nhttp://docs.sqlalchemy.org/en/latest/core/tutorial.html#conjunctions","commit_id":"50427f9a12b970bc1e7549db805924ec7fc25f02"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"1b1511f8a89920c206be48861d6a36fea2ab4ccf","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"},{"line_number":48,"context_line":"    rows_with_pending_shared \u003d (select[images.c.id]"},{"line_number":49,"context_line":"                                .where(images.c.visibility \u003d\u003d \u0027private\u0027)"},{"line_number":50,"context_line":"                                .where(images.c.id.in_("},{"line_number":51,"context_line":"                                    select([image_members.c.image_id])"},{"line_number":52,"context_line":"                                       .distinct()"},{"line_number":53,"context_line":"                                       .where(not_(image_members.c.deleted))))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_2ce9c4e6","line":50,"range":{"start_line":50,"start_character":33,"end_line":50,"end_character":38},"updated":"2017-01-30 22:04:10.000000000","message":"maybe use and_() to be consistent with all existing migrations? I know it\u0027s the same query that gets run, just not sure which reads better and is clearer. This pattern is repeated throughout this file.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"af26e6e3b575e3268e2c5a4b3ba39067ab4bf1c7","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"},{"line_number":48,"context_line":"    rows_with_pending_shared \u003d (select[images.c.id]"},{"line_number":49,"context_line":"                                .where(images.c.visibility \u003d\u003d \u0027private\u0027)"},{"line_number":50,"context_line":"                                .where(images.c.id.in_("},{"line_number":51,"context_line":"                                    select([image_members.c.image_id])"},{"line_number":52,"context_line":"                                       .distinct()"},{"line_number":53,"context_line":"                                       .where(not_(image_members.c.deleted))))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_3d155be4","line":50,"range":{"start_line":50,"start_character":33,"end_line":50,"end_character":38},"in_reply_to":"3a461143_2ce9c4e6","updated":"2017-01-31 03:56:04.000000000","message":"Done","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"1b1511f8a89920c206be48861d6a36fea2ab4ccf","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"def _mark_atmost_public_images_with_public_visibility(images, max_rows):"},{"line_number":106,"context_line":"    rows_to_migrate \u003d (select(columns\u003d[images.c.id], for_update\u003dTrue)"},{"line_number":107,"context_line":"                       .where(images.c.visibility \u003d\u003d None)"},{"line_number":108,"context_line":"                       .where(images.c.is_public)"},{"line_number":109,"context_line":"                       .limit(max_rows)"},{"line_number":110,"context_line":"                       .execute())"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_8c8e38ce","line":108,"range":{"start_line":106,"start_character":23,"end_line":108,"end_character":49},"updated":"2017-01-30 22:04:10.000000000","message":"for_update \nDeprecated since version 0.9.0:\nhttp://docs.sqlalchemy.org/en/latest/core/selectable.html?highlight\u003dfor_update#sqlalchemy.sql.expression.select.params.for_update\n\n\nI think you should use .with_for_update(read\u003dTrue) instead\n\nThis pattern is repeated throughout this file.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"af26e6e3b575e3268e2c5a4b3ba39067ab4bf1c7","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"def _mark_atmost_public_images_with_public_visibility(images, max_rows):"},{"line_number":106,"context_line":"    rows_to_migrate \u003d (select(columns\u003d[images.c.id], for_update\u003dTrue)"},{"line_number":107,"context_line":"                       .where(images.c.visibility \u003d\u003d None)"},{"line_number":108,"context_line":"                       .where(images.c.is_public)"},{"line_number":109,"context_line":"                       .limit(max_rows)"},{"line_number":110,"context_line":"                       .execute())"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_1d9fbf5f","line":108,"range":{"start_line":106,"start_character":23,"end_line":108,"end_character":49},"in_reply_to":"3a461143_8c8e38ce","updated":"2017-01-31 03:56:04.000000000","message":"Will tackle this in a separate patch that addresses batching specifically.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"1b1511f8a89920c206be48861d6a36fea2ab4ccf","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                       .limit(max_rows)"},{"line_number":110,"context_line":"                       .execute())"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    if rows_to_migrate.rowcount \u003e 0:"},{"line_number":113,"context_line":"        fetched_rows \u003d rows_to_migrate.fetchall()"},{"line_number":114,"context_line":"        ids_to_migrate \u003d list(itertools.chain(*fetched_rows))"},{"line_number":115,"context_line":"        migrated \u003d (images"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_a7cdc686","line":112,"updated":"2017-01-30 22:04:10.000000000","message":"An error during update will not close the rows resource. Not a big thing since it will cause the process to exit really fast anyway. But the else clause and the lack of error handling make this feel more complex than it needs to be.\n\nMaybe:\n\n  num_rows \u003d 0\n  with rows_to_migrate as rows:\n    if rows.rowcount \u003e 0:\n      fetched_rows \u003d rows.fetchall()\n      ids_to_migrate \u003d list(itertools.chain(*fetched_rows))\n      migrated \u003d [...]\n      num_rows \u003d migrated.rowcount\n\nI\u0027m not sure if this is actually better, it\u0027s just a thought. Use your judgement.\nThis pattern is repeated throughout this file.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"af26e6e3b575e3268e2c5a4b3ba39067ab4bf1c7","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                       .limit(max_rows)"},{"line_number":110,"context_line":"                       .execute())"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    if rows_to_migrate.rowcount \u003e 0:"},{"line_number":113,"context_line":"        fetched_rows \u003d rows_to_migrate.fetchall()"},{"line_number":114,"context_line":"        ids_to_migrate \u003d list(itertools.chain(*fetched_rows))"},{"line_number":115,"context_line":"        migrated \u003d (images"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_1d76df3d","line":112,"in_reply_to":"3a461143_7e71a075","updated":"2017-01-31 03:56:04.000000000","message":"Will tackle this in a separate patch that addresses batching specifically.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"af26e6e3b575e3268e2c5a4b3ba39067ab4bf1c7","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                       .limit(max_rows)"},{"line_number":110,"context_line":"                       .execute())"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    if rows_to_migrate.rowcount \u003e 0:"},{"line_number":113,"context_line":"        fetched_rows \u003d rows_to_migrate.fetchall()"},{"line_number":114,"context_line":"        ids_to_migrate \u003d list(itertools.chain(*fetched_rows))"},{"line_number":115,"context_line":"        migrated \u003d (images"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_fd7af322","line":112,"in_reply_to":"3a461143_a7cdc686","updated":"2017-01-31 03:56:04.000000000","message":"+1\nWill tackle this in a separate patch that addresses batching specifically.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"6b9d70b5a1d36a43eb656d83ee47c4561872735b","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                       .limit(max_rows)"},{"line_number":110,"context_line":"                       .execute())"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    if rows_to_migrate.rowcount \u003e 0:"},{"line_number":113,"context_line":"        fetched_rows \u003d rows_to_migrate.fetchall()"},{"line_number":114,"context_line":"        ids_to_migrate \u003d list(itertools.chain(*fetched_rows))"},{"line_number":115,"context_line":"        migrated \u003d (images"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_7e71a075","line":112,"in_reply_to":"3a461143_a7cdc686","updated":"2017-01-30 23:45:42.000000000","message":"Not sure if this suggestion makes it a lot better. But i like having num_rows \u003d 0 before the if condition and setting it within.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"dc495f7475de7eca23d683acaa0fefa19eb5a0e4","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # First, migrate public images up to max_rows"},{"line_number":182,"context_line":"    num_rows \u003d _mark_atmost_public_images_with_public_visibility(images,"},{"line_number":183,"context_line":"                                                                 max_rows)"},{"line_number":184,"context_line":"    total_rows +\u003d num_rows"},{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_6723dea7","line":183,"updated":"2017-01-30 19:36:17.000000000","message":"if at all there was nothing to migrate at this step and num_rows is returned as 0 as per L144, 146, no point in continuing right?\nCan we have a check for that?","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"af26e6e3b575e3268e2c5a4b3ba39067ab4bf1c7","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # First, migrate public images up to max_rows"},{"line_number":182,"context_line":"    num_rows \u003d _mark_atmost_public_images_with_public_visibility(images,"},{"line_number":183,"context_line":"                                                                 max_rows)"},{"line_number":184,"context_line":"    total_rows +\u003d num_rows"},{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_3d9abb6e","line":183,"in_reply_to":"3a461143_4f5d1878","updated":"2017-01-31 03:56:04.000000000","message":"+1","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"249bfb9451114a5e36fe76e87b9bf2f785ae8bc6","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # First, migrate public images up to max_rows"},{"line_number":182,"context_line":"    num_rows \u003d _mark_atmost_public_images_with_public_visibility(images,"},{"line_number":183,"context_line":"                                                                 max_rows)"},{"line_number":184,"context_line":"    total_rows +\u003d num_rows"},{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_c7d9f223","line":183,"in_reply_to":"3a461143_6723dea7","updated":"2017-01-30 19:50:58.000000000","message":"Yes, it is certainly possible that the number of rows migrated could be zero at some point for the first mini-migration. But, we can\u0027t terminate yet because there are two more mini-migrations left. They may have some work left to do. We are not done until all the mini-migrations have finished.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"1b1511f8a89920c206be48861d6a36fea2ab4ccf","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    # First, migrate public images up to max_rows"},{"line_number":182,"context_line":"    num_rows \u003d _mark_atmost_public_images_with_public_visibility(images,"},{"line_number":183,"context_line":"                                                                 max_rows)"},{"line_number":184,"context_line":"    total_rows +\u003d num_rows"},{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_4f5d1878","line":183,"in_reply_to":"3a461143_c7d9f223","updated":"2017-01-30 22:04:10.000000000","message":"It seems to me that we cannot /ever/ terminate early based on num_rows. I\u0027m going to just add this comment once but it seems to hold for by my reasoning to all the steps below.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"dc495f7475de7eca23d683acaa0fefa19eb5a0e4","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # Next, migrate private images if affected \u003c max_rows"},{"line_number":188,"context_line":"    if max_rows \u003e 0:"},{"line_number":189,"context_line":"        num_rows \u003d _mark_atmost_non_public_images_with_private_visibility("},{"line_number":190,"context_line":"            images, max_rows)"},{"line_number":191,"context_line":"        total_rows +\u003d num_rows"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_a7dce673","line":188,"range":{"start_line":188,"start_character":4,"end_line":188,"end_character":20},"updated":"2017-01-30 19:36:17.000000000","message":"If num_rows came back as 0, this condition will pass and again return zero.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"dfe6ca19da749933b968f90bbe9e95a1ab699193","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # Next, migrate private images if affected \u003c max_rows"},{"line_number":188,"context_line":"    if max_rows \u003e 0:"},{"line_number":189,"context_line":"        num_rows \u003d _mark_atmost_non_public_images_with_private_visibility("},{"line_number":190,"context_line":"            images, max_rows)"},{"line_number":191,"context_line":"        total_rows +\u003d num_rows"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_b91ecab8","line":188,"range":{"start_line":188,"start_character":4,"end_line":188,"end_character":20},"in_reply_to":"3a461143_07c93a26","updated":"2017-01-30 20:23:23.000000000","message":"Maybe I am just over thinking. Correct me if I am wrong.\nThe 2nd and 3rd are linked. For example, if there are no non-public images in our list of selected rows based on the limit of \u0027atmost_rows\u0027, the second mini migration (marking all as private would not happen at all. And if this does not happen, the third mini migration also will not.\nSo that made me think if the repeated checks of max_rows at least for the last 2 mini migrations were required to be done separately.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"afadad13ccdf1c4ced59968b54a3c426a4fad42e","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # Next, migrate private images if affected \u003c max_rows"},{"line_number":188,"context_line":"    if max_rows \u003e 0:"},{"line_number":189,"context_line":"        num_rows \u003d _mark_atmost_non_public_images_with_private_visibility("},{"line_number":190,"context_line":"            images, max_rows)"},{"line_number":191,"context_line":"        total_rows +\u003d num_rows"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_4d7fe065","line":188,"range":{"start_line":188,"start_character":4,"end_line":188,"end_character":20},"in_reply_to":"3a461143_2fe60cb0","updated":"2017-01-30 21:59:11.000000000","message":"Yes, that can definitely happen. Partly, that\u0027s the intent too maybe? Unless we finish our migrations, the data is not going to be completely consistent. Think of this: initially visibility is going to be none, then some public images will be marked with public visibility and others won\u0027t be marked as such. And, then there is the case that you mentioned. Some shared images will be marked private initially. Eventually they\u0027ll be marked shared. So, there are going to be inconsistent responses from the API during this period.\n\nThey maybe a way to fix this though. If we modify the second mini-migration to mark only the private ones as private, we can address what you hare highlighting. Currently, it even marks the shared ones as private, which are eventually marked shared by the 3rd mini-migration.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"e1346716617bce01d26aadc5ad4e2c5bf276b752","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # Next, migrate private images if affected \u003c max_rows"},{"line_number":188,"context_line":"    if max_rows \u003e 0:"},{"line_number":189,"context_line":"        num_rows \u003d _mark_atmost_non_public_images_with_private_visibility("},{"line_number":190,"context_line":"            images, max_rows)"},{"line_number":191,"context_line":"        total_rows +\u003d num_rows"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_adabe473","line":188,"range":{"start_line":188,"start_character":4,"end_line":188,"end_character":20},"in_reply_to":"3a461143_4d7fe065","updated":"2017-01-30 22:09:12.000000000","message":"We probably do need to rework the queries here to only mark private those which will become private. That query is a fair bit harder to write correctly, though.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"38ac233f6d80b6a7f18b9de3d9030a83855c12a7","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # Next, migrate private images if affected \u003c max_rows"},{"line_number":188,"context_line":"    if max_rows \u003e 0:"},{"line_number":189,"context_line":"        num_rows \u003d _mark_atmost_non_public_images_with_private_visibility("},{"line_number":190,"context_line":"            images, max_rows)"},{"line_number":191,"context_line":"        total_rows +\u003d num_rows"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_2fe60cb0","line":188,"range":{"start_line":188,"start_character":4,"end_line":188,"end_character":20},"in_reply_to":"3a461143_5ceb1478","updated":"2017-01-30 21:25:43.000000000","message":"One case that may support the idea of having the check together:\n\nSay we have max_rows as 5. And there was no need of first migration (no public images were present).\nSo we directly finish the second migration and deducting max_rows as in L192.\nThat would make it 0.\nSo our third mini migration wont happen cos of the check in L195. So we are stuck with all images marked as \u0027private\u0027 now.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"af26e6e3b575e3268e2c5a4b3ba39067ab4bf1c7","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # Next, migrate private images if affected \u003c max_rows"},{"line_number":188,"context_line":"    if max_rows \u003e 0:"},{"line_number":189,"context_line":"        num_rows \u003d _mark_atmost_non_public_images_with_private_visibility("},{"line_number":190,"context_line":"            images, max_rows)"},{"line_number":191,"context_line":"        total_rows +\u003d num_rows"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_3d71db44","line":188,"range":{"start_line":188,"start_character":4,"end_line":188,"end_character":20},"in_reply_to":"3a461143_8dd0e8f7","updated":"2017-01-31 03:56:04.000000000","message":"Will tackle this in a separate patch that addresses batching specifically.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"249bfb9451114a5e36fe76e87b9bf2f785ae8bc6","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # Next, migrate private images if affected \u003c max_rows"},{"line_number":188,"context_line":"    if max_rows \u003e 0:"},{"line_number":189,"context_line":"        num_rows \u003d _mark_atmost_non_public_images_with_private_visibility("},{"line_number":190,"context_line":"            images, max_rows)"},{"line_number":191,"context_line":"        total_rows +\u003d num_rows"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_07c93a26","line":188,"range":{"start_line":188,"start_character":4,"end_line":188,"end_character":20},"in_reply_to":"3a461143_a7dce673","updated":"2017-01-30 19:50:58.000000000","message":"True, and we want that behaviour because of what I mentioned above. We still want go ahead and check whether the remaining parts of the migration have rows left to migrate.\n\nLet me know if I\u0027m missing your point here.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"6b9d70b5a1d36a43eb656d83ee47c4561872735b","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # Next, migrate private images if affected \u003c max_rows"},{"line_number":188,"context_line":"    if max_rows \u003e 0:"},{"line_number":189,"context_line":"        num_rows \u003d _mark_atmost_non_public_images_with_private_visibility("},{"line_number":190,"context_line":"            images, max_rows)"},{"line_number":191,"context_line":"        total_rows +\u003d num_rows"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_8dd0e8f7","line":188,"range":{"start_line":188,"start_character":4,"end_line":188,"end_character":20},"in_reply_to":"3a461143_adabe473","updated":"2017-01-30 23:45:42.000000000","message":"Yes we can do that for our \u0027atmost\u0027 bunch of migrations.\nApart from that, I am unable to think of a better solution at the moment.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"d19bfb3b6784d3b49898dc0e7aa6c5860071919d","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    max_rows -\u003d num_rows"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    # Next, migrate private images if affected \u003c max_rows"},{"line_number":188,"context_line":"    if max_rows \u003e 0:"},{"line_number":189,"context_line":"        num_rows \u003d _mark_atmost_non_public_images_with_private_visibility("},{"line_number":190,"context_line":"            images, max_rows)"},{"line_number":191,"context_line":"        total_rows +\u003d num_rows"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_5ceb1478","line":188,"range":{"start_line":188,"start_character":4,"end_line":188,"end_character":20},"in_reply_to":"3a461143_b91ecab8","updated":"2017-01-30 20:53:49.000000000","message":"I think I see your point on how the 2nd and 3rd mini-migrations are related. However, when the 2nd one gets back with nothing, we don\u0027t know if it is because all rows have been migrated or there aren\u0027t any rows to migrate to begin with. If it\u0027s the latter, then we can safely ignore the 3rd one. However, if it\u0027s the former, then we still have to attempt the 3rd mini-migration. So, either way, I\u0027m thinking it\u0027s safe to attempt the 3rd one independently. \n\nMaybe we can link them if we really want to, but it is probably cleaner and simpler to treat them all independently. Well, unless doing so would lead to incorrect migrations. Thoughts?","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"dc495f7475de7eca23d683acaa0fefa19eb5a0e4","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        max_rows -\u003d num_rows"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    # Next, migrate shared images if affected \u003c max_rows"},{"line_number":195,"context_line":"    if max_rows \u003e 0:"},{"line_number":196,"context_line":"        image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"},{"line_number":197,"context_line":"        num_rows \u003d _mark_atmost_private_images_with_members_as_shared_vsblty("},{"line_number":198,"context_line":"            images, image_members, max_rows)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_a7c30688","line":195,"range":{"start_line":195,"start_character":4,"end_line":195,"end_character":20},"updated":"2017-01-30 19:36:17.000000000","message":"same as above.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"249bfb9451114a5e36fe76e87b9bf2f785ae8bc6","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        max_rows -\u003d num_rows"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    # Next, migrate shared images if affected \u003c max_rows"},{"line_number":195,"context_line":"    if max_rows \u003e 0:"},{"line_number":196,"context_line":"        image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"},{"line_number":197,"context_line":"        num_rows \u003d _mark_atmost_private_images_with_members_as_shared_vsblty("},{"line_number":198,"context_line":"            images, image_members, max_rows)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_67d4bebc","line":195,"range":{"start_line":195,"start_character":4,"end_line":195,"end_character":20},"in_reply_to":"3a461143_a7c30688","updated":"2017-01-30 19:50:58.000000000","message":"same as above","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"1b1511f8a89920c206be48861d6a36fea2ab4ccf","unresolved":false,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"def migrate(max_rows):"},{"line_number":205,"context_line":"    \"\"\"Update rows where visibility is NULL with values based on is_public.\"\"\""},{"line_number":206,"context_line":"    if max_rows \u003e 0:"},{"line_number":207,"context_line":"        return _migrate_atmost(max_rows)"},{"line_number":208,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_b2e5ff38","line":205,"updated":"2017-01-30 22:04:10.000000000","message":"this comment isn\u0027t correct","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"af26e6e3b575e3268e2c5a4b3ba39067ab4bf1c7","unresolved":false,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"def migrate(max_rows):"},{"line_number":205,"context_line":"    \"\"\"Update rows where visibility is NULL with values based on is_public.\"\"\""},{"line_number":206,"context_line":"    if max_rows \u003e 0:"},{"line_number":207,"context_line":"        return _migrate_atmost(max_rows)"},{"line_number":208,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_9d980f6c","line":205,"in_reply_to":"3a461143_b2e5ff38","updated":"2017-01-31 03:56:04.000000000","message":"Done","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"dc495f7475de7eca23d683acaa0fefa19eb5a0e4","unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"def migrate(max_rows):"},{"line_number":205,"context_line":"    \"\"\"Update rows where visibility is NULL with values based on is_public.\"\"\""},{"line_number":206,"context_line":"    if max_rows \u003e 0:"},{"line_number":207,"context_line":"        return _migrate_atmost(max_rows)"},{"line_number":208,"context_line":"    else:"},{"line_number":209,"context_line":"        return _migrate_all()"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_6cebc578","line":206,"range":{"start_line":206,"start_character":7,"end_line":206,"end_character":20},"updated":"2017-01-30 19:36:17.000000000","message":"if max_rows is not None: ?","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"af26e6e3b575e3268e2c5a4b3ba39067ab4bf1c7","unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"def migrate(max_rows):"},{"line_number":205,"context_line":"    \"\"\"Update rows where visibility is NULL with values based on is_public.\"\"\""},{"line_number":206,"context_line":"    if max_rows \u003e 0:"},{"line_number":207,"context_line":"        return _migrate_atmost(max_rows)"},{"line_number":208,"context_line":"    else:"},{"line_number":209,"context_line":"        return _migrate_all()"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_bd9b0b65","line":206,"range":{"start_line":206,"start_character":7,"end_line":206,"end_character":20},"in_reply_to":"3a461143_67b75ef5","updated":"2017-01-31 03:56:04.000000000","message":"Done","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"249bfb9451114a5e36fe76e87b9bf2f785ae8bc6","unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"def migrate(max_rows):"},{"line_number":205,"context_line":"    \"\"\"Update rows where visibility is NULL with values based on is_public.\"\"\""},{"line_number":206,"context_line":"    if max_rows \u003e 0:"},{"line_number":207,"context_line":"        return _migrate_atmost(max_rows)"},{"line_number":208,"context_line":"    else:"},{"line_number":209,"context_line":"        return _migrate_all()"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_67b75ef5","line":206,"range":{"start_line":206,"start_character":7,"end_line":206,"end_character":20},"in_reply_to":"3a461143_6cebc578","updated":"2017-01-30 19:50:58.000000000","message":"Yes, I have to make appropriate changes here after making None as default for max_rows.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"b08fc9f41c223b12bc340cfa21e126d953af3978","unresolved":false,"context_lines":[{"line_number":25,"context_line":"     #2 There exists a private image with active members but its visibility"},{"line_number":26,"context_line":"        isn\u0027t set to \u0027shared\u0027 yet."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    Note: This method can return a false positive if data migrations"},{"line_number":29,"context_line":"    are running in the background as it\u0027s being called."},{"line_number":30,"context_line":"    \"\"\""},{"line_number":31,"context_line":"    meta \u003d MetaData(engine)"},{"line_number":32,"context_line":"    images \u003d Table(\u0027images\u0027, meta, autoload\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_682a6e2e","line":29,"range":{"start_line":28,"start_character":4,"end_line":29,"end_character":55},"updated":"2017-02-01 23:07:45.000000000","message":"Just a note that this might be something we need to possibly figure out as to how to handle in a follow up.","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"71a29b2feb91d01538b9c6c3cf83698dfe7b9dac","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                                 .limit(1)"},{"line_number":37,"context_line":"                                 .execute())"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    if rows_with_null_visibility.rowcount \u003e 0:"},{"line_number":40,"context_line":"        return True"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_2925da09","line":39,"range":{"start_line":39,"start_character":42,"end_line":39,"end_character":45},"updated":"2017-02-01 16:57:35.000000000","message":"\u003d\u003d 1?\n(Just to be more clear with our check as we are limiting to 1 anyway above)","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"73d0cc382ab1a27d6af188f7eec904fd22c9111e","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                                 .limit(1)"},{"line_number":37,"context_line":"                                 .execute())"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    if rows_with_null_visibility.rowcount \u003e 0:"},{"line_number":40,"context_line":"        return True"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_1b085291","line":39,"range":{"start_line":39,"start_character":42,"end_line":39,"end_character":45},"in_reply_to":"3a461143_1b93f250","updated":"2017-02-01 23:40:15.000000000","message":"Suggested this as an improvement for clarity. I am okay with \u003e 0 if you are not pushing another PS.","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"30678e05ced1c89a10a2752d606463f02a7e3799","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                                 .limit(1)"},{"line_number":37,"context_line":"                                 .execute())"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    if rows_with_null_visibility.rowcount \u003e 0:"},{"line_number":40,"context_line":"        return True"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_65453716","line":39,"range":{"start_line":39,"start_character":42,"end_line":39,"end_character":45},"in_reply_to":"3a461143_2925da09","updated":"2017-02-01 22:51:05.000000000","message":"I would rather:\n\n    if rows_with_null_visibility.rowcount:\n        return True","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"2e1a595c13fe71eb7ac567c1aba0c0c3d5f68a49","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                                 .limit(1)"},{"line_number":37,"context_line":"                                 .execute())"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    if rows_with_null_visibility.rowcount \u003e 0:"},{"line_number":40,"context_line":"        return True"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_1b93f250","line":39,"range":{"start_line":39,"start_character":42,"end_line":39,"end_character":45},"in_reply_to":"3a461143_65453716","updated":"2017-02-01 23:36:14.000000000","message":"Guessing this is not a major concern. Will address this if I\u0027m pushing another patchset. Let me know if you guys feel otherwise.","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"71a29b2feb91d01538b9c6c3cf83698dfe7b9dac","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                                       )"},{"line_number":51,"context_line":"                                .limit(1)"},{"line_number":52,"context_line":"                                .execute())"},{"line_number":53,"context_line":"    if rows_with_pending_shared.rowcount \u003e 0:"},{"line_number":54,"context_line":"        return True"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    return False"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_69e442ac","line":53,"range":{"start_line":53,"start_character":41,"end_line":53,"end_character":44},"updated":"2017-02-01 16:57:35.000000000","message":"same.","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"30678e05ced1c89a10a2752d606463f02a7e3799","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                                       )"},{"line_number":51,"context_line":"                                .limit(1)"},{"line_number":52,"context_line":"                                .execute())"},{"line_number":53,"context_line":"    if rows_with_pending_shared.rowcount \u003e 0:"},{"line_number":54,"context_line":"        return True"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    return False"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_e568478a","line":53,"range":{"start_line":53,"start_character":41,"end_line":53,"end_character":44},"in_reply_to":"3a461143_69e442ac","updated":"2017-02-01 22:51:05.000000000","message":"ditto","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"30678e05ced1c89a10a2752d606463f02a7e3799","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    images \u003d Table(\u0027images\u0027, meta, autoload\u003dTrue)"},{"line_number":91,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    num_rows \u003d _mark_all_public_images_with_public_visibility(images)"},{"line_number":94,"context_line":"    num_rows +\u003d _mark_all_non_public_images_with_private_visibility(images)"},{"line_number":95,"context_line":"    num_rows +\u003d _mark_all_private_images_with_members_as_shared_visibility("},{"line_number":96,"context_line":"        images, image_members)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    return num_rows"},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_05d2b334","line":96,"range":{"start_line":93,"start_character":4,"end_line":96,"end_character":30},"updated":"2017-02-01 22:51:05.000000000","message":"why 3 assignments here?","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"2e1a595c13fe71eb7ac567c1aba0c0c3d5f68a49","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    images \u003d Table(\u0027images\u0027, meta, autoload\u003dTrue)"},{"line_number":91,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    num_rows \u003d _mark_all_public_images_with_public_visibility(images)"},{"line_number":94,"context_line":"    num_rows +\u003d _mark_all_non_public_images_with_private_visibility(images)"},{"line_number":95,"context_line":"    num_rows +\u003d _mark_all_private_images_with_members_as_shared_visibility("},{"line_number":96,"context_line":"        images, image_members)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    return num_rows"},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_1ba5b2b4","line":96,"range":{"start_line":93,"start_character":4,"end_line":96,"end_character":30},"in_reply_to":"3a461143_05d2b334","updated":"2017-02-01 23:36:14.000000000","message":"Just adding the rows returned by each individual update.","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"73d0cc382ab1a27d6af188f7eec904fd22c9111e","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    images \u003d Table(\u0027images\u0027, meta, autoload\u003dTrue)"},{"line_number":91,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    num_rows \u003d _mark_all_public_images_with_public_visibility(images)"},{"line_number":94,"context_line":"    num_rows +\u003d _mark_all_non_public_images_with_private_visibility(images)"},{"line_number":95,"context_line":"    num_rows +\u003d _mark_all_private_images_with_members_as_shared_visibility("},{"line_number":96,"context_line":"        images, image_members)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    return num_rows"},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_5bebbab0","line":96,"range":{"start_line":93,"start_character":4,"end_line":96,"end_character":30},"in_reply_to":"3a461143_1ba5b2b4","updated":"2017-02-01 23:40:15.000000000","message":"I guess what Nikhil meant is we can do it as a single assignment.\n\nnum_rows \u003d _mark_all_public_images_with_public_visibility(images) + _mark_all_non_public_images_with_private_visibility(images) + _mark_all_private_images_with_members_as_shared_visibility(\n        images, image_members)\n\nSince each of these return the rowcount, it should be good.","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"26a7b58ebeb5070047fda661719e7945d7a620db","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    images \u003d Table(\u0027images\u0027, meta, autoload\u003dTrue)"},{"line_number":91,"context_line":"    image_members \u003d Table(\u0027image_members\u0027, meta, autoload\u003dTrue)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    num_rows \u003d _mark_all_public_images_with_public_visibility(images)"},{"line_number":94,"context_line":"    num_rows +\u003d _mark_all_non_public_images_with_private_visibility(images)"},{"line_number":95,"context_line":"    num_rows +\u003d _mark_all_private_images_with_members_as_shared_visibility("},{"line_number":96,"context_line":"        images, image_members)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    return num_rows"},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_7b5dd647","line":96,"range":{"start_line":93,"start_character":4,"end_line":96,"end_character":30},"in_reply_to":"3a461143_5bebbab0","updated":"2017-02-01 23:54:19.000000000","message":"Ah, ok.\nGuess we could do that too.\nI don\u0027t see a major improvement in terms of readability or performance either way.","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"}],"glance/db/sqlalchemy/alembic_migrations/versions/ocata_contract01_drop_is_public.py":[{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"1b1511f8a89920c206be48861d6a36fea2ab4ccf","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    _drop_column()"},{"line_number":66,"context_line":"    _drop_triggers(migrate_engine)"},{"line_number":67,"context_line":"    _set_nullability_and_default_on_visibility(meta)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_2df1b401","line":67,"updated":"2017-01-30 22:04:10.000000000","message":"regarding order of operations here, don\u0027t we want to set nullability and default before dropping triggers?","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"af26e6e3b575e3268e2c5a4b3ba39067ab4bf1c7","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    _drop_column()"},{"line_number":66,"context_line":"    _drop_triggers(migrate_engine)"},{"line_number":67,"context_line":"    _set_nullability_and_default_on_visibility(meta)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a461143_bd0eab89","line":67,"in_reply_to":"3a461143_2df1b401","updated":"2017-01-31 03:56:04.000000000","message":"If I remember correctly, constraints are applied before firing the trigger. So, if we change the constraints while the triggers are still active, triggers will fail. That\u0027s my reasoning for the order.","commit_id":"55b7c6c7f90555443875fa478a710e20c971e5db"}],"glance/tests/functional/db/migrations/test_ocata_contract01.py":[{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"71a29b2feb91d01538b9c6c3cf83698dfe7b9dac","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        images.insert().values(shared_temp).execute()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _check_ocata_contract01(self, engine, data):"},{"line_number":56,"context_line":"        # check that after migration, \u0027visibility\u0027 column is introduced"},{"line_number":57,"context_line":"        images \u003d db_utils.get_table(engine, \u0027images\u0027)"},{"line_number":58,"context_line":"        self.assertNotIn(\u0027is_public\u0027, images.c)"},{"line_number":59,"context_line":"        self.assertIn(\u0027visibility\u0027, images.c)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_8ce17c9d","line":56,"range":{"start_line":56,"start_character":10,"end_line":56,"end_character":71},"updated":"2017-02-01 16:57:35.000000000","message":"\"check that after contract, \u0027is_public\u0027 column is dropped\" ?","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"a923243f9a2385ebbf58e65c7d593562aa3bba25","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        images.insert().values(shared_temp).execute()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _check_ocata_contract01(self, engine, data):"},{"line_number":56,"context_line":"        # check that after migration, \u0027visibility\u0027 column is introduced"},{"line_number":57,"context_line":"        images \u003d db_utils.get_table(engine, \u0027images\u0027)"},{"line_number":58,"context_line":"        self.assertNotIn(\u0027is_public\u0027, images.c)"},{"line_number":59,"context_line":"        self.assertIn(\u0027visibility\u0027, images.c)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a461143_bbac9e8d","line":56,"range":{"start_line":56,"start_character":10,"end_line":56,"end_character":71},"in_reply_to":"3a461143_8ce17c9d","updated":"2017-02-01 23:37:09.000000000","message":"ah, good one.\nGuess I\u0027ll push a new PS anyway.","commit_id":"b7b658e98e75c25eb0fe06f5a633612aa01c0fec"}]}