)]}'
{"zun/service/periodic.py":[{"author":{"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"},"change_message_id":"1cc05e1f675faba72944d171e09c50f6d0ccde13","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    def sync_container_status(self, ctx):"},{"line_number":52,"context_line":"        LOG.debug(\u0027Update container status start\u0027)"},{"line_number":53,"context_line":"        all_containers \u003d objects.Container.list(ctx)"},{"line_number":54,"context_line":"        status_to_filter \u003d [\u0027Creating\u0027]"},{"line_number":55,"context_line":"        local_containers \u003d self._get_container_on_local_host(all_containers)"},{"line_number":56,"context_line":"        containers \u003d self._filter_containers_on_status("},{"line_number":57,"context_line":"            local_containers, status_to_filter)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_95de08db","line":54,"range":{"start_line":54,"start_character":8,"end_line":54,"end_character":39},"updated":"2017-03-13 08:58:56.000000000","message":"Why are we only filtering on \u0027creating\u0027 status? I think there could be other valid statuses also. For example: a container in \u0027Running\u0027 state can go to \u0027Exited\u0027 after some failure.\n\nWDYT ?","commit_id":"c67d90e4a0bf51a7afe4caa0aa22caf0ccbeca49"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"2dd2510ea0e5d57a8221954c74d4a3529a0872f6","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    def sync_container_status(self, ctx):"},{"line_number":52,"context_line":"        LOG.debug(\u0027Update container status start\u0027)"},{"line_number":53,"context_line":"        all_containers \u003d objects.Container.list(ctx)"},{"line_number":54,"context_line":"        status_to_filter \u003d [\u0027Creating\u0027]"},{"line_number":55,"context_line":"        local_containers \u003d self._get_container_on_local_host(all_containers)"},{"line_number":56,"context_line":"        containers \u003d self._filter_containers_on_status("},{"line_number":57,"context_line":"            local_containers, status_to_filter)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_b575cccb","line":54,"range":{"start_line":54,"start_character":8,"end_line":54,"end_character":39},"in_reply_to":"9a30ddce_95de08db","updated":"2017-03-13 09:03:14.000000000","message":"@Madhuri, Intent of the filter is not to check the status of container in \u0027Creating\u0027 state by calling docker API i.e. If any container  is in \u0027Creating\u0027 state this patch is not going to check and update its status. Do you think  we need to skip any other state?","commit_id":"c67d90e4a0bf51a7afe4caa0aa22caf0ccbeca49"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"0a96f5b6d694f7cb44b2dc35d42c498a1f7c799e","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        containers \u003d self._filter_containers_on_status_and_host(all_containers)"},{"line_number":53,"context_line":"        for container in containers:"},{"line_number":54,"context_line":"            old_status \u003d container.status"},{"line_number":55,"context_line":"            updated_container \u003d self.driver.show(container)"},{"line_number":56,"context_line":"            if old_status !\u003d updated_container.status:"},{"line_number":57,"context_line":"                updated_container.save(ctx)"},{"line_number":58,"context_line":"                message \u003d \u0027Status of container %s changed from %s to %s\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a25e98e_ac1c8778","line":55,"range":{"start_line":55,"start_character":44,"end_line":55,"end_character":48},"updated":"2017-03-13 19:01:09.000000000","message":"This is fine as the first iteration, although I would have concern about the sociability. Suppose there are 100 containers, it will calls \"docker inspect\" 100 times, which is sub-optimal. I would look for an optimization (could be done in the future) to use \"docker ps -a\" to collect the states of all docker containers in one call.","commit_id":"71bc28b5f4ef939a6ceadd2308f284756172ddb2"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"a69ecb3ca2bf281ade6f3267f6f9a1a951bc2ba7","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            container.host \u003d\u003d self.host, containers)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def _find_changed_containers(self, current_state):"},{"line_number":51,"context_line":"        new_containers \u003d list(set(current_state) - set(self.previous_state))"},{"line_number":52,"context_line":"        changed_containers \u003d [k for k in set(self.previous_state) \u0026"},{"line_number":53,"context_line":"                              set(current_state)"},{"line_number":54,"context_line":"                              if current_state[k] !\u003d self.previous_state[k]]"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_9ff27d99","line":51,"range":{"start_line":51,"start_character":8,"end_line":51,"end_character":22},"updated":"2017-03-14 21:47:00.000000000","message":"For completeness, it might be a good idea to process deleted_containers as well. If the periodic tasks find a container that was removed but the db entry is still there, the state should be set to \"Error\" or \"Unknown\".","commit_id":"b0996993dcb5863b52dd0c65f7b0fd26e3f282e6"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"ebb8f4d49580a265f4ec2a5ad1629e29684ae2a2","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            container.host \u003d\u003d self.host, containers)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def _find_changed_containers(self, current_state):"},{"line_number":51,"context_line":"        new_containers \u003d list(set(current_state) - set(self.previous_state))"},{"line_number":52,"context_line":"        changed_containers \u003d [k for k in set(self.previous_state) \u0026"},{"line_number":53,"context_line":"                              set(current_state)"},{"line_number":54,"context_line":"                              if current_state[k] !\u003d self.previous_state[k]]"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_abcfd4ae","line":51,"range":{"start_line":51,"start_character":8,"end_line":51,"end_character":22},"in_reply_to":"3a1ff146_20f32575","updated":"2017-03-15 17:26:17.000000000","message":"@Hongbin, I have thought about this case, and found that this case can happen only if container deletion is in progress, i believe that no one logs in to compute node and deletes the container using docker API.  I think once https://review.openstack.org/#/c/445259/ is merged then i can check the db for current task state and skip it if is in deleting state otherwise i can mark it as Unknown state.\nWhat do you think?","commit_id":"b0996993dcb5863b52dd0c65f7b0fd26e3f282e6"},{"author":{"_account_id":8264,"name":"Shunli Zhou","username":"ShunliZhou"},"change_message_id":"c3f7ce2bcbcb3ba41fd7e165c69ecb1acd050e56","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            container.host \u003d\u003d self.host, containers)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def _find_changed_containers(self, current_state):"},{"line_number":51,"context_line":"        new_containers \u003d list(set(current_state) - set(self.previous_state))"},{"line_number":52,"context_line":"        changed_containers \u003d [k for k in set(self.previous_state) \u0026"},{"line_number":53,"context_line":"                              set(current_state)"},{"line_number":54,"context_line":"                              if current_state[k] !\u003d self.previous_state[k]]"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_20f32575","line":51,"range":{"start_line":51,"start_character":8,"end_line":51,"end_character":22},"in_reply_to":"3a1ff146_9ff27d99","updated":"2017-03-15 02:03:24.000000000","message":"+1","commit_id":"b0996993dcb5863b52dd0c65f7b0fd26e3f282e6"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"054aa89a626f8f7ccfc7a108ed07bce339ef368c","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            container.host \u003d\u003d self.host, containers)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def _find_changed_containers(self, current_state):"},{"line_number":51,"context_line":"        new_containers \u003d list(set(current_state) - set(self.previous_state))"},{"line_number":52,"context_line":"        changed_containers \u003d [k for k in set(self.previous_state) \u0026"},{"line_number":53,"context_line":"                              set(current_state)"},{"line_number":54,"context_line":"                              if current_state[k] !\u003d self.previous_state[k]]"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_dbc77710","line":51,"range":{"start_line":51,"start_character":8,"end_line":51,"end_character":22},"in_reply_to":"3a1ff146_abcfd4ae","updated":"2017-03-15 20:19:43.000000000","message":"I see. Then, never mind.","commit_id":"b0996993dcb5863b52dd0c65f7b0fd26e3f282e6"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"a69ecb3ca2bf281ade6f3267f6f9a1a951bc2ba7","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        current_state \u003d {container[\u0027Id\u0027]: container[\u0027State\u0027]"},{"line_number":63,"context_line":"                         for container in self.driver.list()"},{"line_number":64,"context_line":"                         if \u0027zun-sandbox-\u0027 not in container[\u0027Names\u0027][0]}"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        changed_containers \u003d self._find_changed_containers(current_state)"},{"line_number":67,"context_line":"        if not changed_containers:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_fc1c5f79","line":64,"range":{"start_line":64,"start_character":29,"end_line":64,"end_character":40},"updated":"2017-03-14 21:47:00.000000000","message":"I think it would be better if self.driver.list() will filter out all the infra containers. As a result, we don\u0027t need to do this check manually.","commit_id":"b0996993dcb5863b52dd0c65f7b0fd26e3f282e6"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"ebb8f4d49580a265f4ec2a5ad1629e29684ae2a2","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        current_state \u003d {container[\u0027Id\u0027]: container[\u0027State\u0027]"},{"line_number":63,"context_line":"                         for container in self.driver.list()"},{"line_number":64,"context_line":"                         if \u0027zun-sandbox-\u0027 not in container[\u0027Names\u0027][0]}"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        changed_containers \u003d self._find_changed_containers(current_state)"},{"line_number":67,"context_line":"        if not changed_containers:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_2bbcc404","line":64,"range":{"start_line":64,"start_character":29,"end_line":64,"end_character":40},"in_reply_to":"3a1ff146_fc1c5f79","updated":"2017-03-15 17:26:17.000000000","message":"Agree, will fix it in next patch.","commit_id":"b0996993dcb5863b52dd0c65f7b0fd26e3f282e6"},{"author":{"_account_id":8264,"name":"Shunli Zhou","username":"ShunliZhou"},"change_message_id":"215d58ad01fb54d801b57c74b0058b66178e7ee4","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        db_containers_map \u003d {container.container_id: container"},{"line_number":76,"context_line":"                             for container in containers}"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        for container_id in changed_containers:"},{"line_number":79,"context_line":"            if db_containers_map.get(container_id):"},{"line_number":80,"context_line":"                old_status \u003d db_containers_map.get(container_id).status"},{"line_number":81,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_222f77f7","line":78,"range":{"start_line":78,"start_character":28,"end_line":78,"end_character":46},"updated":"2017-03-16 01:11:33.000000000","message":"I think here should loop by the db containers which is created by the zun. There maybe containers not created by zun, we should not sync these and write them to db. Leave the containers not created by zun as it is.","commit_id":"b0996993dcb5863b52dd0c65f7b0fd26e3f282e6"},{"author":{"_account_id":8264,"name":"Shunli Zhou","username":"ShunliZhou"},"change_message_id":"a425ae537511c4e9b310a4654f9a7aedc2d179f2","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        db_containers_map \u003d {container.container_id: container"},{"line_number":76,"context_line":"                             for container in containers}"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        for container_id in changed_containers:"},{"line_number":79,"context_line":"            if db_containers_map.get(container_id):"},{"line_number":80,"context_line":"                old_status \u003d db_containers_map.get(container_id).status"},{"line_number":81,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_b28bae1d","line":78,"range":{"start_line":78,"start_character":28,"end_line":78,"end_character":46},"in_reply_to":"3a1ff146_1e8ad3bf","updated":"2017-03-16 07:19:10.000000000","message":"Oh, yes, checked by L79. LGTM:)","commit_id":"b0996993dcb5863b52dd0c65f7b0fd26e3f282e6"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"87bf890ef49df252cc0b64be0c32e43b3b00d775","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        db_containers_map \u003d {container.container_id: container"},{"line_number":76,"context_line":"                             for container in containers}"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        for container_id in changed_containers:"},{"line_number":79,"context_line":"            if db_containers_map.get(container_id):"},{"line_number":80,"context_line":"                old_status \u003d db_containers_map.get(container_id).status"},{"line_number":81,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_1e8ad3bf","line":78,"range":{"start_line":78,"start_character":28,"end_line":78,"end_character":46},"in_reply_to":"3a1ff146_222f77f7","updated":"2017-03-16 03:43:02.000000000","message":"line 79 checks the same thing.","commit_id":"b0996993dcb5863b52dd0c65f7b0fd26e3f282e6"},{"author":{"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"},"change_message_id":"1db2f12962a8322a9dfd2f3f85d8de3d962bf708","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                         db_containers_map.get(container_id).task_state !\u003d"},{"line_number":102,"context_line":"                         fields.TaskState.SANDBOX_DELETING)):"},{"line_number":103,"context_line":"                        old_status \u003d db_containers_map.get(container_id).status"},{"line_number":104,"context_line":"                        updated_container \u003d self.driver.show("},{"line_number":105,"context_line":"                            db_containers_map.get(container_id))"},{"line_number":106,"context_line":"                        updated_container.save(ctx)"},{"line_number":107,"context_line":"                        msg \u003d \u0027Status of container %s changed from %s to %s\u0027"},{"line_number":108,"context_line":"                        LOG.info(msg % (updated_container.uuid, old_status,"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a1ced50_44203a4a","line":105,"range":{"start_line":104,"start_character":24,"end_line":105,"end_character":64},"updated":"2017-03-17 04:51:44.000000000","message":"I think this will fail with 404 error as the container was deleted from docker. Am I right?","commit_id":"3f195afe4e8bc7e40e329bfaaac7d523ee6e7963"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"118bd4b82ce439f8549548a90d1d9de5f6cdf61b","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                         db_containers_map.get(container_id).task_state !\u003d"},{"line_number":102,"context_line":"                         fields.TaskState.SANDBOX_DELETING)):"},{"line_number":103,"context_line":"                        old_status \u003d db_containers_map.get(container_id).status"},{"line_number":104,"context_line":"                        updated_container \u003d self.driver.show("},{"line_number":105,"context_line":"                            db_containers_map.get(container_id))"},{"line_number":106,"context_line":"                        updated_container.save(ctx)"},{"line_number":107,"context_line":"                        msg \u003d \u0027Status of container %s changed from %s to %s\u0027"},{"line_number":108,"context_line":"                        LOG.info(msg % (updated_container.uuid, old_status,"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a1ced50_64237606","line":105,"range":{"start_line":104,"start_character":24,"end_line":105,"end_character":64},"in_reply_to":"1a1ced50_44203a4a","updated":"2017-03-17 05:19:17.000000000","message":"404 is handled here, https://github.com/openstack/zun/blob/master/zun/container/docker/driver.py#L131, I can simply set Error status here but i dont want to use the logic used in show method here.","commit_id":"3f195afe4e8bc7e40e329bfaaac7d523ee6e7963"},{"author":{"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"},"change_message_id":"570707e7d539abbe00206df0bf3b20efcd4893de","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                         db_containers_map.get(container_id).task_state !\u003d"},{"line_number":102,"context_line":"                         fields.TaskState.SANDBOX_DELETING)):"},{"line_number":103,"context_line":"                        old_status \u003d db_containers_map.get(container_id).status"},{"line_number":104,"context_line":"                        updated_container \u003d self.driver.show("},{"line_number":105,"context_line":"                            db_containers_map.get(container_id))"},{"line_number":106,"context_line":"                        updated_container.save(ctx)"},{"line_number":107,"context_line":"                        msg \u003d \u0027Status of container %s changed from %s to %s\u0027"},{"line_number":108,"context_line":"                        LOG.info(msg % (updated_container.uuid, old_status,"}],"source_content_type":"text/x-python","patch_set":12,"id":"1a1ced50_0412c262","line":105,"range":{"start_line":104,"start_character":24,"end_line":105,"end_character":64},"in_reply_to":"1a1ced50_64237606","updated":"2017-03-17 05:40:15.000000000","message":"I see. Don\u0027t you think that we should delete the container when 404 in show() method. This change is not in scope of this patch.","commit_id":"3f195afe4e8bc7e40e329bfaaac7d523ee6e7963"}]}