)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16962,"name":"Federico Ceratto","email":"federico.ceratto@gmail.com","username":"ceratto"},"change_message_id":"04af60137c3475f8cafc72dba2425560c8646f54","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The goal is to find a better place for the transaction decorator and"},{"line_number":10,"context_line":"adjust the implementation to allow disabling the decorator. This will"},{"line_number":11,"context_line":"allow testing transactional methods in solation without having to mock"},{"line_number":12,"context_line":"out all the code that tests retries and handles transactions."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Here is an example of disabling the transaction:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7af24918_b4536638","line":11,"updated":"2016-03-03 16:12:01.000000000","message":"s/solation/isolation/","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":15171,"name":"Eric Larson","email":"eric.larson@rackspace.com","username":"elarson"},"change_message_id":"485da50a90931bb90a600546da1af4552052e912","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The goal is to find a better place for the transaction decorator and"},{"line_number":10,"context_line":"adjust the implementation to allow disabling the decorator. This will"},{"line_number":11,"context_line":"allow testing transactional methods in solation without having to mock"},{"line_number":12,"context_line":"out all the code that tests retries and handles transactions."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Here is an example of disabling the transaction:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7af24918_400a157a","line":11,"in_reply_to":"7af24918_b4536638","updated":"2016-03-03 16:26:23.000000000","message":"Done","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":16962,"name":"Federico Ceratto","email":"federico.ceratto@gmail.com","username":"ceratto"},"change_message_id":"04af60137c3475f8cafc72dba2425560c8646f54","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Here is an example of disabling the transaction:"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"   @mock.patch(\u0027designate.storage.transaction.ENABLE_TRANSACTION\u0027, False)"},{"line_number":17,"context_line":"   def test_method_in_isolation(self):"},{"line_number":18,"context_line":"       assert self.central_api.create_zone()"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7af24918_0fbe8364","line":16,"updated":"2016-03-03 16:12:01.000000000","message":"IMO calling the flag \"DISABLE_TRANSACTION\" would be more explicit given that transactions are enabled by default","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":15171,"name":"Eric Larson","email":"eric.larson@rackspace.com","username":"elarson"},"change_message_id":"485da50a90931bb90a600546da1af4552052e912","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Here is an example of disabling the transaction:"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"   @mock.patch(\u0027designate.storage.transaction.ENABLE_TRANSACTION\u0027, False)"},{"line_number":17,"context_line":"   def test_method_in_isolation(self):"},{"line_number":18,"context_line":"       assert self.central_api.create_zone()"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7af24918_200f0989","line":16,"in_reply_to":"7af24918_0fbe8364","updated":"2016-03-03 16:26:23.000000000","message":"Done","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":15171,"name":"Eric Larson","email":"eric.larson@rackspace.com","username":"elarson"},"change_message_id":"d7dcf2fa75f3129686229c3638914fb68af6f436","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Here is an example of disabling the transaction:"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"   @mock.patch(\u0027designate.storage.transaction.ENABLE_TRANSACTION\u0027, False)"},{"line_number":17,"context_line":"   def test_method_in_isolation(self):"},{"line_number":18,"context_line":"       assert self.central_api.create_zone()"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7af24918_4c39aed6","line":16,"in_reply_to":"7af24918_200f0989","updated":"2016-03-03 17:09:51.000000000","message":"So in trying to refactor this, due to the wrapt semantics, doing DISABLE_TRANSACTION would mean either 1) enable\u003dnot DISABLE_TRANSCTION or 2) a small function (ie partial(is_disable, DISABLE_*) or something similar. So, while I agree the usage is a little clearer, it adds a bit of awkward redirection in the code that is easy to get wrong. \n\nCan we live with the ENABLE_*?","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"}],"designate/central/service.py":[{"author":{"_account_id":16962,"name":"Federico Ceratto","email":"federico.ceratto@gmail.com","username":"ceratto"},"change_message_id":"04af60137c3475f8cafc72dba2425560c8646f54","unresolved":false,"context_lines":[{"line_number":140,"context_line":"def notification(notification_type):"},{"line_number":141,"context_line":"    @wrapt.decorator"},{"line_number":142,"context_line":"    def notification_wrapper(wrapped, instance, args, kwargs):"},{"line_number":143,"context_line":"        if not hasattr(NOTIFICATION_BUFFER, \u0027queue\u0027):"},{"line_number":144,"context_line":"            # Create the notifications queue if necessary"},{"line_number":145,"context_line":"            NOTIFICATION_BUFFER.stack \u003d 0"},{"line_number":146,"context_line":"            NOTIFICATION_BUFFER.queue \u003d collections.deque()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_da18db20","line":143,"updated":"2016-03-03 16:12:01.000000000","message":"This could be done with a try: NOTIFICATION_BUFFER.stack +\u003d1 except ...: \u003ccreate .stack and .queue\u003e to save doing hasattr() on every call\n\n(albeit the missing .stack is not, technically, an error)","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":15171,"name":"Eric Larson","email":"eric.larson@rackspace.com","username":"elarson"},"change_message_id":"485da50a90931bb90a600546da1af4552052e912","unresolved":false,"context_lines":[{"line_number":140,"context_line":"def notification(notification_type):"},{"line_number":141,"context_line":"    @wrapt.decorator"},{"line_number":142,"context_line":"    def notification_wrapper(wrapped, instance, args, kwargs):"},{"line_number":143,"context_line":"        if not hasattr(NOTIFICATION_BUFFER, \u0027queue\u0027):"},{"line_number":144,"context_line":"            # Create the notifications queue if necessary"},{"line_number":145,"context_line":"            NOTIFICATION_BUFFER.stack \u003d 0"},{"line_number":146,"context_line":"            NOTIFICATION_BUFFER.queue \u003d collections.deque()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_60011138","line":143,"in_reply_to":"7af24918_da18db20","updated":"2016-03-03 16:26:23.000000000","message":"I agree. I hadn\u0027t tried to change the contents of the wrapper, so that is something we can do in another patch.","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":16962,"name":"Federico Ceratto","email":"federico.ceratto@gmail.com","username":"ceratto"},"change_message_id":"04af60137c3475f8cafc72dba2425560c8646f54","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            NOTIFICATION_BUFFER.stack \u003d 0"},{"line_number":146,"context_line":"            NOTIFICATION_BUFFER.queue \u003d collections.deque()"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        NOTIFICATION_BUFFER.stack +\u003d 1"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        try:"},{"line_number":151,"context_line":"            # Find the context argument"}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_7a76e7d7","line":148,"updated":"2016-03-03 16:12:01.000000000","message":"\"stack\" sounds like a sequence of items. s/stack/stack_depth/ maybe?","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"}],"designate/storage/transaction.py":[{"author":{"_account_id":16962,"name":"Federico Ceratto","email":"federico.ceratto@gmail.com","username":"ceratto"},"change_message_id":"04af60137c3475f8cafc72dba2425560c8646f54","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def retry(cb\u003dNone, retries\u003d50, delay\u003d150):"},{"line_number":47,"context_line":"    \"\"\"A retry decorator that ignores attempts at creating nested retries\"\"\""},{"line_number":48,"context_line":"    @wrapt.decorator(enabled\u003dENABLE_RETRIES)"},{"line_number":49,"context_line":"    def retry_wrapper(wrapped, instance, args, kwargs):"},{"line_number":50,"context_line":"        if not hasattr(RETRY_STATE, \u0027held\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_557adc77","line":47,"updated":"2016-03-03 16:12:01.000000000","message":"+ \"\"\"Delay is in milliseconds\"\"\"","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":15171,"name":"Eric Larson","email":"eric.larson@rackspace.com","username":"elarson"},"change_message_id":"485da50a90931bb90a600546da1af4552052e912","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def retry(cb\u003dNone, retries\u003d50, delay\u003d150):"},{"line_number":47,"context_line":"    \"\"\"A retry decorator that ignores attempts at creating nested retries\"\"\""},{"line_number":48,"context_line":"    @wrapt.decorator(enabled\u003dENABLE_RETRIES)"},{"line_number":49,"context_line":"    def retry_wrapper(wrapped, instance, args, kwargs):"},{"line_number":50,"context_line":"        if not hasattr(RETRY_STATE, \u0027held\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_e09b6116","line":47,"in_reply_to":"7af24918_557adc77","updated":"2016-03-03 16:26:23.000000000","message":"Done","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":16962,"name":"Federico Ceratto","email":"federico.ceratto@gmail.com","username":"ceratto"},"change_message_id":"04af60137c3475f8cafc72dba2425560c8646f54","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            RETRY_STATE.held \u003d False"},{"line_number":53,"context_line":"            RETRY_STATE.retries \u003d 0"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        if not RETRY_STATE.held:"},{"line_number":56,"context_line":"            # We\u0027re the outermost retry decorator"},{"line_number":57,"context_line":"            RETRY_STATE.held \u003d True"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_dd9bb216","line":55,"updated":"2016-03-03 16:12:01.000000000","message":"This nested retries mechanism should have some unit tests :)","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":15171,"name":"Eric Larson","email":"eric.larson@rackspace.com","username":"elarson"},"change_message_id":"485da50a90931bb90a600546da1af4552052e912","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            RETRY_STATE.held \u003d False"},{"line_number":53,"context_line":"            RETRY_STATE.retries \u003d 0"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        if not RETRY_STATE.held:"},{"line_number":56,"context_line":"            # We\u0027re the outermost retry decorator"},{"line_number":57,"context_line":"            RETRY_STATE.held \u003d True"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_20b9c979","line":55,"in_reply_to":"7af24918_dd9bb216","updated":"2016-03-03 16:26:23.000000000","message":"Totally agree! I think we can do that in another patch as this isn\u0027t new code other than changing the decorator to use wrapt.","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":16962,"name":"Federico Ceratto","email":"federico.ceratto@gmail.com","username":"ceratto"},"change_message_id":"04af60137c3475f8cafc72dba2425560c8646f54","unresolved":false,"context_lines":[{"line_number":72,"context_line":"                            raise"},{"line_number":73,"context_line":"                        else:"},{"line_number":74,"context_line":"                            # Retry, with a delay."},{"line_number":75,"context_line":"                            time.sleep(delay / float(1000))"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"            finally:"},{"line_number":78,"context_line":"                RETRY_STATE.held \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_b52fe086","line":75,"updated":"2016-03-03 16:12:01.000000000","message":"1000.0 instead of calling float()","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":15171,"name":"Eric Larson","email":"eric.larson@rackspace.com","username":"elarson"},"change_message_id":"485da50a90931bb90a600546da1af4552052e912","unresolved":false,"context_lines":[{"line_number":72,"context_line":"                            raise"},{"line_number":73,"context_line":"                        else:"},{"line_number":74,"context_line":"                            # Retry, with a delay."},{"line_number":75,"context_line":"                            time.sleep(delay / float(1000))"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"            finally:"},{"line_number":78,"context_line":"                RETRY_STATE.held \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_e069a1fa","line":75,"in_reply_to":"7af24918_b52fe086","updated":"2016-03-03 16:26:23.000000000","message":"Done","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"},{"author":{"_account_id":15452,"name":"Rahman Syed","email":"rahman.syed@gmail.com","username":"rahmansyed"},"change_message_id":"d216acdf4cb86d23e481e46adaad9ee56ac21e9f","unresolved":false,"context_lines":[{"line_number":72,"context_line":"                            raise"},{"line_number":73,"context_line":"                        else:"},{"line_number":74,"context_line":"                            # Retry, with a delay."},{"line_number":75,"context_line":"                            time.sleep(delay / float(1000))"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"            finally:"},{"line_number":78,"context_line":"                RETRY_STATE.held \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"7af24918_3b33be63","line":75,"in_reply_to":"7af24918_e069a1fa","updated":"2016-03-03 16:35:14.000000000","message":"Hm, as with the other comments can this be saved for another change?  Leaving the existing decorator code unmodified?","commit_id":"d641df97b67ea1d6c2e5b426c8a2ef5c7eae12fd"}]}