Cleanup and coverage

changelog/+legacy_states_catch_errors.fixed.md

@@ -0,0 +1 @@
+Fixed legacy `github` state functions rendering a traceback instead of a failed state when an unexpected `CommandExecutionError` was raised, e.g. caused by missing profile configuration

changelog/+partial_changes.fixed.md

@@ -0,0 +1 @@
+Fixed compound states (`github_repo.present`, `github_org.team_present`, environment management) reporting no changes when failing after some changes had already been applied

docs/ref/utils/index.rst

@@ -24,6 +24,7 @@ _________
     utils.gh.profile
     utils.gh.pulls
     utils.gh.repos
+    utils.gh.resource
     utils.gh.rulesets
     utils.gh.statecores
     utils.gh.statecores.actions

docs/ref/utils/saltext.github.utils.gh.resource.rst

@@ -0,0 +1,5 @@
+saltext.github.utils.gh.resource
+================================
+
+.. automodule:: saltext.github.utils.gh.resource
+    :members:

pyproject.toml

@@ -81,17 +81,23 @@ docs = [
 ]
 docsauto = ["sphinx-autobuild"]
 lint = [
-    "pylint==4.0.4",
+    "pylint==4.0.9",
 ]
 tests = [
     "pytest>=7.2.0",
     "pytest-salt-factories>=1.0.0",
     "pytest-instafail",
+    "pytest-xdist",
 ]
 
 [project.entry-points."salt.loader"]
 "saltext.github" = "saltext.github"
 
+[tool.pytest.ini_options]
+# Best test distribution for the module-scoped functional fixtures.
+# Only effective when parallelizing via ``-n <workers>``/``-n auto``.
+addopts = "--dist=loadscope"
+
 [tool.uv]
 # Salt's legacy setup.py requires old setuptools.
 # Since uv 0.6.0, it does not fall back to reading requirements from egg-info.

src/saltext/github/modules/github.py

@@ -59,13 +59,43 @@ def __virtual__():
     )
 
 
+# Anything below here is deprecated. Replacement functions are in github_org/github_repo.
+# This module is intended for non-org/non-repo functionality now.
+
+
 def _get_conn(profile):
     """
     Return the GitHub connection, cached into __context__ for performance
     """
     return gh_profile.get_conn(profile, opts=__opts__, context=__context__)
 
 
+def _get_conn_repo(profile, repo_name) -> tuple[object, str]:
+    return _get_conn(profile), repo_name or _get_config_value(profile, "repo_name")
+
+
+def _get_config_value(profile, config_name):
+    """
+    Helper function that returns a profile's configuration value based on
+    the supplied configuration name.
+
+    profile
+        The profile name that contains configuration information.
+
+    config_name
+        The configuration item's name to use to return configuration values.
+    """
+    return gh_profile.get_config_value(profile, config_name, opts=__opts__, context=__context__)
+
+
+def _get_owner(profile):
+    """
+    Helper function that returns a profile's default repository owner
+    (``owner``, legacy alias ``org_name``), or None when unset.
+    """
+    return gh_profile.get_owner(profile, opts=__opts__, context=__context__)
+
+
 def get_user(name, profile="github", user_details=False):
     """
     Get a GitHub user by name.
@@ -107,32 +137,6 @@ def get_user(name, profile="github", user_details=False):
         return False
 
 
-# Anything below here is deprecated. Replacement functions are in github_org/github_repo.
-# This module is intended for non-org/non-repo functionality now.
-
-
-def _get_config_value(profile, config_name):
-    """
-    Helper function that returns a profile's configuration value based on
-    the supplied configuration name.
-
-    profile
-        The profile name that contains configuration information.
-
-    config_name
-        The configuration item's name to use to return configuration values.
-    """
-    return gh_profile.get_config_value(profile, config_name, opts=__opts__, context=__context__)
-
-
-def _get_owner(profile):
-    """
-    Helper function that returns a profile's default repository owner
-    (``owner``, legacy alias ``org_name``), or None when unset.
-    """
-    return gh_profile.get_owner(profile, opts=__opts__, context=__context__)
-
-
 def list_users(profile="github", ignore_cache=False):
     """
     List all users within the organization.
@@ -240,12 +244,9 @@ def get_issue(issue_number, repo_name=None, profile="github", output="min"):
         salt myminion github.get_issue 514
         salt myminion github.get_issue 514 repo_name=salt
     """
-    if repo_name is None:
-        repo_name = _get_config_value(profile, "repo_name")
-
     ret = {}
     try:
-        issue_data = gh_issues.get(_get_conn(profile), repo_name, issue_number)
+        issue_data = gh_issues.get(*_get_conn_repo(profile, repo_name), issue_number)
     except GitHubApiError:
         return ret
 
@@ -294,10 +295,9 @@ def get_issue_comments(issue_number, repo_name=None, profile="github", since=Non
         salt myminion github.get_issue_comments 514
         salt myminion github.get_issue 514 repo_name=salt
     """
