)]}'
{"glance/db/sqlalchemy/migrate_repo/versions/031_remove_duplicated_locations.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"76610a433d2b2fdfdd7cb8285a4312160644b088","unresolved":false,"context_lines":[{"line_number":16,"context_line":"#    under the License."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import sqlalchemy"},{"line_number":19,"context_line":"from itertools import groupby"},{"line_number":20,"context_line":"from operator import itemgetter"},{"line_number":21,"context_line":"from sqlalchemy import func"},{"line_number":22,"context_line":"from sqlalchemy import orm"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BCfw%3D","line":19,"updated":"2013-12-03 06:41:37.000000000","message":"pls put them by correct order","commit_id":"b669cc91bf67517ca9358f3b97cdb449e13d7bb2"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"39a2e807ad204fa43e3dca8de141dc8880c4570d","unresolved":false,"context_lines":[{"line_number":16,"context_line":"#    under the License."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import sqlalchemy"},{"line_number":19,"context_line":"from itertools import groupby"},{"line_number":20,"context_line":"from operator import itemgetter"},{"line_number":21,"context_line":"from sqlalchemy import func"},{"line_number":22,"context_line":"from sqlalchemy import orm"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BCdo%3D","line":19,"in_reply_to":"AAAATn%2F%2BCfw%3D","updated":"2013-12-03 06:48:05.000000000","message":"Done","commit_id":"b669cc91bf67517ca9358f3b97cdb449e13d7bb2"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"76610a433d2b2fdfdd7cb8285a4312160644b088","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import sqlalchemy"},{"line_number":19,"context_line":"from itertools import groupby"},{"line_number":20,"context_line":"from operator import itemgetter"},{"line_number":21,"context_line":"from sqlalchemy import func"},{"line_number":22,"context_line":"from sqlalchemy import orm"},{"line_number":23,"context_line":"from sqlalchemy import Table"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BCgc%3D","line":20,"updated":"2013-12-03 06:41:37.000000000","message":"unused var","commit_id":"b669cc91bf67517ca9358f3b97cdb449e13d7bb2"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"39a2e807ad204fa43e3dca8de141dc8880c4570d","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import sqlalchemy"},{"line_number":19,"context_line":"from itertools import groupby"},{"line_number":20,"context_line":"from operator import itemgetter"},{"line_number":21,"context_line":"from sqlalchemy import func"},{"line_number":22,"context_line":"from sqlalchemy import orm"},{"line_number":23,"context_line":"from sqlalchemy import Table"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BCdk%3D","line":20,"in_reply_to":"AAAATn%2F%2BCgc%3D","updated":"2013-12-03 06:48:05.000000000","message":"Done","commit_id":"b669cc91bf67517ca9358f3b97cdb449e13d7bb2"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"b27bb99f7c492f77aecdc96a92a8e6aa258e7568","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    # NOTE(flaper87): Get all duplicated rows"},{"line_number":39,"context_line":"    qry \u003d (session.query(*grp)"},{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BCac%3D","line":41,"updated":"2013-12-03 07:00:18.000000000","message":"How about remove this blank line or update the comment line 38 so as to indicate that we\u0027re getting the duplicated rows by this block instead of only line 39, the sqlalchemy query? You know, the old comments is added for old implement.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"27bee09c7ce37afdc2a102fa6413e56e7dae70c6","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    # NOTE(flaper87): Get all duplicated rows"},{"line_number":39,"context_line":"    qry \u003d (session.query(*grp)"},{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BCZU%3D","line":41,"in_reply_to":"AAAATn%2F%2BCac%3D","updated":"2013-12-03 09:29:39.000000000","message":"Yes,need to update the old comment. \n\nDone. thanks.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"4251d8f72f8cba6a13fb303bb2aab442e8ccbc16","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    for row in qry:"},{"line_number":46,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BCZw%3D","line":43,"updated":"2013-12-03 07:03:00.000000000","message":"This approach is simple and straightforward but I still have the similar concern which I mentioned in @flaper87\u0027s original patch [1]. I don\u0027t think this approach can work well for a large online Cloud deployment in the real world. If glance database contains a lot of location records even though there are only a little duplicated locations (URI+metadata), this approach still load all of location records to glance-api node into glance-api pid, the latter will consume much memory, this is different than original code. Are you sure migration python pid can work as expected? especially 32bit arch cpu/system since each pid only has 1.8G available memory space in userspace. I\u0027m thinking is there no better way to do it in DB2?\n\n[1] https://review.openstack.org/#/c/56663/4/glance/db/sqlalchemy/migrate_repo/versions/031_remove_duplicated_locations.py\n\nthanks.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"d2b1a70103e54f185d9c5246e046e7e50dac11ff","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    for row in qry:"},{"line_number":46,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BBhA%3D","line":43,"in_reply_to":"AAAATn%2F%2BCQ4%3D","updated":"2013-12-03 10:51:32.000000000","message":"I am doing the work on implementing a new query to workaround the GROUPBY.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"7118b5a7c0d6affae4bc1b0713ac186fca0757ff","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    for row in qry:"},{"line_number":46,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F97g8%3D","line":43,"in_reply_to":"AAAATn%2F%2BCRc%3D","updated":"2013-12-04 11:00:43.000000000","message":"As flwang required let me explain my two-layer-nest-loop approach: outer loop primarily query location\u0027s image_id, value (uri) and metadata (maybe others, it based on your implementation), using inner loop to query duplicated location entries by those values. In outer loop we can use follow way to avoid loading all location records. Since location\u0027s id is an integer/number column [1], so we are sure all dbms implementation can support order-by op on it.\n\na. session.query(models.ImageLocation).order_by(sqlalchemy.asc(models.ImageLocation.id)).filter_by(deleted\u003dFalse).filter_by(id \u003e ID_VAR).limit(1)\n\nb. ID_VAR initial value is -1, and update it to id column value from query result on each loop.\n\n[1] https://github.com/openstack/glance/blob/master/glance/db/sqlalchemy/models.py#L179\n\nAnd I\u0027m not sure whether sqlalchemy can support such lazy loading optimization on result rows, but under my investigation seems it only support lazy loading on relationship tables. IMHO if it can support rows lazy loading then it\u0027s an alternative also.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"01f805e1e4df62dae46a36bb16a8a1425c83ab5d","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    for row in qry:"},{"line_number":46,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BCRc%3D","line":43,"in_reply_to":"AAAATn%2F%2BCTY%3D","updated":"2013-12-03 11:00:46.000000000","message":"So, maybe there is a solution to balance applicability and performance:\n\n1. Using original for mysql\n2. Using two-layer-nest-loop approach for others (db2+oracle, as you said), traverse all location record in outer loop, and check location is duplicated one by one in inner loop, this approach can prevent load all records to local from database.\n\nThis is my initial idea around this issue, any thoughts/better approach? thanks.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":6981,"name":"Yang Yu","email":"yangyu@vmware.com","username":"yuyangbj"},"change_message_id":"6677c2d32855edaef956366a9031863f8dd9082f","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    for row in qry:"},{"line_number":46,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BCTY%3D","line":43,"in_reply_to":"AAAATn%2F%2BCZw%3D","updated":"2013-12-03 07:32:32.000000000","message":"@Zhi Yan, yes, your concern is make sense definitely, but frankly speaking, it is also weired to use DB \u0027GROUP BY\u0027 on table column with TEXT/CLOB data type. As you known, for these types, they could support 32K/64K data, so it does not make sense to use GROUP BY on these such big data. And BTW, it is not a DB2 special case, ORACLE has also same problems, any business DB has same considering on these problem, not sure why Mysql could do it on such big data type.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"2916f2d6b156032b5b0a1ca785c04553471c5a55","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    for row in qry:"},{"line_number":46,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BCQ4%3D","line":43,"in_reply_to":"AAAATn%2F%2BCZw%3D","updated":"2013-12-03 07:44:11.000000000","message":"Most of the enterprise level DB doesn\u0027t support GROUP BY TEXT, BLOB and so soon given the performance issue, such as DB2, Oracle. I just had a quick discussion with zhiyan, maybe there are two way we can try:\n1. Implement a new query to wrokaround the GROUP BY\n2. Maybe just follow the initial way flavio87 did in PS4.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"d26711c32deab62317a406b14830e3926e3cec4c","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    for row in qry:"},{"line_number":46,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F964E%3D","line":43,"in_reply_to":"AAAATn%2F97g8%3D","updated":"2013-12-04 13:44:02.000000000","message":"I wouldn\u0027t make this more complicated than what it is already. IMHO, we should have a separate function handling migrations for DB2 and handle this special case there. The function could create a generator and iterate over it instead of loading everything in memory or it could execute a query with limit 1 for each iteration until there are no more records to upgrade.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"a51fdc119240fb63296cc0e537c8f7fa983f318f","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                  .filter(image_locations.c.deleted \u003d\u003d False))"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    key_func \u003d lambda q: (q[0], q[1], q[2])"},{"line_number":43,"context_line":"    qry \u003d [ key for key, items in groupby(qry, key_func) if len(list(items))\u003e1 ]"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    for row in qry:"},{"line_number":46,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F92hc%3D","line":43,"in_reply_to":"AAAATn%2F97g8%3D","updated":"2013-12-05 08:40:20.000000000","message":"zhiyan, your approach inspired me ,  I provide an alternative method to get all duplicated rows and then delete these rows.\n\nplease review, thanks.","commit_id":"948c33356260b213dcb1b94fd7b98a2eb2a2d572"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"b55a58f613b2bd4fa6acade07027900de314e59a","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                   .where(image_locations.c.image_id \u003d\u003d il.c.image_id)"},{"line_number":41,"context_line":"                   .where(image_locations.c.value \u003d\u003d il.c.value)"},{"line_number":42,"context_line":"                   .where(image_locations.c.meta_data \u003d\u003d il.c.meta_data)) \u003e 1)"},{"line_number":43,"context_line":"           .execute()"},{"line_number":44,"context_line":"           )"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    for row in qry:"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F92Jo%3D","line":43,"updated":"2013-12-05 10:10:23.000000000","message":"I have one input for your approach, and two open questions around this issue resolving.\n\n1. For this query, we need to add a key filter condition: \"image_locations.c.deleted \u003d\u003d False\" and \"il.c.deleted \u003d\u003d False\".\n\n2. As flaper87 suggested in PS#2, not sure do you think we should have a separate function handling migrations for DB2/Oracle and handle this special case there?\n\n3. You know this approach has an assumption: duplicated location entry amount is not too large. I\u0027m ok for your new approach firstly since duplication should be less, but, share my thoughts with you, I\u0027m thinking it will be better if we can make our algorithm itself be able to prevent that assumption, like check location entries one by one via db.\n\nThanks.","commit_id":"8e1353c0723c5b65980e9cb65b798a3e5569663a"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"6500a1ad695108222b54e67a9174dedb93ea8b59","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                  .where(image_locations.c.image_id \u003d\u003d il.c.image_id)"},{"line_number":37,"context_line":"                  .where(image_locations.c.value \u003d\u003d il.c.value)"},{"line_number":38,"context_line":"                  .where(image_locations.c.meta_data \u003d\u003d il.c.meta_data)"},{"line_number":39,"context_line":"                  .where(image_locations.c.deleted \u003d\u003d False)))"},{"line_number":40,"context_line":"           .where((sql.select([func.count(image_locations.c.id)])"},{"line_number":41,"context_line":"                   .where(image_locations.c.image_id \u003d\u003d il.c.image_id)"},{"line_number":42,"context_line":"                   .where(image_locations.c.value \u003d\u003d il.c.value)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F9xXs%3D","line":39,"updated":"2013-12-06 05:04:18.000000000","message":"1. We need a \"from\" section for sql.\n\n2. We need put \"il.c.deleted \u003d\u003d False\" to where section also since other \"duplicated\" records can be \"deleted\" already, I posted that in my former comments.","commit_id":"52a9f33da740705fdaaabdc86416b1d46b0544d4"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"deefa217623d3ceb3147111738e57e717a4ed239","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                  .where(image_locations.c.image_id \u003d\u003d il.c.image_id)"},{"line_number":37,"context_line":"                  .where(image_locations.c.value \u003d\u003d il.c.value)"},{"line_number":38,"context_line":"                  .where(image_locations.c.meta_data \u003d\u003d il.c.meta_data)"},{"line_number":39,"context_line":"                  .where(image_locations.c.deleted \u003d\u003d False)))"},{"line_number":40,"context_line":"           .where((sql.select([func.count(image_locations.c.id)])"},{"line_number":41,"context_line":"                   .where(image_locations.c.image_id \u003d\u003d il.c.image_id)"},{"line_number":42,"context_line":"                   .where(image_locations.c.value \u003d\u003d il.c.value)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F9qjU%3D","line":39,"in_reply_to":"AAAATn%2F9qnE%3D","updated":"2013-12-09 03:45:42.000000000","message":"I\u0027m saying \"select from\". thanks.\n\nhttp://logs.openstack.org/30/59630/7/check/gate-glance-python27/19d6937/console.html.gz#_2013-12-06_03_31_55_516","commit_id":"52a9f33da740705fdaaabdc86416b1d46b0544d4"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"a4dd33ae29bbb1af19873309fd64b46ceb99190d","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                  .where(image_locations.c.image_id \u003d\u003d il.c.image_id)"},{"line_number":37,"context_line":"                  .where(image_locations.c.value \u003d\u003d il.c.value)"},{"line_number":38,"context_line":"                  .where(image_locations.c.meta_data \u003d\u003d il.c.meta_data)"},{"line_number":39,"context_line":"                  .where(image_locations.c.deleted \u003d\u003d False)))"},{"line_number":40,"context_line":"           .where((sql.select([func.count(image_locations.c.id)])"},{"line_number":41,"context_line":"                   .where(image_locations.c.image_id \u003d\u003d il.c.image_id)"},{"line_number":42,"context_line":"                   .where(image_locations.c.value \u003d\u003d il.c.value)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F9qnE%3D","line":39,"in_reply_to":"AAAATn%2F9xXs%3D","updated":"2013-12-09 02:56:51.000000000","message":"il.c.deleted \u003d\u003d False has added, about \"from\" section , do you mean \"select from\" or \"from sqlalchemy.sql import sql\" ?\n\nfirstly submitted , if any problem, I will modify it.\n\nThanks. zhiyan.","commit_id":"52a9f33da740705fdaaabdc86416b1d46b0544d4"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"4e5bd5aaf29aaaf63eb6ba203a4ee0cd57af320a","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        stmt \u003d (image_locations.delete()"},{"line_number":51,"context_line":"                .where(image_locations.c.id \u003d\u003d row[0])"},{"line_number":52,"context_line":"                .where(image_locations.c.deleted \u003d\u003d False))"},{"line_number":53,"context_line":"        stmt.execute()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    session.close()"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F9qD8%3D","line":53,"updated":"2013-12-09 07:35:23.000000000","message":"Exception \"OperationalError: (OperationalError) database is locked\" happened here [1], so seems the reason for it is that above select query locked records, we need to force the select query to finish and store all of the image location id result data then perform this deletion loop, like: adding \"dup_loc_ids \u003d [row[0] for row in qry]\" at line #48. Can we have a try?\n\n[1] http://logs.openstack.org/30/59630/8/check/gate-glance-python27/0969bbc/console.html#_2013-12-09_06_39_31_731","commit_id":"0b7105d2f9b85bc4b3d9fea3ed7954a90cd037f8"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"048dbb3e1e627b3ecdddb21fe18a800c61271eea","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        stmt \u003d (image_locations.delete()"},{"line_number":51,"context_line":"                .where(image_locations.c.id \u003d\u003d row[0])"},{"line_number":52,"context_line":"                .where(image_locations.c.deleted \u003d\u003d False))"},{"line_number":53,"context_line":"        stmt.execute()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    session.close()"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F9k7c%3D","line":53,"in_reply_to":"AAAATn%2F9qD8%3D","updated":"2013-12-10 02:03:27.000000000","message":"jenkins failed due to \"OperationalError: (OperationalError) database is locked None None\".\n\nThis is a  limitation of open connection/transactions to sqlite database, when transaction is open the db file is locked for writing. \n\nI work on it","commit_id":"0b7105d2f9b85bc4b3d9fea3ed7954a90cd037f8"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"fa8aa3ab1668453fe6dea7d7079430191229ff59","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        stmt \u003d (image_locations.delete()"},{"line_number":51,"context_line":"                .where(image_locations.c.id \u003d\u003d row[0])"},{"line_number":52,"context_line":"                .where(image_locations.c.deleted \u003d\u003d False))"},{"line_number":53,"context_line":"        stmt.execute()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    session.close()"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F9pjQ%3D","line":53,"in_reply_to":"AAAATn%2F9qD8%3D","updated":"2013-12-09 10:01:31.000000000","message":"ok, I will try it , thanks.","commit_id":"0b7105d2f9b85bc4b3d9fea3ed7954a90cd037f8"},{"author":{"_account_id":7703,"name":"Venkatesh Sampath","email":"venkatesh.sampath@outlook.com","username":"venkateshsampath"},"change_message_id":"9c9d5a84f5f0e1b0c5b10937b29f7b00d43110e2","unresolved":false,"context_lines":[{"line_number":47,"context_line":"           )"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    for row in qry:"},{"line_number":50,"context_line":"        stmt \u003d (image_locations.delete()"},{"line_number":51,"context_line":"                .where(image_locations.c.id \u003d\u003d row[0])"},{"line_number":52,"context_line":"                .where(image_locations.c.deleted \u003d\u003d False))"},{"line_number":53,"context_line":"        stmt.execute()"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F9hX0%3D","line":50,"updated":"2013-12-10 16:35:32.000000000","message":"just a query, can this be a single delete call with where condition?","commit_id":"b2f4dc68c26cb1abf4f856efc8feb2c506f4b40b"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"263b0d168cd13c9260ff53d4896a5f5c594ffd90","unresolved":false,"context_lines":[{"line_number":47,"context_line":"           )"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    for row in qry:"},{"line_number":50,"context_line":"        stmt \u003d (image_locations.delete()"},{"line_number":51,"context_line":"                .where(image_locations.c.id \u003d\u003d row[0])"},{"line_number":52,"context_line":"                .where(image_locations.c.deleted \u003d\u003d False))"},{"line_number":53,"context_line":"        stmt.execute()"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F9e58%3D","line":50,"in_reply_to":"AAAATn%2F9hX0%3D","updated":"2013-12-11 01:54:39.000000000","message":"only a query will make the code more complex, not easy to understand and maintain.","commit_id":"b2f4dc68c26cb1abf4f856efc8feb2c506f4b40b"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"2e1b882daccb80b971727500916681b2030a6688","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from sqlalchemy import func"},{"line_number":20,"context_line":"from sqlalchemy import orm"},{"line_number":21,"context_line":"from sqlalchemy import Table"},{"line_number":22,"context_line":"from sqlalchemy import sql"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def upgrade(migrate_engine):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAT3%2F%2F2pY%3D","line":22,"updated":"2013-12-30 05:36:14.000000000","message":"alphabetical order?","commit_id":"f66e8e7b5bec33bb421cbf6a343fac951630fcc2"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"ddf311181bf9e740fdcbc57faefbe11025be7bf2","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from sqlalchemy import func"},{"line_number":20,"context_line":"from sqlalchemy import orm"},{"line_number":21,"context_line":"from sqlalchemy import Table"},{"line_number":22,"context_line":"from sqlalchemy import sql"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"def upgrade(migrate_engine):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAT3%2F%2F1wg%3D","line":22,"in_reply_to":"AAAAT3%2F%2F2pY%3D","updated":"2013-12-30 12:23:37.000000000","message":"Done","commit_id":"f66e8e7b5bec33bb421cbf6a343fac951630fcc2"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"2e1b882daccb80b971727500916681b2030a6688","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    grp \u003d [image_locations.c.image_id,"},{"line_number":33,"context_line":"           image_locations.c.value,"},{"line_number":34,"context_line":"           image_locations.c.meta_data]"},{"line_number":35,"context_line":"    il \u003d orm.aliased(image_locations)"},{"line_number":36,"context_line":"    qry \u003d None"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    if str(migrate_engine).__contains__(\u0027ibm_db_sa\u0027):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAT3%2F%2F2pg%3D","line":35,"updated":"2013-12-30 05:36:14.000000000","message":"Seems this variable is only used by db2 branch, so how about move it under that?","commit_id":"f66e8e7b5bec33bb421cbf6a343fac951630fcc2"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"ddf311181bf9e740fdcbc57faefbe11025be7bf2","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    grp \u003d [image_locations.c.image_id,"},{"line_number":33,"context_line":"           image_locations.c.value,"},{"line_number":34,"context_line":"           image_locations.c.meta_data]"},{"line_number":35,"context_line":"    il \u003d orm.aliased(image_locations)"},{"line_number":36,"context_line":"    qry \u003d None"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    if str(migrate_engine).__contains__(\u0027ibm_db_sa\u0027):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAT3%2F%2F1wY%3D","line":35,"in_reply_to":"AAAAT3%2F%2F2pg%3D","updated":"2013-12-30 12:23:37.000000000","message":"Done","commit_id":"f66e8e7b5bec33bb421cbf6a343fac951630fcc2"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"2e1b882daccb80b971727500916681b2030a6688","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    il \u003d orm.aliased(image_locations)"},{"line_number":36,"context_line":"    qry \u003d None"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    if str(migrate_engine).__contains__(\u0027ibm_db_sa\u0027):"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        # NOTE(wenchma): Get all duplicated rows."},{"line_number":41,"context_line":"        qry \u003d (sql.select(*grp)"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAT3%2F%2F2p0%3D","line":38,"updated":"2013-12-30 05:36:14.000000000","message":"I prefer to use: if migrate_engine.name \u003d\u003d \"ibm_db_sa\":","commit_id":"f66e8e7b5bec33bb421cbf6a343fac951630fcc2"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"ddf311181bf9e740fdcbc57faefbe11025be7bf2","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    il \u003d orm.aliased(image_locations)"},{"line_number":36,"context_line":"    qry \u003d None"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    if str(migrate_engine).__contains__(\u0027ibm_db_sa\u0027):"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        # NOTE(wenchma): Get all duplicated rows."},{"line_number":41,"context_line":"        qry \u003d (sql.select(*grp)"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAT3%2F%2F1wU%3D","line":38,"in_reply_to":"AAAAT3%2F%2F2p0%3D","updated":"2013-12-30 12:23:37.000000000","message":"Done","commit_id":"f66e8e7b5bec33bb421cbf6a343fac951630fcc2"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"cae56a9548ca90e5fa5d7729445d5cd53cfad12f","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."},{"line_number":64,"context_line":"        # This is the best way to do it since sqlalchemy"},{"line_number":65,"context_line":"        # has a bug around delete + limit."},{"line_number":66,"context_line":"        s \u003d (sql.select([image_locations.c.id])"},{"line_number":67,"context_line":"             .where(image_locations.c.image_id \u003d\u003d row[0])"},{"line_number":68,"context_line":"             .where(image_locations.c.value \u003d\u003d row[1])"},{"line_number":69,"context_line":"             .where(image_locations.c.meta_data \u003d\u003d row[2])"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAT3%2F%2F2Z0%3D","line":66,"updated":"2013-12-30 07:29:41.000000000","message":"I have two suggestions:\n\n1. How about do this part be separated also? since we can avoid do this secondary query in db2 branch, we can get location entry by its id value directly, which already been received on main query (line #41), as you did in previous PS. IMO this way can prevent unnecessary migration performance deduction.\n\n2. If we move this two stuff to two function separately will be more clear IMO.\n\nthanks.","commit_id":"f66e8e7b5bec33bb421cbf6a343fac951630fcc2"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"ddf311181bf9e740fdcbc57faefbe11025be7bf2","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        # NOTE(flaper87): Not the fastest way to do it."},{"line_number":64,"context_line":"        # This is the best way to do it since sqlalchemy"},{"line_number":65,"context_line":"        # has a bug around delete + limit."},{"line_number":66,"context_line":"        s \u003d (sql.select([image_locations.c.id])"},{"line_number":67,"context_line":"             .where(image_locations.c.image_id \u003d\u003d row[0])"},{"line_number":68,"context_line":"             .where(image_locations.c.value \u003d\u003d row[1])"},{"line_number":69,"context_line":"             .where(image_locations.c.meta_data \u003d\u003d row[2])"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAT3%2F%2F1qc%3D","line":66,"in_reply_to":"AAAAT3%2F%2F2Z0%3D","updated":"2013-12-30 12:23:37.000000000","message":"Thanks for your suggestions, I already take the first one, about the second, IMO two function will look clear, but complicate the function intercommunicating,  and it is also good for the two stuff.","commit_id":"f66e8e7b5bec33bb421cbf6a343fac951630fcc2"}],"glance/tests/unit/test_migrations.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"2cfefdbd72098d74c7cea7ee726e9da8423cb983","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        self.engines \u003d {}"},{"line_number":154,"context_line":"        for key, value in self.test_databases.items():"},{"line_number":155,"context_line":"            self.engines[key] \u003d sqlalchemy.create_engine("},{"line_number":156,"context_line":"                                    value, strategy\u003d\u0027threadlocal\u0027)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        # We start each test case with a completely blank slate."},{"line_number":159,"context_line":"        self._reset_databases()"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F9Xqk%3D","line":156,"updated":"2013-12-12 06:13:34.000000000","message":"From the doc [1] it seem this approach has some limitation and is discouraged. Actually I still prefer do my approach [2], can you tell me why you think it is wrong? btw, as an alternative I think Venkatesh\u0027s suggestion is make sense also, I know it probably will be complex than your current implementation but I think it\u0027s a self-contains fixing but change the common code at here (other migration code is workable currently).\n\n[1] http://docs.sqlalchemy.org/en/rel_0_9/core/connections.html#threadlocal-strategy\n\n[2] https://review.openstack.org/#/c/59630/8/glance/db/sqlalchemy/migrate_repo/versions/031_remove_duplicated_locations.py","commit_id":"b2f4dc68c26cb1abf4f856efc8feb2c506f4b40b"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"bf5ac6515fc1ad2a21c2f4e72525cd849f432cf3","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        self.engines \u003d {}"},{"line_number":154,"context_line":"        for key, value in self.test_databases.items():"},{"line_number":155,"context_line":"            self.engines[key] \u003d sqlalchemy.create_engine("},{"line_number":156,"context_line":"                                    value, strategy\u003d\u0027threadlocal\u0027)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        # We start each test case with a completely blank slate."},{"line_number":159,"context_line":"        self._reset_databases()"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F9Xio%3D","line":156,"in_reply_to":"AAAATn%2F9Xqk%3D","updated":"2013-12-12 07:08:55.000000000","message":"I have verified your approach [1], it is still error with \"database locked\", this is a limitation of of open connection/transactions to sqlite database, when transaction is open the db file is locked for writing. I am confused that other migration code is workable, but I did encounter this problem. and threadlocal can fix the issue of non-ORM applications and sqlite.\n\nAs an alternative the threadlocal strategy can be loaded when check that sqlite database is used.\n\nI am considering Venkatesh\u0027s suggention as you said, and will do the code change work.\n\n[1] https://review.openstack.org/#/c/59630/8/glance/db/sqlalchemy/migrate_repo/versions/031_remove_duplicated_locations.py","commit_id":"b2f4dc68c26cb1abf4f856efc8feb2c506f4b40b"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"0153dda9b6f36e232b74af94bdc50c0c6f2a5cf0","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        self.engines \u003d {}"},{"line_number":154,"context_line":"        for key, value in self.test_databases.items():"},{"line_number":155,"context_line":"            self.engines[key] \u003d sqlalchemy.create_engine("},{"line_number":156,"context_line":"                                    value, strategy\u003d\u0027threadlocal\u0027)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        # We start each test case with a completely blank slate."},{"line_number":159,"context_line":"        self._reset_databases()"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F9W%2FQ%3D","line":156,"updated":"2013-12-12 09:28:30.000000000","message":"still need this?","commit_id":"230362bdf1621f793094419449828846d8bcd5e7"},{"author":{"_account_id":8112,"name":"Ma Wen Cheng","email":"wenchma@cn.ibm.com","username":"wenchma"},"change_message_id":"0a2035eb91200c774a19926ca707ae5b18df8534","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        self.engines \u003d {}"},{"line_number":154,"context_line":"        for key, value in self.test_databases.items():"},{"line_number":155,"context_line":"            self.engines[key] \u003d sqlalchemy.create_engine("},{"line_number":156,"context_line":"                                    value, strategy\u003d\u0027threadlocal\u0027)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        # We start each test case with a completely blank slate."},{"line_number":159,"context_line":"        self._reset_databases()"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F9W8M%3D","line":156,"in_reply_to":"AAAATn%2F9W%2FQ%3D","updated":"2013-12-12 09:37:00.000000000","message":"yes, if not, it also throws the database locked error.","commit_id":"230362bdf1621f793094419449828846d8bcd5e7"}]}