launchpad-reviewers team mailing list archive

[Guruprasad <mp+413581@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
> index 1a0c380..1902f8a 100644
> --- a/lib/lp/registry/model/person.py
> +++ b/lib/lp/registry/model/person.py
> @@ -1342,6 +1342,48 @@ class Person(
>          self._inTeam_cache[team.id] = in_team
>          return in_team
>  
> +    def inAnyTeam(self, teams):
> +        """See `IPerson`."""
> +        if not teams:
> +            return False
> +
> +        # Translate each team name to an ITeam if we were passed
> +        # a list of teams.
> +        if all(isinstance(team, str) for team in teams):
> +            teams = PersonSet.getByNames(teams)
> +            if not teams:
> +                return False
> +
> +        team_ids = {team.id for team in teams if team.is_team}
> +        if self.id in team_ids:
> +            # A team is always a member of itself.
> +            return True
> +
> +        if self._inTeam_cache is None:
> +            # Initialize cache
> +            self._inTeam_cache = {}
> +        else:
> +            if any(self._inTeam_cache.get(team_id) for team_id in team_ids):
> +                return True
> +
> +        team_participations = IStore(TeamParticipation).find(
> +            TeamParticipation,
> +            And(
> +                TeamParticipation.teamID.is_in(team_ids),
> +                TeamParticipation.personID == self.id
> +            )
> +        )
> +
> +        in_any_team = False
> +
> +        for team_participation in team_participations:
> +            in_any_team = True
> +            if team_participation.team.id not in self._inTeam_cache:
> +                self._inTeam_cache[team_participation.team.id] = True

@cjwatson, this part I understood already. What I don't understand is that whether it makes sense to populate the cache even if the condition above in the line 97 in the diff viewer is true. Currently, the function returns 'True' at that point.

If that condition in line 97 is false, all the items in 'team_ids' are missing and there is no need to calculate 'missing_team_ids' again.

> +
> +        return in_any_team
> +
> +
>      def hasParticipationEntryFor(self, team):
>          """See `IPerson`."""
>          return bool(TeamParticipation.selectOneBy(person=self, team=team))


-- 
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/413581
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:optimize-has-any-bug-role-bulk-team-membership-checks into launchpad:master.