)]}'
{"novaclient/tests/functional/base.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"7bb6f2184a8fa9403e5f83452df5914588e52fd0","unresolved":false,"context_lines":[{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def wait_for_server_os_boot(self, server_id, timeout\u003d60,"},{"line_number":206,"context_line":"                                poll_interval\u003d1):"},{"line_number":207,"context_line":"        \"\"\"Wait until instance\u0027s operational system  is completely booted."},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        :param server_id: uuid4 id of given instance"},{"line_number":210,"context_line":"        :param timeout: timeout in seconds"}],"source_content_type":"text/x-python","patch_set":3,"id":"dae33548_8a4726bb","line":207,"range":{"start_line":207,"start_character":33,"end_line":207,"end_character":44},"updated":"2016-02-17 17:07:33.000000000","message":"s/operational/operating/ (femtonit)","commit_id":"14e07f27d63deb78914fd53a6442fc75f57df5be"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"7bb6f2184a8fa9403e5f83452df5914588e52fd0","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        :param timeout: timeout in seconds"},{"line_number":211,"context_line":"        :param poll_interval: poll interval in seconds"},{"line_number":212,"context_line":"        \"\"\""},{"line_number":213,"context_line":"        BOOT_IS_COMPLETE \u003d (\"login as \u0027cirros\u0027 user. default password: \""},{"line_number":214,"context_line":"                            \"\u0027cubswin:)\u0027. use \u0027sudo\u0027 for root.\")"},{"line_number":215,"context_line":"        start_time \u003d time.time()"},{"line_number":216,"context_line":"        while time.time() - start_time \u003c timeout:"}],"source_content_type":"text/x-python","patch_set":3,"id":"dae33548_aacdca32","line":213,"updated":"2016-02-17 17:07:33.000000000","message":"I prefer constants be defined in global scope, so that the definition only needs to be processed once.  Here, this variable is set each time this method is called.","commit_id":"14e07f27d63deb78914fd53a6442fc75f57df5be"}],"novaclient/tests/functional/v2/test_trigger_crash_dump.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"e5ea9679cc276ea77ac9aeaa790250237586ca2a","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    # Here, the \"light\" version of functional test is proposed."},{"line_number":29,"context_line":"    # It\u0027s based on knowledge that trigger-crash-dump uses a NMI injection,"},{"line_number":30,"context_line":"    # and when the \u0027trigger-crash-dump\u0027 operation is executed,"},{"line_number":31,"context_line":"    # instance\u0027s kernel receives the MNI signal, and an appropriate"},{"line_number":32,"context_line":"    # message will appear in the instance\u0027s log."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    # The server status must be ACTIVE, PAUSED, RESCUED, RESIZED or ERROR."}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_3756b929","line":31,"range":{"start_line":31,"start_character":37,"end_line":31,"end_character":40},"updated":"2016-02-15 20:48:25.000000000","message":"s/MNI/NMI/ (femtonit)","commit_id":"e355e88bbd5efe25f390fad301807fee02f7e99c"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"e5ea9679cc276ea77ac9aeaa790250237586ca2a","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def test_trigger_crash_dump_in_active_state(self):"},{"line_number":47,"context_line":"        server \u003d self._create_server()"},{"line_number":48,"context_line":"        while \"login as \u0027cirros\u0027 user. default password: \u0027cubswin:)\u0027. \" \\"},{"line_number":49,"context_line":"              \"use \u0027sudo\u0027 for root.\" \\"},{"line_number":50,"context_line":"                not in self.nova(\u0027console-log %s \u0027 % server.id):"},{"line_number":51,"context_line":"            time.sleep(5)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_0adee86c","line":48,"updated":"2016-02-15 20:48:25.000000000","message":"There is no need for using a backslash continuation here; the entire string could be encapsulated in parentheses—or, perhaps better, a top-level constant could be defined with the contents of this string, since it appears on all of the test methods.\n\nAnother concern I have is that there are circumstances where this test will never exit.  That\u0027s simply not acceptable: if the test fails, we need a fail message, not an infinite loop.  At a minimum, you need some sort of timeout on this.\n\nFinally, I\u0027m concerned about the overuse of time.sleep().  The vast majority of time.sleep() uses throughout the rest of the functional test suite only sleep for 1 second, even in a tight polling loop.\n\nI\u0027d suggest that you pull the polling logic implemented here out into a single method, modelled on one of the existing wait_*() methods defined on ClientTestBase (novaclient/tests/functional/base.py).  You may also want to consider a secondary polling method to eliminate the time.sleep(5) that you have on line 54","commit_id":"e355e88bbd5efe25f390fad301807fee02f7e99c"},{"author":{"_account_id":16437,"name":"Anna Babich","email":"anna.v.babich@gmail.com","username":"ababich"},"change_message_id":"2c5ddc69c1a15d67ecca1ebd812882e473e2a559","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def test_trigger_crash_dump_in_active_state(self):"},{"line_number":47,"context_line":"        server \u003d self._create_server()"},{"line_number":48,"context_line":"        while \"login as \u0027cirros\u0027 user. default password: \u0027cubswin:)\u0027. \" \\"},{"line_number":49,"context_line":"              \"use \u0027sudo\u0027 for root.\" \\"},{"line_number":50,"context_line":"                not in self.nova(\u0027console-log %s \u0027 % server.id):"},{"line_number":51,"context_line":"            time.sleep(5)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_8402cd09","line":48,"in_reply_to":"dae33548_0adee86c","updated":"2016-02-16 13:42:58.000000000","message":"Kevin, thanks for comments, will start working on them","commit_id":"e355e88bbd5efe25f390fad301807fee02f7e99c"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"7bb6f2184a8fa9403e5f83452df5914588e52fd0","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from novaclient.v2 import shell"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class TestTrigCrashDumpNovaClientV217(base.TenantTestBase):"},{"line_number":20,"context_line":"    \"\"\"Functional tests for trigger crash dump\"\"\""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    COMPUTE_API_VERSION \u003d \"2.17\""}],"source_content_type":"text/x-python","patch_set":3,"id":"dae33548_aa502a33","line":19,"range":{"start_line":19,"start_character":10,"end_line":19,"end_character":14},"updated":"2016-02-17 17:07:33.000000000","message":"In the world of bike-shedding, how about s/Trig/Trigger/?  :)  I\u0027ll call that one a femtonit, though…","commit_id":"14e07f27d63deb78914fd53a6442fc75f57df5be"},{"author":{"_account_id":16437,"name":"Anna Babich","email":"anna.v.babich@gmail.com","username":"ababich"},"change_message_id":"c26e5c6c893c3ab9ac63cf850e470722b810ae2a","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from novaclient.v2 import shell"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class TestTrigCrashDumpNovaClientV217(base.TenantTestBase):"},{"line_number":20,"context_line":"    \"\"\"Functional tests for trigger crash dump\"\"\""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    COMPUTE_API_VERSION \u003d \"2.17\""}],"source_content_type":"text/x-python","patch_set":3,"id":"dae33548_232ffba1","line":19,"range":{"start_line":19,"start_character":10,"end_line":19,"end_character":14},"in_reply_to":"dae33548_aa502a33","updated":"2016-02-18 11:19:00.000000000","message":"\u003ebike-shedding\nsuch kind of review broadens up my horizons:)","commit_id":"14e07f27d63deb78914fd53a6442fc75f57df5be"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"7bb6f2184a8fa9403e5f83452df5914588e52fd0","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    # The server status must be ACTIVE, PAUSED, RESCUED, RESIZED or ERROR."},{"line_number":35,"context_line":"    # If not, the conflictingRequest(409) code is returned"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def _create_server(self):"},{"line_number":38,"context_line":"        name \u003d self.name_generate(prefix\u003d\u0027server\u0027)"},{"line_number":39,"context_line":"        server \u003d self.client.servers.create(name, self.image, self.flavor)"},{"line_number":40,"context_line":"        self.addCleanup(server.delete)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dae33548_05078918","line":37,"updated":"2016-02-17 17:07:33.000000000","message":"I could swear I remember a pending change that pulls methods like this into the base test class.  You may want to look for that change and keep a watch on it, as you would want to rebase if it merges before yours.","commit_id":"14e07f27d63deb78914fd53a6442fc75f57df5be"},{"author":{"_account_id":16437,"name":"Anna Babich","email":"anna.v.babich@gmail.com","username":"ababich"},"change_message_id":"c26e5c6c893c3ab9ac63cf850e470722b810ae2a","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    # The server status must be ACTIVE, PAUSED, RESCUED, RESIZED or ERROR."},{"line_number":35,"context_line":"    # If not, the conflictingRequest(409) code is returned"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def _create_server(self):"},{"line_number":38,"context_line":"        name \u003d self.name_generate(prefix\u003d\u0027server\u0027)"},{"line_number":39,"context_line":"        server \u003d self.client.servers.create(name, self.image, self.flavor)"},{"line_number":40,"context_line":"        self.addCleanup(server.delete)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dae33548_23fc1b3d","line":37,"in_reply_to":"dae33548_05078918","updated":"2016-02-18 11:19:00.000000000","message":"Yep, we discussed it with Andrew Kurilin, owner of this change, and made an agreement that I\u0027d refactor my tests when his patch would be merged.","commit_id":"14e07f27d63deb78914fd53a6442fc75f57df5be"}]}