From f615c1ad6ab28218058d423bc6628a6c2aa7f4c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Gillot-Lamure?= Date: Wed, 27 Nov 2024 10:57:09 +0100 Subject: [PATCH 1/3] Fix some PEP8 violations Left are a lot of E501 line too long and a few W503 line break before binary operator --- main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.py b/main.py index 6f9f134..22ab2a7 100755 --- a/main.py +++ b/main.py @@ -761,7 +761,7 @@ def get_organization_repositories(self, org: str) -> Iterable[Repository]: n = len(repos) for i, repo in enumerate(repos): name = repo["name"] - print_status_stderr(f"[{i+1} / {n}] Getting access on {name}") + print_status_stderr(f"[{i + 1} / {n}] Getting access on {name}") user_access = tuple(sorted(self.get_repository_users(org, name))) team_access = tuple(sorted(self.get_repository_teams(org, name))) yield Repository( @@ -914,6 +914,7 @@ def print_diff( return has_diff + def print_team_members_diff( *, team_name: str, @@ -954,7 +955,6 @@ def print_team_members_diff( return has_diff - def main() -> None: if "--help" in sys.argv: print(__doc__) From a9c92b29509cab660eb73e963e7de6ae58d9456b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Gillot-Lamure?= Date: Wed, 27 Nov 2024 11:33:12 +0100 Subject: [PATCH 2/3] Extract business logic out of main() --- main.py | 45 ++++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/main.py b/main.py index 22ab2a7..31fc79a 100755 --- a/main.py +++ b/main.py @@ -955,29 +955,10 @@ def print_team_members_diff( return has_diff -def main() -> None: - if "--help" in sys.argv: - print(__doc__) - sys.exit(0) - - github_token = os.getenv("GITHUB_TOKEN") - if github_token is None: - print("Expected GITHUB_TOKEN environment variable to be set.") - print("See also --help.") - sys.exit(1) - - if len(sys.argv) < 2: - print("Expected file name of config toml as first argument.") - print("See also --help.") - sys.exit(1) - +def has_changes(target: Configuration, client: GithubClient) -> bool: has_changes = False - target_fname = sys.argv[1] - target = Configuration.from_toml_file(target_fname) org_name = target.organization.name - client = GithubClient.new(github_token) - actual_repos = set(client.get_organization_repositories(org_name)) target_repos = set(target.repos_by_id.values()) | { target.get_repository_target(r) for r in actual_repos @@ -1035,7 +1016,29 @@ def main() -> None: actual_members=set(client.get_team_members(org_name, team)), ) - if has_changes: + return has_changes + + +def main() -> None: + if "--help" in sys.argv: + print(__doc__) + sys.exit(0) + + if len(sys.argv) < 2: + print("Expected file name of config toml as first argument.") + print("See also --help.") + sys.exit(1) + target_fname = sys.argv[1] + target = Configuration.from_toml_file(target_fname) + + github_token = os.getenv("GITHUB_TOKEN") + if github_token is None: + print("Expected GITHUB_TOKEN environment variable to be set.") + print("See also --help.") + sys.exit(1) + client = GithubClient.new(github_token) + + if has_changes(target, client): sys.exit(2) From c642d4bf60b47f8119bb3b5c8d3a167908650d24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Gillot-Lamure?= Date: Wed, 27 Nov 2024 12:36:06 +0100 Subject: [PATCH 3/3] Split business logic in a sequence of functions --- main.py | 69 ++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/main.py b/main.py index 31fc79a..2d85198 100755 --- a/main.py +++ b/main.py @@ -381,9 +381,10 @@ class Configuration(NamedTuple): default_repo_settings: Repository repos_by_id: Dict[int, Repository] repos_by_name: Dict[str, Repository] + fname: Optional[str] @staticmethod - def from_toml_dict(data: Dict[str, Any]) -> Configuration: + def from_toml_dict(data: Dict[str, Any], fname: Optional[str]) -> Configuration: org = Organization.from_toml_dict(data["organization"]) members = {OrganizationMember.from_toml_dict(m) for m in data["member"]} teams = {Team.from_toml_dict(m) for m in data["team"]} @@ -408,13 +409,14 @@ def from_toml_dict(data: Dict[str, Any]) -> Configuration: default_repo_settings=default_repo_settings, repos_by_id=repos_by_id, repos_by_name=repos_by_name, + fname=fname, ) @staticmethod def from_toml_file(fname: str) -> Configuration: with open(fname, "rb") as f: data = tomllib.load(f) - return Configuration.from_toml_dict(data) + return Configuration.from_toml_dict(data, fname) def get_repository_target(self, actual: Repository) -> Repository: """ @@ -955,48 +957,55 @@ def print_team_members_diff( return has_diff -def has_changes(target: Configuration, client: GithubClient) -> bool: - has_changes = False - org_name = target.organization.name - - actual_repos = set(client.get_organization_repositories(org_name)) +def diff_repos(target: Configuration, client: GithubClient) -> bool: + actual_repos = set(client.get_organization_repositories(target.organization.name)) target_repos = set(target.repos_by_id.values()) | { target.get_repository_target(r) for r in actual_repos } repos_diff = Diff.new(target=target_repos, actual=actual_repos) - has_changes |= repos_diff.print_diff( - f"The following repositories are specified in {target_fname} but not present on GitHub:", + + return repos_diff.print_diff( + f"The following repositories are specified in {target.fname} but not present on GitHub:", # Even though we generate the targets form the actuals using the default # settings, it can happen that we match on repository name but not id # (when the id in the config file is wrong). Then the repo will be # missing from the targets. - f"The following repositories are not specified in {target_fname} but present on GitHub:", - f"The following repositories on GitHub need to be changed to match {target_fname}:", + f"The following repositories are not specified in {target.fname} but present on GitHub:", + f"The following repositories on GitHub need to be changed to match {target.fname}:", ) - current_org = client.get_organization(org_name) - if current_org != target.organization: - has_changes = True + +def diff_org(target: Configuration, client: GithubClient) -> bool: + current_org = client.get_organization(target.organization.name) + has_change = current_org != target.organization + if has_change: print("The organization-level settings need to be changed as follows:\n") print_simple_diff( actual=current_org.format_toml(), target=target.organization.format_toml(), ) + return has_change + - current_members = set(client.get_organization_members(org_name)) +def diff_members(target: Configuration, client: GithubClient) -> bool: + current_members = set(client.get_organization_members(target.organization.name)) members_diff = Diff.new(target=target.members, actual=current_members) - has_changes |= members_diff.print_diff( - f"The following members are specified in {target_fname} but not a member of the GitHub organization:", - f"The following members are not specified in {target_fname} but are a member of the GitHub organization:", - f"The following members on GitHub need to be changed to match {target_fname}:", + return members_diff.print_diff( + f"The following members are specified in {target.fname} but not a member of the GitHub organization:", + f"The following members are not specified in {target.fname} but are a member of the GitHub organization:", + f"The following members on GitHub need to be changed to match {target.fname}:", ) - current_teams = set(client.get_organization_teams(org_name)) + +def diff_teams(target: Configuration, client: GithubClient) -> bool: + has_changes = False + + current_teams = set(client.get_organization_teams(target.organization.name)) teams_diff = Diff.new(target=target.teams, actual=current_teams) has_changes |= teams_diff.print_diff( - f"The following teams specified in {target_fname} are not present on GitHub:", - f"The following teams are not specified in {target_fname} but are present on GitHub:", - f"The following teams on GitHub need to be changed to match {target_fname}:", + f"The following teams specified in {target.fname} are not present on GitHub:", + f"The following teams are not specified in {target.fname} but are present on GitHub:", + f"The following teams on GitHub need to be changed to match {target.fname}:", ) # For all the teams which we want to exist, and which do actually exist, @@ -1009,16 +1018,26 @@ def has_changes(target: Configuration, client: GithubClient) -> bool: for team in existing_desired_teams: has_changes |= print_team_members_diff( team_name=team.name, - target_fname=target_fname, + target_fname=target.fname, target_members={ m for m in target.team_memberships if m.team_name == team.name }, - actual_members=set(client.get_team_members(org_name, team)), + actual_members=set(client.get_team_members(target.organization.name, team)), ) return has_changes +def has_changes(target: Configuration, client: GithubClient) -> bool: + has_changes = False + has_changes |= diff_repos(target, client) + has_changes |= diff_org(target, client) + has_changes |= diff_members(target, client) + has_changes |= diff_teams(target, client) + + return has_changes + + def main() -> None: if "--help" in sys.argv: print(__doc__)