Skip to content

Commit fb7757e

Browse files
committed
enh: use cached repo object
Signed-off-by: habeck <habeck@us.ibm.com>
1 parent 677cfdf commit fb7757e

3 files changed

Lines changed: 32 additions & 31 deletions

File tree

cpex/framework/utils.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ def render(self, content: Any) -> bytes:
411411
)
412412

413413

414-
def find_package_path( package_name: str) -> Path:
414+
def find_package_path(package_name: str) -> Path:
415415
"""Locate installed package directory using importlib.metadata.
416416
417417
Args:
@@ -448,4 +448,4 @@ def find_package_path( package_name: str) -> Path:
448448
except Exception as e:
449449
if isinstance(e, RuntimeError):
450450
raise
451-
raise RuntimeError(f"Error locating package {package_name}: {str(e)}") from e
451+
raise RuntimeError(f"Error locating package {package_name}: {str(e)}") from e

cpex/tools/catalog.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ def download_contents(self, git_url: str, headers, path: str, repo_url: httpx.UR
151151
else:
152152
logger.error("Failed to download file: %s status_code: %d", git_url, result.status_code)
153153

154-
def download_file(self, repo_path: str, item: dict, headers) -> str | None:
154+
def download_file(self, repo_path: str, item: dict, headers, gh_repo) -> str | None:
155155
"""Download the content of a github file
156156
157157
Args:
@@ -163,7 +163,6 @@ def download_file(self, repo_path: str, item: dict, headers) -> str | None:
163163
"""
164164
# Get the repository using PyGithub
165165
try:
166-
gh_repo = self.gh.get_repo(repo_path)
167166
file_content = gh_repo.get_contents(item["path"])
168167
content = file_content.decoded_content.decode("utf-8")
169168
return content
@@ -245,6 +244,7 @@ def _process_manifest_item(
245244
headers,
246245
relpath: Path,
247246
repo_path: str,
247+
gh_repo,
248248
) -> bool:
249249
"""Process a single manifest search result item.
250250
@@ -265,7 +265,7 @@ def _process_manifest_item(
265265
return False
266266

267267
# manifest_data = self.download_file(repo_path=repo_path, git_url=item["git_url"], headers=headers)
268-
manifest_data = self.download_file(repo_path=repo_path, item=item, headers=headers)
268+
manifest_data = self.download_file(repo_path=repo_path, item=item, headers=headers, gh_repo=gh_repo)
269269
if manifest_data is None:
270270
logger.error("Failed to download plugin-manifest from %s", member)
271271
return False
@@ -278,7 +278,7 @@ def _process_manifest_item(
278278
return True
279279

280280
def find_and_save_plugin_manifest(
281-
self, member: str, name: str, repo_url: httpx.URL, headers
281+
self, member: str, name: str, repo_url: httpx.URL, headers, gh_repo
282282
) -> PluginManifest | None:
283283
"""Find the plugin-manifest.yaml relative to the supplied member folder,
284284
download and save the manifest, updating the monorepo's package_folder, package_source and repo_url attributes
@@ -303,7 +303,7 @@ def find_and_save_plugin_manifest(
303303
return None
304304

305305
for item in items:
306-
if self._process_manifest_item(item, name, member, repo_url, headers, relpath, repo_path):
306+
if self._process_manifest_item(item, name, member, repo_url, headers, relpath, repo_path, gh_repo):
307307
break # Successfully processed first valid manifest
308308

309309
return None
@@ -336,7 +336,7 @@ def _process_pyproject(self, gh_repo, item, repo_url: httpx.URL, headers) -> Non
336336

337337
# Find and save the plugin manifest
338338
self.find_and_save_plugin_manifest(
339-
member=member, name=project_data["project"]["name"], repo_url=repo_url, headers=headers
339+
member=member, name=project_data["project"]["name"], repo_url=repo_url, headers=headers, gh_repo=gh_repo
340340
)
341341

342342
def update_catalog_with_pyproject(self) -> bool:

tests/unit/cpex/tools/test_catalog.py

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import subprocess
1414
import sys
1515
from pathlib import Path
16+
from unittest import mock
1617
from unittest.mock import MagicMock, Mock, patch, mock_open
1718

1819
# Third-Party
@@ -424,7 +425,7 @@ def test_install_from_pypi_success(self, tmp_path, mock_github_env):
424425
"""Test successful installation from PyPI."""
425426
with (
426427
patch("cpex.tools.catalog.subprocess.run") as mock_subprocess,
427-
patch("cpex.tools.catalog.importlib.metadata.distributions") as mock_distributions,
428+
patch("cpex.framework.utils.importlib.metadata.distributions") as mock_distributions,
428429
):
429430
# Create manifest file
430431
package_dir = tmp_path / "test_package"
@@ -469,8 +470,8 @@ def test_install_from_pypi_package_not_found(self, mock_github_env):
469470
"""Test when package is not found after installation."""
470471
with (
471472
patch("cpex.tools.catalog.subprocess.run"),
472-
patch("cpex.tools.catalog.importlib.metadata.distributions", return_value=[]),
473-
patch("cpex.tools.catalog.importlib.util.find_spec", return_value=None),
473+
patch("cpex.framework.utils.importlib.metadata.distributions", return_value=[]),
474+
patch("cpex.framework.utils.importlib.util.find_spec", return_value=None),
474475
):
475476
catalog = PluginCatalog()
476477
with pytest.raises(RuntimeError, match="Could not find installed package"):
@@ -480,7 +481,7 @@ def test_install_from_pypi_manifest_not_found(self, tmp_path, mock_github_env):
480481
"""Test when manifest file is not found in package."""
481482
with (
482483
patch("cpex.tools.catalog.subprocess.run"),
483-
patch("cpex.tools.catalog.importlib.metadata.distributions") as mock_distributions,
484+
patch("cpex.framework.utils.importlib.metadata.distributions") as mock_distributions,
484485
):
485486
# Setup mock distribution without plugin-manifest.yaml file
486487
mock_dist = Mock()
@@ -503,7 +504,7 @@ def test_install_from_pypi_invalid_manifest(self, tmp_path, mock_github_env):
503504
"""Test when manifest file is invalid."""
504505
with (
505506
patch("cpex.tools.catalog.subprocess.run"),
506-
patch("cpex.tools.catalog.importlib.metadata.distributions") as mock_distributions,
507+
patch("cpex.framework.utils.importlib.metadata.distributions") as mock_distributions,
507508
):
508509
package_dir = tmp_path / "test_package"
509510
package_dir.mkdir()
@@ -605,20 +606,19 @@ def test_download_file_success(self, mock_github_env):
605606
catalog.gh.get_repo = Mock(return_value=mock_repo)
606607

607608
item = {"path": "test_plugin/plugin-manifest.yaml"}
608-
result = catalog.download_file("org/repo", item, {})
609+
result = catalog.download_file("org/repo", item, {}, mock_repo)
609610

610611
assert result == manifest_content
611612

612613
def test_download_file_failure(self, mock_github_env):
613614
"""Test failed file download."""
614615
with patch("cpex.tools.catalog.logger") as mock_logger:
615616
catalog = PluginCatalog()
616-
617617
# Mock the GitHub repository to raise an exception
618-
catalog.gh.get_repo = Mock(side_effect=Exception("Not found"))
619-
618+
mock_repo = Mock(side_effect=Exception("Not found"))
619+
mock_repo.get_contents.return_value = Exception("Not found")
620620
item = {"path": "test_plugin/plugin-manifest.yaml"}
621-
result = catalog.download_file("org/repo", item, {})
621+
result = catalog.download_file("org/repo", item, {}, mock_repo)
622622

623623
assert result is None
624624
mock_logger.error.assert_called_once()
@@ -654,7 +654,7 @@ def test_find_and_save_plugin_manifest_success(self, tmp_path, mock_github_env):
654654
catalog.gh.get_repo = Mock(return_value=mock_repo)
655655

656656
repo_url = httpx.URL("https://github.com/org/repo")
657-
catalog.find_and_save_plugin_manifest("test_plugin", "test_plugin", repo_url, {})
657+
catalog.find_and_save_plugin_manifest("test_plugin", "test_plugin", repo_url, {}, mock_repo)
658658

659659
saved_file = tmp_path / "catalog" / "test_plugin" / "plugin-manifest.yaml"
660660
assert saved_file.exists()
@@ -738,7 +738,7 @@ def test_install_from_pypi_with_version_constraint(self, tmp_path, mock_github_e
738738
"""Test installation with version constraint."""
739739
with (
740740
patch("cpex.tools.catalog.subprocess.run") as mock_subprocess,
741-
patch("cpex.tools.catalog.importlib.metadata.distributions") as mock_distributions,
741+
patch("cpex.framework.utils.importlib.metadata.distributions") as mock_distributions,
742742
):
743743
package_dir = tmp_path / "test_package"
744744
package_dir.mkdir()
@@ -777,7 +777,7 @@ def test_install_from_pypi_with_default_configs(self, tmp_path, mock_github_env)
777777
"""Test installation with default_configs field."""
778778
with (
779779
patch("cpex.tools.catalog.subprocess.run") as mock_subprocess,
780-
patch("cpex.tools.catalog.importlib.metadata.distributions") as mock_distributions,
780+
patch("cpex.framework.utils.importlib.metadata.distributions") as mock_distributions,
781781
):
782782
package_dir = tmp_path / "test_package"
783783
package_dir.mkdir()
@@ -813,7 +813,7 @@ def test_install_from_pypi_with_existing_package_info(self, tmp_path, mock_githu
813813
"""Test installation with existing package_info."""
814814
with (
815815
patch("cpex.tools.catalog.subprocess.run") as mock_subprocess,
816-
patch("cpex.tools.catalog.importlib.metadata.distributions") as mock_distributions,
816+
patch("cpex.framework.utils.importlib.metadata.distributions") as mock_distributions,
817817
):
818818
package_dir = tmp_path / "test_package"
819819
package_dir.mkdir()
@@ -855,7 +855,7 @@ def test_install_from_pypi_with_null_default_configs_in_manifest(self, tmp_path,
855855
"""Test installation with null default_configs in manifest."""
856856
with (
857857
patch("cpex.tools.catalog.subprocess.run") as mock_subprocess,
858-
patch("cpex.tools.catalog.importlib.metadata.distributions") as mock_distributions,
858+
patch("cpex.framework.utils.importlib.metadata.distributions") as mock_distributions,
859859
):
860860
package_dir = tmp_path / "test_package"
861861
package_dir.mkdir()
@@ -1001,8 +1001,8 @@ def test_process_manifest_item_not_yaml(self, tmp_path, mock_github_env):
10011001

10021002
repo_url = httpx.URL("https://github.com/org/repo")
10031003
relpath = tmp_path / "catalog" / "plugin1" / "plugin-manifest.yaml"
1004-
1005-
result = catalog._process_manifest_item(item, "plugin1", "plugin1", repo_url, {}, relpath, "org/repo")
1004+
mock_repo = Mock()
1005+
result = catalog._process_manifest_item(item, "plugin1", "plugin1", repo_url, {}, relpath, "org/repo", gh_repo=mock_repo)
10061006

10071007
assert result is False
10081008
mock_logger.warning.assert_called()
@@ -1021,11 +1021,11 @@ def test_process_manifest_item_download_failure(self, tmp_path, mock_github_env)
10211021
"path": "plugin1/plugin-manifest.yaml",
10221022
"git_url": "https://api.github.com/file"
10231023
}
1024-
1024+
mock_repo = Mock()
10251025
repo_url = httpx.URL("https://github.com/org/repo")
10261026
relpath = tmp_path / "catalog" / "plugin1" / "plugin-manifest.yaml"
10271027

1028-
result = catalog._process_manifest_item(item, "plugin1", "plugin1", repo_url, {}, relpath, "org/repo")
1028+
result = catalog._process_manifest_item(item, "plugin1", "plugin1", repo_url, {}, relpath, "org/repo", gh_repo=mock_repo)
10291029

10301030
assert result is False
10311031
mock_logger.error.assert_called()
@@ -1041,9 +1041,9 @@ def test_find_and_save_plugin_manifest_search_returns_none(self, tmp_path, mock_
10411041

10421042
# Mock _search_github_code to return None
10431043
catalog._search_github_code = Mock(return_value=None)
1044-
1044+
mock_repo = Mock()
10451045
repo_url = httpx.URL("https://github.com/org/repo")
1046-
result = catalog.find_and_save_plugin_manifest("plugin1", "plugin1", repo_url, {})
1046+
result = catalog.find_and_save_plugin_manifest("plugin1", "plugin1", repo_url, {}, mock_repo)
10471047

10481048
assert result is None
10491049

@@ -1132,9 +1132,10 @@ def test_download_file_with_exception_message(self, mock_github_env):
11321132

11331133
# Mock to raise exception
11341134
catalog.gh.get_repo = Mock(side_effect=Exception("API error"))
1135-
1135+
mock_repo = Mock()
1136+
mock_repo.get_contents = Mock(side_effect=Exception("API error"))
11361137
item = {"path": "test/file.yaml"}
1137-
result = catalog.download_file("org/repo", item, {})
1138+
result = catalog.download_file("org/repo", item, {}, gh_repo=mock_repo)
11381139

11391140
assert result is None
11401141
# Check that error was logged with the item path

0 commit comments

Comments
 (0)