)]}'
{"zun/api/controllers/v1/containers.py":[{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"2dbddbffbcb754c11c3e6da6a33fb9ed381cddd6","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        security_group_ids \u003d [item[\u0027id\u0027] for item in security_groups_list"},{"line_number":205,"context_line":"                              if item[\u0027name\u0027] in security_groups]"},{"line_number":206,"context_line":"        if len(security_group_ids) !\u003d len(security_groups):"},{"line_number":207,"context_line":"            raise exception.ZunException(_("},{"line_number":208,"context_line":"                \"Any of the security group in %s is not found \") %"},{"line_number":209,"context_line":"                security_groups)"},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_925818fa","line":207,"range":{"start_line":207,"start_character":28,"end_line":207,"end_character":40},"updated":"2017-06-17 22:44:34.000000000","message":"Raise this exception will return a 500 response. In our case, we should return 400 response, so better to use a 400 Exception (i.e. Invalid) instead.","commit_id":"4cd6c574af7693f886556ef84a7bc86395174b7f"},{"author":{"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"},"change_message_id":"2ab2b7033c7b27420395541ab76be945096696b1","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            **search_opts).get(\u0027security_groups\u0027, [])"},{"line_number":204,"context_line":"        security_group_ids \u003d [item[\u0027id\u0027] for item in security_groups_list"},{"line_number":205,"context_line":"                              if item[\u0027name\u0027] in security_groups]"},{"line_number":206,"context_line":"        if len(security_group_ids) !\u003d len(security_groups):"},{"line_number":207,"context_line":"            raise exception.InvalidValue(_("},{"line_number":208,"context_line":"                \"Any of the security group in %s is not found \") %"},{"line_number":209,"context_line":"                security_groups)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_b62c0bac","line":206,"updated":"2017-06-19 16:47:25.000000000","message":"I think this check is wrong here as the no of security groups doesn\u0027t verify if it is valid or not.\nI suggest to do a get on each of the security groups or check the security group ids in the neutron.list_security_groups(..)","commit_id":"680194d2408265b70da61b65269da831c1c4dd33"},{"author":{"_account_id":10206,"name":"Madhuri Kumari","email":"madhuri.kumari@intel.com","username":"Madhuri"},"change_message_id":"b76043b105ba0e556473e9819efaa4a47100b327","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            **search_opts).get(\u0027security_groups\u0027, [])"},{"line_number":204,"context_line":"        security_group_ids \u003d [item[\u0027id\u0027] for item in security_groups_list"},{"line_number":205,"context_line":"                              if item[\u0027name\u0027] in security_groups]"},{"line_number":206,"context_line":"        if len(security_group_ids) !\u003d len(security_groups):"},{"line_number":207,"context_line":"            raise exception.InvalidValue(_("},{"line_number":208,"context_line":"                \"Any of the security group in %s is not found \") %"},{"line_number":209,"context_line":"                security_groups)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_44684cb9","line":206,"in_reply_to":"7f231b9d_6297bb3c","updated":"2017-06-20 06:01:19.000000000","message":"Ah my bad. Sorry I missed it.\nThanks Pradeep for pointing out.","commit_id":"680194d2408265b70da61b65269da831c1c4dd33"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"43e35266e8a289016e4896dc0426eef3fea889fe","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            **search_opts).get(\u0027security_groups\u0027, [])"},{"line_number":204,"context_line":"        security_group_ids \u003d [item[\u0027id\u0027] for item in security_groups_list"},{"line_number":205,"context_line":"                              if item[\u0027name\u0027] in security_groups]"},{"line_number":206,"context_line":"        if len(security_group_ids) !\u003d len(security_groups):"},{"line_number":207,"context_line":"            raise exception.InvalidValue(_("},{"line_number":208,"context_line":"                \"Any of the security group in %s is not found \") %"},{"line_number":209,"context_line":"                security_groups)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_6297bb3c","line":206,"in_reply_to":"7f231b9d_b62c0bac","updated":"2017-06-20 05:29:12.000000000","message":"@Madhuri, same thing is done in line #204 and 205.","commit_id":"680194d2408265b70da61b65269da831c1c4dd33"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"c4955378b76c92150d96dc9defbbc1ced8bc0617","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        elif name in [\u0027no\u0027]:"},{"line_number":197,"context_line":"            container_dict.get(\u0027restart_policy\u0027)[\u0027MaximumRetryCount\u0027] \u003d \u00270\u0027"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _check_for_security_group(self, context, security_groups):"},{"line_number":200,"context_line":"        neutron \u003d clients.OpenStackClients(context).neutron()"},{"line_number":201,"context_line":"        search_opts \u003d {\u0027tenant_id\u0027: context.project_id}"},{"line_number":202,"context_line":"        security_groups_list \u003d neutron.list_security_groups("},{"line_number":203,"context_line":"            **search_opts).get(\u0027security_groups\u0027, [])"},{"line_number":204,"context_line":"        security_group_ids \u003d [item[\u0027id\u0027] for item in security_groups_list"},{"line_number":205,"context_line":"                              if item[\u0027name\u0027] in security_groups]"},{"line_number":206,"context_line":"        if len(security_group_ids) !\u003d len(security_groups):"},{"line_number":207,"context_line":"            raise exception.InvalidValue(_("},{"line_number":208,"context_line":"                \"Any of the security group in %s is not found \") %"},{"line_number":209,"context_line":"                security_groups)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @pecan.expose(\u0027json\u0027)"},{"line_number":212,"context_line":"    @api_utils.enforce_content_types([\u0027application/json\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_0c053514","line":209,"range":{"start_line":199,"start_character":4,"end_line":209,"end_character":32},"updated":"2017-06-27 07:59:01.000000000","message":"we are getting the security group ids here by using neutron, again we are getting the security group ids using neutron in docker driver, can we reuse the security group ids which we get here in docker driver, instead of making another call in neutron?\n\nWhat do you think? Or do you  think sharing  security group name is better that its ID, because that exists in zun DB?","commit_id":"0d39a60042eac1f4ca9d34ee0ed7fba40178e1f1"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"259c217ffba49464c8b4ec774b07d345c8625723","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        elif name in [\u0027no\u0027]:"},{"line_number":197,"context_line":"            container_dict.get(\u0027restart_policy\u0027)[\u0027MaximumRetryCount\u0027] \u003d \u00270\u0027"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _check_for_security_group(self, context, security_groups):"},{"line_number":200,"context_line":"        neutron \u003d clients.OpenStackClients(context).neutron()"},{"line_number":201,"context_line":"        search_opts \u003d {\u0027tenant_id\u0027: context.project_id}"},{"line_number":202,"context_line":"        security_groups_list \u003d neutron.list_security_groups("},{"line_number":203,"context_line":"            **search_opts).get(\u0027security_groups\u0027, [])"},{"line_number":204,"context_line":"        security_group_ids \u003d [item[\u0027id\u0027] for item in security_groups_list"},{"line_number":205,"context_line":"                              if item[\u0027name\u0027] in security_groups]"},{"line_number":206,"context_line":"        if len(security_group_ids) !\u003d len(security_groups):"},{"line_number":207,"context_line":"            raise exception.InvalidValue(_("},{"line_number":208,"context_line":"                \"Any of the security group in %s is not found \") %"},{"line_number":209,"context_line":"                security_groups)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @pecan.expose(\u0027json\u0027)"},{"line_number":212,"context_line":"    @api_utils.enforce_content_types([\u0027application/json\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_decc6b36","line":209,"range":{"start_line":199,"start_character":4,"end_line":209,"end_character":32},"in_reply_to":"5f201791_0c053514","updated":"2017-06-27 19:11:28.000000000","message":"Yes. If we persist security group ids into DB, we can save one neutron call, which is a pro. A drawback is possibly the user experience. User might need to look up the name given a id. The third approach is storing both name and id into DB. The name will be presented to end-user and the id will be used for internal API calls.\n\nPersonally, I am open to either approach.","commit_id":"0d39a60042eac1f4ca9d34ee0ed7fba40178e1f1"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"2610e164793de3ffa990c5eeddcbdf5155a52318","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        elif name in [\u0027no\u0027]:"},{"line_number":197,"context_line":"            container_dict.get(\u0027restart_policy\u0027)[\u0027MaximumRetryCount\u0027] \u003d \u00270\u0027"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _check_for_security_group(self, context, security_groups):"},{"line_number":200,"context_line":"        neutron \u003d clients.OpenStackClients(context).neutron()"},{"line_number":201,"context_line":"        search_opts \u003d {\u0027tenant_id\u0027: context.project_id}"},{"line_number":202,"context_line":"        security_groups_list \u003d neutron.list_security_groups("},{"line_number":203,"context_line":"            **search_opts).get(\u0027security_groups\u0027, [])"},{"line_number":204,"context_line":"        security_group_ids \u003d [item[\u0027id\u0027] for item in security_groups_list"},{"line_number":205,"context_line":"                              if item[\u0027name\u0027] in security_groups]"},{"line_number":206,"context_line":"        if len(security_group_ids) !\u003d len(security_groups):"},{"line_number":207,"context_line":"            raise exception.InvalidValue(_("},{"line_number":208,"context_line":"                \"Any of the security group in %s is not found \") %"},{"line_number":209,"context_line":"                security_groups)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @pecan.expose(\u0027json\u0027)"},{"line_number":212,"context_line":"    @api_utils.enforce_content_types([\u0027application/json\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_08928557","line":209,"range":{"start_line":199,"start_character":4,"end_line":209,"end_character":32},"in_reply_to":"5f201791_308015f5","updated":"2017-06-28 14:46:25.000000000","message":"I see. That sounds good to me as well.","commit_id":"0d39a60042eac1f4ca9d34ee0ed7fba40178e1f1"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"703beb26d1e6c25cb0d14273a60a337aed50c06a","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        elif name in [\u0027no\u0027]:"},{"line_number":197,"context_line":"            container_dict.get(\u0027restart_policy\u0027)[\u0027MaximumRetryCount\u0027] \u003d \u00270\u0027"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _check_for_security_group(self, context, security_groups):"},{"line_number":200,"context_line":"        neutron \u003d clients.OpenStackClients(context).neutron()"},{"line_number":201,"context_line":"        search_opts \u003d {\u0027tenant_id\u0027: context.project_id}"},{"line_number":202,"context_line":"        security_groups_list \u003d neutron.list_security_groups("},{"line_number":203,"context_line":"            **search_opts).get(\u0027security_groups\u0027, [])"},{"line_number":204,"context_line":"        security_group_ids \u003d [item[\u0027id\u0027] for item in security_groups_list"},{"line_number":205,"context_line":"                              if item[\u0027name\u0027] in security_groups]"},{"line_number":206,"context_line":"        if len(security_group_ids) !\u003d len(security_groups):"},{"line_number":207,"context_line":"            raise exception.InvalidValue(_("},{"line_number":208,"context_line":"                \"Any of the security group in %s is not found \") %"},{"line_number":209,"context_line":"                security_groups)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @pecan.expose(\u0027json\u0027)"},{"line_number":212,"context_line":"    @api_utils.enforce_content_types([\u0027application/json\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_308015f5","line":209,"range":{"start_line":199,"start_character":4,"end_line":209,"end_character":32},"in_reply_to":"5f201791_decc6b36","updated":"2017-06-28 10:13:13.000000000","message":"@Hongbin, It is good if we store security groups names in Zun-db. For reducing one neutron call, i think we can pass security group IDs, to docker driver from API as feng suggested.","commit_id":"0d39a60042eac1f4ca9d34ee0ed7fba40178e1f1"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"c4955378b76c92150d96dc9defbbc1ced8bc0617","unresolved":false,"context_lines":[{"line_number":297,"context_line":"            container.as_dict(), \"container:add_security_group\")"},{"line_number":298,"context_line":"        utils.validate_container_state(container, \u0027add_security_group\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        # check if security group already presnt in container"},{"line_number":301,"context_line":"        if security_group[\u0027name\u0027] in container.security_groups:"},{"line_number":302,"context_line":"            msg \u003d \"security_group \u0027%s\u0027 already present in container\""},{"line_number":303,"context_line":"            raise exception.InvalidValue(msg % security_group[\u0027name\u0027])"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        context \u003d pecan.request.context"},{"line_number":306,"context_line":"        compute_api \u003d pecan.request.compute_api"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_8c6a65ea","line":303,"range":{"start_line":300,"start_character":8,"end_line":303,"end_character":70},"updated":"2017-06-27 07:59:01.000000000","message":"What will happen if you provide the security group that does not exists in zun DB, and does not exists in neutron, would not that call fail in zun-compute? I think same thing you are preventing in container create operation, right?","commit_id":"0d39a60042eac1f4ca9d34ee0ed7fba40178e1f1"},{"author":{"_account_id":16277,"name":"Pradeep Kumar Singh","email":"ps4openstack@gmail.com","username":"pradeep_singh"},"change_message_id":"703beb26d1e6c25cb0d14273a60a337aed50c06a","unresolved":false,"context_lines":[{"line_number":297,"context_line":"            container.as_dict(), \"container:add_security_group\")"},{"line_number":298,"context_line":"        utils.validate_container_state(container, \u0027add_security_group\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        # check if security group already presnt in container"},{"line_number":301,"context_line":"        if security_group[\u0027name\u0027] in container.security_groups:"},{"line_number":302,"context_line":"            msg \u003d \"security_group \u0027%s\u0027 already present in container\""},{"line_number":303,"context_line":"            raise exception.InvalidValue(msg % security_group[\u0027name\u0027])"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        context \u003d pecan.request.context"},{"line_number":306,"context_line":"        compute_api \u003d pecan.request.compute_api"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_90948938","line":303,"range":{"start_line":300,"start_character":8,"end_line":303,"end_character":70},"in_reply_to":"5f201791_1e7ae3b8","updated":"2017-06-28 10:13:13.000000000","message":"@Hongbin, here my concern was to check if the security group provided by user exists in neutron or not just like we are doing in container create.","commit_id":"0d39a60042eac1f4ca9d34ee0ed7fba40178e1f1"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"259c217ffba49464c8b4ec774b07d345c8625723","unresolved":false,"context_lines":[{"line_number":297,"context_line":"            container.as_dict(), \"container:add_security_group\")"},{"line_number":298,"context_line":"        utils.validate_container_state(container, \u0027add_security_group\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        # check if security group already presnt in container"},{"line_number":301,"context_line":"        if security_group[\u0027name\u0027] in container.security_groups:"},{"line_number":302,"context_line":"            msg \u003d \"security_group \u0027%s\u0027 already present in container\""},{"line_number":303,"context_line":"            raise exception.InvalidValue(msg % security_group[\u0027name\u0027])"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        context \u003d pecan.request.context"},{"line_number":306,"context_line":"        compute_api \u003d pecan.request.compute_api"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_1e7ae3b8","line":303,"range":{"start_line":300,"start_character":8,"end_line":303,"end_character":70},"in_reply_to":"5f201791_8c6a65ea","updated":"2017-06-27 19:11:28.000000000","message":"I think that is right. There is a synchronized problem between Zun and Neutron. To resolve the potential out-of-sync issue, one option is to catch all kinds of out-of-sync exception and handle it. Another option is to have a periodic tasks to sync both database. Yet another option is to avoid storing security groups in Zun DB and always retrieve them from Neutron. Thoughts?","commit_id":"0d39a60042eac1f4ca9d34ee0ed7fba40178e1f1"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"2610e164793de3ffa990c5eeddcbdf5155a52318","unresolved":false,"context_lines":[{"line_number":297,"context_line":"            container.as_dict(), \"container:add_security_group\")"},{"line_number":298,"context_line":"        utils.validate_container_state(container, \u0027add_security_group\u0027)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        # check if security group already presnt in container"},{"line_number":301,"context_line":"        if security_group[\u0027name\u0027] in container.security_groups:"},{"line_number":302,"context_line":"            msg \u003d \"security_group \u0027%s\u0027 already present in container\""},{"line_number":303,"context_line":"            raise exception.InvalidValue(msg % security_group[\u0027name\u0027])"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        context \u003d pecan.request.context"},{"line_number":306,"context_line":"        compute_api \u003d pecan.request.compute_api"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_28d30106","line":303,"range":{"start_line":300,"start_character":8,"end_line":303,"end_character":70},"in_reply_to":"5f201791_90948938","updated":"2017-06-28 14:46:25.000000000","message":"Yes, sounds good to me.","commit_id":"0d39a60042eac1f4ca9d34ee0ed7fba40178e1f1"}]}