)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"f78f56980c01ce3994df2f241807d4716777037f","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Adds the ability to clean up rows that are already marked as"},{"line_number":10,"context_line":"deleted of a certain specified age. Age is calculated as timedelta"},{"line_number":11,"context_line":"from now() in days, which are given at command line"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ia50ab0dc4aa1547a5a6a2430f7941aab194e4baf"},{"line_number":14,"context_line":"Implements: blueprint database-purge"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"da86d52c_1b7146e0","line":12,"updated":"2015-02-04 05:06:45.000000000","message":"Please add DocImpact","commit_id":"435052bb9a8b9b95ea516fa4d30ab30fd7d3eb76"}],"cinder/cmd/manage.py":[{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"0abd283d635fbab73dda4cd06d6ab3adf21978c4","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                                   db_migration.INIT_VERSION))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @args(\u0027age\u0027, nargs\u003d\u0027?\u0027, default\u003dNone,"},{"line_number":232,"context_line":"            help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""},{"line_number":235,"context_line":"        if age is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_fac74e44","line":232,"updated":"2015-01-13 10:13:45.000000000","message":"Is days sufficient granularity? Why not use seconds and then it\u0027s up to the user.","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"0abd283d635fbab73dda4cd06d6ab3adf21978c4","unresolved":false,"context_lines":[{"line_number":233,"context_line":"    def purge_deleted_rows(self, age):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""},{"line_number":235,"context_line":"        if age is not None:"},{"line_number":236,"context_line":"            age \u003d int(age)"},{"line_number":237,"context_line":"            if age \u003c 0:"},{"line_number":238,"context_line":"                print(_(\"Must supply a positive value for age\"))"},{"line_number":239,"context_line":"                return(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_bab4e682","line":236,"updated":"2015-01-13 10:13:45.000000000","message":"If age isn\u0027t a number this is going to throw an exception.\n\nas_int in cinder/utils.py may be of use.","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":236,"context_line":"            age \u003d int(age)"},{"line_number":237,"context_line":"            if age \u003c 0:"},{"line_number":238,"context_line":"                print(_(\"Must supply a positive value for age\"))"},{"line_number":239,"context_line":"                return(1)"},{"line_number":240,"context_line":"        else:"},{"line_number":241,"context_line":"            print(_(\"Missing age value\"))"},{"line_number":242,"context_line":"            return(1)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_94d86180","line":239,"updated":"2015-01-16 06:49:17.000000000","message":"I think you can use `return 1` to remove redundant parentheses.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                return(1)"},{"line_number":240,"context_line":"        else:"},{"line_number":241,"context_line":"            print(_(\"Missing age value\"))"},{"line_number":242,"context_line":"            return(1)"},{"line_number":243,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":244,"context_line":"        db.purge_deleted_rows(ctxt, age)"},{"line_number":245,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_b4e39d32","line":242,"updated":"2015-01-16 06:49:17.000000000","message":"Same as above. Besides, I think you are not supposed to `return 1` to indicate error. The returned value is never used, see `fn(*fn_args)` in main.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"bd8143de48acb8e5d5728575645fe8d6d4d5679d","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def __init__(self):"},{"line_number":217,"context_line":"        pass"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    @args(\u0027version\u0027, nargs\u003d\u0027?\u0027, default\u003dNone,"},{"line_number":220,"context_line":"          help\u003d\u0027Database version\u0027)"},{"line_number":221,"context_line":"    def sync(self, version\u003dNone):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_76b0c243","line":218,"updated":"2015-01-23 07:31:37.000000000","message":"Not part of your patch, but this arg probably shouldn\u0027t be optional either.  Not sure if it should be changed here.","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"bd8143de48acb8e5d5728575645fe8d6d4d5679d","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                                   db_migration.MIGRATE_REPO_PATH,"},{"line_number":229,"context_line":"                                   db_migration.INIT_VERSION))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @args(\u0027age\u0027, nargs\u003d\u0027?\u0027, type\u003dint, default\u003dNone,"},{"line_number":232,"context_line":"          help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_f69cb2c2","line":231,"updated":"2015-01-23 07:31:37.000000000","message":"Why make this arg optional, only to check that it is set later?\n\nAlso, why days?  Large deployments may need finer granularity.  I think hours would be better.","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"3898b5806d5593c60f982e93cef65ceb071f0e0b","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                                   db_migration.MIGRATE_REPO_PATH,"},{"line_number":229,"context_line":"                                   db_migration.INIT_VERSION))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @args(\u0027age\u0027, nargs\u003d\u0027?\u0027, type\u003dint, default\u003dNone,"},{"line_number":232,"context_line":"          help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_30fd346f","line":231,"in_reply_to":"1a930d6b_0a3f33d7","updated":"2015-01-27 06:21:31.000000000","message":"OK, I will take your word for it.","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"3898b5806d5593c60f982e93cef65ceb071f0e0b","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                                   db_migration.MIGRATE_REPO_PATH,"},{"line_number":229,"context_line":"                                   db_migration.INIT_VERSION))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @args(\u0027age\u0027, nargs\u003d\u0027?\u0027, type\u003dint, default\u003dNone,"},{"line_number":232,"context_line":"          help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_5002c06f","line":231,"in_reply_to":"1a930d6b_15ebd4b0","updated":"2015-01-27 06:21:31.000000000","message":"Doesn\u0027t the nargs and default make it optional?","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"c2c5c5c43cd0045de7d9014d0a7b4c57ed17a55f","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                                   db_migration.MIGRATE_REPO_PATH,"},{"line_number":229,"context_line":"                                   db_migration.INIT_VERSION))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @args(\u0027age\u0027, nargs\u003d\u0027?\u0027, type\u003dint, default\u003dNone,"},{"line_number":232,"context_line":"          help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_0a3f33d7","line":231,"in_reply_to":"1a930d6b_f69cb2c2","updated":"2015-01-23 21:59:05.000000000","message":"I\u0027ve worked at large installations, we want days, not hours. Example, I want to purge deleted records that are 1 month old. well, that\u0027s easy to say \"30 days\", very hard to say \"(30 * 24) Hours\". The point is, you\u0027d want to delete old things that you\u0027re sure you won\u0027t need again later. Can\u0027t envision a large shop like ours, or any of the other public clouds saying \"I\u0027d like to purge things that were deleted 3 hours ago\"","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"8c5046c490052859be562735eead9129a41506f9","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                                   db_migration.MIGRATE_REPO_PATH,"},{"line_number":229,"context_line":"                                   db_migration.INIT_VERSION))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @args(\u0027age\u0027, nargs\u003d\u0027?\u0027, type\u003dint, default\u003dNone,"},{"line_number":232,"context_line":"          help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_15ebd4b0","line":231,"in_reply_to":"1a930d6b_f69cb2c2","updated":"2015-01-23 22:22:51.000000000","message":"Where am I making this optional?","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6f4bb00fcbd87a359a90be8114a0814c2c01661c","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        if age_in_days is not None:"},{"line_number":236,"context_line":"            age_in_days \u003d int(age_in_days)"},{"line_number":237,"context_line":"            if age_in_days \u003c 0:"},{"line_number":238,"context_line":"                print(_(\"Must supply a positive value for age\"))"},{"line_number":239,"context_line":"                return"},{"line_number":240,"context_line":"        else:"},{"line_number":241,"context_line":"            print(_(\"Missing age value\"))"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_bcc2ad50","line":238,"updated":"2015-01-26 23:07:38.000000000","message":"Same here","commit_id":"7da02b8c3e30c84fe9c29a031b3a095ee41dc517"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6f4bb00fcbd87a359a90be8114a0814c2c01661c","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                print(_(\"Must supply a positive value for age\"))"},{"line_number":239,"context_line":"                return"},{"line_number":240,"context_line":"        else:"},{"line_number":241,"context_line":"            print(_(\"Missing age value\"))"},{"line_number":242,"context_line":"            return"},{"line_number":243,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":244,"context_line":"        db.purge_deleted_rows(ctxt, age_in_days)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_dc8bf91c","line":241,"updated":"2015-01-26 23:07:38.000000000","message":"It\u0027s not normal behavior, so needs add \u0027exit(1)\u0027 instead of \u0027return\u0027","commit_id":"7da02b8c3e30c84fe9c29a031b3a095ee41dc517"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"f28b7e33ec61a94bbe303680c9c13d3618587a9b","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                                   db_migration.MIGRATE_REPO_PATH,"},{"line_number":229,"context_line":"                                   db_migration.INIT_VERSION))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @args(\u0027age_in_days\u0027, nargs\u003d1, type\u003dint,"},{"line_number":232,"context_line":"          help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age_in_days):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa81d914_0dc89f73","line":231,"updated":"2015-01-28 05:30:53.000000000","message":"Are you sure this works?\n\n \u003e\u003e parser \u003d argparse.ArgumentParser()\n \u003e\u003e parser.add_argument(\u0027age_in_days\u0027, nargs\u003d1, type\u003dint)\n \u003e\u003e parser.parse_args(\u00271\u0027)\n Namespace(age_in_days\u003d[1])\n\nBecause of nargs\u003d1 you get a list.  Also, there is no way it can be None (the check in line 235).\n\nIf you remove nargs, you get what you want:\n \u003e\u003e parser2 \u003d argparse.ArgumentParser()\n \u003e\u003e parser2.add_argument(\u0027age_in_days\u0027, type\u003dint)\n \u003e\u003e parser2.parse_args(\u00271\u0027)\n Namespace(age_in_days\u003d1)\n\n(And still it cannot have a value of None)","commit_id":"c7afca730effefcd7b7613433bdd5caab039c6c1"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"ba09229d01d7f69f71786cd0c236e4b618b57204","unresolved":false,"context_lines":[{"line_number":232,"context_line":"          help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age_in_days):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""},{"line_number":235,"context_line":"        if age_in_days is not None:"},{"line_number":236,"context_line":"            age_in_days \u003d int(age_in_days)"},{"line_number":237,"context_line":"            if age_in_days \u003c 0:"},{"line_number":238,"context_line":"                print(_(\"Must supply a positive value for age\"))"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa81d914_e23a7535","line":235,"updated":"2015-01-28 06:41:04.000000000","message":"I still don\u0027t understand why this check is here...","commit_id":"0dc3be767d2b28ed4ea56b1bb5ec7d65c8bade40"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"5c5d528eee571427422f3a1b45f6083b4280e5e3","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                                   db_migration.INIT_VERSION))"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @args(\u0027age_in_days\u0027, type\u003dint,"},{"line_number":232,"context_line":"          help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age_in_days):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""},{"line_number":235,"context_line":"        age_in_days \u003d int(age_in_days)"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_360baa26","line":232,"updated":"2015-02-03 11:40:38.000000000","message":"Docstring contains a good help text but this one is not a user friendly","commit_id":"241c95fb6cee0a901634ceb0e6cedccdcb6a7c41"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"5c5d528eee571427422f3a1b45f6083b4280e5e3","unresolved":false,"context_lines":[{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @args(\u0027age_in_days\u0027, type\u003dint,"},{"line_number":232,"context_line":"          help\u003d\u0027Number of days deleted to purge\u0027)"},{"line_number":233,"context_line":"    def purge_deleted_rows(self, age_in_days):"},{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""},{"line_number":235,"context_line":"        age_in_days \u003d int(age_in_days)"},{"line_number":236,"context_line":"        if age_in_days \u003c 0:"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_53175051","line":233,"updated":"2015-02-03 11:40:38.000000000","message":"From a user point of view, I propose to rename it to \u0027purge\u0027 to make this command less complex.","commit_id":"241c95fb6cee0a901634ceb0e6cedccdcb6a7c41"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"5c5d528eee571427422f3a1b45f6083b4280e5e3","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        \"\"\"Purge deleted rows older than a given age from cinder tables.\"\"\""},{"line_number":235,"context_line":"        age_in_days \u003d int(age_in_days)"},{"line_number":236,"context_line":"        if age_in_days \u003c 0:"},{"line_number":237,"context_line":"            print(_(\"Must supply a positive value for age\"))"},{"line_number":238,"context_line":"            exit(1)"},{"line_number":239,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":240,"context_line":"        db.purge_deleted_rows(ctxt, age_in_days)"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_d6196e7a","line":237,"updated":"2015-02-03 11:40:38.000000000","message":"Actually, it works good with \u00270\u0027. Please, add it to an error message","commit_id":"241c95fb6cee0a901634ceb0e6cedccdcb6a7c41"}],"cinder/db/api.py":[{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":935,"context_line":"def cgsnapshot_destroy(context, cgsnapshot_id):"},{"line_number":936,"context_line":"    \"\"\"Destroy the cgsnapshot or raise if it does not exist.\"\"\""},{"line_number":937,"context_line":"    return IMPL.cgsnapshot_destroy(context, cgsnapshot_id)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"###############"},{"line_number":940,"context_line":""},{"line_number":941,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_5458b9f8","line":938,"updated":"2015-01-16 06:49:17.000000000","message":"I think you need an extra blank line here to be consistent with other \u0027#########\u0027s in the same file.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"08016bfb8b8ee80af44845da31e27158e9a1c115","unresolved":false,"context_lines":[{"line_number":936,"context_line":"    \"\"\"Destroy the cgsnapshot or raise if it does not exist.\"\"\""},{"line_number":937,"context_line":"    return IMPL.cgsnapshot_destroy(context, cgsnapshot_id)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"###############"},{"line_number":940,"context_line":""},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"def purge_deleted_rows(context, age\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_b9aef2e5","line":939,"updated":"2015-01-21 08:10:41.000000000","message":"Is this line really necessary here?","commit_id":"0b77ff67fb9197c4c4a42252457bb79125f968f1"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"bd8143de48acb8e5d5728575645fe8d6d4d5679d","unresolved":false,"context_lines":[{"line_number":937,"context_line":"    return IMPL.cgsnapshot_destroy(context, cgsnapshot_id)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":941,"context_line":"    \"\"\"Purge deleted rows older than given age from cinder tables"},{"line_number":942,"context_line":""},{"line_number":943,"context_line":"    :returns: number of deleted rows"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_7697e290","line":940,"updated":"2015-01-23 07:31:37.000000000","message":"Please rename \u0027age\u0027 to be more specific, like age_in_hours","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"8c5046c490052859be562735eead9129a41506f9","unresolved":false,"context_lines":[{"line_number":937,"context_line":"    return IMPL.cgsnapshot_destroy(context, cgsnapshot_id)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":941,"context_line":"    \"\"\"Purge deleted rows older than given age from cinder tables"},{"line_number":942,"context_line":""},{"line_number":943,"context_line":"    :returns: number of deleted rows"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_b5068084","line":940,"in_reply_to":"1a930d6b_7697e290","updated":"2015-01-23 22:22:51.000000000","message":"age_in_days, as the spec and blueprint say \"days\"","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"3898b5806d5593c60f982e93cef65ceb071f0e0b","unresolved":false,"context_lines":[{"line_number":937,"context_line":"    return IMPL.cgsnapshot_destroy(context, cgsnapshot_id)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":941,"context_line":"    \"\"\"Purge deleted rows older than given age from cinder tables"},{"line_number":942,"context_line":""},{"line_number":943,"context_line":"    :returns: number of deleted rows"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_70fa5c76","line":940,"in_reply_to":"1a930d6b_b5068084","updated":"2015-01-27 06:21:31.000000000","message":"I\u0027m OK with days","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"0abd283d635fbab73dda4cd06d6ab3adf21978c4","unresolved":false,"context_lines":[{"line_number":41,"context_line":"from sqlalchemy.sql.expression import literal_column"},{"line_number":42,"context_line":"from sqlalchemy.sql.expression import true"},{"line_number":43,"context_line":"from sqlalchemy.sql import func"},{"line_number":44,"context_line":"from sqlalchemy.schema import Table"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"from cinder.common import sqlalchemyutils"},{"line_number":47,"context_line":"from cinder.db.sqlalchemy import models"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_154dcbc5","line":44,"updated":"2015-01-13 10:13:45.000000000","message":"I think imports are supposed to be in alphabetical order","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"bc734770dc8c6432fa9f0541cef20eeffedb265b","unresolved":false,"context_lines":[{"line_number":41,"context_line":"from sqlalchemy.sql.expression import literal_column"},{"line_number":42,"context_line":"from sqlalchemy.sql.expression import true"},{"line_number":43,"context_line":"from sqlalchemy.sql import func"},{"line_number":44,"context_line":"from sqlalchemy.schema import Table"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"from cinder.common import sqlalchemyutils"},{"line_number":47,"context_line":"from cinder.db.sqlalchemy import models"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_a8600fd0","line":44,"in_reply_to":"3a961159_154dcbc5","updated":"2015-01-13 17:37:14.000000000","message":"Done","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"0abd283d635fbab73dda4cd06d6ab3adf21978c4","unresolved":false,"context_lines":[{"line_number":3291,"context_line":"@require_admin_context"},{"line_number":3292,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":3293,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3294,"context_line":"    from datetime import datetime, timedelta"},{"line_number":3295,"context_line":""},{"line_number":3296,"context_line":"    engine \u003d get_engine()"},{"line_number":3297,"context_line":"    session \u003d get_session()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_facbce70","line":3294,"updated":"2015-01-13 10:13:45.000000000","message":"Why is this not at the top of the file?","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"bc734770dc8c6432fa9f0541cef20eeffedb265b","unresolved":false,"context_lines":[{"line_number":3291,"context_line":"@require_admin_context"},{"line_number":3292,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":3293,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3294,"context_line":"    from datetime import datetime, timedelta"},{"line_number":3295,"context_line":""},{"line_number":3296,"context_line":"    engine \u003d get_engine()"},{"line_number":3297,"context_line":"    session \u003d get_session()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_c85b9b20","line":3294,"in_reply_to":"3a961159_facbce70","updated":"2015-01-13 17:37:14.000000000","message":"Done","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"1486ed1b2d9cdc022b26fd3b6b4caf95522f0b81","unresolved":false,"context_lines":[{"line_number":3303,"context_line":"    for model_class in models.__dict__.itervalues():"},{"line_number":3304,"context_line":"        if hasattr(model_class, \"__tablename__\"):"},{"line_number":3305,"context_line":"            tables.append(model_class.__tablename__)"},{"line_number":3306,"context_line":"            "},{"line_number":3307,"context_line":"    # Reorder the list so that the volumes table is last"},{"line_number":3308,"context_line":"    tables.remove(\"volumes\")"},{"line_number":3309,"context_line":"    tables.append(\"volumes\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_8bbb66e3","line":3306,"updated":"2015-01-13 06:08:57.000000000","message":"Trailing white space","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"bc734770dc8c6432fa9f0541cef20eeffedb265b","unresolved":false,"context_lines":[{"line_number":3303,"context_line":"    for model_class in models.__dict__.itervalues():"},{"line_number":3304,"context_line":"        if hasattr(model_class, \"__tablename__\"):"},{"line_number":3305,"context_line":"            tables.append(model_class.__tablename__)"},{"line_number":3306,"context_line":"            "},{"line_number":3307,"context_line":"    # Reorder the list so that the volumes table is last"},{"line_number":3308,"context_line":"    tables.remove(\"volumes\")"},{"line_number":3309,"context_line":"    tables.append(\"volumes\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_e89937d1","line":3306,"in_reply_to":"3a961159_8bbb66e3","updated":"2015-01-13 17:37:14.000000000","message":"Done","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"1486ed1b2d9cdc022b26fd3b6b4caf95522f0b81","unresolved":false,"context_lines":[{"line_number":3304,"context_line":"        if hasattr(model_class, \"__tablename__\"):"},{"line_number":3305,"context_line":"            tables.append(model_class.__tablename__)"},{"line_number":3306,"context_line":"            "},{"line_number":3307,"context_line":"    # Reorder the list so that the volumes table is last"},{"line_number":3308,"context_line":"    tables.remove(\"volumes\")"},{"line_number":3309,"context_line":"    tables.append(\"volumes\")"},{"line_number":3310,"context_line":"    for table in tables:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_0bc77666","line":3307,"updated":"2015-01-13 06:08:57.000000000","message":"Why?","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"bc734770dc8c6432fa9f0541cef20eeffedb265b","unresolved":false,"context_lines":[{"line_number":3304,"context_line":"        if hasattr(model_class, \"__tablename__\"):"},{"line_number":3305,"context_line":"            tables.append(model_class.__tablename__)"},{"line_number":3306,"context_line":"            "},{"line_number":3307,"context_line":"    # Reorder the list so that the volumes table is last"},{"line_number":3308,"context_line":"    tables.remove(\"volumes\")"},{"line_number":3309,"context_line":"    tables.append(\"volumes\")"},{"line_number":3310,"context_line":"    for table in tables:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_c8b2bb48","line":3307,"in_reply_to":"3a961159_0bc77666","updated":"2015-01-13 17:37:14.000000000","message":"This is to avoid potential FK constraints, as other tables use volume_id as a FK joined to volumes.id","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"0abd283d635fbab73dda4cd06d6ab3adf21978c4","unresolved":false,"context_lines":[{"line_number":3309,"context_line":"    tables.append(\"volumes\")"},{"line_number":3310,"context_line":"    for table in tables:"},{"line_number":3311,"context_line":"        t \u003d Table(table, metadata, autoload\u003dTrue)"},{"line_number":3312,"context_line":"        msg \u003d _(\"Purging deleted rows older than %s days from %s\") % (age, table)"},{"line_number":3313,"context_line":"        LOG.info(msg)"},{"line_number":3314,"context_line":"        default_deleted_value \u003d \"0\""},{"line_number":3315,"context_line":"        deleted_age \u003d datetime.now() - timedelta(days\u003dage)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_3f56282b","line":3312,"updated":"2015-01-13 10:13:45.000000000","message":"You should use named interpolation here\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-translated-messages\n\nIt\u0027s also better to delay the interpolation\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-log-messages\n\n_LI() should be used instead of _()\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#log-translation","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"bc734770dc8c6432fa9f0541cef20eeffedb265b","unresolved":false,"context_lines":[{"line_number":3309,"context_line":"    tables.append(\"volumes\")"},{"line_number":3310,"context_line":"    for table in tables:"},{"line_number":3311,"context_line":"        t \u003d Table(table, metadata, autoload\u003dTrue)"},{"line_number":3312,"context_line":"        msg \u003d _(\"Purging deleted rows older than %s days from %s\") % (age, table)"},{"line_number":3313,"context_line":"        LOG.info(msg)"},{"line_number":3314,"context_line":"        default_deleted_value \u003d \"0\""},{"line_number":3315,"context_line":"        deleted_age \u003d datetime.now() - timedelta(days\u003dage)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_0884c35b","line":3312,"in_reply_to":"3a961159_3f56282b","updated":"2015-01-13 17:37:14.000000000","message":"Done","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"0abd283d635fbab73dda4cd06d6ab3adf21978c4","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"        t \u003d Table(table, metadata, autoload\u003dTrue)"},{"line_number":3312,"context_line":"        msg \u003d _(\"Purging deleted rows older than %s days from %s\") % (age, table)"},{"line_number":3313,"context_line":"        LOG.info(msg)"},{"line_number":3314,"context_line":"        default_deleted_value \u003d \"0\""},{"line_number":3315,"context_line":"        deleted_age \u003d datetime.now() - timedelta(days\u003dage)"},{"line_number":3316,"context_line":"        try:"},{"line_number":3317,"context_line":"            with conn.begin():"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_1fd40cba","line":3314,"updated":"2015-01-13 10:13:45.000000000","message":"What\u0027s this variable for? It doesn\u0027t seem to be used.","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"bc734770dc8c6432fa9f0541cef20eeffedb265b","unresolved":false,"context_lines":[{"line_number":3311,"context_line":"        t \u003d Table(table, metadata, autoload\u003dTrue)"},{"line_number":3312,"context_line":"        msg \u003d _(\"Purging deleted rows older than %s days from %s\") % (age, table)"},{"line_number":3313,"context_line":"        LOG.info(msg)"},{"line_number":3314,"context_line":"        default_deleted_value \u003d \"0\""},{"line_number":3315,"context_line":"        deleted_age \u003d datetime.now() - timedelta(days\u003dage)"},{"line_number":3316,"context_line":"        try:"},{"line_number":3317,"context_line":"            with conn.begin():"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_487ecb6b","line":3314,"in_reply_to":"3a961159_1fd40cba","updated":"2015-01-13 17:37:14.000000000","message":"Done","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"0abd283d635fbab73dda4cd06d6ab3adf21978c4","unresolved":false,"context_lines":[{"line_number":3315,"context_line":"        deleted_age \u003d datetime.now() - timedelta(days\u003dage)"},{"line_number":3316,"context_line":"        try:"},{"line_number":3317,"context_line":"            with conn.begin():"},{"line_number":3318,"context_line":"                result_delete \u003d conn.execute(t.delete().where(t.c.deleted_at \u003c deleted_age))"},{"line_number":3319,"context_line":"        except db_exc.DBError:"},{"line_number":3320,"context_line":"            msg \u003d _(\"IntegrityError detected when purging from table %s\") % table"},{"line_number":3321,"context_line":"            LOG.info(msg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_3a747635","line":3318,"updated":"2015-01-13 10:13:45.000000000","message":"timeutils has some helper functions, such as is_older_than which may be appropriate here\n\nhttps://github.com/openstack/oslo.utils/blob/master/oslo_utils/timeutils.py#L76","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"0abd283d635fbab73dda4cd06d6ab3adf21978c4","unresolved":false,"context_lines":[{"line_number":3317,"context_line":"            with conn.begin():"},{"line_number":3318,"context_line":"                result_delete \u003d conn.execute(t.delete().where(t.c.deleted_at \u003c deleted_age))"},{"line_number":3319,"context_line":"        except db_exc.DBError:"},{"line_number":3320,"context_line":"            msg \u003d _(\"IntegrityError detected when purging from table %s\") % table"},{"line_number":3321,"context_line":"            LOG.info(msg)"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        rows_purged \u003d result_delete.rowcount"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_9a272ab5","line":3320,"updated":"2015-01-13 10:13:45.000000000","message":"Why do you mention an IntegrityError in the message when you catch DBError?\n\nIt\u0027s also better to delay the interpolation\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-log-messages\n\n_LI() should be used instead of _()\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#log-translation","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":8475,"name":"AmitKumarDas","email":"amit.das@cloudbyte.com","username":"AmitKumarDas"},"change_message_id":"67152e32a50e3f70c491e70cb7bfebf655ceca6c","unresolved":false,"context_lines":[{"line_number":3318,"context_line":"                result_delete \u003d conn.execute(t.delete().where(t.c.deleted_at \u003c deleted_age))"},{"line_number":3319,"context_line":"        except db_exc.DBError:"},{"line_number":3320,"context_line":"            msg \u003d _(\"IntegrityError detected when purging from table %s\") % table"},{"line_number":3321,"context_line":"            LOG.info(msg)"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        rows_purged \u003d result_delete.rowcount"},{"line_number":3324,"context_line":"        msg \u003d _(\"Deleted %s rows from table %s\") % (rows_purged, table)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_6e16485a","line":3321,"updated":"2015-01-13 07:28:26.000000000","message":"Should this be an INFO log ?","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"bc734770dc8c6432fa9f0541cef20eeffedb265b","unresolved":false,"context_lines":[{"line_number":3318,"context_line":"                result_delete \u003d conn.execute(t.delete().where(t.c.deleted_at \u003c deleted_age))"},{"line_number":3319,"context_line":"        except db_exc.DBError:"},{"line_number":3320,"context_line":"            msg \u003d _(\"IntegrityError detected when purging from table %s\") % table"},{"line_number":3321,"context_line":"            LOG.info(msg)"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        rows_purged \u003d result_delete.rowcount"},{"line_number":3324,"context_line":"        msg \u003d _(\"Deleted %s rows from table %s\") % (rows_purged, table)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_083923a3","line":3321,"in_reply_to":"3a961159_6e16485a","updated":"2015-01-13 17:37:14.000000000","message":"You\u0027re right, this should be LOG.exception","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":8475,"name":"AmitKumarDas","email":"amit.das@cloudbyte.com","username":"AmitKumarDas"},"change_message_id":"67152e32a50e3f70c491e70cb7bfebf655ceca6c","unresolved":false,"context_lines":[{"line_number":3320,"context_line":"            msg \u003d _(\"IntegrityError detected when purging from table %s\") % table"},{"line_number":3321,"context_line":"            LOG.info(msg)"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        rows_purged \u003d result_delete.rowcount"},{"line_number":3324,"context_line":"        msg \u003d _(\"Deleted %s rows from table %s\") % (rows_purged, table)"},{"line_number":3325,"context_line":"        LOG.info(msg)"},{"line_number":3326,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_c158ed33","line":3323,"updated":"2015-01-13 07:28:26.000000000","message":"Will result_delete have rowcount set to 0 in-case of errors while executing the SQL ?","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"0abd283d635fbab73dda4cd06d6ab3adf21978c4","unresolved":false,"context_lines":[{"line_number":3321,"context_line":"            LOG.info(msg)"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        rows_purged \u003d result_delete.rowcount"},{"line_number":3324,"context_line":"        msg \u003d _(\"Deleted %s rows from table %s\") % (rows_purged, table)"},{"line_number":3325,"context_line":"        LOG.info(msg)"},{"line_number":3326,"context_line":""},{"line_number":3327,"context_line":"        "}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_7f2fd0a7","line":3324,"updated":"2015-01-13 10:13:45.000000000","message":"You should use named interpolation here\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-translated-messages\n\nIt\u0027s also better to delay the interpolation\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-log-messages\n\n_LI() should be used instead of _()\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#log-translation","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"bc734770dc8c6432fa9f0541cef20eeffedb265b","unresolved":false,"context_lines":[{"line_number":3321,"context_line":"            LOG.info(msg)"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        rows_purged \u003d result_delete.rowcount"},{"line_number":3324,"context_line":"        msg \u003d _(\"Deleted %s rows from table %s\") % (rows_purged, table)"},{"line_number":3325,"context_line":"        LOG.info(msg)"},{"line_number":3326,"context_line":""},{"line_number":3327,"context_line":"        "}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_c8655bb0","line":3324,"in_reply_to":"3a961159_7f2fd0a7","updated":"2015-01-13 17:37:14.000000000","message":"Done","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"1486ed1b2d9cdc022b26fd3b6b4caf95522f0b81","unresolved":false,"context_lines":[{"line_number":3324,"context_line":"        msg \u003d _(\"Deleted %s rows from table %s\") % (rows_purged, table)"},{"line_number":3325,"context_line":"        LOG.info(msg)"},{"line_number":3326,"context_line":""},{"line_number":3327,"context_line":"        "}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_cbb56eee","line":3327,"updated":"2015-01-13 06:08:57.000000000","message":"Trailing white space","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"bc734770dc8c6432fa9f0541cef20eeffedb265b","unresolved":false,"context_lines":[{"line_number":3324,"context_line":"        msg \u003d _(\"Deleted %s rows from table %s\") % (rows_purged, table)"},{"line_number":3325,"context_line":"        LOG.info(msg)"},{"line_number":3326,"context_line":""},{"line_number":3327,"context_line":"        "}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_e803f7bb","line":3327,"in_reply_to":"3a961159_cbb56eee","updated":"2015-01-13 17:37:14.000000000","message":"Done","commit_id":"cb734f764d0e2d02fa7411466c68eaf5d3403a13"},{"author":{"_account_id":7219,"name":"git-harry","email":"git-harry@live.co.uk","username":"git-harry"},"change_message_id":"7c3c0a640fd392b509d5ae8229242716166fc1ca","unresolved":false,"context_lines":[{"line_number":3320,"context_line":"                    .where(t.c.deleted_at \u003c deleted_age))"},{"line_number":3321,"context_line":"        except db_exc.DBError:"},{"line_number":3322,"context_line":"            LOG.exception(_LE(\u0027DBError detected when purging from \u0027"},{"line_number":3323,"context_line":"                              \u0027table\u003d%(table)s\u0027) % {\u0027table\u0027: table})"},{"line_number":3324,"context_line":""},{"line_number":3325,"context_line":"        rows_purged \u003d result_delete.rowcount"},{"line_number":3326,"context_line":"        LOG.info(_LI(\"Deleted %(row)s rows from table\u003d%(table)s\"),"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_08b014fc","line":3323,"updated":"2015-01-14 20:26:29.000000000","message":"\u0027%\u0027 should be a \u0027,\u0027 so that the interpolation only happens when necessary.","commit_id":"7cd0f59f6a8358105eb9b135d80702fe392e20c4"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":19,"context_line":"\"\"\"Implementation of SQLAlchemy backend.\"\"\""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from datetime import datetime"},{"line_number":23,"context_line":"from datetime import timedelta"},{"line_number":24,"context_line":"import functools"},{"line_number":25,"context_line":"import sys"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_afbfcafa","line":22,"updated":"2015-01-16 06:49:17.000000000","message":"The least Import granularity is module, according to http://docs.openstack.org/developer/hacking/#imports. The existing code doesn\u0027t follow but new code cannot avoid.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from datetime import datetime"},{"line_number":23,"context_line":"from datetime import timedelta"},{"line_number":24,"context_line":"import functools"},{"line_number":25,"context_line":"import sys"},{"line_number":26,"context_line":"import threading"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_cfba560a","line":23,"updated":"2015-01-16 06:49:17.000000000","message":"Same as above.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":7860,"name":"Ryan Lucio","email":"ryan.lucio@fireeye.com","username":"rlucio"},"change_message_id":"1b6fe6daaadcb6d0a87a8f6847398d395e575d8b","unresolved":false,"context_lines":[{"line_number":3292,"context_line":""},{"line_number":3293,"context_line":""},{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":""},{"line_number":3298,"context_line":"    engine \u003d get_engine()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_48de953f","line":3295,"updated":"2015-01-14 23:56:32.000000000","message":"Why is the age value optional?  It doesn\u0027t appear that there is a use case for not including age.  Also, shouldn\u0027t you be doing data validation here?","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"d30405adfb798fba47459aef2e3cf6024f464319","unresolved":false,"context_lines":[{"line_number":3292,"context_line":""},{"line_number":3293,"context_line":""},{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":""},{"line_number":3298,"context_line":"    engine \u003d get_engine()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_e8239abb","line":3295,"in_reply_to":"3a961159_283789ff","updated":"2015-01-15 20:31:39.000000000","message":"If is it not optional, why default value is None?","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":7860,"name":"Ryan Lucio","email":"ryan.lucio@fireeye.com","username":"rlucio"},"change_message_id":"a4ddd35535b2c478df4ee3e5e180621d16540277","unresolved":false,"context_lines":[{"line_number":3292,"context_line":""},{"line_number":3293,"context_line":""},{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":""},{"line_number":3298,"context_line":"    engine \u003d get_engine()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_48b027dc","line":3295,"in_reply_to":"3a961159_37b41af0","updated":"2015-01-15 23:59:13.000000000","message":"I think you are missing the point. The fact you set age\u003dNone means your arg is a keyword arg with a default value of None.  All keyword args are optional in python.  So if its not optional, then you shouldn\u0027t set a default value, eg., purge_deleted_rows(context, age).\n\nWRT data validation I was just thinking from a best practices perspective, it would seem best to do data validation in the api because there may be multiple front ends that you don\u0027t have control over and the data may be dirty.   That said, it appears that isn\u0027t standard practice for db functions written here so nevermind...","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3292,"context_line":""},{"line_number":3293,"context_line":""},{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":""},{"line_number":3298,"context_line":"    engine \u003d get_engine()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_4f1d8661","line":3295,"in_reply_to":"3a961159_48b027dc","updated":"2015-01-16 06:49:17.000000000","message":"I agree with Ryan. No need to set default value for `age`. db.api also has the same issue. If we need to add parameter validation, we can add it to db.api.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"f74fcf43e418a60f51a7cf655c58d845d6f75425","unresolved":false,"context_lines":[{"line_number":3292,"context_line":""},{"line_number":3293,"context_line":""},{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":""},{"line_number":3298,"context_line":"    engine \u003d get_engine()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_283789ff","line":3295,"in_reply_to":"3a961159_48de953f","updated":"2015-01-15 00:11:57.000000000","message":"The age isn\u0027t optional, and it\u0027s checked in the cmd/manage.py","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"d9d0a5da2c6959a361ef35e38a491245b4935046","unresolved":false,"context_lines":[{"line_number":3292,"context_line":""},{"line_number":3293,"context_line":""},{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":""},{"line_number":3298,"context_line":"    engine \u003d get_engine()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_37b41af0","line":3295,"in_reply_to":"3a961159_e8239abb","updated":"2015-01-15 22:06:20.000000000","message":"I wouldn\u0027t set a default value, this is intended for an operator to run from cinder-manage with a non-optional argument for age.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3295,"context_line":"def purge_deleted_rows(context, age\u003dNone):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":""},{"line_number":3298,"context_line":"    engine \u003d get_engine()"},{"line_number":3299,"context_line":"    conn \u003d engine.connect()"},{"line_number":3300,"context_line":"    metadata \u003d MetaData()"},{"line_number":3301,"context_line":"    metadata.bind \u003d engine"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_2f083a90","line":3298,"updated":"2015-01-16 06:49:17.000000000","message":"The engine and connection is managed by get_session and uses lazy loading. It is better to use them rather than write twice.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3301,"context_line":"    metadata.bind \u003d engine"},{"line_number":3302,"context_line":"    tables \u003d []"},{"line_number":3303,"context_line":""},{"line_number":3304,"context_line":"    for model_class in models.__dict__.itervalues():"},{"line_number":3305,"context_line":"        if hasattr(model_class, \"__tablename__\"):"},{"line_number":3306,"context_line":"            tables.append(model_class.__tablename__)"},{"line_number":3307,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_efb0f241","line":3304,"updated":"2015-01-16 06:49:17.000000000","message":"This code block gets all tables in models. I think you can extract it as a common function.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3302,"context_line":"    tables \u003d []"},{"line_number":3303,"context_line":""},{"line_number":3304,"context_line":"    for model_class in models.__dict__.itervalues():"},{"line_number":3305,"context_line":"        if hasattr(model_class, \"__tablename__\"):"},{"line_number":3306,"context_line":"            tables.append(model_class.__tablename__)"},{"line_number":3307,"context_line":""},{"line_number":3308,"context_line":"    # Reorder the list so the volumes table is last to avoid FK constraints"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_cfe1769c","line":3305,"updated":"2015-01-16 06:49:17.000000000","message":"It looks not good to depend on __private_member__ of a class.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3306,"context_line":"            tables.append(model_class.__tablename__)"},{"line_number":3307,"context_line":""},{"line_number":3308,"context_line":"    # Reorder the list so the volumes table is last to avoid FK constraints"},{"line_number":3309,"context_line":"    tables.remove(\"volumes\")"},{"line_number":3310,"context_line":"    tables.append(\"volumes\")"},{"line_number":3311,"context_line":"    for table in tables:"},{"line_number":3312,"context_line":"        t \u003d Table(table, metadata, autoload\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_cf5176da","line":3309,"updated":"2015-01-16 06:49:17.000000000","message":"What if someone adds a new table or changes some table which breaks the above? The trick take advantages of the special information which should be hidden in models, rather than its public interface provided.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3307,"context_line":""},{"line_number":3308,"context_line":"    # Reorder the list so the volumes table is last to avoid FK constraints"},{"line_number":3309,"context_line":"    tables.remove(\"volumes\")"},{"line_number":3310,"context_line":"    tables.append(\"volumes\")"},{"line_number":3311,"context_line":"    for table in tables:"},{"line_number":3312,"context_line":"        t \u003d Table(table, metadata, autoload\u003dTrue)"},{"line_number":3313,"context_line":"        LOG.info(_LI(\u0027Purging deleted rows older than age\u003d%(age)s days \u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_efa1d23a","line":3310,"updated":"2015-01-16 06:49:17.000000000","message":"What if row A foreign key refers to row B, B is old and to be deleted, but A is newer and should not be deleted?","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3309,"context_line":"    tables.remove(\"volumes\")"},{"line_number":3310,"context_line":"    tables.append(\"volumes\")"},{"line_number":3311,"context_line":"    for table in tables:"},{"line_number":3312,"context_line":"        t \u003d Table(table, metadata, autoload\u003dTrue)"},{"line_number":3313,"context_line":"        LOG.info(_LI(\u0027Purging deleted rows older than age\u003d%(age)s days \u0027"},{"line_number":3314,"context_line":"                     \u0027from table\u003d%(table)s\u0027), {\u0027age\u0027: age, \u0027table\u0027: table})"},{"line_number":3315,"context_line":"        deleted_age \u003d datetime.now() - timedelta(days\u003dage)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_8ff92eca","line":3312,"updated":"2015-01-16 06:49:17.000000000","message":"It is better to use more meaningful variable names.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3310,"context_line":"    tables.append(\"volumes\")"},{"line_number":3311,"context_line":"    for table in tables:"},{"line_number":3312,"context_line":"        t \u003d Table(table, metadata, autoload\u003dTrue)"},{"line_number":3313,"context_line":"        LOG.info(_LI(\u0027Purging deleted rows older than age\u003d%(age)s days \u0027"},{"line_number":3314,"context_line":"                     \u0027from table\u003d%(table)s\u0027), {\u0027age\u0027: age, \u0027table\u0027: table})"},{"line_number":3315,"context_line":"        deleted_age \u003d datetime.now() - timedelta(days\u003dage)"},{"line_number":3316,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_8f720e7a","line":3313,"updated":"2015-01-16 06:49:17.000000000","message":"`age` is an integer. I think it is better to use %(age)d. Also we usually don\u0027t use \u0027\u003d\u0027 to indicate a value in log message. You can refer to other log messages.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3315,"context_line":"        deleted_age \u003d datetime.now() - timedelta(days\u003dage)"},{"line_number":3316,"context_line":"        try:"},{"line_number":3317,"context_line":"            with conn.begin():"},{"line_number":3318,"context_line":"                result_delete \u003d conn.execute("},{"line_number":3319,"context_line":"                    t.delete()"},{"line_number":3320,"context_line":"                    .where(t.c.deleted_at \u003c deleted_age))"},{"line_number":3321,"context_line":"        except db_exc.DBError:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_efa472a4","line":3318,"updated":"2015-01-16 06:49:17.000000000","message":"I think it is better to use model_query and delete, which is more consistent with existing code and avoid using low level api directly.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3317,"context_line":"            with conn.begin():"},{"line_number":3318,"context_line":"                result_delete \u003d conn.execute("},{"line_number":3319,"context_line":"                    t.delete()"},{"line_number":3320,"context_line":"                    .where(t.c.deleted_at \u003c deleted_age))"},{"line_number":3321,"context_line":"        except db_exc.DBError:"},{"line_number":3322,"context_line":"            LOG.exception(_LE(\u0027DBError detected when purging from \u0027"},{"line_number":3323,"context_line":"                              \u0027table\u003d%(table)s\u0027), {\u0027table\u0027: table})"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_cf1c16c5","line":3320,"updated":"2015-01-16 06:49:17.000000000","message":"What if a table doesn\u0027t have column `deleted_at`? For now they inherit from CinderBase who has `deleted_at` but we\u0027d better not make an assumption of that.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3320,"context_line":"                    .where(t.c.deleted_at \u003c deleted_age))"},{"line_number":3321,"context_line":"        except db_exc.DBError:"},{"line_number":3322,"context_line":"            LOG.exception(_LE(\u0027DBError detected when purging from \u0027"},{"line_number":3323,"context_line":"                              \u0027table\u003d%(table)s\u0027), {\u0027table\u0027: table})"},{"line_number":3324,"context_line":""},{"line_number":3325,"context_line":"        rows_purged \u003d result_delete.rowcount"},{"line_number":3326,"context_line":"        LOG.info(_LI(\"Deleted %(row)s rows from table\u003d%(table)s\"),"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_6f26c268","line":3323,"updated":"2015-01-16 06:49:17.000000000","message":"I think it is not good to swallow an exception.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":3323,"context_line":"                              \u0027table\u003d%(table)s\u0027), {\u0027table\u0027: table})"},{"line_number":3324,"context_line":""},{"line_number":3325,"context_line":"        rows_purged \u003d result_delete.rowcount"},{"line_number":3326,"context_line":"        LOG.info(_LI(\"Deleted %(row)s rows from table\u003d%(table)s\"),"},{"line_number":3327,"context_line":"                 {\u0027row\u0027: rows_purged, \u0027table\u0027: table})"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_6f8322cb","line":3326,"updated":"2015-01-16 06:49:17.000000000","message":"Same with above problem: use %(row)d and avoid using \u0027\u003d\u0027 to indicate value.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13636,"name":"aarefiev","display_name":"Anton","email":"arefyevanton@gmail.com","username":"aarefiev"},"change_message_id":"09e21a25e52b554cd93bb54aecce96de02343068","unresolved":false,"context_lines":[{"line_number":3295,"context_line":"def purge_deleted_rows(context, age):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":""},{"line_number":3298,"context_line":"    if context is None:"},{"line_number":3299,"context_line":"        raise exception.NotAuthorized()"},{"line_number":3300,"context_line":""},{"line_number":3301,"context_line":"    if age is not None:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_a3ab2088","line":3298,"updated":"2015-01-21 09:58:39.000000000","message":"This check is redundant.Use of empty request context is deprecated. It should check require_admin_context decorator.","commit_id":"0b77ff67fb9197c4c4a42252457bb79125f968f1"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"bd8143de48acb8e5d5728575645fe8d6d4d5679d","unresolved":false,"context_lines":[{"line_number":3292,"context_line":""},{"line_number":3293,"context_line":""},{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":"    if age is not None:"},{"line_number":3298,"context_line":"        age \u003d int(age)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_f6eaf20d","line":3295,"updated":"2015-01-23 07:31:37.000000000","message":"age_in_hours","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"3898b5806d5593c60f982e93cef65ceb071f0e0b","unresolved":false,"context_lines":[{"line_number":3292,"context_line":""},{"line_number":3293,"context_line":""},{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":"    if age is not None:"},{"line_number":3298,"context_line":"        age \u003d int(age)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_b0f04492","line":3295,"in_reply_to":"1a930d6b_d5322c5e","updated":"2015-01-27 06:21:31.000000000","message":"Sure","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"8c5046c490052859be562735eead9129a41506f9","unresolved":false,"context_lines":[{"line_number":3292,"context_line":""},{"line_number":3293,"context_line":""},{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":"    if age is not None:"},{"line_number":3298,"context_line":"        age \u003d int(age)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_d5322c5e","line":3295,"in_reply_to":"1a930d6b_f6eaf20d","updated":"2015-01-23 22:22:51.000000000","message":"age_in_days, as that\u0027s what the spec is for.","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"bd8143de48acb8e5d5728575645fe8d6d4d5679d","unresolved":false,"context_lines":[{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":"    if age is not None:"},{"line_number":3298,"context_line":"        age \u003d int(age)"},{"line_number":3299,"context_line":"        if age \u003c 0:"},{"line_number":3300,"context_line":"            raise ValueError(_(\u0027Must supply a positive value for age\u0027))"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_36439a15","line":3297,"updated":"2015-01-23 07:31:37.000000000","message":"It would be safer to check like this:\n\n try:\n     age \u003d int(age)\n except ValueError:\n     raise...\n if age \u003c 0:\n     raise...","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"6ef17d18fd18159b8ef466e4b8bcd7d7cd4b6e07","unresolved":false,"context_lines":[{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":"    if age is not None:"},{"line_number":3298,"context_line":"        age \u003d int(age)"},{"line_number":3299,"context_line":"        if age \u003c 0:"},{"line_number":3300,"context_line":"            raise ValueError(_(\u0027Must supply a positive value for age\u0027))"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_7a5a5c51","line":3297,"in_reply_to":"1a930d6b_36439a15","updated":"2015-01-23 12:18:25.000000000","message":"I think you might add it to db.api so that other drivers can use.","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"3898b5806d5593c60f982e93cef65ceb071f0e0b","unresolved":false,"context_lines":[{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":"    if age is not None:"},{"line_number":3298,"context_line":"        age \u003d int(age)"},{"line_number":3299,"context_line":"        if age \u003c 0:"},{"line_number":3300,"context_line":"            raise ValueError(_(\u0027Must supply a positive value for age\u0027))"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_1033d863","line":3297,"in_reply_to":"1a930d6b_75f09891","updated":"2015-01-27 06:21:31.000000000","message":"Abel, I understand your point.  Either way is fine IMO - I will leave that up to you.  Please consider my code change here though.","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"b597bd0a5efcaa6c9770c69b4ac2510cb91f6aea","unresolved":false,"context_lines":[{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":"    if age is not None:"},{"line_number":3298,"context_line":"        age \u003d int(age)"},{"line_number":3299,"context_line":"        if age \u003c 0:"},{"line_number":3300,"context_line":"            raise ValueError(_(\u0027Must supply a positive value for age\u0027))"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_75734fe7","line":3297,"in_reply_to":"1a930d6b_7a5a5c51","updated":"2015-01-23 12:43:49.000000000","message":"Agreed","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"8c5046c490052859be562735eead9129a41506f9","unresolved":false,"context_lines":[{"line_number":3294,"context_line":"@require_admin_context"},{"line_number":3295,"context_line":"def purge_deleted_rows(context, age):"},{"line_number":3296,"context_line":"    \"\"\"Purge deleted rows older than age from cinder tables.\"\"\""},{"line_number":3297,"context_line":"    if age is not None:"},{"line_number":3298,"context_line":"        age \u003d int(age)"},{"line_number":3299,"context_line":"        if age \u003c 0:"},{"line_number":3300,"context_line":"            raise ValueError(_(\u0027Must supply a positive value for age\u0027))"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_75f09891","line":3297,"in_reply_to":"1a930d6b_7a5a5c51","updated":"2015-01-23 22:22:51.000000000","message":"When I looked at db.api, everything was just a simple definition and pass-through to sqlalchemy. I was just being consistent there.","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"6ef17d18fd18159b8ef466e4b8bcd7d7cd4b6e07","unresolved":false,"context_lines":[{"line_number":3301,"context_line":"    else:"},{"line_number":3302,"context_line":"        raise TypeError(_(\u0027Missing age value\u0027))"},{"line_number":3303,"context_line":""},{"line_number":3304,"context_line":"    engine \u003d get_engine()"},{"line_number":3305,"context_line":"    session \u003d get_session()"},{"line_number":3306,"context_line":"    metadata \u003d MetaData()"},{"line_number":3307,"context_line":"    metadata.bind \u003d engine"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_f59f3fb2","line":3304,"updated":"2015-01-23 12:18:25.000000000","message":"I don\u0027t think you\u0027ll need explicit invoke to get_engine() in a db api method.","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"c2c5c5c43cd0045de7d9014d0a7b4c57ed17a55f","unresolved":false,"context_lines":[{"line_number":3301,"context_line":"    else:"},{"line_number":3302,"context_line":"        raise TypeError(_(\u0027Missing age value\u0027))"},{"line_number":3303,"context_line":""},{"line_number":3304,"context_line":"    engine \u003d get_engine()"},{"line_number":3305,"context_line":"    session \u003d get_session()"},{"line_number":3306,"context_line":"    metadata \u003d MetaData()"},{"line_number":3307,"context_line":"    metadata.bind \u003d engine"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_5512bc57","line":3304,"in_reply_to":"1a930d6b_f59f3fb2","updated":"2015-01-23 21:59:05.000000000","message":"without this get_engine(), I can\u0027t get the metadata.bind to work.","commit_id":"3abc0d45a3fb65addb5119b7d928102770637c6c"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6f4bb00fcbd87a359a90be8114a0814c2c01661c","unresolved":false,"context_lines":[{"line_number":36,"context_line":"import osprofiler.sqlalchemy"},{"line_number":37,"context_line":"import six"},{"line_number":38,"context_line":"import sqlalchemy"},{"line_number":39,"context_line":"from sqlalchemy import MetaData"},{"line_number":40,"context_line":"from sqlalchemy import or_"},{"line_number":41,"context_line":"from sqlalchemy.orm import joinedload, joinedload_all"},{"line_number":42,"context_line":"from sqlalchemy.orm import RelationshipProperty"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_5cfc69b8","line":39,"updated":"2015-01-26 23:07:38.000000000","message":"According to a Hacking rules, please, import modules, not objects","commit_id":"7da02b8c3e30c84fe9c29a031b3a095ee41dc517"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6f4bb00fcbd87a359a90be8114a0814c2c01661c","unresolved":false,"context_lines":[{"line_number":3299,"context_line":"    except ValueError:"},{"line_number":3300,"context_line":"        raise"},{"line_number":3301,"context_line":"    if age_in_days \u003c 0:"},{"line_number":3302,"context_line":"        raise ValueError(_(\u0027Must supply a positive value for age\u0027))"},{"line_number":3303,"context_line":""},{"line_number":3304,"context_line":"    engine \u003d get_engine()"},{"line_number":3305,"context_line":"    session \u003d get_session()"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_3cb79df6","line":3302,"updated":"2015-01-26 23:07:38.000000000","message":"Please, specify that passing zero will purge all deleted data","commit_id":"7da02b8c3e30c84fe9c29a031b3a095ee41dc517"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"5c5d528eee571427422f3a1b45f6083b4280e5e3","unresolved":false,"context_lines":[{"line_number":36,"context_line":"import osprofiler.sqlalchemy"},{"line_number":37,"context_line":"import six"},{"line_number":38,"context_line":"import sqlalchemy"},{"line_number":39,"context_line":"from sqlalchemy import MetaData"},{"line_number":40,"context_line":"from sqlalchemy import or_"},{"line_number":41,"context_line":"from sqlalchemy.orm import joinedload, joinedload_all"},{"line_number":42,"context_line":"from sqlalchemy.orm import RelationshipProperty"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_d647ae49","line":39,"updated":"2015-02-03 11:40:38.000000000","message":"According to a Hacking rules, please, import modules, not objects","commit_id":"241c95fb6cee0a901634ceb0e6cedccdcb6a7c41"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"b285ec2d79c160a30613e24366261393243ad25d","unresolved":false,"context_lines":[{"line_number":36,"context_line":"import osprofiler.sqlalchemy"},{"line_number":37,"context_line":"import six"},{"line_number":38,"context_line":"import sqlalchemy"},{"line_number":39,"context_line":"from sqlalchemy import MetaData"},{"line_number":40,"context_line":"from sqlalchemy import or_"},{"line_number":41,"context_line":"from sqlalchemy.orm import joinedload, joinedload_all"},{"line_number":42,"context_line":"from sqlalchemy.orm import RelationshipProperty"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_8f026709","line":39,"in_reply_to":"da86d52c_d647ae49","updated":"2015-02-03 17:12:01.000000000","message":"The Hacking rules state an exception for sqlalchemy","commit_id":"241c95fb6cee0a901634ceb0e6cedccdcb6a7c41"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"f78f56980c01ce3994df2f241807d4716777037f","unresolved":false,"context_lines":[{"line_number":3297,"context_line":"    try:"},{"line_number":3298,"context_line":"        age_in_days \u003d int(age_in_days)"},{"line_number":3299,"context_line":"    except ValueError:"},{"line_number":3300,"context_line":"        raise"},{"line_number":3301,"context_line":"    if age_in_days \u003c 0:"},{"line_number":3302,"context_line":"        raise ValueError(_(\u0027Must supply a positive value for age,\u0027"},{"line_number":3303,"context_line":"                           \u00270 purges all deleted rows\u0027))"}],"source_content_type":"text/x-python","patch_set":13,"id":"da86d52c_7b9162f0","line":3300,"updated":"2015-02-04 05:06:45.000000000","message":"Why bother catching the exception just to raise?  Please log something useful.","commit_id":"435052bb9a8b9b95ea516fa4d30ab30fd7d3eb76"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"f78f56980c01ce3994df2f241807d4716777037f","unresolved":false,"context_lines":[{"line_number":3298,"context_line":"        age_in_days \u003d int(age_in_days)"},{"line_number":3299,"context_line":"    except ValueError:"},{"line_number":3300,"context_line":"        raise"},{"line_number":3301,"context_line":"    if age_in_days \u003c 0:"},{"line_number":3302,"context_line":"        raise ValueError(_(\u0027Must supply a positive value for age,\u0027"},{"line_number":3303,"context_line":"                           \u00270 purges all deleted rows\u0027))"},{"line_number":3304,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"da86d52c_5baf3e2a","line":3301,"updated":"2015-02-04 05:06:45.000000000","message":"Here you allow 0, but in the command line you don\u0027t.  Please choose one and be consistent.  If you do allow 0, please write that it purges all deleted rows in the command line argument help.","commit_id":"435052bb9a8b9b95ea516fa4d30ab30fd7d3eb76"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"f78f56980c01ce3994df2f241807d4716777037f","unresolved":false,"context_lines":[{"line_number":3328,"context_line":"                    .where(t.c.deleted_at \u003c deleted_age))"},{"line_number":3329,"context_line":"        except db_exc.DBError:"},{"line_number":3330,"context_line":"            LOG.exception(_LE(\u0027DBError detected when purging from \u0027"},{"line_number":3331,"context_line":"                              \u0027table\u003d%(table)s\u0027), {\u0027table\u0027: table})"},{"line_number":3332,"context_line":""},{"line_number":3333,"context_line":"        rows_purged \u003d result.rowcount"},{"line_number":3334,"context_line":"        LOG.info(_LI(\"Deleted %(row)d rows from table\u003d%(table)s\"),"}],"source_content_type":"text/x-python","patch_set":13,"id":"da86d52c_9ba53649","line":3331,"updated":"2015-02-04 05:06:45.000000000","message":"Don\u0027t want to raise an exception here?","commit_id":"435052bb9a8b9b95ea516fa4d30ab30fd7d3eb76"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"09c2afd597dcd9d699c95030b02493cb666de610","unresolved":false,"context_lines":[{"line_number":3297,"context_line":"    try:"},{"line_number":3298,"context_line":"        age_in_days \u003d int(age_in_days)"},{"line_number":3299,"context_line":"    except ValueError:"},{"line_number":3300,"context_line":"        LOG.exception(_LE(\u0027Invalid value for age\u0027))"},{"line_number":3301,"context_line":"        raise ValueError"},{"line_number":3302,"context_line":"    if age_in_days \u003c\u003d 0:"},{"line_number":3303,"context_line":"        LOG.exception(_LE(\u0027Must supply a positive value for age\u0027))"}],"source_content_type":"text/x-python","patch_set":14,"id":"da86d52c_87c23532","line":3300,"updated":"2015-02-04 08:17:58.000000000","message":"Please, add exception message to log","commit_id":"fbfcfa6b5f8740bc51542bd0a90408464fd98302"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"09c2afd597dcd9d699c95030b02493cb666de610","unresolved":false,"context_lines":[{"line_number":3298,"context_line":"        age_in_days \u003d int(age_in_days)"},{"line_number":3299,"context_line":"    except ValueError:"},{"line_number":3300,"context_line":"        LOG.exception(_LE(\u0027Invalid value for age\u0027))"},{"line_number":3301,"context_line":"        raise ValueError"},{"line_number":3302,"context_line":"    if age_in_days \u003c\u003d 0:"},{"line_number":3303,"context_line":"        LOG.exception(_LE(\u0027Must supply a positive value for age\u0027))"},{"line_number":3304,"context_line":"        raise ValueError"}],"source_content_type":"text/x-python","patch_set":14,"id":"da86d52c_477e7d1a","line":3301,"updated":"2015-02-04 08:17:58.000000000","message":"Raising exception without a message makes it painful in a debugging time. You could use just \u0027raise\u0027 here to re-raise exception","commit_id":"fbfcfa6b5f8740bc51542bd0a90408464fd98302"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"09c2afd597dcd9d699c95030b02493cb666de610","unresolved":false,"context_lines":[{"line_number":3301,"context_line":"        raise ValueError"},{"line_number":3302,"context_line":"    if age_in_days \u003c\u003d 0:"},{"line_number":3303,"context_line":"        LOG.exception(_LE(\u0027Must supply a positive value for age\u0027))"},{"line_number":3304,"context_line":"        raise ValueError"},{"line_number":3305,"context_line":""},{"line_number":3306,"context_line":"    engine \u003d get_engine()"},{"line_number":3307,"context_line":"    session \u003d get_session()"}],"source_content_type":"text/x-python","patch_set":14,"id":"da86d52c_a771f945","line":3304,"updated":"2015-02-04 08:17:58.000000000","message":"Same here","commit_id":"fbfcfa6b5f8740bc51542bd0a90408464fd98302"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"09c2afd597dcd9d699c95030b02493cb666de610","unresolved":false,"context_lines":[{"line_number":3330,"context_line":"        except db_exc.DBReferenceError:"},{"line_number":3331,"context_line":"            LOG.exception(_LE(\u0027DBError detected when purging from \u0027"},{"line_number":3332,"context_line":"                              \u0027table\u003d%(table)s\u0027), {\u0027table\u0027: table})"},{"line_number":3333,"context_line":"            raise db_exc.DBReferenceError"},{"line_number":3334,"context_line":""},{"line_number":3335,"context_line":"        rows_purged \u003d result.rowcount"},{"line_number":3336,"context_line":"        LOG.info(_LI(\"Deleted %(row)d rows from table\u003d%(table)s\"),"}],"source_content_type":"text/x-python","patch_set":14,"id":"da86d52c_e7e4d1e2","line":3333,"updated":"2015-02-04 08:17:58.000000000","message":"And here","commit_id":"fbfcfa6b5f8740bc51542bd0a90408464fd98302"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"51004ab5a77fbc45919ec3056f64c6f52627cd60","unresolved":false,"context_lines":[{"line_number":3298,"context_line":"        age_in_days \u003d int(age_in_days)"},{"line_number":3299,"context_line":"    except ValueError:"},{"line_number":3300,"context_line":"        LOG.exception(_LE(\u0027Invalid value for age, %(age)s\u0027),"},{"line_number":3301,"context_line":"                      {\u0027age\u0027: age_in_days})"},{"line_number":3302,"context_line":"        raise ValueError(_(\u0027Invalid value for age, %(age)s\u0027),"},{"line_number":3303,"context_line":"                         {\u0027age\u0027: age_in_days})"},{"line_number":3304,"context_line":"    if age_in_days \u003c\u003d 0:"}],"source_content_type":"text/x-python","patch_set":15,"id":"da86d52c_7e421969","line":3301,"updated":"2015-02-04 16:42:49.000000000","message":"Could you, please, remove duplicated strings in logging and exception raising? I\u0027ll be happy +2 it once it fixed","commit_id":"fe7d9ddaa6f57a9faa66308f25b2291ced929b63"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"9b58f451a988a752635e9676d3b05104189c77e0","unresolved":false,"context_lines":[{"line_number":3302,"context_line":"        raise"},{"line_number":3303,"context_line":"    if age_in_days \u003c\u003d 0:"},{"line_number":3304,"context_line":"        LOG.exception(_LE(\u0027Must supply a positive value for age\u0027))"},{"line_number":3305,"context_line":"        raise ValueError"},{"line_number":3306,"context_line":""},{"line_number":3307,"context_line":"    engine \u003d get_engine()"},{"line_number":3308,"context_line":"    session \u003d get_session()"}],"source_content_type":"text/x-python","patch_set":16,"id":"da86d52c_6ff50156","line":3305,"updated":"2015-02-04 17:42:14.000000000","message":"Please, don\u0027t raise exception without error message","commit_id":"b992221294a554e46a8b1afbee820debfe5e6ad1"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"41535e22e7170ab3d4b1292f5e484577908cff55","unresolved":false,"context_lines":[{"line_number":3297,"context_line":"    try:"},{"line_number":3298,"context_line":"        age_in_days \u003d int(age_in_days)"},{"line_number":3299,"context_line":"    except ValueError:"},{"line_number":3300,"context_line":"        LOG.exception(_LE(\u0027Invalid value for age, %(age)s\u0027),"},{"line_number":3301,"context_line":"                      {\u0027age\u0027: age_in_days})"},{"line_number":3302,"context_line":"        raise"},{"line_number":3303,"context_line":"    if age_in_days \u003c\u003d 0:"}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_feec44bd","line":3300,"updated":"2015-02-04 18:22:09.000000000","message":"As decided in IRC with thingee, it\u0027s our API so we need to raise specific exception. In current cases, InvalidParameterValue will be better than re-raising ValueError","commit_id":"f27fa290711bfa9c2e79b34be597817de956e651"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"41535e22e7170ab3d4b1292f5e484577908cff55","unresolved":false,"context_lines":[{"line_number":3301,"context_line":"                      {\u0027age\u0027: age_in_days})"},{"line_number":3302,"context_line":"        raise"},{"line_number":3303,"context_line":"    if age_in_days \u003c\u003d 0:"},{"line_number":3304,"context_line":"        LOG.exception(_LE(\u0027Must supply a positive value for age\u0027))"},{"line_number":3305,"context_line":"        raise ValueError(_(\u0027Must supply a positive value for age\u0027))"},{"line_number":3306,"context_line":""},{"line_number":3307,"context_line":"    engine \u003d get_engine()"}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_fe1a84c4","line":3304,"updated":"2015-02-04 18:22:09.000000000","message":"msg \u003d \u0027Must supply a positive value for age\u0027\nLOG.exception(_LE(msg))\nraise exceptions.InvalidParameterValue(_(msg))","commit_id":"f27fa290711bfa9c2e79b34be597817de956e651"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"7b838d78bf4281fa8c67de3fd2700bbce857ea88","unresolved":false,"context_lines":[{"line_number":3297,"context_line":"    try:"},{"line_number":3298,"context_line":"        age_in_days \u003d int(age_in_days)"},{"line_number":3299,"context_line":"    except ValueError:"},{"line_number":3300,"context_line":"        msg \u003d _LE(\u0027Invalid valude for age, %(age)s\u0027)"},{"line_number":3301,"context_line":"        LOG.exception(msg, {\u0027age\u0027: age_in_days})"},{"line_number":3302,"context_line":"        raise exception.InvalidParameterValue(msg % {\u0027age\u0027: age_in_days})"},{"line_number":3303,"context_line":"    if age_in_days \u003c\u003d 0:"}],"source_content_type":"text/x-python","patch_set":20,"id":"da86d52c_032a998f","line":3300,"updated":"2015-02-04 21:29:52.000000000","message":"If a message is being used for both logging and for an exception it should use _(), not _LE().","commit_id":"7587e194a05f43cb2d654c0287bfdd00dd28302e"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"7b838d78bf4281fa8c67de3fd2700bbce857ea88","unresolved":false,"context_lines":[{"line_number":3301,"context_line":"        LOG.exception(msg, {\u0027age\u0027: age_in_days})"},{"line_number":3302,"context_line":"        raise exception.InvalidParameterValue(msg % {\u0027age\u0027: age_in_days})"},{"line_number":3303,"context_line":"    if age_in_days \u003c\u003d 0:"},{"line_number":3304,"context_line":"        msg \u003d _LE(\u0027Must supply a positive value for age\u0027)"},{"line_number":3305,"context_line":"        LOG.exception(msg)"},{"line_number":3306,"context_line":"        raise exception.InvalidParameterValue(msg)"},{"line_number":3307,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"da86d52c_431f316c","line":3304,"updated":"2015-02-04 21:29:52.000000000","message":"Same","commit_id":"7587e194a05f43cb2d654c0287bfdd00dd28302e"}],"cinder/tests/db/test_purge.py":[{"author":{"_account_id":7860,"name":"Ryan Lucio","email":"ryan.lucio@fireeye.com","username":"rlucio"},"change_message_id":"1b6fe6daaadcb6d0a87a8f6847398d395e575d8b","unresolved":false,"context_lines":[{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class PurgeDeletedTest(test.TestCase):"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def setUp(self):"},{"line_number":37,"context_line":"        super(PurgeDeletedTest, self).setUp()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_e88e6126","line":34,"updated":"2015-01-14 23:56:32.000000000","message":"If my comments on the db api function are correct then you should add a case where invalid age data or no age data is passed.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":12092,"name":"Abel Lopez","email":"abelopez@cisco.com","username":"alopgeek"},"change_message_id":"d9d0a5da2c6959a361ef35e38a491245b4935046","unresolved":false,"context_lines":[{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class PurgeDeletedTest(test.TestCase):"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def setUp(self):"},{"line_number":37,"context_line":"        super(PurgeDeletedTest, self).setUp()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_d70596c2","line":34,"in_reply_to":"3a961159_e88e6126","updated":"2015-01-15 22:06:20.000000000","message":"The values are checked in cmd/manage.py, which is the intended usage. it checks there that the user passed an int, and that it\u0027s greater than 0. I suppose that I could replicate this check in the api layer as well, but I\u0027d rather first get this functionality in at the cinder-manage layer.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        for unused in range(6):"},{"line_number":46,"context_line":"            self.uuidstrs.append(uuid.uuid4().hex)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    def test_purge_deleted_rows(self):"},{"line_number":49,"context_line":"        # Add 6 rows to table"},{"line_number":50,"context_line":"        for uuidstr in self.uuidstrs:"},{"line_number":51,"context_line":"            ins_stmt \u003d self.volumes.insert().values(id\u003duuidstr)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_ef4f128a","line":48,"updated":"2015-01-16 06:49:17.000000000","message":"Did you test foreign key condition here?","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        db.purge_deleted_rows(self.context, age\u003d30)"},{"line_number":66,"context_line":"        rows \u003d self.session.query(self.volumes).count()"},{"line_number":67,"context_line":"        # Verify that we only deleted 2"},{"line_number":68,"context_line":"        self.assertEqual(rows, 4)"},{"line_number":69,"context_line":"        # Purge at 10 days old now, should delete 2 more rows"},{"line_number":70,"context_line":"        db.purge_deleted_rows(self.context, age\u003d10)"},{"line_number":71,"context_line":"        rows \u003d self.session.query(self.volumes).count()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_0f4b7e87","line":68,"updated":"2015-01-16 06:49:17.000000000","message":"Expected value should be the first parameter.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"7e60002f88b08a9e1331b41717410336bbf7524e","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        db.purge_deleted_rows(self.context, age\u003d10)"},{"line_number":71,"context_line":"        rows \u003d self.session.query(self.volumes).count()"},{"line_number":72,"context_line":"        # Verify that we only have 2 rows now"},{"line_number":73,"context_line":"        self.assertEqual(rows, 2)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_4f410662","line":73,"updated":"2015-01-16 06:49:17.000000000","message":"Same as above.","commit_id":"14d815a2d088153e54b6dccd80c947e11e9730fc"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"08016bfb8b8ee80af44845da31e27158e9a1c115","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from cinder.openstack.common import log as logging"},{"line_number":26,"context_line":"from cinder import test"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"from oslo.db.sqlalchemy import utils as sqlalchemyutils"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_3974c29b","line":28,"updated":"2015-01-21 08:10:41.000000000","message":"./cinder/tests/db/test_purge.py:28:1: N333  \u0027from oslo_db.sqlalchemy import utils as sqlalchemyutils\u0027 must be used instead of \u0027from oslo.db.sqlalchemy import utils as sqlalchemyutils\u0027.","commit_id":"0b77ff67fb9197c4c4a42252457bb79125f968f1"},{"author":{"_account_id":13636,"name":"aarefiev","display_name":"Anton","email":"arefyevanton@gmail.com","username":"aarefiev"},"change_message_id":"09e21a25e52b554cd93bb54aecce96de02343068","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        for unused in range(6):"},{"line_number":49,"context_line":"            self.uuidstrs.append(uuid.uuid4().hex)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def test_purge_deleted_rows(self):"},{"line_number":52,"context_line":"        # Add 6 rows to table"},{"line_number":53,"context_line":"        for uuidstr in self.uuidstrs:"},{"line_number":54,"context_line":"            ins_stmt \u003d self.volumes.insert().values(id\u003duuidstr)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_03f6b4d7","line":51,"updated":"2015-01-21 09:58:39.000000000","message":"I think you should split this test into several smaller. Proper unit tests should fail for exactly one reason, that’s why you should be using one assert( no more 3) per unit test.","commit_id":"0b77ff67fb9197c4c4a42252457bb79125f968f1"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"6f4bb00fcbd87a359a90be8114a0814c2c01661c","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (C) 2013 Hewlett-Packard Development Company, L.P."},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_dc92194b","line":1,"updated":"2015-01-26 23:07:38.000000000","message":"Wrong copyrights","commit_id":"7da02b8c3e30c84fe9c29a031b3a095ee41dc517"}]}