)]}'
{"blazar/manager/service.py":[{"author":{"_account_id":15197,"name":"Pierre Riteau","email":"pierre@stackhpc.com","username":"priteau","status":"StackHPC"},"change_message_id":"3a87887a55909ee032903c9a7353df03c9f8aaa1","unresolved":false,"context_lines":[{"line_number":403,"context_line":"                        raise"},{"line_number":404,"context_line":"                db_api.lease_destroy(lease_id)"},{"line_number":405,"context_line":"                self._send_notification(lease, ctx, events\u003d[\u0027delete\u0027])"},{"line_number":406,"context_line":"        else:"},{"line_number":407,"context_line":"            event \u003d db_api.event_get_first_sorted_by_filters("},{"line_number":408,"context_line":"                \u0027lease_id\u0027,"},{"line_number":409,"context_line":"                \u0027asc\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f044301_a50660cd","line":406,"updated":"2017-05-11 15:47:03.000000000","message":"I think there might be a race condition in this code block: if delete_lease is called within 10 seconds after start_date, the manager loop might not have run on_start yet. So this code would run on_end first, which might be an issue.","commit_id":"a1c17f8827837d743f259d0b17367e912e09cc55"},{"author":{"_account_id":23840,"name":"Hiroaki Kobayashi","email":"kobayashi.hiroaki@lab.ntt.co.jp","username":"hiro-kobayashi"},"change_message_id":"da6f7b78b6413b5cbfb2cf4b9915d014a0948233","unresolved":false,"context_lines":[{"line_number":403,"context_line":"                        raise"},{"line_number":404,"context_line":"                db_api.lease_destroy(lease_id)"},{"line_number":405,"context_line":"                self._send_notification(lease, ctx, events\u003d[\u0027delete\u0027])"},{"line_number":406,"context_line":"        else:"},{"line_number":407,"context_line":"            event \u003d db_api.event_get_first_sorted_by_filters("},{"line_number":408,"context_line":"                \u0027lease_id\u0027,"},{"line_number":409,"context_line":"                \u0027asc\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_42972292","line":406,"in_reply_to":"1f013ff3_f61714ed","updated":"2017-05-15 08:02:48.000000000","message":"I\u0027ve modified here to check the status of the start_lease event. The delete_lease fails if the start_lease event has not completed yet.","commit_id":"a1c17f8827837d743f259d0b17367e912e09cc55"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"b2d66a2b14b6afeca6355ded9e07b2504197b80f","unresolved":false,"context_lines":[{"line_number":403,"context_line":"                        raise"},{"line_number":404,"context_line":"                db_api.lease_destroy(lease_id)"},{"line_number":405,"context_line":"                self._send_notification(lease, ctx, events\u003d[\u0027delete\u0027])"},{"line_number":406,"context_line":"        else:"},{"line_number":407,"context_line":"            event \u003d db_api.event_get_first_sorted_by_filters("},{"line_number":408,"context_line":"                \u0027lease_id\u0027,"},{"line_number":409,"context_line":"                \u0027asc\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_f61714ed","line":406,"in_reply_to":"3f044301_a50660cd","updated":"2017-05-15 04:08:28.000000000","message":"I agree. It might cause race conditions.\n\nOther ideas are followrings\n* Wait on_start task finishes because plugin developers need to consider different race condition between on_start and on_end.\n* Fail delete_lease only during on_start is running","commit_id":"a1c17f8827837d743f259d0b17367e912e09cc55"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"b2d66a2b14b6afeca6355ded9e07b2504197b80f","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                raise common_ex.BlazarException("},{"line_number":419,"context_line":"                    \u0027Invalid end lease event status\u0027)"},{"line_number":420,"context_line":"            db_api.event_update(event[\u0027id\u0027], {\u0027status\u0027: \u0027IN_PROGRESS\u0027})"},{"line_number":421,"context_line":"            with trusts.create_ctx_from_trust(lease[\u0027trust_id\u0027]) as ctx:"},{"line_number":422,"context_line":"                for reservation in lease[\u0027reservations\u0027]:"},{"line_number":423,"context_line":"                    plugin \u003d self.plugins[reservation[\u0027resource_type\u0027]]"},{"line_number":424,"context_line":"                    try:"},{"line_number":425,"context_line":"                        plugin.on_end(reservation[\u0027resource_id\u0027])"},{"line_number":426,"context_line":"                    except (db_ex.BlazarDBException, RuntimeError):"},{"line_number":427,"context_line":"                        LOG.exception(\"Failed to delete a reservation \""},{"line_number":428,"context_line":"                                      \"for a lease.\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_96b1f0e4","line":425,"range":{"start_line":421,"start_character":0,"end_line":425,"end_character":65},"updated":"2017-05-15 04:08:28.000000000","message":"on_end event should be skipped when the event\u0027s status is already \"IN_PROGRESS\".  Current on_end method in host_plugin.py doesn\u0027t support manager process calls on_end method more than two time in parallel.","commit_id":"a1c17f8827837d743f259d0b17367e912e09cc55"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"87d25caa5e7455b6337e03198540dbe3a9ec3141","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                raise common_ex.BlazarException("},{"line_number":419,"context_line":"                    \u0027Invalid end lease event status\u0027)"},{"line_number":420,"context_line":"            db_api.event_update(event[\u0027id\u0027], {\u0027status\u0027: \u0027IN_PROGRESS\u0027})"},{"line_number":421,"context_line":"            with trusts.create_ctx_from_trust(lease[\u0027trust_id\u0027]) as ctx:"},{"line_number":422,"context_line":"                for reservation in lease[\u0027reservations\u0027]:"},{"line_number":423,"context_line":"                    plugin \u003d self.plugins[reservation[\u0027resource_type\u0027]]"},{"line_number":424,"context_line":"                    try:"},{"line_number":425,"context_line":"                        plugin.on_end(reservation[\u0027resource_id\u0027])"},{"line_number":426,"context_line":"                    except (db_ex.BlazarDBException, RuntimeError):"},{"line_number":427,"context_line":"                        LOG.exception(\"Failed to delete a reservation \""},{"line_number":428,"context_line":"                                      \"for a lease.\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_cec57890","line":425,"range":{"start_line":421,"start_character":0,"end_line":425,"end_character":65},"in_reply_to":"1f013ff3_4794d444","updated":"2017-05-16 10:22:53.000000000","message":"sorry, I missed it.","commit_id":"a1c17f8827837d743f259d0b17367e912e09cc55"},{"author":{"_account_id":23840,"name":"Hiroaki Kobayashi","email":"kobayashi.hiroaki@lab.ntt.co.jp","username":"hiro-kobayashi"},"change_message_id":"da6f7b78b6413b5cbfb2cf4b9915d014a0948233","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                raise common_ex.BlazarException("},{"line_number":419,"context_line":"                    \u0027Invalid end lease event status\u0027)"},{"line_number":420,"context_line":"            db_api.event_update(event[\u0027id\u0027], {\u0027status\u0027: \u0027IN_PROGRESS\u0027})"},{"line_number":421,"context_line":"            with trusts.create_ctx_from_trust(lease[\u0027trust_id\u0027]) as ctx:"},{"line_number":422,"context_line":"                for reservation in lease[\u0027reservations\u0027]:"},{"line_number":423,"context_line":"                    plugin \u003d self.plugins[reservation[\u0027resource_type\u0027]]"},{"line_number":424,"context_line":"                    try:"},{"line_number":425,"context_line":"                        plugin.on_end(reservation[\u0027resource_id\u0027])"},{"line_number":426,"context_line":"                    except (db_ex.BlazarDBException, RuntimeError):"},{"line_number":427,"context_line":"                        LOG.exception(\"Failed to delete a reservation \""},{"line_number":428,"context_line":"                                      \"for a lease.\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f013ff3_4794d444","line":425,"range":{"start_line":421,"start_character":0,"end_line":425,"end_character":65},"in_reply_to":"1f013ff3_96b1f0e4","updated":"2017-05-15 08:02:48.000000000","message":"The above code (L417~) skips the on_end() if the event\u0027s status is already \"IN_PROGRESS.\"","commit_id":"a1c17f8827837d743f259d0b17367e912e09cc55"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"2fadb4f071c6c36c5da437190f7d7344119e3af6","unresolved":false,"context_lines":[{"line_number":412,"context_line":"                    \u0027event_type\u0027: \u0027start_lease\u0027"},{"line_number":413,"context_line":"                }"},{"line_number":414,"context_line":"            )"},{"line_number":415,"context_line":"            if not on_start_evt:"},{"line_number":416,"context_line":"                raise common_ex.BlazarException(\u0027Start lease event not found\u0027)"},{"line_number":417,"context_line":"            if on_start_evt[\u0027status\u0027] !\u003d \u0027DONE\u0027:"},{"line_number":418,"context_line":"                raise common_ex.BlazarException("},{"line_number":419,"context_line":"                    \u0027Start lease is not completed\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f013ff3_1346e97d","line":416,"range":{"start_line":415,"start_character":0,"end_line":416,"end_character":78},"updated":"2017-05-16 10:28:50.000000000","message":"IMO, all lease have its start_lease event. This check is not need.","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"},{"author":{"_account_id":23840,"name":"Hiroaki Kobayashi","email":"kobayashi.hiroaki@lab.ntt.co.jp","username":"hiro-kobayashi"},"change_message_id":"132042ed640e291a551ee2e4080b78f9946d0289","unresolved":false,"context_lines":[{"line_number":412,"context_line":"                    \u0027event_type\u0027: \u0027start_lease\u0027"},{"line_number":413,"context_line":"                }"},{"line_number":414,"context_line":"            )"},{"line_number":415,"context_line":"            if not on_start_evt:"},{"line_number":416,"context_line":"                raise common_ex.BlazarException(\u0027Start lease event not found\u0027)"},{"line_number":417,"context_line":"            if on_start_evt[\u0027status\u0027] !\u003d \u0027DONE\u0027:"},{"line_number":418,"context_line":"                raise common_ex.BlazarException("},{"line_number":419,"context_line":"                    \u0027Start lease is not completed\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_d22f73f4","line":416,"range":{"start_line":415,"start_character":0,"end_line":416,"end_character":78},"in_reply_to":"1f013ff3_1346e97d","updated":"2017-05-18 03:08:19.000000000","message":"Agree. I\u0027ve removed this check.","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"2fadb4f071c6c36c5da437190f7d7344119e3af6","unresolved":false,"context_lines":[{"line_number":415,"context_line":"            if not on_start_evt:"},{"line_number":416,"context_line":"                raise common_ex.BlazarException(\u0027Start lease event not found\u0027)"},{"line_number":417,"context_line":"            if on_start_evt[\u0027status\u0027] !\u003d \u0027DONE\u0027:"},{"line_number":418,"context_line":"                raise common_ex.BlazarException("},{"line_number":419,"context_line":"                    \u0027Start lease is not completed\u0027)"},{"line_number":420,"context_line":"            on_end_evt \u003d db_api.event_get_first_sorted_by_filters("},{"line_number":421,"context_line":"                \u0027lease_id\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f013ff3_934af952","line":418,"range":{"start_line":418,"start_character":32,"end_line":418,"end_character":47},"updated":"2017-05-16 10:28:50.000000000","message":"It\u0027s better to set 400 status code to the exception here at least. Or it\u0027s ok to create a new BadRequest exception in exceptions.py\n\nDefault value of BlazarException returns \u0027500 Internal Server Error\u0027, now.","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"},{"author":{"_account_id":23840,"name":"Hiroaki Kobayashi","email":"kobayashi.hiroaki@lab.ntt.co.jp","username":"hiro-kobayashi"},"change_message_id":"132042ed640e291a551ee2e4080b78f9946d0289","unresolved":false,"context_lines":[{"line_number":415,"context_line":"            if not on_start_evt:"},{"line_number":416,"context_line":"                raise common_ex.BlazarException(\u0027Start lease event not found\u0027)"},{"line_number":417,"context_line":"            if on_start_evt[\u0027status\u0027] !\u003d \u0027DONE\u0027:"},{"line_number":418,"context_line":"                raise common_ex.BlazarException("},{"line_number":419,"context_line":"                    \u0027Start lease is not completed\u0027)"},{"line_number":420,"context_line":"            on_end_evt \u003d db_api.event_get_first_sorted_by_filters("},{"line_number":421,"context_line":"                \u0027lease_id\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_f2627716","line":418,"range":{"start_line":418,"start_character":32,"end_line":418,"end_character":47},"in_reply_to":"1f013ff3_934af952","updated":"2017-05-18 03:08:19.000000000","message":"IMO, this exception is not due to a client. This exception occurs if the Blazar manager is just processing the on_start event (status \u003d IN_PROGRESS) or failed to process on_start event (status \u003d ERROR). That means the exception is not because of the incorrectness of the request itself. So I think returning 500 is better than 400 here.","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"2fadb4f071c6c36c5da437190f7d7344119e3af6","unresolved":false,"context_lines":[{"line_number":417,"context_line":"            if on_start_evt[\u0027status\u0027] !\u003d \u0027DONE\u0027:"},{"line_number":418,"context_line":"                raise common_ex.BlazarException("},{"line_number":419,"context_line":"                    \u0027Start lease is not completed\u0027)"},{"line_number":420,"context_line":"            on_end_evt \u003d db_api.event_get_first_sorted_by_filters("},{"line_number":421,"context_line":"                \u0027lease_id\u0027,"},{"line_number":422,"context_line":"                \u0027asc\u0027,"},{"line_number":423,"context_line":"                {"},{"line_number":424,"context_line":"                    \u0027lease_id\u0027: lease_id,"},{"line_number":425,"context_line":"                    \u0027event_type\u0027: \u0027end_lease\u0027"},{"line_number":426,"context_line":"                }"},{"line_number":427,"context_line":"            )"},{"line_number":428,"context_line":"            if not on_end_evt:"},{"line_number":429,"context_line":"                raise common_ex.BlazarException(\u0027End lease event not found\u0027)"},{"line_number":430,"context_line":"            if on_end_evt[\u0027status\u0027] !\u003d \u0027UNDONE\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f013ff3_f3713de2","line":427,"range":{"start_line":420,"start_character":0,"end_line":427,"end_character":13},"updated":"2017-05-16 10:28:50.000000000","message":"Could you encapsulate L420-433 and use same db session for the logic?  Or could you add some barriers to _event() to check event\u0027s status? I still have a concern of race condition in following scenario.\n\n1. the API thread queries it\u0027s end_lease event, then get the event with \u0027UNDONE\u0027 status.\n2. context switch to _event() thread happens because of timer, then it get the event.\n3. _event() thread updates the event\u0027s status to IN_PROGRESS and start on_end method, then do context switch.\n4. the API thread checks the old event\u0027s status, then tries to start on_end action.","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"},{"author":{"_account_id":23840,"name":"Hiroaki Kobayashi","email":"kobayashi.hiroaki@lab.ntt.co.jp","username":"hiro-kobayashi"},"change_message_id":"132042ed640e291a551ee2e4080b78f9946d0289","unresolved":false,"context_lines":[{"line_number":417,"context_line":"            if on_start_evt[\u0027status\u0027] !\u003d \u0027DONE\u0027:"},{"line_number":418,"context_line":"                raise common_ex.BlazarException("},{"line_number":419,"context_line":"                    \u0027Start lease is not completed\u0027)"},{"line_number":420,"context_line":"            on_end_evt \u003d db_api.event_get_first_sorted_by_filters("},{"line_number":421,"context_line":"                \u0027lease_id\u0027,"},{"line_number":422,"context_line":"                \u0027asc\u0027,"},{"line_number":423,"context_line":"                {"},{"line_number":424,"context_line":"                    \u0027lease_id\u0027: lease_id,"},{"line_number":425,"context_line":"                    \u0027event_type\u0027: \u0027end_lease\u0027"},{"line_number":426,"context_line":"                }"},{"line_number":427,"context_line":"            )"},{"line_number":428,"context_line":"            if not on_end_evt:"},{"line_number":429,"context_line":"                raise common_ex.BlazarException(\u0027End lease event not found\u0027)"},{"line_number":430,"context_line":"            if on_end_evt[\u0027status\u0027] !\u003d \u0027UNDONE\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_e82de646","line":427,"range":{"start_line":420,"start_character":0,"end_line":427,"end_character":13},"in_reply_to":"1f013ff3_f3713de2","updated":"2017-05-18 03:08:19.000000000","message":"I agree with you. But it is rare case and the problem is fundamental to Blazar. e.g. API thread of update_lease can also handle the same record at the same time with _event() thread. How about dealing with this problem in another patch? Because It may require much more changes than this patch\u0027s main scope itself. Maybe it\u0027s related to the Atomic transaction blueprint.","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"2fadb4f071c6c36c5da437190f7d7344119e3af6","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                    \u0027event_type\u0027: \u0027end_lease\u0027"},{"line_number":426,"context_line":"                }"},{"line_number":427,"context_line":"            )"},{"line_number":428,"context_line":"            if not on_end_evt:"},{"line_number":429,"context_line":"                raise common_ex.BlazarException(\u0027End lease event not found\u0027)"},{"line_number":430,"context_line":"            if on_end_evt[\u0027status\u0027] !\u003d \u0027UNDONE\u0027:"},{"line_number":431,"context_line":"                raise common_ex.BlazarException("},{"line_number":432,"context_line":"                    \u0027Invalid end lease event status\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f013ff3_d35f613f","line":429,"range":{"start_line":428,"start_character":0,"end_line":429,"end_character":76},"updated":"2017-05-16 10:28:50.000000000","message":"same comment for \u0027start_lease\u0027 event.","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"},{"author":{"_account_id":23840,"name":"Hiroaki Kobayashi","email":"kobayashi.hiroaki@lab.ntt.co.jp","username":"hiro-kobayashi"},"change_message_id":"132042ed640e291a551ee2e4080b78f9946d0289","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                    \u0027event_type\u0027: \u0027end_lease\u0027"},{"line_number":426,"context_line":"                }"},{"line_number":427,"context_line":"            )"},{"line_number":428,"context_line":"            if not on_end_evt:"},{"line_number":429,"context_line":"                raise common_ex.BlazarException(\u0027End lease event not found\u0027)"},{"line_number":430,"context_line":"            if on_end_evt[\u0027status\u0027] !\u003d \u0027UNDONE\u0027:"},{"line_number":431,"context_line":"                raise common_ex.BlazarException("},{"line_number":432,"context_line":"                    \u0027Invalid end lease event status\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_725f8741","line":429,"range":{"start_line":428,"start_character":0,"end_line":429,"end_character":76},"in_reply_to":"1f013ff3_d35f613f","updated":"2017-05-18 03:08:19.000000000","message":"I\u0027ve removed this check.","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"2fadb4f071c6c36c5da437190f7d7344119e3af6","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                raise common_ex.BlazarException("},{"line_number":432,"context_line":"                    \u0027Invalid end lease event status\u0027)"},{"line_number":433,"context_line":"            db_api.event_update(on_end_evt[\u0027id\u0027], {\u0027status\u0027: \u0027IN_PROGRESS\u0027})"},{"line_number":434,"context_line":"            with trusts.create_ctx_from_trust(lease[\u0027trust_id\u0027]) as ctx:"},{"line_number":435,"context_line":"                for reservation in lease[\u0027reservations\u0027]:"},{"line_number":436,"context_line":"                    plugin \u003d self.plugins[reservation[\u0027resource_type\u0027]]"},{"line_number":437,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f013ff3_0ef3600c","line":434,"updated":"2017-05-16 10:28:50.000000000","message":"This codes looks same with L395-405. IMO, completely same logic/codes putted in one method should be pulled out from the method as other method. It\u0027s easy to maintain and to test the method","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"},{"author":{"_account_id":23840,"name":"Hiroaki Kobayashi","email":"kobayashi.hiroaki@lab.ntt.co.jp","username":"hiro-kobayashi"},"change_message_id":"132042ed640e291a551ee2e4080b78f9946d0289","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                raise common_ex.BlazarException("},{"line_number":432,"context_line":"                    \u0027Invalid end lease event status\u0027)"},{"line_number":433,"context_line":"            db_api.event_update(on_end_evt[\u0027id\u0027], {\u0027status\u0027: \u0027IN_PROGRESS\u0027})"},{"line_number":434,"context_line":"            with trusts.create_ctx_from_trust(lease[\u0027trust_id\u0027]) as ctx:"},{"line_number":435,"context_line":"                for reservation in lease[\u0027reservations\u0027]:"},{"line_number":436,"context_line":"                    plugin \u003d self.plugins[reservation[\u0027resource_type\u0027]]"},{"line_number":437,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff0f0b1f_68c5568e","line":434,"in_reply_to":"1f013ff3_0ef3600c","updated":"2017-05-18 03:08:19.000000000","message":"I\u0027ve changed the sequence of if statements to remove the duplication.","commit_id":"ff4e1e931befa3732b994144c5a0ec58f37b7ec8"}]}