)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"b88ea5d22ebb1da57b44b63994de8880e26d2653","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Deleting the expired execution in chunk"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If there is a huge number of expired executions, only one"},{"line_number":10,"context_line":"sqlalchemy session could not afford it. This patch will"},{"line_number":11,"context_line":"truncate the expried executions in chunk."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I894db17b1c2fc5c292e7979ab46ea00fd14ff833"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5a3905b3_5c0af726","line":10,"range":{"start_line":9,"start_character":49,"end_line":10,"end_character":38},"updated":"2017-01-24 15:56:33.000000000","message":"IMO the wording is a bit confusing. The bug says there is a timeout, so maybe consider writing it here instead (only if you want to).","commit_id":"92c7ee6c4a434def0c72668ef2486e5a184b71a6"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"c56b3cdfbe8df6a990187ae3f0cd48e3d9947aff","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Deleting the expired execution in chunk"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If there is a huge number of expired executions, only one"},{"line_number":10,"context_line":"sqlalchemy session could not afford it. This patch will"},{"line_number":11,"context_line":"truncate the expried executions in chunk."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I894db17b1c2fc5c292e7979ab46ea00fd14ff833"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5a3905b3_a633c512","line":10,"range":{"start_line":9,"start_character":49,"end_line":10,"end_character":38},"in_reply_to":"5a3905b3_5c0af726","updated":"2017-01-24 16:04:45.000000000","message":"Valid point Mike :). Thanks for reviewing:)","commit_id":"92c7ee6c4a434def0c72668ef2486e5a184b71a6"}],"mistral/config.py":[{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"3a9f75c69bbe8aa28c9680afb10b5a3f7260d331","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":230,"context_line":"               \u0027The default value is None. If it is default\u0027"},{"line_number":231,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":232,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":233,"context_line":"    )"},{"line_number":234,"context_line":"]"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_40c87374","line":232,"updated":"2017-01-31 12:00:39.000000000","message":"Wouldn\u0027t it be safer for us to default to set number? I can\u0027t see why it would be a problem to enable it by default? It would avoid people finding the setting when it is too late.","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"5599f8f5342603a8d4214529cdeaf011036e606e","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":230,"context_line":"               \u0027The default value is None. If it is default\u0027"},{"line_number":231,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":232,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":233,"context_line":"    )"},{"line_number":234,"context_line":"]"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1a430d35_2a9d4fbc","line":232,"in_reply_to":"1a430d35_0b639eb2","updated":"2017-02-07 07:48:36.000000000","message":"IMHO, it depends on concrete scenarios. There is no information about this tuning in sqlalchemy. This behavior of tuning is also sensitive since in sqlalchemy, there is also a default value of worker is 5, but it can be changed due to the purpose of tuning based on specific scenarios and of course no examples for it.\n\nI tried to find some use cases but there is no information about number of entities in one session of sqlalchemy. Therefore, i would like to put it None as default (it means this feature does not affect to normal behavior). If in specific scenarios, people want to tune it in providing batch, it depends on their specific cases.","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"aa2ad7e481305ebf454a26356362e786e7982e83","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":230,"context_line":"               \u0027The default value is None. If it is default\u0027"},{"line_number":231,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":232,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":233,"context_line":"    )"},{"line_number":234,"context_line":"]"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1a430d35_61621040","line":232,"in_reply_to":"1a430d35_2a9d4fbc","updated":"2017-02-07 14:15:01.000000000","message":"Wouldn\u0027t it be better if default value was \u00270\u0027? Therefore it would have consistent type no matter if someone wants to delete in chunks or not.","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"f7e3c2ed85dcd18290648a9569f2a40aa5af76e8","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":230,"context_line":"               \u0027The default value is None. If it is default\u0027"},{"line_number":231,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":232,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":233,"context_line":"    )"},{"line_number":234,"context_line":"]"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1a430d35_39d118fb","line":232,"in_reply_to":"1a430d35_61621040","updated":"2017-02-07 15:40:42.000000000","message":"It makes sense with 0 value. I am going to update the new patch","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"85630646274fbe41b8414311cd2ca1e405c3b4d4","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":230,"context_line":"               \u0027The default value is None. If it is default\u0027"},{"line_number":231,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":232,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":233,"context_line":"    )"},{"line_number":234,"context_line":"]"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_ecc31936","line":232,"in_reply_to":"3a461143_40c87374","updated":"2017-01-31 12:59:30.000000000","message":"From very beginning i made it as mandatory however i think that this topic only happens in a huge expired executions which have big size (in our telco case), otherwise, working as previous behavior does not show any problems and we do not change any thing.\n\nWhat do you think about it?","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"001ab4ee19b46e4d28a400f62814116d1634b25e","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":230,"context_line":"               \u0027The default value is None. If it is default\u0027"},{"line_number":231,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":232,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":233,"context_line":"    )"},{"line_number":234,"context_line":"]"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1a430d35_0b639eb2","line":232,"in_reply_to":"3a461143_ecc31936","updated":"2017-02-03 16:32:34.000000000","message":"I don\u0027t have a strong opinion. Is there any reason that this would be a bad idea even if you are deleting a small number each time? If there is no downside then I think we should have a default. What size of chunk do you plan on using?","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"ef5607c7a6d21ba28089d28b77fd1733d38b39b6","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is None. If it is default\u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a430d35_5192d4f7","line":231,"range":{"start_line":231,"start_character":16,"end_line":231,"end_character":59},"updated":"2017-02-09 15:14:08.000000000","message":"The default value is 0. If it is set to 0","commit_id":"32bd54cd810cf990a23fb976bfe0b6957eaec198"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"1a8413fa62ea64d1f0b49760b4fd70816a5ae067","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is None. If it is default\u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a430d35_ba858d5a","line":231,"range":{"start_line":231,"start_character":16,"end_line":231,"end_character":59},"in_reply_to":"1a430d35_5192d4f7","updated":"2017-02-09 15:53:00.000000000","message":"Done","commit_id":"32bd54cd810cf990a23fb976bfe0b6957eaec198"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"88d63e88cbcf916b667065635dedc1635fa09ba5","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"},{"line_number":235,"context_line":"]"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_5a4829cd","line":233,"range":{"start_line":231,"start_character":40,"end_line":233,"end_character":44},"updated":"2017-02-09 16:03:40.000000000","message":"is this right? am I missing something?","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"fa59253684f61ab1cf97308ba16e392b30916a04","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"},{"line_number":235,"context_line":"]"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_a0ce40b5","line":233,"range":{"start_line":231,"start_character":40,"end_line":233,"end_character":44},"in_reply_to":"1a430d35_0502c66f","updated":"2017-02-09 16:35:51.000000000","message":"well, until some UX expert will join the project, we\u0027ll have to live with it. haha.","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"ab6d29c65dcecb4986ee5da18a38f1b440dcc7b9","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"},{"line_number":235,"context_line":"]"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_0502c66f","line":233,"range":{"start_line":231,"start_character":40,"end_line":233,"end_character":44},"in_reply_to":"1a430d35_05ce46b4","updated":"2017-02-09 16:23:34.000000000","message":"No you are rite about it does not seem very intuitive literally :D but if we set it is 1, therefore for each of item in database, sqlalchemy will open one session to delete 1. It is different to 0, check the code then you will see the different","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"775516dc0556f5888f3d0bf15d6cab657ce339a7","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"},{"line_number":235,"context_line":"]"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_05ce46b4","line":233,"range":{"start_line":231,"start_character":40,"end_line":233,"end_character":44},"in_reply_to":"1a430d35_453daeba","updated":"2017-02-09 16:17:12.000000000","message":"I think it\u0027s not very intuitive, but I don\u0027t want to block the fix.","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"365fe02047814e69956669ad4895294976bd87e0","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"},{"line_number":235,"context_line":"]"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_ba4d8d9b","line":233,"range":{"start_line":231,"start_character":40,"end_line":233,"end_character":44},"in_reply_to":"1a430d35_5a4829cd","updated":"2017-02-09 16:08:40.000000000","message":"Hi Mike, Yes it is. We need something consistent therefore value of 0 is better than None.","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"7501c082045200333695d6bc86e7f0f20201f458","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"},{"line_number":235,"context_line":"]"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_d7506f0d","line":233,"range":{"start_line":231,"start_character":40,"end_line":233,"end_character":44},"in_reply_to":"1a430d35_a0ce40b5","updated":"2017-02-09 17:11:33.000000000","message":"Well yeah, cest la vie:)","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"81f3205a10d242f9e35c8142d67849cc6c5f12fb","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"},{"line_number":235,"context_line":"]"},{"line_number":236,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_453daeba","line":233,"range":{"start_line":231,"start_character":40,"end_line":233,"end_character":44},"in_reply_to":"1a430d35_ba4d8d9b","updated":"2017-02-09 16:12:26.000000000","message":"so if it is 0, it is possible more will be deleted, compared to what would have been deleted if the value was 1?","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":225,"context_line":"               \u0027( SUCCESS / ERROR ).\u0027)"},{"line_number":226,"context_line":"    ),"},{"line_number":227,"context_line":"    cfg.IntOpt("},{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_60928b59","line":228,"range":{"start_line":228,"start_character":9,"end_line":228,"end_character":23},"updated":"2017-02-10 05:19:40.000000000","message":"Change \"chunk\" to \"batch\". \"Chunk\" looks a little bit sloppy. \"batch\" is a standard term used in programming for cases like this.\n\nActually, I almost insist that we call it simply \"batch_size\". There won\u0027t be any confusions about the meaning because all expiration policy is about deletion. And the word \"size\" should present, it\u0027s more informative.","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":227,"context_line":"    cfg.IntOpt("},{"line_number":228,"context_line":"        \u0027deletion_chunk\u0027,"},{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_a09c836a","line":230,"range":{"start_line":230,"start_character":41,"end_line":230,"end_character":50},"updated":"2017-02-10 05:19:40.000000000","message":"-\u003e executions (plural)","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        default\u003d0,"},{"line_number":230,"context_line":"        help\u003d_(\u0027Size of batch of expired execution to be deleted.\u0027"},{"line_number":231,"context_line":"               \u0027The default value is 0. If it is set to 0, \u0027"},{"line_number":232,"context_line":"               \u0027size of batch is total number of expired chunk\u0027"},{"line_number":233,"context_line":"               \u0027that is going to be deleted.\u0027)"},{"line_number":234,"context_line":"    )"},{"line_number":235,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_c0b5f7d0","line":232,"range":{"start_line":232,"start_character":57,"end_line":232,"end_character":62},"updated":"2017-02-10 05:19:40.000000000","message":"-\u003e executions","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"}],"mistral/db/v2/sqlalchemy/api.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"3a7e69fd7e1d01f54b3b678c867a65c2f0d1f32c","unresolved":false,"context_lines":[{"line_number":1052,"context_line":"            models.WorkflowExecution.state \u003d\u003d states.CANCELLED"},{"line_number":1053,"context_line":"        )"},{"line_number":1054,"context_line":"    )"},{"line_number":1055,"context_line":"    if limit:"},{"line_number":1056,"context_line":"        query \u003d query.limit(limit)"},{"line_number":1057,"context_line":""},{"line_number":1058,"context_line":"    return query.all()"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba2be162_c6c29752","line":1055,"updated":"2017-03-06 07:52:49.000000000","message":"Soft: we usually insert blank lines before all control structures like \"if\", \"try\", \"return\" etc.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"}],"mistral/services/expiration_policy.py":[{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"b88ea5d22ebb1da57b44b63994de8880e26d2653","unresolved":false,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":73,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":74,"context_line":"    execution_deleted_groups \u003d _group_into_chunk(total_exp_executions,"},{"line_number":75,"context_line":"                                                deletion_chunk)"},{"line_number":76,"context_line":"    for execution_group in execution_deleted_groups:"},{"line_number":77,"context_line":"        with db_api.transaction():"},{"line_number":78,"context_line":"            # TODO(gpaz): In the future should use generic method with"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a3905b3_bcc91b28","line":75,"range":{"start_line":74,"start_character":31,"end_line":75,"end_character":63},"updated":"2017-01-24 15:56:33.000000000","message":"There is a code convention in mistral on how a call for a function should look. If a function call takes more than on line each argument should be in a new line and the closing brackets should be in a new line as well (see example in lines 83-88).\n\nthe call should look like so:\nexecution_deleted_groups \u003d _group_into_chunk(\n    total_exp_executions,\n    deletion_chunk\n)","commit_id":"92c7ee6c4a434def0c72668ef2486e5a184b71a6"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"a0f0d25692001aeaceaee8563809a1577dac2144","unresolved":false,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":73,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":74,"context_line":"    execution_deleted_groups \u003d _group_into_chunk(total_exp_executions,"},{"line_number":75,"context_line":"                                                deletion_chunk)"},{"line_number":76,"context_line":"    for execution_group in execution_deleted_groups:"},{"line_number":77,"context_line":"        with db_api.transaction():"},{"line_number":78,"context_line":"            # TODO(gpaz): In the future should use generic method with"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a3905b3_69fb6e47","line":75,"range":{"start_line":74,"start_character":31,"end_line":75,"end_character":63},"in_reply_to":"5a3905b3_6680fd8b","updated":"2017-01-24 16:15:24.000000000","message":"It\u0027s fine to push code while it\u0027s still in progress (e.g not all tests pass yet), but you should mark it as work in progress (http://pasteboard.co/eaoosT0R.png).\n\nThat way people can see your code early and supply feedback, while they know it is not fully ready yet. This allows those who don\u0027t won\u0027t to review before it\u0027s ready the option to skip it for now.","commit_id":"92c7ee6c4a434def0c72668ef2486e5a184b71a6"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"c56b3cdfbe8df6a990187ae3f0cd48e3d9947aff","unresolved":false,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":73,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":74,"context_line":"    execution_deleted_groups \u003d _group_into_chunk(total_exp_executions,"},{"line_number":75,"context_line":"                                                deletion_chunk)"},{"line_number":76,"context_line":"    for execution_group in execution_deleted_groups:"},{"line_number":77,"context_line":"        with db_api.transaction():"},{"line_number":78,"context_line":"            # TODO(gpaz): In the future should use generic method with"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a3905b3_6680fd8b","line":75,"range":{"start_line":74,"start_character":31,"end_line":75,"end_character":63},"in_reply_to":"5a3905b3_bcc91b28","updated":"2017-01-24 16:04:45.000000000","message":"Thank Michal, i had problem with my local tox environment then i had to submit the patch without any testings :(.","commit_id":"92c7ee6c4a434def0c72668ef2486e5a184b71a6"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"2d51a271c459ab5d6a369b6120310baad5cd67f9","unresolved":false,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":73,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":74,"context_line":"    execution_deleted_groups \u003d _group_into_chunk(total_exp_executions,"},{"line_number":75,"context_line":"                                                 deletion_chunk)"},{"line_number":76,"context_line":"    for execution_group in execution_deleted_groups:"},{"line_number":77,"context_line":"        with db_api.transaction():"},{"line_number":78,"context_line":"            # TODO(gpaz): In the future should use generic method with"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a461143_6b0fb6e1","line":75,"range":{"start_line":74,"start_character":0,"end_line":75,"end_character":64},"updated":"2017-01-26 08:40:02.000000000","message":"the code should look like this:\n\n    execution_deleted_groups \u003d _group_into_chunk(\n        total_exp_executions,\n        deletion_chunk\n    )\n\nsorry for nitpicking, it\u0027s just that I like that function calls looks the same everywhere (you can look at other function calls in this file. for example lines 91, 110 and 117)\n\nI know it is different in other projects, but this is the code standard in mistral.","commit_id":"d94d279e8469f5ad860fdc941fe30b28c2d8bb2b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"2a1a0d547ba077c356920826efaa00119e03be1d","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                - datetime.timedelta(minutes\u003dolder_than))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is None"},{"line_number":106,"context_line":"    # If it is not provided, size of batch will be the size"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_778e222f","line":103,"updated":"2017-01-31 01:36:53.000000000","message":"From your reply, if the problem lies in query large number of executions, then the problem still exists here.","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"3a9f75c69bbe8aa28c9680afb10b5a3f7260d331","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                - datetime.timedelta(minutes\u003dolder_than))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is None"},{"line_number":106,"context_line":"    # If it is not provided, size of batch will be the size"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_c0fd436f","line":103,"in_reply_to":"3a461143_778e222f","updated":"2017-01-31 12:00:39.000000000","message":"Done","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"5090917340c174981854d1a5157f72ce6ab39782","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                - datetime.timedelta(minutes\u003dolder_than))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is None"},{"line_number":106,"context_line":"    # If it is not provided, size of batch will be the size"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_aa501833","line":103,"in_reply_to":"3a461143_778e222f","updated":"2017-01-31 11:16:42.000000000","message":"Well, it is actually the db behavior, if mysql or sqlite3 or something else slow in reaction, it is itself behavior and i think we could not change it.\n\nThe only thing we can change here is the number of batch in exp_execution deletion via sqlalchemy since previously, we only opened one session to delete a huge number of exp_executions (each of them has also big size)\n\nThis topic does not state that whether or not it is the problem of sqlalchemy or database. It is just the refactor of behavior of dealing with deletion in batch.","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"3a9f75c69bbe8aa28c9680afb10b5a3f7260d331","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                - datetime.timedelta(minutes\u003dolder_than))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is None"},{"line_number":106,"context_line":"    # If it is not provided, size of batch will be the size"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_0043cbb2","line":103,"in_reply_to":"3a461143_aa501833","updated":"2017-01-31 12:00:39.000000000","message":"Wouldn\u0027t it be better to change the get_expired_executions method and add a SQL LIMIT that is the size of the deletion_chunk? Then the SELECT and the DELETE will be much smaller and quicker.","commit_id":"72aa086f615309772b980c5914a515d9405a14d8"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"fa59253684f61ab1cf97308ba16e392b30916a04","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is None"},{"line_number":106,"context_line":"    # If it is not provided, size of batch will be the size"},{"line_number":107,"context_line":"    # of totoal number of expired executions."},{"line_number":108,"context_line":"    if deletion_chunk:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_20125059","line":105,"range":{"start_line":105,"start_character":45,"end_line":105,"end_character":49},"updated":"2017-02-09 16:35:51.000000000","message":"soft: should be 0","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"7501c082045200333695d6bc86e7f0f20201f458","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is None"},{"line_number":106,"context_line":"    # If it is not provided, size of batch will be the size"},{"line_number":107,"context_line":"    # of totoal number of expired executions."},{"line_number":108,"context_line":"    if deletion_chunk:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_170d9737","line":105,"range":{"start_line":105,"start_character":45,"end_line":105,"end_character":49},"in_reply_to":"1a430d35_20125059","updated":"2017-02-09 17:11:33.000000000","message":"Done","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"fa59253684f61ab1cf97308ba16e392b30916a04","unresolved":false,"context_lines":[{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is None"},{"line_number":106,"context_line":"    # If it is not provided, size of batch will be the size"},{"line_number":107,"context_line":"    # of totoal number of expired executions."},{"line_number":108,"context_line":"    if deletion_chunk:"},{"line_number":109,"context_line":"        execution_deleted_groups \u003d _group_into_chunk("}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_e00da8e9","line":106,"range":{"start_line":106,"start_character":9,"end_line":106,"end_character":11},"updated":"2017-02-09 16:35:51.000000000","message":"soft: it -\u003e other value","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"7501c082045200333695d6bc86e7f0f20201f458","unresolved":false,"context_lines":[{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is None"},{"line_number":106,"context_line":"    # If it is not provided, size of batch will be the size"},{"line_number":107,"context_line":"    # of totoal number of expired executions."},{"line_number":108,"context_line":"    if deletion_chunk:"},{"line_number":109,"context_line":"        execution_deleted_groups \u003d _group_into_chunk("}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_b7fda30a","line":106,"range":{"start_line":106,"start_character":9,"end_line":106,"end_character":11},"in_reply_to":"1a430d35_e00da8e9","updated":"2017-02-09 17:11:33.000000000","message":"Done","commit_id":"422f07ec12dc25e6c20b241189b7cb7d1507cc40"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                      \"is not configured or older_than \u003c \u00271\u0027.\")"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def _group_into_chunk(total_executions, chunk_num):"},{"line_number":61,"context_line":"    return (total_executions[i:i + chunk_num]"},{"line_number":62,"context_line":"            for i in range(0, len(total_executions), chunk_num))"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_603c4b30","line":60,"range":{"start_line":60,"start_character":16,"end_line":60,"end_character":21},"updated":"2017-02-10 05:19:40.000000000","message":"-\u003e batch","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                      \"is not configured or older_than \u003c \u00271\u0027.\")"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def _group_into_chunk(total_executions, chunk_num):"},{"line_number":61,"context_line":"    return (total_executions[i:i + chunk_num]"},{"line_number":62,"context_line":"            for i in range(0, len(total_executions), chunk_num))"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_a04643bd","line":60,"range":{"start_line":60,"start_character":40,"end_line":60,"end_character":45},"updated":"2017-02-10 05:19:40.000000000","message":"-\u003e batch","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def _group_into_chunk(total_executions, chunk_num):"},{"line_number":61,"context_line":"    return (total_executions[i:i + chunk_num]"},{"line_number":62,"context_line":"            for i in range(0, len(total_executions), chunk_num))"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_80aa1f18","line":61,"range":{"start_line":61,"start_character":12,"end_line":61,"end_character":17},"updated":"2017-02-10 05:19:40.000000000","message":"total -\u003e all (better english), \"total\" is usually an adjective meaning slightly different thing.","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":62,"context_line":"            for i in range(0, len(total_executions), chunk_num))"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"def _delete_exp_execs(exp_executions):"},{"line_number":66,"context_line":"    with db_api.transaction():"},{"line_number":67,"context_line":"        # TODO(gpaz): In the future should use generic method with"},{"line_number":68,"context_line":"        # filters params and not specific method that filter by time."}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_1b2b0432","line":65,"range":{"start_line":65,"start_character":4,"end_line":65,"end_character":21},"updated":"2017-02-10 05:19:40.000000000","message":"-\u003e delete_expired_executions. We don\u0027t shorten method names except using well-known acronyms.","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        # filters params and not specific method that filter by time."},{"line_number":69,"context_line":"        for execution in exp_executions:"},{"line_number":70,"context_line":"            try:"},{"line_number":71,"context_line":"                # Setup project_id for _secure_query delete execution."},{"line_number":72,"context_line":"                ctx \u003d auth_ctx.MistralContext("},{"line_number":73,"context_line":"                    user_id\u003dNone,"},{"line_number":74,"context_line":"                    project_id\u003dexecution.project_id,"},{"line_number":75,"context_line":"                    auth_token\u003dNone,"},{"line_number":76,"context_line":"                    is_admin\u003dTrue"},{"line_number":77,"context_line":"                )"},{"line_number":78,"context_line":"                auth_ctx.set_ctx(ctx)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"                LOG.debug("},{"line_number":81,"context_line":"                    \u0027DELETE execution id : %s from date : %s \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_dbbeccd3","line":78,"range":{"start_line":71,"start_character":16,"end_line":78,"end_character":37},"updated":"2017-02-10 05:19:40.000000000","message":"DB transaction needs to be as short as possible. We need to move manipulations with auth context out of transaction scope. \"is_admin\" should be enough so that a DB layer allows to perform the operation, hence no need to do it in a loop for every execution object. However, not all necessary changes are yet made in DB layer itself. I understand that this issue comes from the previous implementation so it\u0027s just a note to fix it later. Please leave a TODO for this.","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"def run_execution_expiration_policy(self, ctx):"},{"line_number":96,"context_line":"    LOG.debug(\"Starting expiration policy task.\")"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    older_than \u003d CONF.execution_expiration_policy.older_than"},{"line_number":99,"context_line":"    exp_time \u003d (datetime.datetime.now()"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_5bdadc79","line":96,"range":{"start_line":96,"start_character":42,"end_line":96,"end_character":46},"updated":"2017-02-10 05:19:40.000000000","message":"I think we need to remove the word \"task\" from this sentence so that we don\u0027t confuse it with workflow task.","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    exp_time \u003d (datetime.datetime.now()"},{"line_number":100,"context_line":"                - datetime.timedelta(minutes\u003dolder_than))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is 0"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_3b960895","line":102,"range":{"start_line":102,"start_character":4,"end_line":102,"end_character":18},"updated":"2017-02-10 05:19:40.000000000","message":"-\u003e batch_size","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    deletion_chunk \u003d CONF.execution_expiration_policy.deletion_chunk"},{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is 0"},{"line_number":106,"context_line":"    # If other value not provided, size of batch will be the size"},{"line_number":107,"context_line":"    # of total number of expired executions."},{"line_number":108,"context_line":"    if deletion_chunk:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_1baa24d5","line":105,"range":{"start_line":105,"start_character":27,"end_line":105,"end_character":41},"updated":"2017-02-10 05:19:40.000000000","message":"All comments and docstrings must be compliant with regular English grammar (start with a capital, end with a period etc.)\n\nSo:\ndeletion_chunk -\u003e \"batch size\" without underscore. If you want to mention it exactly as in the config then use quotes and then underscore is ok. If you are using it in a free speaking manner then underscore is confusing.","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":103,"context_line":"    total_exp_executions \u003d db_api.get_expired_executions(exp_time)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    # The default value of deletion_chunk is 0"},{"line_number":106,"context_line":"    # If other value not provided, size of batch will be the size"},{"line_number":107,"context_line":"    # of total number of expired executions."},{"line_number":108,"context_line":"    if deletion_chunk:"},{"line_number":109,"context_line":"        execution_deleted_groups \u003d _group_into_chunk("}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_9bb6143c","line":106,"range":{"start_line":106,"start_character":21,"end_line":106,"end_character":24},"updated":"2017-02-10 05:19:40.000000000","message":"not -\u003e is not","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":105,"context_line":"    # The default value of deletion_chunk is 0"},{"line_number":106,"context_line":"    # If other value not provided, size of batch will be the size"},{"line_number":107,"context_line":"    # of total number of expired executions."},{"line_number":108,"context_line":"    if deletion_chunk:"},{"line_number":109,"context_line":"        execution_deleted_groups \u003d _group_into_chunk("},{"line_number":110,"context_line":"            total_exp_executions,"},{"line_number":111,"context_line":"            deletion_chunk"},{"line_number":112,"context_line":"        )"},{"line_number":113,"context_line":"        for execution_group in execution_deleted_groups:"},{"line_number":114,"context_line":"            _delete_exp_execs(execution_group)"},{"line_number":115,"context_line":"    else:"},{"line_number":116,"context_line":"        _delete_exp_execs(total_exp_executions)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"def setup():"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_fb0e30e4","line":116,"range":{"start_line":108,"start_character":0,"end_line":116,"end_character":47},"updated":"2017-02-10 05:19:40.000000000","message":"Sorry, I can\u0027t accept this algorithm due to performance and memory footprint considerations.\n\nYou implemented the simplest but less efficient algorithm. Please keep in mind what problem we\u0027re trying to solve. We\u0027re trying to avoid processing many objects in one transaction. So here, you are solving a \u0027delete\u0027 part but leaving \u0027read\u0027 part as is. I can easily imagine the situation when we read too many objects so that they may not even fit into memory. \n\nInstead of reading all objects at once and keeping them in memory while you\u0027re deleting batches, you need to be reading objects as batches from DB. Then it will be faster and and consume less memory. In order to do that you simply need to use \"limit\" SQL clause for this.","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"057eee7bc2add76b0faf0cd6457a52a6df53b55e","unresolved":false,"context_lines":[{"line_number":105,"context_line":"    # The default value of deletion_chunk is 0"},{"line_number":106,"context_line":"    # If other value not provided, size of batch will be the size"},{"line_number":107,"context_line":"    # of total number of expired executions."},{"line_number":108,"context_line":"    if deletion_chunk:"},{"line_number":109,"context_line":"        execution_deleted_groups \u003d _group_into_chunk("},{"line_number":110,"context_line":"            total_exp_executions,"},{"line_number":111,"context_line":"            deletion_chunk"},{"line_number":112,"context_line":"        )"},{"line_number":113,"context_line":"        for execution_group in execution_deleted_groups:"},{"line_number":114,"context_line":"            _delete_exp_execs(execution_group)"},{"line_number":115,"context_line":"    else:"},{"line_number":116,"context_line":"        _delete_exp_execs(total_exp_executions)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"def setup():"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_8cec07c9","line":116,"range":{"start_line":108,"start_character":0,"end_line":116,"end_character":47},"in_reply_to":"fa31d9ce_fb0e30e4","updated":"2017-02-10 13:06:11.000000000","message":"Thanks Renat, good point. Sorry for my bad, i did not focus on this point and was distracted by other private tasks. I will upload the new fix of this algo as well as other problems.","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"be117b7afbb38fe03a120ddf2a0aa416a5d7bbd8","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_725f8541","line":70,"updated":"2017-02-12 09:18:22.000000000","message":"are we only deleting one batch now?\nin PS 7, if I understand correctly, we looped all the chunks (PS 7 line 113).","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"18edaf93654fd2262629d3b35f8171e94a8720f9","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba2be162_8e86df14","line":70,"in_reply_to":"ba2be162_46b86775","updated":"2017-03-06 16:52:56.000000000","message":"Thank you guys for your comments.\n\nI also agree with this idea since sqlalchemy will have no problem when open/close multiple sessions. I will implement this idea and unittest later.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"3a7e69fd7e1d01f54b3b678c867a65c2f0d1f32c","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba2be162_06d90f57","line":70,"in_reply_to":"fa31d9ce_068dfd7a","updated":"2017-03-06 07:52:49.000000000","message":"I completely agree with Jozsef and Michal. If you don\u0027t delete all expired executions on one triggering event it simply breaks semantics of how this function works. We have to delete all of them. The fact that we decided to split all set of executions into batches is purely an implementation thing just to reduce memory footprint and not hold DB transaction open for too long (btw, under certain conditions it can even timeout).\n\nAgain, the criteria here is simple: the decision to delete only one batch at a time affects user semantics of this feature which is \"Upon a timer event we delete all expired executions\".\n\nRegarding your code snippet. I personally hate using \"while True\"  but seems like yes, it\u0027s the most convenient thing to use here. Just to slightly rewrite the snippet:\n\n\nwhile True:\n    with db_api.transaction():\n        batch \u003d db_api.get_expired_executions(expired_time, limit\u003dbatch_size)\n\n\n        if not batch:\n            break\n    \n        _delete_batch(batch)\n\n\nAs far as opening multiple SQLAlchemy sessions, no worries here:\n\n1. Behind the scenes SQLAlchemy uses a pool of DB connections (to cache the most expensive operation of opening a DB connection)\n\n2. These sessions will be opened by one thread one-by-one so we won\u0027t create any serious DB contention.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"10a7001b9bd972c55796e6aeae527a7448d8587c","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_e5d706bd","line":70,"in_reply_to":"fa31d9ce_1639cb41","updated":"2017-02-13 15:28:58.000000000","message":"IMHO, it really depends on each concrete scenario. It is hard to say that batch_size (5 in your example) is suitable or not, it should depends on operator. Here we just provide a flexible way for operators in order to tune performance.\n\nBTW, the new name makes sense to me:). Another thing is (if i understand well your concern about \"extra_memory\" :D): Do you mean that does multiple session opening with batch_size take more RAM than only one session with no batch_size?","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"d27faf3ef134eb2fdf44bdfc7e745a0253a90143","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_e050ed0c","line":70,"in_reply_to":"fa31d9ce_725f8541","updated":"2017-02-13 07:12:15.000000000","message":"Hi Mike,\n\nYeap in the PS 7, we looped it in chunks, however, we just looped it when we executed its deletion, the process of reading was in memory and this amount of memory was held to be used until the chunks were deleted. It showed a bad performance.\n\nTherefore i think we should open one session, collect size of one batch, delete it, close session. Next time with periodic_task, we open another session, collect a new batch. IMHO, it is a suitable solution in this situation.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"61e5dd9db65eb1933e0567aa34ae7c3ce462ce34","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba2be162_46b86775","line":70,"in_reply_to":"fa31d9ce_8583aa03","updated":"2017-03-06 08:09:31.000000000","message":"Is the concern that if we loop through deleting actions that the number of expired actions will continue to increase as we delete and thus the deletion will never finish? If this happens, then we are in trouble anyway because eventually something will run out of space :)\n\nI think Renats proposed code looks good.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"10a7001b9bd972c55796e6aeae527a7448d8587c","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_8583aa03","line":70,"in_reply_to":"fa31d9ce_d13add97","updated":"2017-02-13 15:28:58.000000000","message":"It is hard to say \"more\" or \"less\" expired executions in one execution_interval. We collect the number of expired executions based on the time, not based on the how often we check it.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"314d4ec751bd6ce3e7864aefc01de9ebfcd94be8","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_1639cb41","line":70,"in_reply_to":"fa31d9ce_e050ed0c","updated":"2017-02-13 08:43:41.000000000","message":"Hi Iuong,\n\nIsn\u0027t there a way to do something like pagination to delete all expired execution in batches, without using extra memory?\n\nI am worried the user might have something like 100 execution in an hour, policy that runs each hour and batch size of 5 (numbers not that important). I think the DB will not have enough memory and it is not user friendly IMO.\n\nAlso, doesn\u0027t it make the parameter more like \"max execution to delete in cycle\" than batch size\"?\n\nI think we need to see what Renat thinks.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":23211,"name":"Kupai József","email":"jozsef.kupai@nokia.com","username":"kupsef"},"change_message_id":"4baa10558bb1c1ba97e22c2245e6db78829031f5","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_d13add97","line":70,"in_reply_to":"fa31d9ce_e050ed0c","updated":"2017-02-13 08:53:28.000000000","message":"The problem with that it would break the functionality. \n\nConsider a small batch size with long execution_interval. If more executions expire during an execution_interval than the batch_size, then the size of the expired_executions will never actually decrease.\n\nYou have to delete all the expired executions each run regardless of the batch_size.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"cc90713acf980c6f550ee7aa2d60d15e7a9dce4c","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                limit\u003dbatch_size)"},{"line_number":68,"context_line":"        else:"},{"line_number":69,"context_line":"            expired_executions \u003d db_api.get_expired_executions(expired_time)"},{"line_number":70,"context_line":"        _delete(expired_executions)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"def _delete(exp_executions):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_068dfd7a","line":70,"in_reply_to":"fa31d9ce_e5d706bd","updated":"2017-02-13 17:08:41.000000000","message":"Mike and Jozsef,\n\nThank for your comments. Of course we can do pagination in this situation but we have to open quite lot of sessions in the execution_interval, a snippet as below:\n\nWhile True:\n  with open_session:\n     _delete_in_batch_size(limit\u003d5)\n     no_more_expired_execs \u003d query_db_for_remained_exp_exec()\n     if no_more_expired_execs:\n        break\n\nI have no information that opening/closing sqlalchemy session multiple times (imagine we have 10000 expired and batch_size is 5, so we need open/close 2000 times in execution interval) does affect to the performance. IF NOT, we can easily apply this idea for pagination.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"be117b7afbb38fe03a120ddf2a0aa416a5d7bbd8","unresolved":false,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    batch_size \u003d CONF.execution_expiration_policy.batch_size"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    # The default value of batch_size is 0"},{"line_number":112,"context_line":"    # If it is not set, size of batch will be the size"},{"line_number":113,"context_line":"    # of total number of expired executions."},{"line_number":114,"context_line":"    _delete_expired_executions(batch_size, exp_time)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_5271a1df","line":111,"range":{"start_line":111,"start_character":27,"end_line":111,"end_character":37},"updated":"2017-02-12 09:18:22.000000000","message":"soft \u0026 mostly copy-paste from Renat\u0027s comment in PS 7:\nAll comments and docstrings must be compliant with regular English grammar (start with a capital, end with a period etc.)\nSo:\nbatch_size -\u003e `batch size` without underscore. If you want to mention it exactly as in the config then use quotes (`\"batch_size\"`) and then underscore is ok. If you are using it in a free speaking manner then underscore is confusing.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"d27faf3ef134eb2fdf44bdfc7e745a0253a90143","unresolved":false,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    batch_size \u003d CONF.execution_expiration_policy.batch_size"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    # The default value of batch_size is 0"},{"line_number":112,"context_line":"    # If it is not set, size of batch will be the size"},{"line_number":113,"context_line":"    # of total number of expired executions."},{"line_number":114,"context_line":"    _delete_expired_executions(batch_size, exp_time)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_4042993f","line":111,"range":{"start_line":111,"start_character":27,"end_line":111,"end_character":37},"in_reply_to":"fa31d9ce_5271a1df","updated":"2017-02-13 07:12:15.000000000","message":"Done","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"842fb55b38005f8ddd712b8f0291751507446ecd","unresolved":false,"context_lines":[{"line_number":65,"context_line":"                # filters params and not specific method that filter by time."},{"line_number":66,"context_line":"                expired_executions \u003d db_api.get_expired_executions("},{"line_number":67,"context_line":"                    expired_time,"},{"line_number":68,"context_line":"                    limit\u003dbatch_size)"},{"line_number":69,"context_line":"                if not expired_executions:"},{"line_number":70,"context_line":"                    break"},{"line_number":71,"context_line":"                _delete(expired_executions)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a30ddce_15738a81","line":68,"range":{"start_line":68,"start_character":36,"end_line":68,"end_character":37},"updated":"2017-03-08 12:03:44.000000000","message":"should be in a new line","commit_id":"ebe34a7868014fac534a59df199144a658dc8e8f"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"d44630681771865e9c058f6ec472eabcafada4bf","unresolved":false,"context_lines":[{"line_number":65,"context_line":"                # filters params and not specific method that filter by time."},{"line_number":66,"context_line":"                expired_executions \u003d db_api.get_expired_executions("},{"line_number":67,"context_line":"                    expired_time,"},{"line_number":68,"context_line":"                    limit\u003dbatch_size)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"                if not expired_executions:"},{"line_number":71,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a30ddce_cd95e6e4","line":68,"range":{"start_line":68,"start_character":36,"end_line":68,"end_character":37},"updated":"2017-03-08 14:26:57.000000000","message":"should be in a new line.\nlike so:\n                expired_executions \u003d db_api.get_expired_executions(\n                    expired_time,\n                    limit\u003dbatch_size\n                )","commit_id":"43961d140ce937bd8551b528c7d9c7310b909a1d"}],"mistral/tests/unit/services/test_expired_executions_policy.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8c65445f4879fa178bd3be43412b874cabd8df71","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def test_deletion_chunk_of_exp_executions(self):"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        cfg.CONF.set_default(\u0027deletion_chunk\u0027,"},{"line_number":165,"context_line":"                             2,"},{"line_number":166,"context_line":"                             group\u003d\u0027execution_expiration_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_7bf2409c","line":163,"updated":"2017-02-10 05:19:40.000000000","message":"Remove this blank line.","commit_id":"c7eeb93836ebf070347c51822edb40bdb713b27d"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"be117b7afbb38fe03a120ddf2a0aa416a5d7bbd8","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        _set_expiration_policy_config(1, 5)"},{"line_number":199,"context_line":"        expiration_policy.run_execution_expiration_policy(self, ctx)"},{"line_number":200,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":201,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def test_negative_wrong_conf_values(self):"},{"line_number":204,"context_line":"        _set_expiration_policy_config(None, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_523fe1d5","line":201,"updated":"2017-02-12 09:18:22.000000000","message":"nice test. please consider adding another test where the number of expired executions is bigger than the limit.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"e0e0b3b786feb0f257fad6dc909665f7865809ca","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        _set_expiration_policy_config(1, 5)"},{"line_number":199,"context_line":"        expiration_policy.run_execution_expiration_policy(self, ctx)"},{"line_number":200,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":201,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def test_negative_wrong_conf_values(self):"},{"line_number":204,"context_line":"        _set_expiration_policy_config(None, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba2be162_7d5ebb13","line":201,"in_reply_to":"ba2be162_8e033fbe","updated":"2017-03-06 18:13:13.000000000","message":"Of course, this one should be also kept :)","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"ef1a704454eaf3454c8de93690a35140fe9622d7","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        _set_expiration_policy_config(1, 5)"},{"line_number":199,"context_line":"        expiration_policy.run_execution_expiration_policy(self, ctx)"},{"line_number":200,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":201,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def test_negative_wrong_conf_values(self):"},{"line_number":204,"context_line":"        _set_expiration_policy_config(None, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_92d2eb5a","line":201,"in_reply_to":"fa31d9ce_11f715f6","updated":"2017-03-06 10:39:05.000000000","message":"from what I see this is the scenario here:\n- there are 5 execution that have expired (line 183)\n- set policy function called (line 185)\n- now there are X execution that have expired according to new policy (no line checks the new number)\n- expired execution are being deleted (line 191)\n- we are left with 2 expired executions (line 196)\n- set policy function called again for some reason (line 198)\n- expired execution are being deleted (line 199)\n- we are left with 0 expired executions (line 201)\n\nIMHO, even if we already have the test I wanted in here somewhere, I thing we need to add another test that follows the steps I suggested and only has one call to delete the executions and deletes all of them.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"cb612bcc31229ac7dc72a6595880377488e89b51","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        _set_expiration_policy_config(1, 5)"},{"line_number":199,"context_line":"        expiration_policy.run_execution_expiration_policy(self, ctx)"},{"line_number":200,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":201,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def test_negative_wrong_conf_values(self):"},{"line_number":204,"context_line":"        _set_expiration_policy_config(None, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_11f715f6","line":201,"in_reply_to":"fa31d9ce_36032f44","updated":"2017-02-13 15:19:34.000000000","message":"Actually we had it:\n- When we set the expiration_policy_config(1,5), it means all the 5 created executions will be expired. And then we run it by calling run_execution_expiration_policy() with batch_size is set to 3 from very beginning.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"4abb4dbc4c2f4c981f4ad3784adbd436e77330ad","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        _set_expiration_policy_config(1, 5)"},{"line_number":199,"context_line":"        expiration_policy.run_execution_expiration_policy(self, ctx)"},{"line_number":200,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":201,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def test_negative_wrong_conf_values(self):"},{"line_number":204,"context_line":"        _set_expiration_policy_config(None, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_c0884925","line":201,"in_reply_to":"fa31d9ce_523fe1d5","updated":"2017-02-13 07:28:13.000000000","message":"Sorry Mike but i do not understand well this comment. Does \"expired executions\" mean the expired executions remaining in db after deleting or something else?\n\n\"expiration_policy._delete_expired_executions\": This function will delete once in batch. \n\n\"expiration_policy.run_execution_expiration_policy\": This function will delete all the expired execution in periodic task.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":14025,"name":"luong tuan","email":"tuan.luong_huu@nokia.com","username":"tuantuluong"},"change_message_id":"18edaf93654fd2262629d3b35f8171e94a8720f9","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        _set_expiration_policy_config(1, 5)"},{"line_number":199,"context_line":"        expiration_policy.run_execution_expiration_policy(self, ctx)"},{"line_number":200,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":201,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def test_negative_wrong_conf_values(self):"},{"line_number":204,"context_line":"        _set_expiration_policy_config(None, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba2be162_8e033fbe","line":201,"in_reply_to":"fa31d9ce_92d2eb5a","updated":"2017-03-06 16:52:56.000000000","message":"Hi Michal,\n\nWhen we apply the new code snippet, the semantics totally the same with what you suggested. It means that we will have a while loop that will help us delete all of the expired executions using batch by opening/closing session multiple times.\n\nThis unittest is just for previous patch, as you see that it will open and close session in multiple times. In the new patch, the unittest should be modified.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"314d4ec751bd6ce3e7864aefc01de9ebfcd94be8","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        _set_expiration_policy_config(1, 5)"},{"line_number":199,"context_line":"        expiration_policy.run_execution_expiration_policy(self, ctx)"},{"line_number":200,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":201,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def test_negative_wrong_conf_values(self):"},{"line_number":204,"context_line":"        _set_expiration_policy_config(None, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa31d9ce_36032f44","line":201,"in_reply_to":"fa31d9ce_c0884925","updated":"2017-02-13 08:43:41.000000000","message":"Hi luong,\n\nI want a test that simulate the next scenario:\n\n1. user has defined batch_size to be 2.\n2. user created 5 executions.\n3. the 5 execution expired.\n4. expiration policy is about to run\n5. after expiration policy runs once (or more) all 5 expired execution are deleted.","commit_id":"5d55fd80b59607d7061e9ace9f8d2ad1d8e55260"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"842fb55b38005f8ddd712b8f0291751507446ecd","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def test_deletion_of_expired_executions_with_batch_size_scenario1(self):"},{"line_number":163,"context_line":"        cfg.CONF.set_default(\u0027batch_size\u0027,"},{"line_number":164,"context_line":"                             3,"},{"line_number":165,"context_line":"                             group\u003d\u0027execution_expiration_policy\u0027)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        # Since we are removing other projects execution,"},{"line_number":168,"context_line":"        # we want to load the executions with other project_id."}],"source_content_type":"text/x-python","patch_set":9,"id":"9a30ddce_f5b356e7","line":165,"range":{"start_line":163,"start_character":0,"end_line":165,"end_character":65},"updated":"2017-03-08 12:03:44.000000000","message":"If you are changing the function please consider improving these line of code as well.\n\nThere is a code convention in mistral on how a call for a function should look like. If a function call takes more than one line, each argument should be in a new line and the closing brackets should be in a new line as well (see example in lines 191-194 in this file).\nThis function call should look like so:\n\n        cfg.CONF.set_default(\n            \u0027batch_size\u0027,\n            3,\n            group\u003d\u0027execution_expiration_policy\u0027\n        )","commit_id":"ebe34a7868014fac534a59df199144a658dc8e8f"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"842fb55b38005f8ddd712b8f0291751507446ecd","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":201,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def test_deletion_of_expired_executions_with_batch_size_scenario2(self):"},{"line_number":204,"context_line":"        cfg.CONF.set_default(\u0027batch_size\u0027,"},{"line_number":205,"context_line":"                             2,"},{"line_number":206,"context_line":"                             group\u003d\u0027execution_expiration_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a30ddce_35508e5e","line":203,"range":{"start_line":203,"start_character":60,"end_line":203,"end_character":69},"updated":"2017-03-08 12:03:44.000000000","message":"I think the difference between scenario1 and scenario2 should be in the name of the function explicitly or as a comment if it\u0027s too long. If possible, I want another way to know what the test does other than reading all of it.","commit_id":"ebe34a7868014fac534a59df199144a658dc8e8f"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"842fb55b38005f8ddd712b8f0291751507446ecd","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def test_deletion_of_expired_executions_with_batch_size_scenario2(self):"},{"line_number":204,"context_line":"        cfg.CONF.set_default(\u0027batch_size\u0027,"},{"line_number":205,"context_line":"                             2,"},{"line_number":206,"context_line":"                             group\u003d\u0027execution_expiration_policy\u0027)"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        # Since we are removing other projects execution,"},{"line_number":209,"context_line":"        # we want to load the executions with other project_id."}],"source_content_type":"text/x-python","patch_set":9,"id":"9a30ddce_f5ea76ab","line":206,"range":{"start_line":204,"start_character":0,"end_line":206,"end_character":65},"updated":"2017-03-08 12:03:44.000000000","message":"There is a code convention in mistral on how a call for a function should look like. If a function call takes more than one line, each argument should be in a new line and the closing brackets should be in a new line as well (see example in lines 191-194 in this file).\nThis function call should look like so:\n        cfg.CONF.set_default(\n            \u0027batch_size\u0027,\n            2,\n            group\u003d\u0027execution_expiration_policy\u0027\n        )","commit_id":"ebe34a7868014fac534a59df199144a658dc8e8f"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"842fb55b38005f8ddd712b8f0291751507446ecd","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        )"},{"line_number":236,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":237,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    def test_negative_wrong_conf_values(self):"},{"line_number":240,"context_line":"        _set_expiration_policy_config(None, None)"},{"line_number":241,"context_line":"        e_policy \u003d expiration_policy.ExecutionExpirationPolicy(cfg.CONF)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a30ddce_55826295","line":238,"updated":"2017-03-08 12:03:44.000000000","message":"great tests, IMO we still need a test of the scenario where batch size is smaller than the number of expired executions.\n\nThis the scenario I mentioned before: \n1. user has defined batch_size to be X.\n2. user created Y executions (Y is at least 2X).\n3. the at east Y execution expired.\n4. expiration policy is about to run\n5. after expiration policy runs once all Y expired execution are deleted.","commit_id":"ebe34a7868014fac534a59df199144a658dc8e8f"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"81a7696607a4e7d9a0b92412ab5ba71d9921ec5d","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        )"},{"line_number":236,"context_line":"        execs \u003d db_api.get_expired_executions(now)"},{"line_number":237,"context_line":"        self.assertEqual(0, len(execs))"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    def test_negative_wrong_conf_values(self):"},{"line_number":240,"context_line":"        _set_expiration_policy_config(None, None)"},{"line_number":241,"context_line":"        e_policy \u003d expiration_policy.ExecutionExpirationPolicy(cfg.CONF)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a30ddce_fbb9857a","line":238,"in_reply_to":"9a30ddce_55826295","updated":"2017-03-08 13:00:44.000000000","message":"to clarify, the line that is not aligned with the scenario I described is line 226.\n\ncombining the scenario I mentioned to this test is also fine by me (and it\u0027s just adding/modding about one or two lines).","commit_id":"ebe34a7868014fac534a59df199144a658dc8e8f"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"d44630681771865e9c058f6ec472eabcafada4bf","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        cfg.CONF.set_default("},{"line_number":169,"context_line":"            \u0027batch_size\u0027,"},{"line_number":170,"context_line":"            3,"},{"line_number":171,"context_line":"            group\u003d\u0027execution_expiration_policy\u0027)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        # Since we are removing other projects execution,"},{"line_number":174,"context_line":"        # we want to load the executions with other project_id."}],"source_content_type":"text/x-python","patch_set":10,"id":"9a30ddce_2dba026f","line":171,"range":{"start_line":171,"start_character":47,"end_line":171,"end_character":48},"updated":"2017-03-08 14:26:57.000000000","message":"should be in a new line.\nshould look like so:\n        cfg.CONF.set_default(\n            \u0027batch_size\u0027,\n            3,\n            group\u003d\u0027execution_expiration_policy\u0027\n        )","commit_id":"43961d140ce937bd8551b528c7d9c7310b909a1d"},{"author":{"_account_id":9408,"name":"Michal Gershenzon","email":"michal.gershenzon@gmail.com","username":"Mgershen"},"change_message_id":"d44630681771865e9c058f6ec472eabcafada4bf","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        cfg.CONF.set_default("},{"line_number":214,"context_line":"            \u0027batch_size\u0027,"},{"line_number":215,"context_line":"            2,"},{"line_number":216,"context_line":"            group\u003d\u0027execution_expiration_policy\u0027)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        # Since we are removing other projects execution,"},{"line_number":219,"context_line":"        # we want to load the executions with other project_id."}],"source_content_type":"text/x-python","patch_set":10,"id":"9a30ddce_cd188687","line":216,"range":{"start_line":216,"start_character":47,"end_line":216,"end_character":48},"updated":"2017-03-08 14:26:57.000000000","message":"should be in a new line.\n\nThere is a code convention in mistral on how a call for a function should look like. \n\nIf a function call takes more than one line, \neach argument should be in a new line \nand the closing brackets should be in a new line as well \n(see example in lines 196-199 in this file).\nThis function call should look like so:\n        cfg.CONF.set_default(\n            \u0027batch_size\u0027,\n            2,\n            group\u003d\u0027execution_expiration_policy\u0027\n        )","commit_id":"43961d140ce937bd8551b528c7d9c7310b909a1d"}]}