)]}'
{"trove/guestagent/datastore/cassandra/service.py":[{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"517d5fc24027865d17bdbd5ebe564ee343e7464d","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        cql_enabled \u003d conf.get(\u0027start_native_transport\u0027)"},{"line_number":83,"context_line":"        LOG.debug(\"CQL enabled: %s.\" % cql_enabled)"},{"line_number":84,"context_line":"        return thrift_enabled, cql_enabled"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def _verify_cassandra_started(self):"},{"line_number":87,"context_line":"        thrift, cql \u003d self.get_enabled_transport_services()"},{"line_number":88,"context_line":"        cql_running, thrift_running \u003d False, False"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_44a1b9f6","line":85,"updated":"2014-11-18 15:19:46.000000000","message":"denis, as I just mentioned on IRC, this code should probably just be factored into _get_actual_db_status() which currently is incomplete in that it doesn\u0027t check thrift status. That seems to be the guts of your change. Yes?","commit_id":"0b4732dc35d8c860af78e508c47142e860ac66fe"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"517d5fc24027865d17bdbd5ebe564ee343e7464d","unresolved":false,"context_lines":[{"line_number":118,"context_line":"            utils.execute_with_timeout(system.START_CASSANDRA,"},{"line_number":119,"context_line":"                                       shell\u003dTrue)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"            utils.poll_until("},{"line_number":122,"context_line":"                self._verify_cassandra_started,"},{"line_number":123,"context_line":"                time_out\u003d1000, sleep_time\u003d5)"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_e4b70538","line":121,"updated":"2014-11-18 15:19:46.000000000","message":"and this polling call would be automatic with wait_for_real_status_to_change_to()","commit_id":"0b4732dc35d8c860af78e508c47142e860ac66fe"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"6da8f5d99eab23f0bd23fcce3ebf4c9ed6f7ed68","unresolved":false,"context_lines":[{"line_number":207,"context_line":"    def _get_enabled_transport_services(self):"},{"line_number":208,"context_line":"        conf \u003d CassandraApp.read_conf()"},{"line_number":209,"context_line":"        thrift_enabled \u003d conf.get(\u0027start_rpc\u0027)"},{"line_number":210,"context_line":"        LOG.debug(\"Thrift enabled: %s.\" % thrift_enabled)"},{"line_number":211,"context_line":"        cql_enabled \u003d conf.get(\u0027start_native_transport\u0027)"},{"line_number":212,"context_line":"        LOG.debug(\"CQL enabled: %s.\" % cql_enabled)"},{"line_number":213,"context_line":"        return thrift_enabled, cql_enabled"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_5136b8e8","line":210,"updated":"2014-11-19 12:11:59.000000000","message":"merge the two log.debug\u0027s and change the argument to avoid string rendering if debug isn\u0027t enabled.\n\ni.e. log.debug ( \"thrift \u003d %(thrift), cql \u003d %(cql)\", ...)","commit_id":"b888586a0ab38b0f06082dfd2ca1e6c1173adfcc"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"6898d71b2f393198e437425e6fbe5193f6d4723b","unresolved":false,"context_lines":[{"line_number":207,"context_line":"    def _get_enabled_transport_services(self):"},{"line_number":208,"context_line":"        conf \u003d CassandraApp.read_conf()"},{"line_number":209,"context_line":"        thrift_enabled \u003d conf.get(\u0027start_rpc\u0027)"},{"line_number":210,"context_line":"        LOG.debug(\"Thrift enabled: %s.\" % thrift_enabled)"},{"line_number":211,"context_line":"        cql_enabled \u003d conf.get(\u0027start_native_transport\u0027)"},{"line_number":212,"context_line":"        LOG.debug(\"CQL enabled: %s.\" % cql_enabled)"},{"line_number":213,"context_line":"        return thrift_enabled, cql_enabled"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_d499b620","line":210,"in_reply_to":"5a890539_5136b8e8","updated":"2014-11-19 13:04:32.000000000","message":"Done","commit_id":"b888586a0ab38b0f06082dfd2ca1e6c1173adfcc"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"6da8f5d99eab23f0bd23fcce3ebf4c9ed6f7ed68","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                system.CQL_TRANSPORT_STATUS, shell\u003dTrue,"},{"line_number":224,"context_line":"                check_exit_code\u003d[1, 0], timeout\u003d100)"},{"line_number":225,"context_line":"            cql_running \u003d True if not cql_err else False"},{"line_number":226,"context_line":"            LOG.debug(\"CQL status %s.\" % cql_running)"},{"line_number":227,"context_line":"        if thrift:"},{"line_number":228,"context_line":"            thrift_out, thrift_err \u003d utils.execute_with_timeout("},{"line_number":229,"context_line":"                system.THRIFT_TRANSPORT_STATUS, shell\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_31676cec","line":226,"updated":"2014-11-19 12:11:59.000000000","message":"similar changes to string rendering.","commit_id":"b888586a0ab38b0f06082dfd2ca1e6c1173adfcc"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"6898d71b2f393198e437425e6fbe5193f6d4723b","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                system.CQL_TRANSPORT_STATUS, shell\u003dTrue,"},{"line_number":224,"context_line":"                check_exit_code\u003d[1, 0], timeout\u003d100)"},{"line_number":225,"context_line":"            cql_running \u003d True if not cql_err else False"},{"line_number":226,"context_line":"            LOG.debug(\"CQL status %s.\" % cql_running)"},{"line_number":227,"context_line":"        if thrift:"},{"line_number":228,"context_line":"            thrift_out, thrift_err \u003d utils.execute_with_timeout("},{"line_number":229,"context_line":"                system.THRIFT_TRANSPORT_STATUS, shell\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_f49eb206","line":226,"in_reply_to":"5a890539_31676cec","updated":"2014-11-19 13:04:32.000000000","message":"Done","commit_id":"b888586a0ab38b0f06082dfd2ca1e6c1173adfcc"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"6fa353c84c4eb1030f8118915e3c860299e89d3e","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def __init__(self, status):"},{"line_number":40,"context_line":"        \"\"\"By default login with root no password for initial setup.\"\"\""},{"line_number":41,"context_line":"        self.state_change_wait_time \u003d 1000"},{"line_number":42,"context_line":"        self.status \u003d status"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def install_if_needed(self, packages):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_1cdd0a43","line":41,"updated":"2014-11-20 01:46:00.000000000","message":"Why are we hard-coding this magic number inline and not using the CONF value?","commit_id":"edc03a2aa2c834c1b4dbdabaa18e0538be554ad8"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"5d66c166c24ad5a8b68db216b24112e457ee4e21","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def __init__(self, status):"},{"line_number":40,"context_line":"        \"\"\"By default login with root no password for initial setup.\"\"\""},{"line_number":41,"context_line":"        self.state_change_wait_time \u003d 1000"},{"line_number":42,"context_line":"        self.status \u003d status"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def install_if_needed(self, packages):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_cb12d157","line":41,"in_reply_to":"5a890539_1cdd0a43","updated":"2014-11-20 07:16:58.000000000","message":"Because given conf timeout is too small. And we should have working status definition out of box, without tuning guest conf file. For example increasing value of given conf attribute may cause problem for those instance datastores that weren\u0027t able to start properly because of waiting too much time to start.\nFor Cassandra 180 secs timeout is not definitely enough because of the way how Cassandra\u0027s start script works.\n\nThe only reasonable way to avoid use of hard coding - make \u0027state_change_wait_time\u0027 option datastore-specific.","commit_id":"edc03a2aa2c834c1b4dbdabaa18e0538be554ad8"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"69c7b1f1fd81389d59a55c434e0da0e13ff48a9e","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        # As common practise start_db method should use"},{"line_number":79,"context_line":"        # CONF.state_change_wait_time, but default value"},{"line_number":80,"context_line":"        # is lower than needed, that\u0027s why next"},{"line_number":81,"context_line":"        # call will use bigger timeout"},{"line_number":82,"context_line":"        if not (self.status."},{"line_number":83,"context_line":"                wait_for_real_status_to_change_to("},{"line_number":84,"context_line":"                rd_instance.ServiceStatuses.RUNNING,"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_9860af36","line":81,"updated":"2014-12-03 15:14:23.000000000","message":"Is this comment still valid? I think not ;)","commit_id":"22aaa03e04c194711e6a067e47e5bc2966080475"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"affef77d6a191d0e790d67bab5e1f421924534ba","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        # As common practise start_db method should use"},{"line_number":79,"context_line":"        # CONF.state_change_wait_time, but default value"},{"line_number":80,"context_line":"        # is lower than needed, that\u0027s why next"},{"line_number":81,"context_line":"        # call will use bigger timeout"},{"line_number":82,"context_line":"        if not (self.status."},{"line_number":83,"context_line":"                wait_for_real_status_to_change_to("},{"line_number":84,"context_line":"                rd_instance.ServiceStatuses.RUNNING,"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_be539314","line":81,"in_reply_to":"5a890539_9860af36","updated":"2014-12-03 15:30:00.000000000","message":"Done","commit_id":"22aaa03e04c194711e6a067e47e5bc2966080475"},{"author":{"_account_id":10725,"name":"Sergey Gotliv","email":"sgotliv@redhat.com","username":"sgotliv"},"change_message_id":"8b39872363db8015743fe45141c28351d15ab424","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # guest should check if Cassandra transports"},{"line_number":233,"context_line":"        # were enabled and started or at least starting"},{"line_number":234,"context_line":"        services_were_started \u003d ("},{"line_number":235,"context_line":"            (cql_running and cql) and (thrift_running and thrift))"},{"line_number":236,"context_line":"        return services_were_started"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _get_actual_db_status(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_df4a9682","line":235,"updated":"2014-12-09 07:32:17.000000000","message":"\u0027OR\u0027 should be used instead of \u0027AND\u0027 between parentheses:\n\n(cql_running and cql) or (thrift_running and thrift)","commit_id":"15f0ff5aae3d9c7d960e05784e9a6a3974aea901"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"3c89071fb46d051e801a5080ea96facaa7958cfd","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # guest should check if Cassandra transports"},{"line_number":233,"context_line":"        # were enabled and started or at least starting"},{"line_number":234,"context_line":"        services_were_started \u003d ("},{"line_number":235,"context_line":"            (cql_running and cql) and (thrift_running and thrift))"},{"line_number":236,"context_line":"        return services_were_started"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _get_actual_db_status(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_eb7c98b7","line":235,"in_reply_to":"3a961159_df4a9682","updated":"2014-12-11 13:52:33.000000000","message":"Done","commit_id":"15f0ff5aae3d9c7d960e05784e9a6a3974aea901"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"d14c8c8546c35fe7085b3db2e340b516063714e5","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                system.CQL_TRANSPORT_STATUS, shell\u003dTrue,"},{"line_number":220,"context_line":"                check_exit_code\u003d[1, 0], timeout\u003d100)"},{"line_number":221,"context_line":"            cql_running \u003d True if not cql_err else False"},{"line_number":222,"context_line":"        if thrift:"},{"line_number":223,"context_line":"            thrift_out, thrift_err \u003d utils.execute_with_timeout("},{"line_number":224,"context_line":"                system.THRIFT_TRANSPORT_STATUS, shell\u003dTrue,"},{"line_number":225,"context_line":"                check_exit_code\u003d[1, 0], timeout\u003d100)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_7ae4e5e0","line":222,"updated":"2014-12-11 18:58:51.000000000","message":"out of cql and thrift, doesn\u0027t at least one of them need to be running? doesn\u0027t look like the logic asserts that.","commit_id":"3d39886186069cc2a223004297416bc58f0390ae"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"b76f170f78d4e2b93d29b6839a66fac63ae6de28","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # guest should check if Cassandra transports"},{"line_number":233,"context_line":"        # were enabled and started or at least starting"},{"line_number":234,"context_line":"        services_were_started \u003d ("},{"line_number":235,"context_line":"            (cql_running and cql) or (thrift_running and thrift))"},{"line_number":236,"context_line":"        return services_were_started"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _get_actual_db_status(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_eff739d1","line":235,"updated":"2014-12-11 18:18:28.000000000","message":"Sorry, this doesn\u0027t work for me. \n\nHere\u0027s a truth table for this logic.\n\ncr \u003d True | cq \u003d True | tr \u003d True | th \u003d True | True\ncr \u003d True | cq \u003d True | tr \u003d True | th \u003d False | True\ncr \u003d True | cq \u003d True | tr \u003d False | th \u003d True | True\ncr \u003d True | cq \u003d True | tr \u003d False | th \u003d False | True\ncr \u003d True | cq \u003d False | tr \u003d True | th \u003d True | True\ncr \u003d True | cq \u003d False | tr \u003d True | th \u003d False | False\ncr \u003d True | cq \u003d False | tr \u003d False | th \u003d True | False\ncr \u003d True | cq \u003d False | tr \u003d False | th \u003d False | False\ncr \u003d False | cq \u003d True | tr \u003d True | th \u003d True | True\ncr \u003d False | cq \u003d True | tr \u003d True | th \u003d False | False\ncr \u003d False | cq \u003d True | tr \u003d False | th \u003d True | False\ncr \u003d False | cq \u003d True | tr \u003d False | th \u003d False | False\ncr \u003d False | cq \u003d False | tr \u003d True | th \u003d True | True\ncr \u003d False | cq \u003d False | tr \u003d True | th \u003d False | False\ncr \u003d False | cq \u003d False | tr \u003d False | th \u003d True | False\ncr \u003d False | cq \u003d False | tr \u003d False | th \u003d False | False\n\nthe issue is this ...\n\nif cql and thrift are both true, then this \"OR\" will incorrectly return True if just one of the services is running.\n\nthe correct code for this is going to have to be a little more complex.\n\nWhat about:\n\n((cql and cql_running) or (not cql)) \nand ((thrift_running and thrift) or (not thrift))","commit_id":"3d39886186069cc2a223004297416bc58f0390ae"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"8aa2e190372e16f672494e3e93ccd3c5b08c5188","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # guest should check if Cassandra transports"},{"line_number":233,"context_line":"        # were enabled and started or at least starting"},{"line_number":234,"context_line":"        services_were_started \u003d ("},{"line_number":235,"context_line":"            (cql_running and cql) or (thrift_running and thrift))"},{"line_number":236,"context_line":"        return services_were_started"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _get_actual_db_status(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_4733d1aa","line":235,"in_reply_to":"3a961159_352a6e90","updated":"2014-12-11 19:58:09.000000000","message":"IMO if one is up then Cassandra is up, but I understand that it might be a problem if the user configured one transport and that transport is not running. I don\u0027t know if this is the patch to make such control though.","commit_id":"3d39886186069cc2a223004297416bc58f0390ae"},{"author":{"_account_id":10725,"name":"Sergey Gotliv","email":"sgotliv@redhat.com","username":"sgotliv"},"change_message_id":"0aba25c1f3b02b41ca2cff37e140e7f1cba8ef00","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # guest should check if Cassandra transports"},{"line_number":233,"context_line":"        # were enabled and started or at least starting"},{"line_number":234,"context_line":"        services_were_started \u003d ("},{"line_number":235,"context_line":"            (cql_running and cql) or (thrift_running and thrift))"},{"line_number":236,"context_line":"        return services_were_started"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _get_actual_db_status(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_352a6e90","line":235,"in_reply_to":"3a961159_da8a39be","updated":"2014-12-11 19:45:31.000000000","message":"I agree with Victoria, that\u0027s my understanding.\n\nIf both services are configured (I am not sure who actually configure them both, but I am not so experienced with Cassandra), but only one is running so Casandra is running, right? I don\u0027t think we should validate configuration, we should check if its running or not.","commit_id":"3d39886186069cc2a223004297416bc58f0390ae"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"d14c8c8546c35fe7085b3db2e340b516063714e5","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # guest should check if Cassandra transports"},{"line_number":233,"context_line":"        # were enabled and started or at least starting"},{"line_number":234,"context_line":"        services_were_started \u003d ("},{"line_number":235,"context_line":"            (cql_running and cql) or (thrift_running and thrift))"},{"line_number":236,"context_line":"        return services_were_started"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _get_actual_db_status(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_daa4f9a7","line":235,"in_reply_to":"3a961159_eff739d1","updated":"2014-12-11 18:58:51.000000000","message":"+1","commit_id":"3d39886186069cc2a223004297416bc58f0390ae"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"f30034ecfccafd8407ab8562b687cac02c216184","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        # guest should check if Cassandra transports"},{"line_number":233,"context_line":"        # were enabled and started or at least starting"},{"line_number":234,"context_line":"        services_were_started \u003d ("},{"line_number":235,"context_line":"            (cql_running and cql) or (thrift_running and thrift))"},{"line_number":236,"context_line":"        return services_were_started"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _get_actual_db_status(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_da8a39be","line":235,"in_reply_to":"3a961159_eff739d1","updated":"2014-12-11 19:08:41.000000000","message":"AFAIK CQL provides an API to Cassandra that is simpler than the Thrift API. Doesn\u0027t that mean that its ok if one of them is down? That statement will be true only if one of the ANDs is true, so it guarantees that either CQL or Thrift will be up and running. Please correct me if I\u0027m wrong.","commit_id":"3d39886186069cc2a223004297416bc58f0390ae"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"a0dd84090c80ade24c30e837c76b4c3dd12bedd3","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        if not (self.status."},{"line_number":79,"context_line":"                wait_for_real_status_to_change_to("},{"line_number":80,"context_line":"                rd_instance.ServiceStatuses.RUNNING,"},{"line_number":81,"context_line":"                self.state_change_wait_time, update_db)):"},{"line_number":82,"context_line":"            try:"},{"line_number":83,"context_line":"                utils.execute_with_timeout(system.CASSANDRA_KILL,"},{"line_number":84,"context_line":"                                           shell\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa81d914_d1733a13","line":81,"updated":"2015-01-29 12:35:50.000000000","message":"is this only a whitespace change?","commit_id":"d40d43ec239883098d13b934cdb66fee34e55fad"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"7aecbaec2bb02280ef4bb2a7d21433e2ff0f7882","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        if not (self.status."},{"line_number":79,"context_line":"                wait_for_real_status_to_change_to("},{"line_number":80,"context_line":"                rd_instance.ServiceStatuses.RUNNING,"},{"line_number":81,"context_line":"                self.state_change_wait_time, update_db)):"},{"line_number":82,"context_line":"            try:"},{"line_number":83,"context_line":"                utils.execute_with_timeout(system.CASSANDRA_KILL,"},{"line_number":84,"context_line":"                                           shell\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa81d914_62b9dae8","line":81,"in_reply_to":"fa81d914_d1733a13","updated":"2015-01-29 13:22:44.000000000","message":"yes","commit_id":"d40d43ec239883098d13b934cdb66fee34e55fad"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"beef6a754622fc3a7dae62db64964061dccb4b18","unresolved":false,"context_lines":[{"line_number":215,"context_line":"class CassandraAppStatus(service.BaseDbStatus):"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def _get_enabled_transport_services(self):"},{"line_number":218,"context_line":"        conf \u003d CassandraApp.read_conf()"},{"line_number":219,"context_line":"        thrift_enabled \u003d conf.get(\u0027start_rpc\u0027)"},{"line_number":220,"context_line":"        cql_enabled \u003d conf.get(\u0027start_native_transport\u0027)"},{"line_number":221,"context_line":"        LOG.debug(\"CQL enabled: %s.Thrift enabled: %s.\""}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_3c9f3b05","line":218,"updated":"2015-02-09 13:57:58.000000000","message":"There appears to be nothing catching a possible failure in read_conf(). If the recently surfaced issue with permission on cassandra.yaml is any indication, this read could fail because the guest is running as a user other than cassandra. Would it make sense to attempt to try on this operation?","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"7830fb9e83019cd4ba411c240339bded45ce642b","unresolved":false,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def _get_enabled_transport_services(self):"},{"line_number":218,"context_line":"        conf \u003d CassandraApp.read_conf()"},{"line_number":219,"context_line":"        thrift_enabled \u003d conf.get(\u0027start_rpc\u0027)"},{"line_number":220,"context_line":"        cql_enabled \u003d conf.get(\u0027start_native_transport\u0027)"},{"line_number":221,"context_line":"        LOG.debug(\"CQL enabled: %s.Thrift enabled: %s.\""},{"line_number":222,"context_line":"                  % (cql_enabled, thrift_enabled))"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_954715bc","line":219,"updated":"2015-02-16 06:22:24.000000000","message":"I would agree to the fact of adding some exception handling so that in case of failure of reading of conf we are not proceeding further and failing back in a planned manner","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"beef6a754622fc3a7dae62db64964061dccb4b18","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                  % (cql_enabled, thrift_enabled))"},{"line_number":223,"context_line":"        return thrift_enabled, cql_enabled"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    def _verify_cassandra_started(self):"},{"line_number":226,"context_line":"        thrift, cql \u003d self._get_enabled_transport_services()"},{"line_number":227,"context_line":"        cql_running, thrift_running \u003d False, False"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_87f530cd","line":225,"updated":"2015-02-09 13:57:58.000000000","message":"This code uses a hard-coded 100s for the check for service start, actually two consecutive 100s timeouts which has its own interesting effects. Why not a configuration parameter?","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"beef6a754622fc3a7dae62db64964061dccb4b18","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        if cql:"},{"line_number":232,"context_line":"            cql_out, cql_err \u003d utils.execute_with_timeout("},{"line_number":233,"context_line":"                system.CQL_TRANSPORT_STATUS, shell\u003dTrue,"},{"line_number":234,"context_line":"                check_exit_code\u003d[1, 0], timeout\u003d100)"},{"line_number":235,"context_line":"            cql_running \u003d True if not cql_err else False"},{"line_number":236,"context_line":"        if thrift:"},{"line_number":237,"context_line":"            thrift_out, thrift_err \u003d utils.execute_with_timeout("}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_bc05eb15","line":234,"updated":"2015-02-09 13:57:58.000000000","message":"This specific invocation makes the assumption that on an exit code of 1, something must be pushed to stderr. This seems a reasonable expectation but seems like an unnecessary one. Why not leave the default exit_code check (i.e. for 0) and allow execute_with_timeout to throw an exception if an exit code of 1 were returned? In any event, you have nothing here to handle a thrown error which seems less than ideal.\n\nIn addition, execute_with_timeout could throw a timeout.","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"beef6a754622fc3a7dae62db64964061dccb4b18","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            cql_running \u003d True if not cql_err else False"},{"line_number":236,"context_line":"        if thrift:"},{"line_number":237,"context_line":"            thrift_out, thrift_err \u003d utils.execute_with_timeout("},{"line_number":238,"context_line":"                system.THRIFT_TRANSPORT_STATUS, shell\u003dTrue,"},{"line_number":239,"context_line":"                check_exit_code\u003d[1, 0], timeout\u003d100)"},{"line_number":240,"context_line":"            thrift_running \u003d True if not thrift_err else False"},{"line_number":241,"context_line":"        LOG.debug(\"CQL is running: %s.Thrift is running: %s.\""}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_475c1826","line":238,"updated":"2015-02-09 13:57:58.000000000","message":"Comments similar to the ones above with CQL status.","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"7830fb9e83019cd4ba411c240339bded45ce642b","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        if thrift:"},{"line_number":237,"context_line":"            thrift_out, thrift_err \u003d utils.execute_with_timeout("},{"line_number":238,"context_line":"                system.THRIFT_TRANSPORT_STATUS, shell\u003dTrue,"},{"line_number":239,"context_line":"                check_exit_code\u003d[1, 0], timeout\u003d100)"},{"line_number":240,"context_line":"            thrift_running \u003d True if not thrift_err else False"},{"line_number":241,"context_line":"        LOG.debug(\"CQL is running: %s.Thrift is running: %s.\""},{"line_number":242,"context_line":"                  % (cql_running, thrift_running))"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_d510adad","line":239,"updated":"2015-02-16 06:22:24.000000000","message":"Multiple hard-coded timeouts","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"eec265b545901cc137c173003da892e79c4bc3ed","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                system.THRIFT_TRANSPORT_STATUS, shell\u003dTrue,"},{"line_number":239,"context_line":"                check_exit_code\u003d[1, 0], timeout\u003d100)"},{"line_number":240,"context_line":"            thrift_running \u003d True if not thrift_err else False"},{"line_number":241,"context_line":"        LOG.debug(\"CQL is running: %s.Thrift is running: %s.\""},{"line_number":242,"context_line":"                  % (cql_running, thrift_running))"},{"line_number":243,"context_line":"        # possibly previous execs were finished with code 1,"},{"line_number":244,"context_line":"        # that\u0027s why guest should verify that Cassandra"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_b5944b8d","line":241,"updated":"2015-02-06 21:22:27.000000000","message":"I think we should note any \u0027cql_err\u0027 or \u0027thrift_err\u0027 here as well.\nI am running into some issues with this in my test environment and it would be really helpful to have the cause in the log.","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"1c489b9b56758b7c1239feb58a31595eecf5d804","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                system.THRIFT_TRANSPORT_STATUS, shell\u003dTrue,"},{"line_number":239,"context_line":"                check_exit_code\u003d[1, 0], timeout\u003d100)"},{"line_number":240,"context_line":"            thrift_running \u003d True if not thrift_err else False"},{"line_number":241,"context_line":"        LOG.debug(\"CQL is running: %s.Thrift is running: %s.\""},{"line_number":242,"context_line":"                  % (cql_running, thrift_running))"},{"line_number":243,"context_line":"        # possibly previous execs were finished with code 1,"},{"line_number":244,"context_line":"        # that\u0027s why guest should verify that Cassandra"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_fd906150","line":241,"in_reply_to":"da86d52c_b5944b8d","updated":"2015-02-07 07:32:08.000000000","message":"It will be there since oslo.concurenncy method already logs stderr and stdout, take a look at that code.","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"beef6a754622fc3a7dae62db64964061dccb4b18","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        # that\u0027s why guest should verify that Cassandra"},{"line_number":245,"context_line":"        # actually starting in a background -"},{"line_number":246,"context_line":"        # guest should check if Cassandra transports"},{"line_number":247,"context_line":"        # were enabled and started or at least starting"},{"line_number":248,"context_line":"        services_were_started \u003d ("},{"line_number":249,"context_line":"            ((cql and cql_running) or (not cql)) and"},{"line_number":250,"context_line":"            ((thrift_running and thrift) or (not thrift))"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_a78f9474","line":247,"updated":"2015-02-09 13:57:58.000000000","message":"This comment is out of sync with the code.","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"beef6a754622fc3a7dae62db64964061dccb4b18","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        # actually starting in a background -"},{"line_number":246,"context_line":"        # guest should check if Cassandra transports"},{"line_number":247,"context_line":"        # were enabled and started or at least starting"},{"line_number":248,"context_line":"        services_were_started \u003d ("},{"line_number":249,"context_line":"            ((cql and cql_running) or (not cql)) and"},{"line_number":250,"context_line":"            ((thrift_running and thrift) or (not thrift))"},{"line_number":251,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_47cfb8b0","line":248,"updated":"2015-02-09 13:57:58.000000000","message":"A comment explaining this conditional may be worthwhile given that it went through at least three iterations in the code review process. Right now I think it is right, fingers crossed.","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"}],"trove/guestagent/datastore/cassandra/system.py":[{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"beef6a754622fc3a7dae62db64964061dccb4b18","unresolved":false,"context_lines":[{"line_number":30,"context_line":"START_CASSANDRA \u003d \"sudo service cassandra start\""},{"line_number":31,"context_line":"STOP_CASSANDRA \u003d \"sudo service cassandra stop\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CQL_TRANSPORT_STATUS \u003d ("},{"line_number":34,"context_line":"    \u0027echo \"use system;\" \u003e/tmp/check; cqlsh -f /tmp/check\u0027)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"THRIFT_TRANSPORT_STATUS \u003d \"nodetool statusthrift\""}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_4717b851","line":33,"updated":"2015-02-09 13:57:58.000000000","message":"It appears that this is incorrect. I\u0027m no expert on cassandra but based on a short IRC chat with someone on #cassandra, it appears that if thirft is enabled and CQL is disabled, this test could pass if thrift is working properly.\n\nThe proper way to do this is, apparently using the \"nodetool statusbinary\".\n\nI would be uncomfortable merging this patch based on my 30 minutes of IRC research and unless someone with good cassandra knowledge is willing to bless this change, I\u0027m not sure how we can be sure that it is a good change.","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"eec265b545901cc137c173003da892e79c4bc3ed","unresolved":false,"context_lines":[{"line_number":31,"context_line":"STOP_CASSANDRA \u003d \"sudo service cassandra stop\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CQL_TRANSPORT_STATUS \u003d ("},{"line_number":34,"context_line":"    \u0027echo \"use system;\" \u003e/tmp/check; cqlsh -f /tmp/check\u0027)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"THRIFT_TRANSPORT_STATUS \u003d \"nodetool statusthrift\""},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_35e85b16","line":34,"updated":"2015-02-06 21:22:27.000000000","message":"\u0027echo \"use system;\" \u003e/tmp/check;\u0027 is platform specific. Should open a new temp file and write into it using Python.","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"1c489b9b56758b7c1239feb58a31595eecf5d804","unresolved":false,"context_lines":[{"line_number":31,"context_line":"STOP_CASSANDRA \u003d \"sudo service cassandra stop\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CQL_TRANSPORT_STATUS \u003d ("},{"line_number":34,"context_line":"    \u0027echo \"use system;\" \u003e/tmp/check; cqlsh -f /tmp/check\u0027)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"THRIFT_TRANSPORT_STATUS \u003d \"nodetool statusthrift\""},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_dd7abd24","line":34,"in_reply_to":"da86d52c_35e85b16","updated":"2015-02-07 07:32:08.000000000","message":"Please explain why is it platform specific?","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"95b30711067eb6d722fcdc5edd96f65708a8fe1e","unresolved":false,"context_lines":[{"line_number":31,"context_line":"STOP_CASSANDRA \u003d \"sudo service cassandra stop\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CQL_TRANSPORT_STATUS \u003d ("},{"line_number":34,"context_line":"    \u0027echo \"use system;\" \u003e/tmp/check; cqlsh -f /tmp/check\u0027)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"THRIFT_TRANSPORT_STATUS \u003d \"nodetool statusthrift\""},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_312b0296","line":34,"in_reply_to":"da86d52c_dd7abd24","updated":"2015-02-07 18:24:52.000000000","message":"We can break it down to two commands.\n\nThe first one uses a shell scripting language \u003d\u003d language other than Python.\n\nIt may work in bash/dash, but can we ever guarantee it would work in any environment?\nWill whatever default shell is used by the underlying OS support (this particular) redirection? Will \u0027echo\u0027 always do exactly what it is supposed to?\nPython already has a well documented method for writing into files which is guaranteed and tested in every Python port.\n\nThe second command references \u0027/tmp/check\u0027. This is very \"Linuxy\"...\n\nWill it work on Windows? Probably not. Will it even work on Ubuntu Linux with some \u0027custom\u0027 filesystem structure?\nAs opposed to Python\u0027s \u0027tempfile.NamedTemporaryFile(...)\u0027 which: \"... generates temporary files and directories. It works on all supported platforms ...\" by obtaining the proper path from the underlying OS?\n\nP. :-)","commit_id":"6b2407e37e612647080b3cf05567b40b41e293f2"}],"trove/tests/unittests/guestagent/test_dbaas.py":[{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"69c7b1f1fd81389d59a55c434e0da0e13ff48a9e","unresolved":false,"context_lines":[{"line_number":1509,"context_line":""},{"line_number":1510,"context_line":"    def test_start_cassandra(self):"},{"line_number":1511,"context_line":""},{"line_number":1512,"context_line":"        cass_service.utils.execute_with_timeout \u003d Mock(return_value\u003d[\"\", \"\"])"},{"line_number":1513,"context_line":"        self.appStatus.set_next_status(rd_instance.ServiceStatuses.RUNNING)"},{"line_number":1514,"context_line":""},{"line_number":1515,"context_line":"        self.cassandra.start_db()"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_38ffbb35","line":1512,"updated":"2014-12-03 15:14:23.000000000","message":"This may be a minor difference and may be immaterial to this change but I believe the proper return value from a Mocked execute[_with_timeout] should be (\"\", \"\"). i.e. a tuple and not a list.","commit_id":"22aaa03e04c194711e6a067e47e5bc2966080475"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"affef77d6a191d0e790d67bab5e1f421924534ba","unresolved":false,"context_lines":[{"line_number":1509,"context_line":""},{"line_number":1510,"context_line":"    def test_start_cassandra(self):"},{"line_number":1511,"context_line":""},{"line_number":1512,"context_line":"        cass_service.utils.execute_with_timeout \u003d Mock(return_value\u003d[\"\", \"\"])"},{"line_number":1513,"context_line":"        self.appStatus.set_next_status(rd_instance.ServiceStatuses.RUNNING)"},{"line_number":1514,"context_line":""},{"line_number":1515,"context_line":"        self.cassandra.start_db()"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_be1c7367","line":1512,"in_reply_to":"5a890539_38ffbb35","updated":"2014-12-03 15:30:00.000000000","message":"Done","commit_id":"22aaa03e04c194711e6a067e47e5bc2966080475"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"69c7b1f1fd81389d59a55c434e0da0e13ff48a9e","unresolved":false,"context_lines":[{"line_number":1530,"context_line":""},{"line_number":1531,"context_line":"    def test_start_db_with_db_update(self):"},{"line_number":1532,"context_line":""},{"line_number":1533,"context_line":"        cass_service.utils.execute_with_timeout \u003d Mock(return_value\u003d[\"\", \"\"])"},{"line_number":1534,"context_line":"        self.appStatus.set_next_status("},{"line_number":1535,"context_line":"            rd_instance.ServiceStatuses.RUNNING)"},{"line_number":1536,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_f8a0d343","line":1533,"updated":"2014-12-03 15:14:23.000000000","message":"This may be a minor difference and may be immaterial to this change but I believe the proper return value from a Mocked execute[_with_timeout] should be (\"\", \"\"). i.e. a tuple and not a list.","commit_id":"22aaa03e04c194711e6a067e47e5bc2966080475"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"affef77d6a191d0e790d67bab5e1f421924534ba","unresolved":false,"context_lines":[{"line_number":1530,"context_line":""},{"line_number":1531,"context_line":"    def test_start_db_with_db_update(self):"},{"line_number":1532,"context_line":""},{"line_number":1533,"context_line":"        cass_service.utils.execute_with_timeout \u003d Mock(return_value\u003d[\"\", \"\"])"},{"line_number":1534,"context_line":"        self.appStatus.set_next_status("},{"line_number":1535,"context_line":"            rd_instance.ServiceStatuses.RUNNING)"},{"line_number":1536,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5a890539_de213f9f","line":1533,"in_reply_to":"5a890539_f8a0d343","updated":"2014-12-03 15:30:00.000000000","message":"Done","commit_id":"22aaa03e04c194711e6a067e47e5bc2966080475"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"a0dd84090c80ade24c30e837c76b4c3dd12bedd3","unresolved":false,"context_lines":[{"line_number":1509,"context_line":""},{"line_number":1510,"context_line":"    def test_start_cassandra(self):"},{"line_number":1511,"context_line":""},{"line_number":1512,"context_line":"        cass_service.utils.execute_with_timeout \u003d Mock(return_value\u003d(\"\", \"\"))"},{"line_number":1513,"context_line":"        self.appStatus.set_next_status(rd_instance.ServiceStatuses.RUNNING)"},{"line_number":1514,"context_line":""},{"line_number":1515,"context_line":"        self.cassandra.start_db()"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa81d914_f1c83e24","line":1512,"updated":"2015-01-29 12:35:50.000000000","message":"This is not a safe way to mock execute_with_timeout. Not that your change makes it any more unsafe but why the change anyway?","commit_id":"d40d43ec239883098d13b934cdb66fee34e55fad"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"a0dd84090c80ade24c30e837c76b4c3dd12bedd3","unresolved":false,"context_lines":[{"line_number":1530,"context_line":""},{"line_number":1531,"context_line":"    def test_start_db_with_db_update(self):"},{"line_number":1532,"context_line":""},{"line_number":1533,"context_line":"        cass_service.utils.execute_with_timeout \u003d Mock(return_value\u003d(\"\", \"\"))"},{"line_number":1534,"context_line":"        self.appStatus.set_next_status("},{"line_number":1535,"context_line":"            rd_instance.ServiceStatuses.RUNNING)"},{"line_number":1536,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"fa81d914_11ccf234","line":1533,"updated":"2015-01-29 12:35:50.000000000","message":"This is not a safe way to mock execute_with_timeout. Not that your change makes it any more unsafe but why the change anyway?","commit_id":"d40d43ec239883098d13b934cdb66fee34e55fad"}]}