Skip to content
Open
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 111 additions & 24 deletions scripts/01-fetch-github.py
Original file line number Diff line number Diff line change
@@ -1,36 +1,123 @@
# SPDX-FileCopyrightText: Copyright (c) 2019-2025 Aibolit
# SPDX-License-Identifier: MIT


import argparse
import os
import subprocess
import sys
from pathlib import Path
from typing import List

import requests as r
import requests
from bs4 import BeautifulSoup


def downloadrepos():
parser = argparse.ArgumentParser(add_help=False)
parser.add_argument('--nrepos', type=int, required=False, default=100)
args = parser.parse_args()
numrepos = args.nrepos
repos = 'target/01'
if not os.path.isdir(repos):
os.makedirs(repos)
result = r.get('https://github.com/trending/java?since=daily', timeout=30)
soup = BeautifulSoup(result.text)
for city in soup.find_all('h1', {'class': 'h3 lh-condensed'}):
if numrepos <= 0:
break
numrepos = numrepos - 1
path = city.a['href'].split('/')
if not os.path.isdir(os.path.join(repos, path[len(path) - 2])):
os.makedirs(os.path.join(repos, path[len(path) - 2]))
if not os.path.isdir(os.path.join(repos, path[len(path) - 2], path[len(path) - 1])):
subprocess.run(['git', 'clone', 'https://github.com' + city.a['href'] + '.git'],
cwd=os.path.join(repos, path[len(path) - 2]), check=False)
class RepositoryDownloader:
def __init__(self, output_dir: str = 'target/01', trend_url, timeout):
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
self.output_dir = Path(output_dir)
self.trending_url = trend_url
self.request_timeout = timeout

def setup_directories(self) -> None:
self.output_dir.mkdir(parents=True, exist_ok=True)

def fetch_trending_repositories(self) -> List[str]:
try:
response = requests.get(self.trending_url, timeout=self.request_timeout)
response.raise_for_status()
except requests.RequestException as e:
print(f'Error fetching trending repositories: {e}', file=sys.stderr)
raise
soup = BeautifulSoup(response.text, 'html.parser')
repositories: List[str] = []
anchors = (
soup.select('article.Box-row h2 a[href]') or
soup.select('h2.h3.lh-condensed a[href]') or
soup.select('h1.h3.lh-condensed a[href]')
)
for a in anchors:
href = (a.get('href') or '').strip()
if not href:
continue
if href.startswith('/'):
href = f'https://github.com{href}'
if not href.endswith('.git'):
href = f'{href}.git'
repositories.append(href)
return repositories
Comment thread
coderabbitai[bot] marked this conversation as resolved.

def clone_repository(self, repo_url: str, owner: str, repo_name: str) -> bool:
owner_dir = self.output_dir / owner
owner_dir.mkdir(exist_ok=True)
if (owner_dir / repo_name).exists():
print(f'Repository {owner}/{repo_name} already exists, skipping...')
return True
try:
result = subprocess.run(
['git', 'clone', repo_url],
cwd=owner_dir,
capture_output=True,
text=True,
check=False
)
if result.returncode == 0:
print(f'Successfully cloned {owner}/{repo_name}')
return True
else:
print(f'Failed to clone {owner}/{repo_name}: {result.stderr}')
return False
except subprocess.SubprocessError as e:
print(f'Error cloning {owner}/{repo_name}: {e}', file=sys.stderr)
return False
Comment on lines +49 to +71

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Address unresolved hardening concerns from previous review.

The clone operation still lacks several critical improvements requested in the previous review:

  1. Line 46: owner_dir.mkdir(exist_ok=True) should use parents=True to handle nested paths.
  2. Lines 52: Missing --depth 1 flag for shallow cloning (reduces bandwidth and time).
  3. Line 51: Missing timeout parameter on subprocess.run (prevents hangs on unresponsive clones).
  4. Line 64: Missing FileNotFoundError handler to gracefully handle missing git executable.

Apply this diff to address all concerns:

     def clone_repository(self, repo_url: str, owner: str, repo_name: str) -> bool:
         owner_dir = self.output_dir / owner
-        owner_dir.mkdir(exist_ok=True)
+        owner_dir.mkdir(parents=True, exist_ok=True)
         if (owner_dir / repo_name).exists():
             print(f'Repository {owner}/{repo_name} already exists, skipping...')
             return True
         try:
             result = subprocess.run(
-                ['git', 'clone', repo_url],
+                ['git', 'clone', '--depth', '1', repo_url],
                 cwd=owner_dir,
                 capture_output=True,
                 text=True,
-                check=False
+                check=False,
+                timeout=300,
             )
             if result.returncode == 0:
                 print(f'Successfully cloned {owner}/{repo_name}')
                 return True
             else:
                 print(f'Failed to clone {owner}/{repo_name}: {result.stderr}')
                 return False
