)]}'
{"neutron/db/securitygroups_db.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"af5aa77fee7063280233cf14f88f2ca44c6b0207","unresolved":false,"context_lines":[{"line_number":720,"context_line":"            sorts\u003dsorts, marker\u003dmarker, limit\u003dlimit, page_reverse\u003dpage_reverse)"},{"line_number":721,"context_line":""},{"line_number":722,"context_line":"        project_id \u003d filters.get(\u0027project_id\u0027) or filters.get(\u0027tenant_id\u0027)"},{"line_number":723,"context_line":"        if project_id:"},{"line_number":724,"context_line":"            project_id \u003d project_id[0]"},{"line_number":725,"context_line":"        else:"},{"line_number":726,"context_line":"            project_id \u003d context.project_id"},{"line_number":727,"context_line":"        if project_id:"},{"line_number":728,"context_line":"            self._ensure_default_security_group(context, project_id)"},{"line_number":729,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_725bb6f7","line":726,"range":{"start_line":723,"start_character":8,"end_line":726,"end_character":43},"updated":"2020-02-21 10:24:33.000000000","message":"nit: \n\n  project_id \u003d project_id[0] or context.project_id","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"6e2a2c13f9110fb35389fedb15e32c3026ffd410","unresolved":false,"context_lines":[{"line_number":720,"context_line":"            sorts\u003dsorts, marker\u003dmarker, limit\u003dlimit, page_reverse\u003dpage_reverse)"},{"line_number":721,"context_line":""},{"line_number":722,"context_line":"        project_id \u003d filters.get(\u0027project_id\u0027) or filters.get(\u0027tenant_id\u0027)"},{"line_number":723,"context_line":"        if project_id:"},{"line_number":724,"context_line":"            project_id \u003d project_id[0]"},{"line_number":725,"context_line":"        else:"},{"line_number":726,"context_line":"            project_id \u003d context.project_id"},{"line_number":727,"context_line":"        if project_id:"},{"line_number":728,"context_line":"            self._ensure_default_security_group(context, project_id)"},{"line_number":729,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_6d019b6c","line":726,"range":{"start_line":723,"start_character":8,"end_line":726,"end_character":43},"in_reply_to":"3fa7e38b_725bb6f7","updated":"2020-02-21 11:48:56.000000000","message":"There is a case where project_id is an empty list, so the if condition is required....","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"10ed1952a4e0f26293973faed25c6d0465233ea1","unresolved":false,"context_lines":[{"line_number":724,"context_line":"            project_id \u003d project_id[0]"},{"line_number":725,"context_line":"        else:"},{"line_number":726,"context_line":"            project_id \u003d context.project_id"},{"line_number":727,"context_line":"        if project_id:"},{"line_number":728,"context_line":"            self._ensure_default_security_group(context, project_id)"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"        # NOTE(slaweq): use admin context here to be able to get all rules"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_f2b80614","line":727,"updated":"2020-02-21 10:06:08.000000000","message":"Is there a case where project_id is still empty? At a glance, L.723-726 ensures project_id and this if clause looks unnecessary. I am missing something perhaps...","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5e6de113625bf05b5325ae04082de8178a0bc8c5","unresolved":false,"context_lines":[{"line_number":724,"context_line":"            project_id \u003d project_id[0]"},{"line_number":725,"context_line":"        else:"},{"line_number":726,"context_line":"            project_id \u003d context.project_id"},{"line_number":727,"context_line":"        if project_id:"},{"line_number":728,"context_line":"            self._ensure_default_security_group(context, project_id)"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"        # NOTE(slaweq): use admin context here to be able to get all rules"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d30aac1d","line":727,"in_reply_to":"3fa7e38b_32577e5c","updated":"2020-02-21 13:35:25.000000000","message":"It happens, I already had this issue when checking this patch locally. As I said before, in L206 there is used function context_lib.get_admin_context() which returns context with tenant_id\u003dNone - see https://github.com/openstack/neutron-lib/blob/master/neutron_lib/context.py#L182 and this will later end up here and will cause an error if there will be no this additional check","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b3e25858b35037150befc033b740f073eba808eb","unresolved":false,"context_lines":[{"line_number":724,"context_line":"            project_id \u003d project_id[0]"},{"line_number":725,"context_line":"        else:"},{"line_number":726,"context_line":"            project_id \u003d context.project_id"},{"line_number":727,"context_line":"        if project_id:"},{"line_number":728,"context_line":"            self._ensure_default_security_group(context, project_id)"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"        # NOTE(slaweq): use admin context here to be able to get all rules"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_32577e5c","line":727,"in_reply_to":"3fa7e38b_d2194a24","updated":"2020-02-21 10:48:58.000000000","message":"Thanks. To create a security group rule, an API user need to know the ID of the default security group, so I think the case with project_id\u003dNone does not actually happens, but too nested assumption is not a good idea and potentially causes a bug. project_id check here makes sense to me now.","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9c5e51568a07e66a5b809fe42a7553c74f619334","unresolved":false,"context_lines":[{"line_number":724,"context_line":"            project_id \u003d project_id[0]"},{"line_number":725,"context_line":"        else:"},{"line_number":726,"context_line":"            project_id \u003d context.project_id"},{"line_number":727,"context_line":"        if project_id:"},{"line_number":728,"context_line":"            self._ensure_default_security_group(context, project_id)"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"        # NOTE(slaweq): use admin context here to be able to get all rules"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d2194a24","line":727,"in_reply_to":"3fa7e38b_f2b80614","updated":"2020-02-21 10:18:53.000000000","message":"Yes, it can be None still. It happens e.g. in case of creation of SG rule, when _check_for_duplicate_rules() is called and that calls get_security_group() and finally get_security_groups() - see L206 in this file. There is admin_context used as argument and it has no tenant_id.","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"af5aa77fee7063280233cf14f88f2ca44c6b0207","unresolved":false,"context_lines":[{"line_number":725,"context_line":"        else:"},{"line_number":726,"context_line":"            project_id \u003d context.project_id"},{"line_number":727,"context_line":"        if project_id:"},{"line_number":728,"context_line":"            self._ensure_default_security_group(context, project_id)"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"        # NOTE(slaweq): use admin context here to be able to get all rules"},{"line_number":731,"context_line":"        # which fits filters\u0027 criteria. Later in policy engine rules will be"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_72b736f9","line":728,"updated":"2020-02-21 10:24:33.000000000","message":"I\u0027m Ok with this code.\n\nnit: But shouldn\u0027t we try to capture the keystone event of project creation instead? We have plenty of those calls in this module.","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5e6de113625bf05b5325ae04082de8178a0bc8c5","unresolved":false,"context_lines":[{"line_number":725,"context_line":"        else:"},{"line_number":726,"context_line":"            project_id \u003d context.project_id"},{"line_number":727,"context_line":"        if project_id:"},{"line_number":728,"context_line":"            self._ensure_default_security_group(context, project_id)"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"        # NOTE(slaweq): use admin context here to be able to get all rules"},{"line_number":731,"context_line":"        # which fits filters\u0027 criteria. Later in policy engine rules will be"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d81c0b7e","line":728,"in_reply_to":"3fa7e38b_72b736f9","updated":"2020-02-21 13:35:25.000000000","message":"I don\u0027t think we can do it now. If that would be possible, probably cleaning of resources after project deletion would be also easier to implement :)","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"636510aba07ea106135296e5397f603e19856d8f","unresolved":false,"context_lines":[{"line_number":725,"context_line":"        else:"},{"line_number":726,"context_line":"            project_id \u003d context.project_id"},{"line_number":727,"context_line":"        if project_id:"},{"line_number":728,"context_line":"            self._ensure_default_security_group(context, project_id)"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"        # NOTE(slaweq): use admin context here to be able to get all rules"},{"line_number":731,"context_line":"        # which fits filters\u0027 criteria. Later in policy engine rules will be"}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_8f99b820","line":728,"in_reply_to":"3fa7e38b_d81c0b7e","updated":"2020-02-24 11:00:59.000000000","message":"That was just a comment, this is out of scope.","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b3e25858b35037150befc033b740f073eba808eb","unresolved":false,"context_lines":[{"line_number":742,"context_line":""},{"line_number":743,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":744,"context_line":"    def get_security_group_rules_count(self, context, filters\u003dNone):"},{"line_number":745,"context_line":"        filters \u003d filters or {}"},{"line_number":746,"context_line":"        return sg_obj.SecurityGroupRule.count("},{"line_number":747,"context_line":"            context, validate_filters\u003dFalse, **filters)"},{"line_number":748,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_329a1e12","line":745,"updated":"2020-02-21 10:48:58.000000000","message":"Would we like to call _ensure_default_security_group here too?\nThe bug is about a case where list_security_group_rules are called before accessing any security groups. The similar case happens for this method.","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5e6de113625bf05b5325ae04082de8178a0bc8c5","unresolved":false,"context_lines":[{"line_number":742,"context_line":""},{"line_number":743,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":744,"context_line":"    def get_security_group_rules_count(self, context, filters\u003dNone):"},{"line_number":745,"context_line":"        filters \u003d filters or {}"},{"line_number":746,"context_line":"        return sg_obj.SecurityGroupRule.count("},{"line_number":747,"context_line":"            context, validate_filters\u003dFalse, **filters)"},{"line_number":748,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d3386ccd","line":745,"in_reply_to":"3fa7e38b_2d796380","updated":"2020-02-21 13:35:25.000000000","message":"But this method is never used. See http://codesearch.openstack.org/?q\u003dget_security_group_rules_count\u0026i\u003dnope\u0026files\u003d\u0026repos\u003d\n\nI will propose a patch to remove it.","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"863daceffa8b6730b92a189f43bd66786d019865","unresolved":false,"context_lines":[{"line_number":742,"context_line":""},{"line_number":743,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":744,"context_line":"    def get_security_group_rules_count(self, context, filters\u003dNone):"},{"line_number":745,"context_line":"        filters \u003d filters or {}"},{"line_number":746,"context_line":"        return sg_obj.SecurityGroupRule.count("},{"line_number":747,"context_line":"            context, validate_filters\u003dFalse, **filters)"},{"line_number":748,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_2d796380","line":745,"in_reply_to":"3fa7e38b_329a1e12","updated":"2020-02-21 11:14:30.000000000","message":"Agree","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"636510aba07ea106135296e5397f603e19856d8f","unresolved":false,"context_lines":[{"line_number":742,"context_line":""},{"line_number":743,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":744,"context_line":"    def get_security_group_rules_count(self, context, filters\u003dNone):"},{"line_number":745,"context_line":"        filters \u003d filters or {}"},{"line_number":746,"context_line":"        return sg_obj.SecurityGroupRule.count("},{"line_number":747,"context_line":"            context, validate_filters\u003dFalse, **filters)"},{"line_number":748,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_2fe9a47f","line":745,"in_reply_to":"3fa7e38b_33a0e0f2","updated":"2020-02-24 11:00:59.000000000","message":"You are right: http://codesearch.openstack.org/?q\u003dget_security_group_rules_count\u0026i\u003dnope\u0026files\u003d\u0026repos\u003d","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"8bee9904c424d5b615b26f01a7ffbad51416a88a","unresolved":false,"context_lines":[{"line_number":742,"context_line":""},{"line_number":743,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":744,"context_line":"    def get_security_group_rules_count(self, context, filters\u003dNone):"},{"line_number":745,"context_line":"        filters \u003d filters or {}"},{"line_number":746,"context_line":"        return sg_obj.SecurityGroupRule.count("},{"line_number":747,"context_line":"            context, validate_filters\u003dFalse, **filters)"},{"line_number":748,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_33a0e0f2","line":745,"in_reply_to":"3fa7e38b_d3386ccd","updated":"2020-02-21 16:01:22.000000000","message":"Patch https://review.opendev.org/709140 proposed :)","commit_id":"43f60812fffd1f7d41fd821dba55f34ffa6e72f0"}]}