)]}'
{"cinder/scheduler/host_manager.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d2f74a3b67dfb2024f5c55db876ef69348c0715b","unresolved":false,"context_lines":[{"line_number":421,"context_line":"            \u0027cinder.scheduler.weights\u0027)"},{"line_number":422,"context_line":"        self.weight_classes \u003d self.weight_handler.get_all_classes()"},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"        self._active_capabilities_backends \u003d set()  # Services with capabilities"},{"line_number":425,"context_line":"        self._no_capabilities_backends \u003d set()  # Services without capabilities"},{"line_number":426,"context_line":"        self._update_backend_state_map(cinder_context.get_admin_context())"},{"line_number":427,"context_line":"        self.service_states_last_update \u003d {}"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_221784f1","line":424,"range":{"start_line":424,"start_character":13,"end_line":424,"end_character":42},"updated":"2018-11-28 11:01:46.000000000","message":"-1: I haven\u0027t touched this code in a while, but isn\u0027t this the same thing as `self.backend_state_map`?","commit_id":"d82a753253a1593540c80993b30a3e650dc2f7d6"},{"author":{"_account_id":24250,"name":"wangwei","email":"wangw88916@126.com","username":"Dream_x"},"change_message_id":"74595f2fd4cd3644154253beea37cc3b084b4dc9","unresolved":false,"context_lines":[{"line_number":421,"context_line":"            \u0027cinder.scheduler.weights\u0027)"},{"line_number":422,"context_line":"        self.weight_classes \u003d self.weight_handler.get_all_classes()"},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"        self._active_capabilities_backends \u003d set()  # Services with capabilities"},{"line_number":425,"context_line":"        self._no_capabilities_backends \u003d set()  # Services without capabilities"},{"line_number":426,"context_line":"        self._update_backend_state_map(cinder_context.get_admin_context())"},{"line_number":427,"context_line":"        self.service_states_last_update \u003d {}"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_3e3786b1","line":424,"range":{"start_line":424,"start_character":13,"end_line":424,"end_character":42},"in_reply_to":"3f79a3b5_221784f1","updated":"2018-11-29 11:06:16.000000000","message":"hi, this variable is used to save the backend of the returned message, similar to self.backend_state_map, but self.backend_state_map will not update when receiving rpc message, so if we want to use self.backend_state_map we need to call _update_backend_state_map method in the update_service_capabilities method, I modified the code to update it.","commit_id":"d82a753253a1593540c80993b30a3e650dc2f7d6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d2f74a3b67dfb2024f5c55db876ef69348c0715b","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        return len(self._no_capabilities_backends) \u003d\u003d 0"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"    def first_receive_capabilities(self):"},{"line_number":590,"context_line":"        if (self.service_states_last_update and"},{"line_number":591,"context_line":"            len(self._active_capabilities_backends) \u003e 0 and"},{"line_number":592,"context_line":"                len(self._no_capabilities_backends) \u003d\u003d 0):"},{"line_number":593,"context_line":"            return True"},{"line_number":594,"context_line":"        return False"},{"line_number":595,"context_line":""},{"line_number":596,"context_line":"    def _update_backend_state_map(self, context):"},{"line_number":597,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_1c83ceab","line":594,"range":{"start_line":590,"start_character":0,"end_line":594,"end_character":20},"updated":"2018-11-28 11:01:46.000000000","message":"Since the evaluation matches the return value, why not return it?\n\n        return (self.service_states_last_update and\n                len(self._active_capabilities_backends) \u003e 0 and\n                len(self._no_capabilities_backends) \u003d\u003d 0)","commit_id":"d82a753253a1593540c80993b30a3e650dc2f7d6"},{"author":{"_account_id":24250,"name":"wangwei","email":"wangw88916@126.com","username":"Dream_x"},"change_message_id":"74595f2fd4cd3644154253beea37cc3b084b4dc9","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        return len(self._no_capabilities_backends) \u003d\u003d 0"},{"line_number":588,"context_line":""},{"line_number":589,"context_line":"    def first_receive_capabilities(self):"},{"line_number":590,"context_line":"        if (self.service_states_last_update and"},{"line_number":591,"context_line":"            len(self._active_capabilities_backends) \u003e 0 and"},{"line_number":592,"context_line":"                len(self._no_capabilities_backends) \u003d\u003d 0):"},{"line_number":593,"context_line":"            return True"},{"line_number":594,"context_line":"        return False"},{"line_number":595,"context_line":""},{"line_number":596,"context_line":"    def _update_backend_state_map(self, context):"},{"line_number":597,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_5e3cc290","line":594,"range":{"start_line":590,"start_character":0,"end_line":594,"end_character":20},"in_reply_to":"3f79a3b5_1c83ceab","updated":"2018-11-29 11:06:16.000000000","message":"Thanks for your detailed reply, I have modified the code.","commit_id":"d82a753253a1593540c80993b30a3e650dc2f7d6"}],"cinder/scheduler/manager.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bad9ed5968049b06c662a825496fa27419a425b1","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        self.request_service_capabilities(ctxt)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        self.run_periodic_tasks(ctxt)"},{"line_number":108,"context_line":"        self._startup_delay \u003d False"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def reset(self):"},{"line_number":111,"context_line":"        super(SchedulerManager, self).reset()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_ca9ac5c1","line":108,"updated":"2018-11-22 08:58:48.000000000","message":"-1: This is not correct.  Even if we are making a request to the volumes services to send their stats, this is an asynchronous call, so we cannot change _startup_delay to False until we have actually received the data.\n\nThis patch would reintroduce bug https://bugs.launchpad.net/cinder/+bug/1409012","commit_id":"3296cffdf291acff4966cfcc1a5a31c32ff04b07"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"d8b6e7e2adb1d969a3ac4c576cfe9798378dd95b","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":105,"context_line":"        self.request_service_capabilities(ctxt)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        for t in range(CONF.periodic_interval):"},{"line_number":108,"context_line":"            if self.driver.is_first_receive():"},{"line_number":109,"context_line":"                break"},{"line_number":110,"context_line":"            eventlet.sleep(1)"},{"line_number":111,"context_line":"        self._startup_delay \u003d False"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    def reset(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_f318a772","line":110,"range":{"start_line":107,"start_character":0,"end_line":110,"end_character":29},"updated":"2018-11-26 19:51:15.000000000","message":"Why is this being done as a range?\n\nAlso, t isn\u0027t being used, this will always just sleep 1.","commit_id":"6e3364f6c0eee076e8bcde50296833394ab9d673"},{"author":{"_account_id":24250,"name":"wangwei","email":"wangw88916@126.com","username":"Dream_x"},"change_message_id":"91a347dc41453201f2c686304eda42f3f08d5c8d","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":105,"context_line":"        self.request_service_capabilities(ctxt)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        for t in range(CONF.periodic_interval):"},{"line_number":108,"context_line":"            if self.driver.is_first_receive():"},{"line_number":109,"context_line":"                break"},{"line_number":110,"context_line":"            eventlet.sleep(1)"},{"line_number":111,"context_line":"        self._startup_delay \u003d False"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    def reset(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_02c86b1d","line":110,"range":{"start_line":107,"start_character":0,"end_line":110,"end_character":29},"in_reply_to":"3f79a3b5_f318a772","updated":"2018-11-27 01:20:31.000000000","message":"I mean there is a loop here, the upper limit of the loop is CONF.periodic_interval, sleep only one second each time, if it is judged that the message is received, it will jump out of the loop, otherwise it will reach the upper limit. You are right, t is not used, and using the \u0027range\u0027 code can be simpler, but with increased memory usage, I will use \u0027while\u0027 instead, thanks for your review.","commit_id":"6e3364f6c0eee076e8bcde50296833394ab9d673"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"236929cc5830e84dc787036ac20aba2aa975700f","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        t \u003d 0"},{"line_number":108,"context_line":"        while t \u003c CONF.periodic_interval:"},{"line_number":109,"context_line":"            if self.driver.is_first_receive():"},{"line_number":110,"context_line":"                break"},{"line_number":111,"context_line":"            eventlet.sleep(1)"},{"line_number":112,"context_line":"            t +\u003d 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_44e2ec00","line":109,"updated":"2018-11-27 08:12:52.000000000","message":"-1, I think self.driver.is_ready() is better to use.That means at least a volume service is ready.","commit_id":"f3f81497188abafd653a63e8b4da89ebe6985c49"},{"author":{"_account_id":24250,"name":"wangwei","email":"wangw88916@126.com","username":"Dream_x"},"change_message_id":"906d7d7fe331f94a5a84a4cd898fdca450013be0","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        t \u003d 0"},{"line_number":108,"context_line":"        while t \u003c CONF.periodic_interval:"},{"line_number":109,"context_line":"            if self.driver.is_first_receive():"},{"line_number":110,"context_line":"                break"},{"line_number":111,"context_line":"            eventlet.sleep(1)"},{"line_number":112,"context_line":"            t +\u003d 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_662b53e8","line":109,"in_reply_to":"3f79a3b5_44e2ec00","updated":"2018-11-28 08:47:57.000000000","message":"Thanks for your review, but there is one case to self.driver.is_ready() method. If the cinder-volume service is not started when the cinder-scheduler service starts, then is_ready will return True because the set is always empty.\n\nI will modify this code.","commit_id":"f3f81497188abafd653a63e8b4da89ebe6985c49"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d2f74a3b67dfb2024f5c55db876ef69348c0715b","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":105,"context_line":"        self.request_service_capabilities(ctxt)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        t \u003d 0"},{"line_number":108,"context_line":"        while t \u003c CONF.periodic_interval:"},{"line_number":109,"context_line":"            if self.driver.is_first_receive():"},{"line_number":110,"context_line":"                break"},{"line_number":111,"context_line":"            eventlet.sleep(1)"},{"line_number":112,"context_line":"            t +\u003d 1"},{"line_number":113,"context_line":"        self._startup_delay \u003d False"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def reset(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_82cc58be","line":112,"range":{"start_line":107,"start_character":0,"end_line":112,"end_character":18},"updated":"2018-11-28 11:01:46.000000000","message":"nit: I liked your approach on patch #7 more, with a range.  To avoid having a non used variable `t` in Python we usually name throwaway variables as `__`.  We do this because following the naming convention of functional programming and naming it as `_` would conflict with the interactive interpreter variable of the same name that holds the result of the last executed expression.\n\nIf we don\u0027t use a range, then we should at leas include the break condition on the while condition, to avoid using the break...\n        t \u003d 0\n        while (not self.driver.is_first_receive() and\n               t \u003c CONF.periodic_interval):\n            eventlet.sleep(1)\n            t +\u003d 1","commit_id":"d82a753253a1593540c80993b30a3e650dc2f7d6"},{"author":{"_account_id":24250,"name":"wangwei","email":"wangw88916@126.com","username":"Dream_x"},"change_message_id":"74595f2fd4cd3644154253beea37cc3b084b4dc9","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":105,"context_line":"        self.request_service_capabilities(ctxt)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        t \u003d 0"},{"line_number":108,"context_line":"        while t \u003c CONF.periodic_interval:"},{"line_number":109,"context_line":"            if self.driver.is_first_receive():"},{"line_number":110,"context_line":"                break"},{"line_number":111,"context_line":"            eventlet.sleep(1)"},{"line_number":112,"context_line":"            t +\u003d 1"},{"line_number":113,"context_line":"        self._startup_delay \u003d False"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def reset(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_3e50e6d1","line":112,"range":{"start_line":107,"start_character":0,"end_line":112,"end_character":18},"in_reply_to":"3f79a3b5_82cc58be","updated":"2018-11-29 11:06:16.000000000","message":"Thanks again for your detailed suggestions, I modified the code to use your first opinion.","commit_id":"d82a753253a1593540c80993b30a3e650dc2f7d6"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"afd37e93b9bac519bd45d2c54daf437310752b0c","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":105,"context_line":"        self.request_service_capabilities(ctxt)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        for __ in range(CONF.periodic_interval):"},{"line_number":108,"context_line":"            if self.driver.is_first_receive():"},{"line_number":109,"context_line":"                break"},{"line_number":110,"context_line":"            eventlet.sleep(1)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_1ae6d24a","line":107,"updated":"2018-12-18 22:25:55.000000000","message":"This confuses me. Why are you doing a range() on the periodic_interval? And then shouldn\u0027t that still be used in the eventlet.sleep call if it isn\u0027t the first time?","commit_id":"0db593d86c2566ee24af8f29b9f2cb2b805dc682"},{"author":{"_account_id":24250,"name":"wangwei","email":"wangw88916@126.com","username":"Dream_x"},"change_message_id":"078f92a56d9825610f7e18c4dbc79b382b976f35","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":105,"context_line":"        self.request_service_capabilities(ctxt)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        for __ in range(CONF.periodic_interval):"},{"line_number":108,"context_line":"            if self.driver.is_first_receive():"},{"line_number":109,"context_line":"                break"},{"line_number":110,"context_line":"            eventlet.sleep(1)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_39b5d9cf","line":107,"in_reply_to":"3f79a3b5_1ae6d24a","updated":"2018-12-19 02:29:26.000000000","message":"Hi, maybe I don\u0027t understand your question very well, so I will talk about what I understand. \nFirst of all, this function is called when cinder-scheduler starts. In this function, first call \u0027self.request_service_capabilities(ctxt)\u0027 to get the information about the cinder-volume backend. This is an rpc.cast method, so in order to ensure that cinder-scheduler starts after receiving the return message of cinder-volume, here is set a sleep time corresponding to CONF.periodic_interval which is a variable that controls the interval between periodic tasks.\n\nBut when we set this value to 3600s for some reason, we need an hour to start cinder-scheduler. In fact, cinder-volume returns the message immediately after receiving the rpc.cast request, so it is unnecessary to wait for the corresponding time of CONF.periodic_interval here.\n\nMy change is to check if the cinder-volume message is received every 1 second. The upper limit of this loop is CONF.periodic_interval. If it receives the message, the program can jump out of the loop and proceed to the following process.","commit_id":"0db593d86c2566ee24af8f29b9f2cb2b805dc682"}]}