+        except FileNotFoundError:
+            print('git not found on PATH; cannot clone repositories.', file=sys.stderr)
+            return False
+        except subprocess.TimeoutExpired:
+            print(f'Timeout cloning {owner}/{repo_name}', file=sys.stderr)
+            return False
         except subprocess.SubprocessError as e:
             print(f'Error cloning {owner}/{repo_name}: {e}', file=sys.stderr)
             return False
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def clone_repository(self, repo_url: str, owner: str, repo_name: str) -> bool:
owner_dir = self.output_dir / owner
owner_dir.mkdir(exist_ok=True)
if (owner_dir / repo_name).exists():
print(f'Repository {owner}/{repo_name} already exists, skipping...')
return True
try:
result = subprocess.run(
['git', 'clone', repo_url],
cwd=owner_dir,
capture_output=True,
text=True,
check=False
)
if result.returncode == 0:
print(f'Successfully cloned {owner}/{repo_name}')
return True
else:
print(f'Failed to clone {owner}/{repo_name}: {result.stderr}')
return False
except subprocess.SubprocessError as e:
print(f'Error cloning {owner}/{repo_name}: {e}', file=sys.stderr)
return False
def clone_repository(self, repo_url: str, owner: str, repo_name: str) -> bool:
owner_dir = self.output_dir / owner
owner_dir.mkdir(parents=True, exist_ok=True)
if (owner_dir / repo_name).exists():
print(f'Repository {owner}/{repo_name} already exists, skipping...')
return True
try:
result = subprocess.run(
['git', 'clone', '--depth', '1', repo_url],
cwd=owner_dir,
capture_output=True,
text=True,
check=False,
timeout=300,
)
if result.returncode == 0:
print(f'Successfully cloned {owner}/{repo_name}')
return True
else:
print(f'Failed to clone {owner}/{repo_name}: {result.stderr}')
return False
except FileNotFoundError:
print('git not found on PATH; cannot clone repositories.', file=sys.stderr)
return False
except subprocess.TimeoutExpired:
print(f'Timeout cloning {owner}/{repo_name}', file=sys.stderr)
return False
except subprocess.SubprocessError as e:
print(f'Error cloning {owner}/{repo_name}: {e}', file=sys.stderr)
return False
🤖 Prompt for AI Agents
In scripts/01-fetch-github.py around lines 44 to 66, the clone_repository
implementation needs hardening: create owner_dir with
owner_dir.mkdir(parents=True, exist_ok=True) to handle nested paths; run git
with a shallow clone flag by adding '--depth', '1' to the git clone args; pass a
sensible timeout to subprocess.run (e.g., timeout=60) to avoid hanging; and add
a separate except FileNotFoundError handler to log a clear error when the git
executable is missing (return False), keeping the existing SubprocessError
handler for other subprocess errors.


def download_repositories(self, max_repositories: int) -> None:
print(f'Fetching {max_repositories} trending Java repositories...')
try:
repositories = self.fetch_trending_repositories()
except requests.RequestException:
print('Failed to fetch repository list. Exiting.')
return
downloaded_count = 0
for repo_url in repositories:
if downloaded_count >= max_repositories:
break
path_parts = repo_url.replace('https://github.com/', '').replace('.git', '').split('/')
if len(path_parts) < 2:
continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

download_repositories increments downloaded_count for every successful return from clone_repository, including the skip-because-already-exists branch on line 41. That double-counts pre-existing clones against --nrepos, so a user with N stale clones in target/01 will end up downloading nothing on the next run. Count only the actual clone, not the skip.

owner, repo_name = path_parts[0], path_parts[1]
Comment thread
kodsurfer marked this conversation as resolved.
Outdated
print(f'Processing {owner}/{repo_name}...')
if self.clone_repository(repo_url, owner, repo_name):
downloaded_count += 1
print(f'Downloaded {downloaded_count} repositories to {self.output_dir}')


def parse_arguments() -> argparse.Namespace:
parser = argparse.ArgumentParser(
description='Download trending Java repositories from GitHub',
add_help=True
)
parser.add_argument(
'--nrepos',
type=int,
required=False,
default=100,
help='Number of repositories to download (default: 100)'
)
parser.add_argument(
'--output-dir',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR body advertises "options for count, output directory, and timeout", but parse_arguments only adds --nrepos and --output-dir. The timeout stays hardcoded at 30 in main(). Either add the --timeout argument the description promises or drop it from the summary.

type=str,
required=False,
default='target/01',
help='Output directory for downloaded repositories (default: target/01)'
)
return parser.parse_args()


def main() -> None:
args = parse_arguments()
downloader = RepositoryDownloader(args.output_dir, 'https://github.com/trending/java?since=daily', 30)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
downloader.setup_directories()
downloader.download_repositories(args.nrepos)


if __name__ == '__main__':
downloadrepos()
main()
Loading