Skip fork PR approval API calls for private repos instead of catching exceptions

Agent-Logs-Url: https://github.com/eclipse-csi/otterdog/sessions/7a7e39a1-6243-4958-8653-fa287e9c1984

Co-authored-by: mbarbero <452625+mbarbero@users.noreply.github.com>

Author: Copilot

otterdog/models/github_organization.py

@@ -681,7 +681,8 @@ async def _process_single_repo(
     github_repo_data = await rest_api.repo.get_repo_data(github_id, repo_name)
     repo = Repository.from_provider_data(github_id, github_repo_data)
 
-    github_repo_workflow_data = await rest_api.repo.get_workflow_settings(github_id, repo_name)
+    is_private = github_repo_data.get("private", False)
+    github_repo_workflow_data = await rest_api.repo.get_workflow_settings(github_id, repo_name, is_private=is_private)
     repo.workflows = RepositoryWorkflowSettings.from_provider_data(github_id, github_repo_workflow_data)
     if repo_permissions is not None:
         repo_permission = repo_permissions.get(repo_name, [])

otterdog/models/repository.py

@@ -1478,6 +1478,7 @@ async def apply_live_patch(
                         org_id,
                         expected_object.name,
                         await expected_object.workflows.dict_to_provider_data(org_id, workflow_data, provider),
+                        is_private=expected_object.private is True,
                     )
 
                 # Team permissions are defined on the repository but originate from the team side.
@@ -1511,7 +1512,12 @@ async def apply_live_patch(
                     data = unwrap(patch.expected_object).workflows.to_model_dict(for_diff=True)
                     github_data = await RepositoryWorkflowSettings.dict_to_provider_data(org_id, data, provider)
 
-                    await provider.update_repo_workflow_settings(org_id, expected_object.name, github_data)
+                    await provider.update_repo_workflow_settings(
+                        org_id,
+                        expected_object.name,
+                        github_data,
+                        is_private=expected_object.private is True,
+                    )
 
                 # Team permissions sit at the intersection of repositories and teams.
                 # A change in `team_permissions` can represent three different operations:

otterdog/providers/github/__init__.py

@@ -360,13 +360,15 @@ async def add_repo_environment(self, org_id: str, repo_name: str, env_name: str,
     async def delete_repo_environment(self, org_id: str, repo_name: str, env_name: str) -> None:
         await self.rest_api.repo.delete_environment(org_id, repo_name, env_name)
 
-    async def get_repo_workflow_settings(self, org_id: str, repo_name: str) -> dict[str, Any]:
-        return await self.rest_api.repo.get_workflow_settings(org_id, repo_name)
+    async def get_repo_workflow_settings(
+        self, org_id: str, repo_name: str, is_private: bool = False
+    ) -> dict[str, Any]:
+        return await self.rest_api.repo.get_workflow_settings(org_id, repo_name, is_private=is_private)
 
     async def update_repo_workflow_settings(
-        self, org_id: str, repo_name: str, workflow_settings: dict[str, Any]
+        self, org_id: str, repo_name: str, workflow_settings: dict[str, Any], is_private: bool = False
     ) -> None:
-        await self.rest_api.repo.update_workflow_settings(org_id, repo_name, workflow_settings)
+        await self.rest_api.repo.update_workflow_settings(org_id, repo_name, workflow_settings, is_private=is_private)
 
     async def get_org_secrets(self, org_id: str) -> list[dict[str, Any]]:
         return await self.rest_api.org.get_secrets(org_id)

otterdog/providers/github/rest/repo_client.py

@@ -971,7 +971,9 @@ async def delete_variable(self, org_id: str, repo_name: str, variable_name: str)
 
         _logger.debug("removed repo variable '%s'", variable_name)
 
-    async def get_workflow_settings(self, org_id: str, repo_name: str) -> dict[str, Any]:
+    async def get_workflow_settings(
+        self, org_id: str, repo_name: str, is_private: bool = False
+    ) -> dict[str, Any]:
         _logger.debug("retrieving workflow settings for repo '%s/%s'", org_id, repo_name)
 
         workflow_settings: dict[str, Any] = {}
@@ -989,11 +991,14 @@ async def get_workflow_settings(self, org_id: str, repo_name: str) -> dict[str,
         if permissions.get("enabled", False) is not False:
             workflow_settings.update(await self._get_default_workflow_permissions(org_id, repo_name))
 
-        workflow_settings.update(await self._get_fork_pr_approval_policy(org_id, repo_name))
+        if not is_private:
+            workflow_settings.update(await self._get_fork_pr_approval_policy(org_id, repo_name))
 
         return workflow_settings
 
-    async def update_workflow_settings(self, org_id: str, repo_name: str, data: dict[str, Any]) -> None:
+    async def update_workflow_settings(
+        self, org_id: str, repo_name: str, data: dict[str, Any], is_private: bool = False
+    ) -> None:
         _logger.debug("updating workflow settings for repo '%s/%s'", org_id, repo_name)
 
         permission_data = {k: data[k] for k in ["enabled", "allowed_actions"] if k in data}
@@ -1023,7 +1028,7 @@ async def update_workflow_settings(self, org_id: str, repo_name: str, data: dict
         if len(default_permission_data) > 0:
             await self._update_default_workflow_permissions(org_id, repo_name, default_permission_data)
 
-        if "approval_policy" in data:
+        if not is_private and "approval_policy" in data:
             await self._update_fork_pr_approval_policy(org_id, repo_name, {"approval_policy": data["approval_policy"]})
 
         _logger.debug("updated %d workflow setting(s)", len(data))

tests/providers/github/rest/test_repo_client.py

@@ -248,3 +248,24 @@ async def mock_get(org, repo):
 
         # Assert result includes the approval policy
         assert set(policy.items()).issubset(result.items())
+
+    async def test_get_workflow_settings_skips_fork_pr_approval_policy_for_private_repo(self):
+        called = []
+
+        async def mock_request(method, url):
+            return {}
+
+        async def mock_get(org, repo):
+            called.append(True)
+            return {"approval_policy": "all_external_contributors"}
+
+        mock_requester = pretend.stub(request_json=mock_request)
+        mock_restapi = pretend.stub(requester=mock_requester)
+        repo_client = RepoClient(mock_restapi)
+        repo_client._get_fork_pr_approval_policy = mock_get
+
+        result = await repo_client.get_workflow_settings("org", "repo", is_private=True)
+
+        # Assert the fork PR approval policy was NOT fetched
+        assert not called
+        assert "approval_policy" not in result