Fix requested changes

Signed-off-by: ziad hany <[email protected]>

Author: ziadhany

minecode_pipelines/miners/cargo.py

@@ -9,6 +9,7 @@
 from datetime import datetime
 
 from minecode_pipelines.pipes import fetch_checkpoint_from_github
+from minecode_pipelines.pipes import get_commit_at_distance_ahead
 from minecode_pipelines.pipes import update_checkpoints_in_github
 from minecode_pipelines.pipes import MINECODE_PIPELINES_CONFIG_REPO
 from minecode_pipelines.pipes import get_changed_files
@@ -20,9 +21,10 @@
 import json
 from pathlib import Path
 
-from minecode_pipelines.utils import get_next_x_commit
 
 PACKAGE_BATCH_SIZE = 500
+COMMIT_BATCH_SIZE = 10
+
 CARGO_CHECKPOINT_PATH = "cargo/checkpoints.json"
 
 
@@ -36,14 +38,14 @@ def process_cargo_packages(cargo_index_repo, cloned_data_repo, config_repo, logg
     base_path = Path(cargo_index_repo.working_tree_dir)
 
     while True:
-        cargo_checkpoints = (
-            fetch_checkpoint_from_github(MINECODE_PIPELINES_CONFIG_REPO, CARGO_CHECKPOINT_PATH)
-            or {}
+        cargo_checkpoints = fetch_checkpoint_from_github(
+            config_repo=MINECODE_PIPELINES_CONFIG_REPO, checkpoint_path=CARGO_CHECKPOINT_PATH
         )
+
         checkpoints_last_commit = cargo_checkpoints.get("last_commit")
 
-        next_commit = get_next_x_commit(
-            cargo_index_repo, checkpoints_last_commit, x=10, branch="master"
+        next_commit = get_commit_at_distance_ahead(
+            cargo_index_repo, checkpoints_last_commit, num_commits_ahead=10, branch_name="master"
         )
 
         if next_commit == checkpoints_last_commit:
@@ -62,10 +64,11 @@ def process_cargo_packages(cargo_index_repo, cloned_data_repo, config_repo, logg
             file_path = base_path / rel_path
             logger(f"Found {file_path}.")
 
-            if not file_path.is_file():
-                continue
-
-            if file_path.name in {"config.json", "README.md", "update-dl-url.yml"}:
+            if not file_path.is_file() or file_path.name in {
+                "config.json",
+                "README.md",
+                "update-dl-url.yml",
+            }:
                 continue
 
             packages = []
@@ -75,6 +78,8 @@ def process_cargo_packages(cargo_index_repo, cloned_data_repo, config_repo, logg
                         packages.append(json.loads(line))
 
             file_counter += 1
+
+            # Commit and push after each full batch or when processing the last file
             commit_and_push = (file_counter % PACKAGE_BATCH_SIZE == 0) or (
                 idx == len(changed_files)
             )
@@ -83,6 +88,7 @@ def process_cargo_packages(cargo_index_repo, cloned_data_repo, config_repo, logg
 
             purl_files.append(purl_file)
             purls.append(str(base_purl))
+
             if not commit_and_push:
                 continue
 
@@ -91,11 +97,10 @@ def process_cargo_packages(cargo_index_repo, cloned_data_repo, config_repo, logg
                 files_to_commit=purl_files,
                 purls=purls,
                 mine_type="packageURL",
-                tool_name="pkg:cargo/minecode-pipelines",
+                tool_name="pkg:pypi/minecode-pipelines",
                 tool_version=VERSION,
             )
 
-            # Push changes to remote repository
             push_changes(repo=cloned_data_repo)
             purl_files = []
             purls = []
@@ -105,15 +110,16 @@ def process_cargo_packages(cargo_index_repo, cloned_data_repo, config_repo, logg
                     f"Updating checkpoint at: {CARGO_CHECKPOINT_PATH} with last commit: {checkpoints_last_commit}"
                 )
 
-            settings_data = {
-                "date": str(datetime.now()),
-                "last_commit": next_commit,
-            }
+            if next_commit != checkpoints_last_commit:
+                settings_data = {
+                    "date": str(datetime.now()),
+                    "last_commit": next_commit,
+                }
 
-            update_checkpoints_in_github(
-                checkpoint=settings_data,
-                cloned_repo=config_repo,
-                path=CARGO_CHECKPOINT_PATH,
-            )
+                update_checkpoints_in_github(
+                    checkpoint=settings_data,
+                    cloned_repo=config_repo,
+                    path=CARGO_CHECKPOINT_PATH,
+                )
 
         logger(f"Pushed batch for commit range {checkpoints_last_commit}:{next_commit}.")

minecode_pipelines/pipelines/mine_cargo.py

@@ -40,7 +40,7 @@ class MineandPublishCargoPURLs(Pipeline):
     def steps(cls):
         return (
             cls.check_federatedcode_eligibility,
-            cls.clone_cargo_repo,
+            cls.clone_cargo_repos,
             cls.mine_and_publish_cargo_packageurls,
             cls.delete_cloned_repos,
         )
@@ -52,9 +52,10 @@ def check_federatedcode_eligibility(self):
         """
         federatedcode.check_federatedcode_configured_and_available(logger=self.log)
 
-    def clone_cargo_repo(self):
+    def clone_cargo_repos(self):
         """
-        Clone the repo at repo_url and return the Repo object
+        Clone the Cargo-related repositories (index, data, and pipelines config)
+        and store their Repo objects in the corresponding instance variables.
         """
         self.cargo_index_repo = federatedcode.clone_repository(MINECODE_CARGO_INDEX_REPO)
         self.cloned_data_repo = federatedcode.clone_repository(MINECODE_DATA_CARGO_REPO)

minecode_pipelines/pipes/__init__.py

@@ -36,7 +36,7 @@ def fetch_checkpoint_from_github(config_repo, checkpoint_path):
     )
     response = requests.get(checkpoints_file)
     if not response.ok:
-        return
+        return {}
 
     checkpoint_data = json.loads(response.text)
     return checkpoint_data
@@ -161,13 +161,19 @@ def get_last_commit(repo, ecosystem):
     return settings_data.get("last_commit")
 
 
-def get_next_x_commit(repo: Repo, current_commit: str, x: int = 10, branch: str = "master") -> str:
+def get_commit_at_distance_ahead(
+    repo: Repo,
+    current_commit: str,
+    num_commits_ahead: int = 10,
+    branch_name: str = "master",
+) -> str:
     """
-    Get the x-th next commit after the current commit in the specified branch.
+    Return the commit hash that is `num_commits_ahead` commits ahead of `current_commit`
+    on the given branch.
     """
     if not current_commit:
         current_commit = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
-    revs = repo.git.rev_list(f"^{current_commit}", branch).splitlines()
-    if len(revs) < x:
+    revs = repo.git.rev_list(f"^{current_commit}", branch_name).splitlines()
+    if len(revs) < num_commits_ahead:
         raise ValueError(f"Not enough commits ahead; only {len(revs)} available.")
-    return revs[-x]
+    return revs[-num_commits_ahead]

minecode_pipelines/tests/pipes/test_cargo_pipes.py

@@ -36,7 +36,7 @@ def test_collect_packages_from_cargo_calls_write(self, mock_write):
 
         self.assertEqual(called_repo, repo)
 
-        expected_base_purl = 'aboutcode-packages-cargo-0/cargo/c5store/purls.yml'
+        expected_base_purl = "aboutcode-packages-cargo-0/cargo/c5store/purls.yml"
         self.assertEqual(str(base_purl), str(expected_base_purl))
         self.assertEqual(written_packages, expected)
 
@@ -59,4 +59,4 @@ def test_add_purl_result_with_mock_repo(self):
 
             with open(written_file, encoding="utf-8") as f:
                 content = saneyaml.load(f)
-            self.assertEqual(content, purls)
+            self.assertEqual(content, purls)

minecode_pipelines/utils.py

@@ -9,7 +9,6 @@
 import tempfile
 import os
 from commoncode.fileutils import create_dir
-from git.repo.base import Repo
 
 from itertools import zip_longest
 
@@ -60,15 +59,3 @@ def get_temp_file(file_name="data", extension=".file", dir_name=""):
     temp_dir = get_temp_dir(dir_name)
     location = os.path.join(temp_dir, file_name)
     return location
-
-
-def get_next_x_commit(repo: Repo, current_commit: str, x: int = 10, branch: str = "master") -> str:
-    """
-    Get the x-th next commit after the current commit in the specified branch.
-    """
-    if not current_commit:
-        current_commit = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
-    revs = repo.git.rev_list(f"^{current_commit}", branch).splitlines()
-    if len(revs) < x:
-        raise ValueError(f"Not enough commits ahead; only {len(revs)} available.")
-    return revs[-x]