)]}'
{"zun/api/controllers/v1/containers.py":[{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"ed77d4d57933cde40902d5dd04a7762de73785f1","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        context \u003d pecan.request.context"},{"line_number":314,"context_line":"        compute_api \u003d pecan.request.compute_api"},{"line_number":315,"context_line":"        security_group_id \u003d compute_api.\\"},{"line_number":316,"context_line":"            get_security_group_ids(context, [security_group[\u0027name\u0027]])[0]"},{"line_number":317,"context_line":"        container_ports_detail \u003d compute_api.container_show_ports(context,"},{"line_number":318,"context_line":"                                                                  container)"},{"line_number":319,"context_line":"        # Does the default security group exist, default to False."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_e8081a29","line":316,"range":{"start_line":316,"start_character":12,"end_line":316,"end_character":34},"updated":"2017-07-24 23:51:07.000000000","message":"I think this needs to go through the compute API, since it is a pure neutron API call to collect sg ids (compute_api is mainly for performing local operations, such as doing a local call to docker API to launch container).","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"4800573f41bb086d227f51ce990ecf9afacea22f","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        context \u003d pecan.request.context"},{"line_number":314,"context_line":"        compute_api \u003d pecan.request.compute_api"},{"line_number":315,"context_line":"        security_group_id \u003d compute_api.\\"},{"line_number":316,"context_line":"            get_security_group_ids(context, [security_group[\u0027name\u0027]])[0]"},{"line_number":317,"context_line":"        container_ports_detail \u003d compute_api.container_show_ports(context,"},{"line_number":318,"context_line":"                                                                  container)"},{"line_number":319,"context_line":"        # Does the default security group exist, default to False."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_c0cbc66f","line":316,"range":{"start_line":316,"start_character":12,"end_line":316,"end_character":34},"in_reply_to":"ff346bd7_e8081a29","updated":"2017-07-25 21:09:13.000000000","message":"Sorry, a typing mistake. I wanted to say\n\n  I think this *doesn\u0027t* need to go through the compute API.","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"ed77d4d57933cde40902d5dd04a7762de73785f1","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        compute_api \u003d pecan.request.compute_api"},{"line_number":315,"context_line":"        security_group_id \u003d compute_api.\\"},{"line_number":316,"context_line":"            get_security_group_ids(context, [security_group[\u0027name\u0027]])[0]"},{"line_number":317,"context_line":"        container_ports_detail \u003d compute_api.container_show_ports(context,"},{"line_number":318,"context_line":"                                                                  container)"},{"line_number":319,"context_line":"        # Does the default security group exist, default to False."},{"line_number":320,"context_line":"        is_exist \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_68630ade","line":317,"range":{"start_line":317,"start_character":45,"end_line":317,"end_character":65},"updated":"2017-07-24 23:51:07.000000000","message":"Ditto. I think you can directly make this call instead of going though compute API.","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"d3981be9002bd148c6c648c8f1258dc81d668da5","unresolved":false,"context_lines":[{"line_number":320,"context_line":"        for container_port_detail in container_ports_detail:"},{"line_number":321,"context_line":"            if security_group_id in container_port_detail[\u0027security_groups\u0027]:"},{"line_number":322,"context_line":"                is_exist \u003d True"},{"line_number":323,"context_line":"        if (security_group[\u0027name\u0027] in container.security_groups) or is_exist:"},{"line_number":324,"context_line":"            msg \u003d _(\"security_group %s already present in container\") % \\"},{"line_number":325,"context_line":"                security_group[\u0027name\u0027]"},{"line_number":326,"context_line":"            raise exception.InvalidValue(msg)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff346bd7_912616ce","line":323,"range":{"start_line":323,"start_character":8,"end_line":323,"end_character":77},"updated":"2017-07-26 15:50:13.000000000","message":"can we move this line up, in case if it already present we can save few calls to other apis, thoughts?","commit_id":"bbd9d7eec440156c61f65d01a3b2d3aee0cdd044"}],"zun/common/utils.py":[{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"d369cf2b8d1df2e1f58f58facd88ee7c628a9e79","unresolved":false,"context_lines":[{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"def list_ports(context, container, **kwargs):"},{"line_number":286,"context_line":"        container_ports \u003d []"},{"line_number":287,"context_line":"        if not container.addresses:"},{"line_number":288,"context_line":"            return container_ports"},{"line_number":289,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_acd98a57","line":286,"range":{"start_line":286,"start_character":0,"end_line":286,"end_character":8},"updated":"2017-07-27 20:07:49.000000000","message":"The indentation looks wrong.","commit_id":"b4f38441e965a8c7caf123be21182232493dbc96"},{"author":{"_account_id":17813,"name":"wangzhh","email":"wzh_1993@126.com","username":"wangzhh"},"change_message_id":"cb57113170b32985a474a1c696c06cb23e11e037","unresolved":false,"context_lines":[{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"def list_ports(context, container, **kwargs):"},{"line_number":286,"context_line":"        container_ports \u003d []"},{"line_number":287,"context_line":"        if not container.addresses:"},{"line_number":288,"context_line":"            return container_ports"},{"line_number":289,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_6fdee9da","line":286,"range":{"start_line":286,"start_character":0,"end_line":286,"end_character":8},"in_reply_to":"ff346bd7_acd98a57","updated":"2017-07-28 03:34:27.000000000","message":"Oh, my god. So sorry.","commit_id":"b4f38441e965a8c7caf123be21182232493dbc96"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"d369cf2b8d1df2e1f58f58facd88ee7c628a9e79","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        else:"},{"line_number":290,"context_line":"            port_list \u003d []"},{"line_number":291,"context_line":"            neutron_client \u003d clients.OpenStackClients(context).neutron()"},{"line_number":292,"context_line":"            container_addresses \u003d [ports for _, ports in"},{"line_number":293,"context_line":"                                   container.addresses.items()]"},{"line_number":294,"context_line":"            for ports in container_addresses:"},{"line_number":295,"context_line":"                port_list.extend([p[\u0027port\u0027] for p in ports])"},{"line_number":296,"context_line":"            port_set \u003d set(port_list)"},{"line_number":297,"context_line":"            for port in port_set:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff346bd7_a78ac366","line":294,"range":{"start_line":292,"start_character":12,"end_line":294,"end_character":45},"updated":"2017-07-27 20:07:49.000000000","message":"Nit: it looks we can combine these two lines as (untested):\n\n    for ports in container.addresses.values():\n        ...","commit_id":"b4f38441e965a8c7caf123be21182232493dbc96"}],"zun/compute/manager.py":[{"author":{"_account_id":22406,"name":"Kien Nguyen","email":"kiennt2609@gmail.com","username":"kiennt"},"change_message_id":"e007b5c3c7b06be8fe9f651c015221568b98acef","unresolved":false,"context_lines":[{"line_number":624,"context_line":""},{"line_number":625,"context_line":"    def get_security_group_ids(self, context, security_group_names):"},{"line_number":626,"context_line":"        security_group_ids \u003d self.driver.\\"},{"line_number":627,"context_line":"            _get_security_group_ids(context, security_group_names)"},{"line_number":628,"context_line":"        return security_group_ids"},{"line_number":629,"context_line":""},{"line_number":630,"context_line":"    def container_show_ports(self, context, container):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_369fc751","line":627,"range":{"start_line":627,"start_character":12,"end_line":627,"end_character":35},"updated":"2017-07-24 03:20:52.000000000","message":"It works fine now, but I have a little concern. This method only exists in container/docker driver, so if the driver is not Docker (at this time, we have only 1 driver - docker, but maybe in the future, we will have more than one) and we didn\u0027t define _get_security_group_ids. It may raise some errors.","commit_id":"d17fb53a7189113228aee3fffd4103f054efa788"}],"zun/container/driver.py":[{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"ed77d4d57933cde40902d5dd04a7762de73785f1","unresolved":false,"context_lines":[{"line_number":239,"context_line":"            return True"},{"line_number":240,"context_line":"        return False"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _get_security_group_ids(self, context, security_groups):"},{"line_number":243,"context_line":"        if security_groups is None:"},{"line_number":244,"context_line":"            return None"},{"line_number":245,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_a897b290","line":242,"range":{"start_line":242,"start_character":8,"end_line":242,"end_character":31},"updated":"2017-07-24 23:51:07.000000000","message":"I think it is better to move this method to neutron network module: https://github.com/openstack/zun/blob/master/zun/network/network.py , which is a wrapper to neutron API calls.","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"},{"author":{"_account_id":17813,"name":"wangzhh","email":"wzh_1993@126.com","username":"wangzhh"},"change_message_id":"cf3ac296abee4dbe04bf369e34b6f30ac164ea82","unresolved":false,"context_lines":[{"line_number":239,"context_line":"            return True"},{"line_number":240,"context_line":"        return False"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _get_security_group_ids(self, context, security_groups):"},{"line_number":243,"context_line":"        if security_groups is None:"},{"line_number":244,"context_line":"            return None"},{"line_number":245,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_b09b98c7","line":242,"range":{"start_line":242,"start_character":8,"end_line":242,"end_character":31},"in_reply_to":"ff346bd7_a897b290","updated":"2017-07-25 03:08:28.000000000","message":"OK, but this method is called by add_security_group in https://github.com/openstack/zun/blob/master/zun/container/docker/driver.py, need I report a new bug or just modify it in next patch?","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"},{"author":{"_account_id":17813,"name":"wangzhh","email":"wzh_1993@126.com","username":"wangzhh"},"change_message_id":"45ea868db9f001dd8ad72bef48ec97c431a153f1","unresolved":false,"context_lines":[{"line_number":239,"context_line":"            return True"},{"line_number":240,"context_line":"        return False"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _get_security_group_ids(self, context, security_groups):"},{"line_number":243,"context_line":"        if security_groups is None:"},{"line_number":244,"context_line":"            return None"},{"line_number":245,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_b331620e","line":242,"range":{"start_line":242,"start_character":8,"end_line":242,"end_character":31},"in_reply_to":"ff346bd7_a897b290","updated":"2017-07-25 03:08:49.000000000","message":"OK, but this method is called by add_security_group in https://github.com/openstack/zun/blob/master/zun/container/docker/driver.py, need I report a new bug or just modify it in next patch?","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"4800573f41bb086d227f51ce990ecf9afacea22f","unresolved":false,"context_lines":[{"line_number":239,"context_line":"            return True"},{"line_number":240,"context_line":"        return False"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _get_security_group_ids(self, context, security_groups):"},{"line_number":243,"context_line":"        if security_groups is None:"},{"line_number":244,"context_line":"            return None"},{"line_number":245,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_15d232d3","line":242,"range":{"start_line":242,"start_character":8,"end_line":242,"end_character":31},"in_reply_to":"ff346bd7_b331620e","updated":"2017-07-25 21:09:13.000000000","message":"It is up to you :)","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"}],"zun/network/kuryr_network.py":[{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"ed77d4d57933cde40902d5dd04a7762de73785f1","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":217,"context_line":"                    LOG.exception(\"Neutron Error:\")"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def list_ports(self, container, **kwargs):"},{"line_number":220,"context_line":"        container_ports \u003d []"},{"line_number":221,"context_line":"        if not container.addresses:"},{"line_number":222,"context_line":"            return container_ports"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_68bf8afb","line":219,"range":{"start_line":219,"start_character":8,"end_line":219,"end_character":18},"updated":"2017-07-24 23:51:07.000000000","message":"How about moving this to a common utils module https://github.com/openstack/zun/blob/master/zun/common/utils.py ? This would allow you to call it without going through compute API.","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"},{"author":{"_account_id":17813,"name":"wangzhh","email":"wzh_1993@126.com","username":"wangzhh"},"change_message_id":"cf3ac296abee4dbe04bf369e34b6f30ac164ea82","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":217,"context_line":"                    LOG.exception(\"Neutron Error:\")"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def list_ports(self, container, **kwargs):"},{"line_number":220,"context_line":"        container_ports \u003d []"},{"line_number":221,"context_line":"        if not container.addresses:"},{"line_number":222,"context_line":"            return container_ports"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_7009b02a","line":219,"range":{"start_line":219,"start_character":8,"end_line":219,"end_character":18},"in_reply_to":"ff346bd7_68bf8afb","updated":"2017-07-25 03:08:28.000000000","message":"Thanks, Hongbin Lu. But list_ports is a network operation like list_networks here, Do u confirm to move it to utils.py?","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"},{"author":{"_account_id":17813,"name":"wangzhh","email":"wzh_1993@126.com","username":"wangzhh"},"change_message_id":"45ea868db9f001dd8ad72bef48ec97c431a153f1","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":217,"context_line":"                    LOG.exception(\"Neutron Error:\")"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def list_ports(self, container, **kwargs):"},{"line_number":220,"context_line":"        container_ports \u003d []"},{"line_number":221,"context_line":"        if not container.addresses:"},{"line_number":222,"context_line":"            return container_ports"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_93341efd","line":219,"range":{"start_line":219,"start_character":8,"end_line":219,"end_character":18},"in_reply_to":"ff346bd7_68bf8afb","updated":"2017-07-25 03:08:49.000000000","message":"Thanks, Hongbin Lu. But list_ports is a network operation like list_networks here, Do u confirm to move it to utils.py?","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"4800573f41bb086d227f51ce990ecf9afacea22f","unresolved":false,"context_lines":[{"line_number":216,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":217,"context_line":"                    LOG.exception(\"Neutron Error:\")"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def list_ports(self, container, **kwargs):"},{"line_number":220,"context_line":"        container_ports \u003d []"},{"line_number":221,"context_line":"        if not container.addresses:"},{"line_number":222,"context_line":"            return container_ports"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_80693e8c","line":219,"range":{"start_line":219,"start_character":8,"end_line":219,"end_character":18},"in_reply_to":"ff346bd7_93341efd","updated":"2017-07-25 21:09:13.000000000","message":"Hi @wangzhh,\n\nYes, I understand that it is odd. However, from what I observed, this method doesn\u0027t have API calls to local docker daemon. In comparison, most methods in this module has Docker API calls (like self.docker.xxx). If we want to call this method, we have to construct a Docker API client and pass it to this class (line #34). This imposes limitation on using this method in API layer, which doesn\u0027t have a Docker client reference. Therefore, it seems to me that we need to move this method to another place, which would allow us to use this method without giving a Docker client.\n\nWhat do you think? Suggestions are welcome.","commit_id":"99a97c47b0eceb622c5c7e6eaf192fb226f3cef9"}]}