)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"dae929c17f13decb5972a6fdaaab6c30fb6ad58c","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2015-11-01 19:06:01 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Correct the status change time"},{"line_number":8,"context_line":"Closes-Bug: 1286379"},{"line_number":9,"context_line":"Author: Petra Sargent"},{"line_number":10,"context_line":"Date: Sun November 1 2015"},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1a26ad4f_963ebd8a","line":8,"updated":"2015-11-02 00:49:06.000000000","message":"Please follow the recommended commit message format as described in https://wiki.openstack.org/wiki/GitCommitMessages\n\nAmong other things, the Author and Date lines are not relevant and the Close-Bug goes at the end, with a # before the bug number.","commit_id":"06e9abfb2143f6bfb7917f262eb749f7c46e9c87"}],"trove/guestagent/datastore/service.py":[{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"a3af9925db804e7fdcc11f295e1b0fe31ec4d9fa","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        \"update_db\" is True."},{"line_number":323,"context_line":"        \"\"\""},{"line_number":324,"context_line":"        end_time \u003d time.time() + max_time"},{"line_number":325,"context_line":"        actual_status \u003d self._get_actual_db_status()"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        while time.time() \u003c\u003d end_time:"},{"line_number":328,"context_line":"            LOG.debug(\"Waiting for DB status to change from %s to %s.\" %"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_2da3e89e","line":325,"updated":"2015-11-10 21:32:40.000000000","message":"Move this in the loop and it only needs to be coded once.","commit_id":"a180bbba22ecf696efd0d9c769fcb7b9e567f97b"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"78bdf4901b03d6f6d569e1a02d484526872bfc1f","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        \"update_db\" is True."},{"line_number":323,"context_line":"        \"\"\""},{"line_number":324,"context_line":"        end_time \u003d time.time() + max_time"},{"line_number":325,"context_line":"        actual_status \u003d self._get_actual_db_status()"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        while time.time() \u003c\u003d end_time:"},{"line_number":328,"context_line":"            LOG.debug(\"Waiting for DB status to change from %s to %s.\" %"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_bd2e1441","line":325,"in_reply_to":"da85f559_2da3e89e","updated":"2015-11-11 10:24:39.000000000","message":"No, you can\u0027t move this into the loop safely because there is nothing to guarantee that a caller doesn\u0027t provide max_time small enough that between line 324 (where end_time is computed above) and where we decide whether to enter the loop or not (line 327 below), enough time does not elapse and the loop never gets entered. therefore you do have to safeguard it and get actual_status set here.\n\nThe reason why this change is required is that I change the message on line 339 in this commit and make it show the status. In the way the code was done earlier, it wouldn\u0027t have mattered but in testing with a timeout of very small value (like 0) I could get this to fail. Hence also the check for time with \u003c\u003d and not \u003c as in the old code.\n\nSo, no. I won\u0027t make the change you recommend.","commit_id":"a180bbba22ecf696efd0d9c769fcb7b9e567f97b"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"8fa60ebf51667c4e7b47898e9678cd56f2412c1a","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        actual_status \u003d self._get_actual_db_status()"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        while time.time() \u003c\u003d end_time:"},{"line_number":328,"context_line":"            LOG.debug(\"Waiting for DB status to change from %s to %s.\" %"},{"line_number":329,"context_line":"                      (actual_status, status))"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"            if actual_status \u003d\u003d status:"},{"line_number":332,"context_line":"                if update_db:"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_7249b2b6","line":329,"range":{"start_line":328,"start_character":0,"end_line":329,"end_character":46},"updated":"2015-11-11 13:49:35.000000000","message":"My only concern here is that you could easily print out a message saying:\n\n    \"Waiting for status to change from \u0027running\u0027 to \u0027running\u0027\"\n\nwhich could be confusing.  Maybe this should be put just before the sleep.  The set_status already prints out what the current status is, so the only reason you\u0027d need to print it out here is if it\u0027s different from the expected one.  For example, the following re-arrangement of code addresses both this and Morgan\u0027 earlier comment:\n\n    end_time \u003d time.time() + max_time\n    change_found \u003d False\n    status_checked \u003d False\n\n    while not status_checked or (\n            not change_found and time.time() \u003c\u003d end_time):\n        actual_status \u003d self._get_actual_db_status()\n        status_checked \u003d True\n        if actual_status \u003d\u003d status:\n            change_found \u003d True\n        else:\n            LOG.debug(\"Waiting for DB status to change from %s to %s.\" %\n                      (actual_status, status))\n            time.sleep(CONF.state_change_poll_time)\n\n    if change_found:\n        if update_db:\n            self.set_status(actual_status)\n    else:\n        LOG.error(_(\"Timeout while waiting for database status to change.\"\n                    \"Expected state %s, current state is %s\") %\n                  (status, actual_status))\n\n    return change_found\n\nJust a thought.","commit_id":"a180bbba22ecf696efd0d9c769fcb7b9e567f97b"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"ebfdfe3572c70fde223f85c2c90127a345197ad0","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        actual_status \u003d self._get_actual_db_status()"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        while time.time() \u003c\u003d end_time:"},{"line_number":328,"context_line":"            LOG.debug(\"Waiting for DB status to change from %s to %s.\" %"},{"line_number":329,"context_line":"                      (actual_status, status))"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"            if actual_status \u003d\u003d status:"},{"line_number":332,"context_line":"                if update_db:"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_e6640323","line":329,"range":{"start_line":328,"start_character":0,"end_line":329,"end_character":46},"in_reply_to":"da85f559_7249b2b6","updated":"2015-11-11 13:53:20.000000000","message":"There is a much easier way to fix this ... move the LOG line to after the check and just before the sleep ;)\n\nNo, I don\u0027t think the code you provided above is \u0027simpler\u0027 for most normal definitions of \u0027simpler\u0027 ;)\n\nI will do that ...","commit_id":"a180bbba22ecf696efd0d9c769fcb7b9e567f97b"},{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"a3af9925db804e7fdcc11f295e1b0fe31ec4d9fa","unresolved":false,"context_lines":[{"line_number":333,"context_line":"                    self.set_status(actual_status)"},{"line_number":334,"context_line":"                return True"},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"            time.sleep(CONF.state_change_poll_time)"},{"line_number":337,"context_line":"            actual_status \u003d self._get_actual_db_status()"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        LOG.error(_(\"Timeout while waiting for database status to change.\""}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_6d857005","line":336,"updated":"2015-11-10 21:32:40.000000000","message":"It seems odd that max_time is passed in, but state_change_poll_time is a CONF value.  Could we make them the same?","commit_id":"a180bbba22ecf696efd0d9c769fcb7b9e567f97b"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"78bdf4901b03d6f6d569e1a02d484526872bfc1f","unresolved":false,"context_lines":[{"line_number":333,"context_line":"                    self.set_status(actual_status)"},{"line_number":334,"context_line":"                return True"},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"            time.sleep(CONF.state_change_poll_time)"},{"line_number":337,"context_line":"            actual_status \u003d self._get_actual_db_status()"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        LOG.error(_(\"Timeout while waiting for database status to change.\""}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_1d1640f1","line":336,"in_reply_to":"da85f559_6d857005","updated":"2015-11-11 10:24:39.000000000","message":"Most callers use a CONF parameter but they could each use a different one (based on their datastore for example).\n\nSo, nothing odd here. We could make them both get passed in if you\u0027d like but that\u0027d be a different class of change.","commit_id":"a180bbba22ecf696efd0d9c769fcb7b9e567f97b"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"0793b1edb5ca4c31b5efce4abd20b8da22ac01f0","unresolved":false,"context_lines":[{"line_number":336,"context_line":"                    self.set_status(actual_status)"},{"line_number":337,"context_line":"                return True"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"            LOG.debug(\"Waiting for DB status to change from \""},{"line_number":340,"context_line":"                      \"%(actual_status)s to %(status)s.\" %"},{"line_number":341,"context_line":"                      {\"actual_status\": actual_status, \"status\": status})"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"            # should we remain in this loop? this is the thing"},{"line_number":344,"context_line":"            # that emulates the do-while construct."}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_d73cc2e2","line":341,"range":{"start_line":339,"start_character":0,"end_line":341,"end_character":73},"updated":"2015-11-12 16:00:11.000000000","message":"This should probably go just before the sleep (inside the \u0027if loop:\u0027 block), but no biggie.","commit_id":"edf1967e4c6d1e01d65899447b531e06558f1aa7"}]}