-    if repo_name is None:
-        repo_name = _get_config_value(profile, "repo_name")
-
-    comments = gh_issues.get_comments(_get_conn(profile), repo_name, issue_number, since=since)
+    comments = gh_issues.get_comments(
+        *_get_conn_repo(profile, repo_name), issue_number, since=since
+    )
 
     ret = {}
     for comment in comments or []:
@@ -400,12 +400,8 @@ def get_issues(
 
         salt myminion github.get_issues my-github-repo
     """
-    if repo_name is None:
-        repo_name = _get_config_value(profile, "repo_name")
-
     issues = gh_issues.list_all(
-        _get_conn(profile),
-        repo_name,
+        *_get_conn_repo(profile, repo_name),
         milestone=milestone,
         state=state,
         assignee=assignee,
@@ -479,12 +475,8 @@ def get_milestones(
 
         salt myminion github.get_milestones
     """
-    if repo_name is None:
-        repo_name = _get_config_value(profile, "repo_name")
-
     milestones = gh_milestones.list_all(
-        _get_conn(profile),
-        repo_name,
+        *_get_conn_repo(profile, repo_name),
         state=state,
         sort=sort,
         direction=direction,
@@ -536,17 +528,14 @@ def get_milestone(number=None, name=None, repo_name=None, profile="github", outp
         salt myminion github.get_milestone 72
         salt myminion github.get_milestone name=my_milestone
     """
-    ret = {}
-
     if not (number or name):
         raise CommandExecutionError("Either a milestone 'name' or 'number' must be provided.")
 
-    if repo_name is None:
-        repo_name = _get_config_value(profile, "repo_name")
+    ret = {}
 
     if number:
         try:
-            milestone_data = gh_milestones.get(_get_conn(profile), repo_name, number)
+            milestone_data = gh_milestones.get(*_get_conn_repo(profile, repo_name), number)
         except GitHubApiError:
             return ret
         milestone_id = (milestone_data or {}).get("id")
@@ -1093,17 +1082,13 @@ def add_team_repo(repo_name, team_name, profile="github", permission=None):
         log.error("Team %s does not exist", team_name)
         return False
     try:
-        if not gh_teams.add_repo(conn, team_info, repo_name, permission=permission):
-            log.error(
-                "Failed setting permission %s on repo %s for team %s",
-                permission,
-                repo_name,
-                team_name,
-            )
-            return False
+        gh_teams.add_repo(conn, team_info, repo_name, permission=permission)
     except GitHubNotFoundError:
         log.error("Resource not found: %s", team_info["id"], exc_info_on_loglevel=logging.DEBUG)
         return False
+    except CommandExecutionError as err:
+        log.error(str(err), exc_info_on_loglevel=logging.DEBUG)
+        return False
     return True
 
 
@@ -1387,12 +1372,8 @@ def get_prs(
         salt myminion github.get_prs
         salt myminion github.get_prs base=2016.11
     """
-    if repo_name is None:
-        repo_name = _get_config_value(profile, "repo_name")
-
     prs = gh_pulls.list_all(
-        _get_conn(profile),
-        repo_name,
+        *_get_conn_repo(profile, repo_name),
         state=state,
         head=head,
         base=base,

src/saltext/github/modules/github_org.py

@@ -33,8 +33,6 @@
 
 import logging
 
-from salt.exceptions import CommandExecutionError
-
 from saltext.github.utils import filter_fields
 from saltext.github.utils.gh import members as gh_members
 from saltext.github.utils.gh import profile as gh_profile
@@ -50,13 +48,6 @@ def _get_conn(profile):
     return gh_profile.get_conn(profile, opts=__opts__, context=__context__)
 
 
-def _get_team(conn, name, owner=None):
-    """
-    Return a team's cached details, raising when it does not exist.
-    """
-    return gh_teams.find(conn, name, owner=owner)
-
-
 def list_members(owner=None, ignore_cache=False, profile="github"):
     """
     List the logins of all members of the organization.
@@ -204,7 +195,7 @@ def get_team(name, owner=None, profile="github"):
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    return _strip_team_caches(_get_team(_get_conn(profile), name, owner=owner))
+    return _strip_team_caches(gh_teams.find(_get_conn(profile), name, owner=owner))
 
 
 def add_team(
@@ -248,17 +239,16 @@ def add_team(
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    conn = _get_conn(profile)
-    gh_teams.create(
-        conn,
+    ght = _get_conn(profile).bind(gh_teams)
+    ght.create(
         name,
         description=description,
         repo_names=repo_names,
         privacy=privacy,
         permission=permission,
         owner=owner,
     )
-    return _strip_team_caches(_get_team(conn, name, owner=owner))
+    return _strip_team_caches(ght.find(name, owner=owner))
 
 
 def edit_team(name, description=None, privacy=None, permission=None, owner=None, profile="github"):
@@ -291,17 +281,16 @@ def edit_team(name, description=None, privacy=None, permission=None, owner=None,
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    conn = _get_conn(profile)
-    gh_teams.update(
-        conn,
-        _get_team(conn, name, owner=owner),
+    ght = _get_conn(profile).bind(gh_teams)
+    ght.update(
+        ght.find(name, owner=owner),
         name,
         description=description,
         privacy=privacy,
         permission=permission,
         owner=owner,
     )
-    return _strip_team_caches(_get_team(conn, name, owner=owner))
+    return _strip_team_caches(ght.find(name, owner=owner))
 
 
 def remove_team(name, owner=None, profile="github"):
@@ -323,8 +312,8 @@ def remove_team(name, owner=None, profile="github"):
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    conn = _get_conn(profile)
-    gh_teams.delete(conn, _get_team(conn, name, owner=owner), owner=owner)
+    ght = _get_conn(profile).bind(gh_teams)
+    ght.delete(ght.find(name, owner=owner), owner=owner)
     return True
 
 
@@ -355,10 +344,8 @@ def list_team_repos(team_name, owner=None, fields=None, ignore_cache=False, prof
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    conn = _get_conn(profile)
-    repos = gh_teams.list_repos(
-        conn, _get_team(conn, team_name, owner=owner), owner=owner, ignore_cache=ignore_cache
-    )
+    ght = _get_conn(profile).bind(gh_teams)
+    repos = ght.list_repos(ght.find(team_name, owner=owner), owner=owner, ignore_cache=ignore_cache)
     return filter_fields({repo["name"]: repo for repo in repos.values()}, fields)
 
 
@@ -389,13 +376,8 @@ def add_team_repo(repo_name, team_name, permission=None, owner=None, profile="gi
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    conn = _get_conn(profile)
-    if not gh_teams.add_repo(
-        conn, _get_team(conn, team_name, owner=owner), repo_name, permission=permission, owner=owner
-    ):
-        raise CommandExecutionError(
-            f"Failed setting permission {permission} on repo {repo_name} for team {team_name}."
-        )
+    ght = _get_conn(profile).bind(gh_teams)
+    ght.add_repo(ght.find(team_name, owner=owner), repo_name, permission=permission, owner=owner)
     return True
 
 
@@ -421,8 +403,8 @@ def remove_team_repo(repo_name, team_name, owner=None, profile="github"):
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    conn = _get_conn(profile)
-    gh_teams.remove_repo(conn, _get_team(conn, team_name, owner=owner), repo_name, owner=owner)
+    ght = _get_conn(profile).bind(gh_teams)
+    ght.remove_repo(ght.find(team_name, owner=owner), repo_name, owner=owner)
     return True
 
 
@@ -448,9 +430,9 @@ def list_team_members(team_name, owner=None, ignore_cache=False, profile="github
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    conn = _get_conn(profile)
-    return gh_teams.list_members(
-        conn, _get_team(conn, team_name, owner=owner), owner=owner, ignore_cache=ignore_cache
+    ght = _get_conn(profile).bind(gh_teams)
+    return ght.list_members(
+        ght.find(team_name, owner=owner), owner=owner, ignore_cache=ignore_cache
     )
 
 
@@ -501,8 +483,8 @@ def add_team_member(name, team_name, owner=None, profile="github"):
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    conn = _get_conn(profile)
-    gh_teams.add_member(conn, _get_team(conn, team_name, owner=owner), name, owner=owner)
+    ght = _get_conn(profile).bind(gh_teams)
+    ght.add_member(ght.find(team_name, owner=owner), name, owner=owner)
     return True
 
 
@@ -528,8 +510,8 @@ def remove_team_member(name, team_name, owner=None, profile="github"):
     profile
         Profile configuration name to use. Defaults to ``github``.
     """
-    conn = _get_conn(profile)
-    gh_teams.remove_member(conn, _get_team(conn, team_name, owner=owner), name, owner=owner)
+    ght = _get_conn(profile).bind(gh_teams)
+    ght.remove_member(ght.find(team_name, owner=owner), name, owner=owner)
     return True