feat: show compat and commit version - #25
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCollect_plugins now gathers git-backed commit metadata and docs for plugins, builds a PyPI-derived Snakemake compatibility index, computes per-plugin minimum Snakemake versions, converts plugin Markdown docs to RST, and exposes commit/sn akemake_version data to templates; templates, CSS, tests, CI, and the collector API were updated. Changes
Sequence DiagramsequenceDiagram
participant Main as Collection Flow
participant PyPI as PyPI API
participant Index as Compat Index Builder
participant Collector as Plugin Collector
participant Repo as Git Repository
participant Template as Template Renderer
Main->>PyPI: request Snakemake release metadata
PyPI-->>Index: return releases & requires_dist
Index->>Index: build snakemake_compat_index
Index-->>Main: provide snakemake_compat_index
Main->>Collector: collect_plugins(..., snakemake_compat_index)
Collector->>Collector: compute plugin min Snakemake version
Collector->>Repo: clone/fetch plugin repo docs & latest commit
Repo-->>Collector: return commit sha, date, docs
Collector->>Collector: compute commit_age_color and date label
Collector->>Template: render plugin page with snakemake_version + commit_info
Template-->>Main: produced enriched documentation output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
btraven00
force-pushed
the
fetch-commit
branch
5 times, most recently
from
March 9, 2026 14:55
b9799d7 to
cd4e4a5
Compare
btraven00
force-pushed
the
fetch-commit
branch
2 times, most recently
from
March 9, 2026 16:23
ab06b8b to
6d6e55e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/_templates/plugin_base.rst.j2`:
- Around line 15-20: The badges currently target the repository page; update
both image targets so they point to the specific commit page instead: change the
:target: for the last_commit and last_updated images to the commit URL using the
commit SHA (e.g., construct repository + '/commit/' + commit_info["sha"] or use
commit_info["url"] if available) instead of the generic repository variable;
ensure both badges reference commit_info["sha"] (or commit_info["url"]) so
clicking them opens the exact revision shown.
- Around line 32-35: The Snakemake badge URL uses raw snakemake_version (the
template block referencing snakemake_version) which can contain characters like
'>' or '=' that must be URL-encoded; update the template code that builds the
badge (the conditional block using snakemake_version) to apply the same
URL-encoding transformation used for the author badge (e.g., replace or
URL-encode special characters before inserting into the image URL) so the badge
path is safe for all version strings.
In `@source/collect_plugins.py`:
- Around line 420-438: The code only captures plugin_lower from SpecifierSet but
ignores upper bounds; update the loop that iterates
SpecifierSet(m.group(2).strip()) to also capture plugin_upper when s.operator is
"<" or "<=" (convert s.version to Version and set plugin_upper to the smallest
such Version), keep plugin_lower logic for ">" and ">=", and then in the
compat_index matching loop (which checks iface_pkg == plugin_iface) change the
final compatibility check to verify full range overlap by ensuring plugin_lower
< upper AND (plugin_upper is None or plugin_upper > _lower) before returning the
Snakemake version string; use the same identifiers (plugin_lower, plugin_upper,
plugin_iface, compat_index, SpecifierSet, Version, _INTERFACE_PKG_RE,
requires_dist) to locate and modify the code.
- Around line 525-548: The commit-fetching code in get_commit() currently makes
requests without a timeout and swallows all exceptions; update both the GitHub
and GitLab request blocks to pass a reasonable timeout (e.g., timeout=5), catch
only requests.RequestException (and related timeout exceptions), and avoid a
broad except; treat a 404 response as a quiet miss (return None), but for any
other non-200 status or caught exception log an error including repository,
branch and api_url, and then return None so the failure is bounded and visible;
refer to symbols repository_type, api_url, owner, repo, branch, and the
get_commit() function when applying these changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5392192f-ddd3-4cc4-af37-5ddd57a26bfc
📒 Files selected for processing (7)
source/_static/custom.csssource/_templates/executor_plugin.rst.j2source/_templates/logger_plugin.rst.j2source/_templates/plugin_base.rst.j2source/_templates/report_plugin.rst.j2source/_templates/scheduler_plugin.rst.j2source/collect_plugins.py
Comment on lines
+525
to
+548
| api_url = f"https://api.github.com/repos/{owner}/{repo}/commits/{branch}?per_page=1&page=1" | ||
| response = requests.get(api_url) | ||
| if response.status_code == 200: | ||
| data = response.json() | ||
| return { | ||
| "sha": data["sha"][:7], | ||
| "date": data["commit"]["author"]["date"], | ||
| } | ||
|
|
||
| elif repository_type == "gitlab": | ||
| match = re.match(r"https://gitlab\.com/(.+?)/?$", repository) | ||
| if not match: | ||
| return None | ||
| project_path = match.group(1) | ||
|
|
||
| encoded_path = urllib.parse.quote(project_path, safe="") | ||
| api_url = f"https://gitlab.com/api/v4/projects/{encoded_path}/repository/commits/{branch}" | ||
| response = requests.get(api_url) | ||
| if response.status_code == 200: | ||
| data = response.json() | ||
| return { | ||
| "sha": data["short_id"], | ||
| "date": data["committed_date"], | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check its size
find . -name "collect_plugins.py" -type f | head -5Repository: snakemake/snakemake-plugin-catalog
Length of output: 105
🏁 Script executed:
# Read the relevant sections of the file
wc -l source/collect_plugins.pyRepository: snakemake/snakemake-plugin-catalog
Length of output: 107
🏁 Script executed:
# Read lines around 525-548 to see the GitHub API code
sed -n '515,560p' source/collect_plugins.pyRepository: snakemake/snakemake-plugin-catalog
Length of output: 1647
🏁 Script executed:
# Read lines around 566-573 to see the GitLab section and broader context
sed -n '560,580p' source/collect_plugins.pyRepository: snakemake/snakemake-plugin-catalog
Length of output: 630
🏁 Script executed:
# Find the full get_commit function to understand exception handling
rg -n "def get_commit" source/collect_plugins.py -A 100 | head -150Repository: snakemake/snakemake-plugin-catalog
Length of output: 906
Bound and surface commit metadata failures.
These API requests have no timeout, and get_commit() swallows every exception. One slow or broken repo can stall the catalog build or silently remove commit badges. Treat 404 as the only quiet miss; everything else should be bounded and logged.
🔧 Suggested change
- response = requests.get(api_url)
- if response.status_code == 200:
- data = response.json()
- return {
- "sha": data["sha"][:7],
- "date": data["commit"]["author"]["date"],
- }
+ response = requests.get(api_url, timeout=10)
+ if response.status_code == 404:
+ return None
+ response.raise_for_status()
+ data = response.json()
+ return {
+ "sha": data["sha"][:7],
+ "date": data["commit"]["author"]["date"],
+ }
@@
- response = requests.get(api_url)
- if response.status_code == 200:
- data = response.json()
- return {
- "sha": data["short_id"],
- "date": data["committed_date"],
- }
+ response = requests.get(api_url, timeout=10)
+ if response.status_code == 404:
+ return None
+ response.raise_for_status()
+ data = response.json()
+ return {
+ "sha": data["short_id"],
+ "date": data["committed_date"],
+ }
@@
- except Exception:
- # Continue to next branch on any error
+ except (requests.RequestException, ValueError, KeyError) as e:
+ print(
+ f"Unable to fetch commit info for {repository} ({branch}): {e}",
+ file=sys.stderr,
+ )
continue🧰 Tools
🪛 Ruff (0.15.4)
[error] 526-526: Probable use of requests call without timeout
(S113)
[error] 542-542: Probable use of requests call without timeout
(S113)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@source/collect_plugins.py` around lines 525 - 548, The commit-fetching code
in get_commit() currently makes requests without a timeout and swallows all
exceptions; update both the GitHub and GitLab request blocks to pass a
reasonable timeout (e.g., timeout=5), catch only requests.RequestException (and
related timeout exceptions), and avoid a broad except; treat a 404 response as a
quiet miss (return None), but for any other non-200 status or caught exception
log an error including repository, branch and api_url, and then return None so
the failure is bounded and visible; refer to symbols repository_type, api_url,
owner, repo, branch, and the get_commit() function when applying these changes.
Contributor
There was a problem hiding this comment.
As in #26 Github may rate limit. Is it possible to use the same cloned repo from retrieve_plugin_markdown_files to extract the commit info?
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look at the retrieve_plugin_markdown_files function and surrounding code
rg -n "retrieve_plugin_markdown_files\|def get_docs\|def retrieve\|git clone\|subprocess.*git\|tempdir\|TemporaryDir" source/collect_plugins.py -C 3
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for git-related operations and clone in the file
rg -n "git|clone|TemporaryDirectory|tempdir" source/collect_plugins.py -C 2
echo "---"
# Also check referenced line 343 area
sed -n '330,380p' source/collect_plugins.py
Length of output: 5197
🏁 Script executed:
#!/bin/bash
# Check PR `#26` for retrieve_plugin_markdown_files
gh pr view 26 --repo snakemake/snakemake-plugin-catalog --json title,body,headRefName | jq .
echo "---"
# Look for retrieve_plugin_markdown_files in main branch or any other branch
gh api repos/snakemake/snakemake-plugin-catalog/git/refs | jq '.[].ref'
echo "---"
# Search for the function in the PR `#26` branch
gh pr diff 26 --repo snakemake/snakemake-plugin-catalog | grep -A 30 "retrieve_plugin_markdown\|git clone\|git log\|git rev-parse" | head -80
Length of output: 3255
@Wytamma, great point! PR #26 uses gitpython with a bare clone (git.Repo.clone_from(..., bare=True)) to fetch the docs. That same bare clone already contains the commit history, so yes — the commit SHA and date can be read directly from it with zero extra API calls:
repo.head.commit.hexsha[:7] # short SHA
repo.head.commit.committed_datetime.isoformat() # commit dateThe natural refactor (once PR #25 is rebased on top of PR #26) would be to clone once per plugin and pass the resulting Repo object to both the doc-fetching and commit-info logic:
def _get_plugin_git_info(repo_url: str, branches=["main", "master"]):
"""Clone the plugin repo once (bare) and return docs + commit info."""
import git as gitpython
for branch in branches:
try:
with tempfile.TemporaryDirectory() as tmpdir:
repo = gitpython.Repo.clone_from(repo_url, to_path=tmpdir, bare=True)
# commit metadata — no API call needed
commit = repo.commit(branch)
commit_info = {
"sha": commit.hexsha[:7],
"date": commit.committed_datetime.isoformat(),
}
# docs
def _show(section):
try:
return repo.git.show(f"{branch}:docs/{section}.md")
except gitpython.GitCommandError:
return None
return commit_info, _show("intro"), _show("further")
except gitpython.GitCommandError:
continue
return None, None, NoneThis eliminates the GitHub/GitLab REST API calls for commit metadata entirely and resolves the rate-limiting concern — it would make both my original suggestion (timeout/error handling) and the API approach moot. I'd suggest coordinating with the PR #26 author so both branches can share the clone, or simply rebasing PR #25 on top of PR #26.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
source/_templates/plugin_base.rst.j2 (2)
15-20:⚠️ Potential issue | 🟡 MinorLink the commit badges to the displayed revision.
Lines 17 and 20 still target
repository, so clicking either badge opens the repo root instead of the commit being shown. Sincesource/collect_plugins.py:510-552only providesshaanddatetoday, either add a commit URL there or branch onrepository_typehere before rendering:target:.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/_templates/plugin_base.rst.j2` around lines 15 - 20, The commit badges in plugin_base.rst.j2 currently link to the repository root via :target: {{ repository }} instead of the specific revision; update the template so the badge :target uses a commit-specific URL (e.g., build from commit_info["sha"] or a new commit_url) or branch on repository_type to render the correct commit link; if chosen, add a commit_url field in source/collect_plugins.py (around the code block that produces commit_info in the 510-552 area) so commit_info contains the full URL, and change the template to use that commit_url (or use repository + commit_info["sha"]) for the :target attributes for both badges.
32-35:⚠️ Potential issue | 🟠 MajorURL-encode
snakemake_versionbefore inserting it into the badge path.
source/collect_plugins.py:412-440produces values like>=8.5. Rendering that raw string into the Shields URL breaks the Snakemake badge as soon as compatibility data is present. The author badge already uses the safe pattern.🔧 Suggested change
-.. image:: https://img.shields.io/badge/snakemake-{{ snakemake_version|replace("-", "--") }}-blue?color=%230ea5e9 +.. image:: https://img.shields.io/badge/snakemake-{{ snakemake_version|replace("-", "--")|urlencode }}-blue?color=%230ea5e9🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/_templates/plugin_base.rst.j2` around lines 32 - 35, The Snakemake badge URL currently injects raw snakemake_version (from template variable snakemake_version) which can contain characters like ">=" and breaks Shields URLs; update the template (plugin_base.rst.j2) to URL-encode the snakemake_version before embedding it in the image path (use the Jinja2 urlencode filter on snakemake_version, e.g. apply |urlencode when rendering the badge URL) so values like ">=8.5" become safe for the Shields URL.source/collect_plugins.py (2)
527-545:⚠️ Potential issue | 🟠 MajorBound commit metadata requests and stop swallowing real failures.
Lines 528 and 544 call external APIs without a timeout, and Lines 569-575 catch every exception and silently continue. One slow or rate-limited repository can stall the build or make badges disappear with no signal. Treat
404as the only quiet miss, add a timeout, and log the other request/parsing failures before trying the next branch.🔧 Suggested change
- response = requests.get(api_url) - if response.status_code == 200: - data = response.json() - return { - "sha": data["sha"][:7], - "date": data["commit"]["author"]["date"], - } + response = requests.get(api_url, timeout=10) + if response.status_code == 404: + return None + response.raise_for_status() + data = response.json() + return { + "sha": data["sha"][:7], + "date": data["commit"]["author"]["date"], + } @@ - response = requests.get(api_url) - if response.status_code == 200: - data = response.json() - return { - "sha": data["short_id"], - "date": data["committed_date"], - } + response = requests.get(api_url, timeout=10) + if response.status_code == 404: + return None + response.raise_for_status() + data = response.json() + return { + "sha": data["short_id"], + "date": data["committed_date"], + } @@ - except Exception: - # Continue to next branch on any error + except (requests.RequestException, ValueError, KeyError) as e: + print( + f"Unable to fetch commit info for {repository} ({branch}): {e}", + file=sys.stderr, + ) continueAlso applies to: 568-575
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/collect_plugins.py` around lines 527 - 545, The commit-metadata fetches call requests.get without timeouts and then silently swallow all exceptions; update the GitHub and GitLab request paths (the blocks building api_url and calling requests.get) to pass a reasonable timeout (e.g. timeout=5), check response.status_code and treat 404 as a quiet miss but log any other non-200 status; replace the broad except that currently swallows errors with targeted catches (requests.RequestException for network/timeouts and json.JSONDecodeError or KeyError for parsing) and log the error details (including repository, branch, status code or exception) before continuing so failures are visible instead of silent.
420-438:⚠️ Potential issue | 🟠 MajorCheck full interface-range overlap before returning a Snakemake floor.
_plugin_min_snakemake()still only records the plugin's lower bound and then testsplugin_lower < upper. A constraint like>=8,<8.5can therefore match a Snakemake entry whose interface window starts at8.5, even though the ranges do not overlap. Please preserve the plugin upper bound and compare both sides of the interval before returning a compatibility label.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@source/_templates/plugin_base.rst.j2`:
- Around line 15-20: The commit badges in plugin_base.rst.j2 currently link to
the repository root via :target: {{ repository }} instead of the specific
revision; update the template so the badge :target uses a commit-specific URL
(e.g., build from commit_info["sha"] or a new commit_url) or branch on
repository_type to render the correct commit link; if chosen, add a commit_url
field in source/collect_plugins.py (around the code block that produces
commit_info in the 510-552 area) so commit_info contains the full URL, and
change the template to use that commit_url (or use repository +
commit_info["sha"]) for the :target attributes for both badges.
- Around line 32-35: The Snakemake badge URL currently injects raw
snakemake_version (from template variable snakemake_version) which can contain
characters like ">=" and breaks Shields URLs; update the template
(plugin_base.rst.j2) to URL-encode the snakemake_version before embedding it in
the image path (use the Jinja2 urlencode filter on snakemake_version, e.g. apply
|urlencode when rendering the badge URL) so values like ">=8.5" become safe for
the Shields URL.
In `@source/collect_plugins.py`:
- Around line 527-545: The commit-metadata fetches call requests.get without
timeouts and then silently swallow all exceptions; update the GitHub and GitLab
request paths (the blocks building api_url and calling requests.get) to pass a
reasonable timeout (e.g. timeout=5), check response.status_code and treat 404 as
a quiet miss but log any other non-200 status; replace the broad except that
currently swallows errors with targeted catches (requests.RequestException for
network/timeouts and json.JSONDecodeError or KeyError for parsing) and log the
error details (including repository, branch, status code or exception) before
continuing so failures are visible instead of silent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f7cef4b-5ba0-4197-b4e8-11bccf0d064b
📒 Files selected for processing (7)
source/_static/custom.csssource/_templates/executor_plugin.rst.j2source/_templates/logger_plugin.rst.j2source/_templates/plugin_base.rst.j2source/_templates/report_plugin.rst.j2source/_templates/scheduler_plugin.rst.j2source/collect_plugins.py
🚧 Files skipped from review as they are similar to previous changes (3)
- source/_templates/executor_plugin.rst.j2
- source/_templates/scheduler_plugin.rst.j2
- source/_static/custom.css
Wytamma
self-requested a review
March 10, 2026 09:28
Wytamma
reviewed
Mar 10, 2026
Wytamma
left a comment
Wytamma
left a comment
Contributor
There was a problem hiding this comment.
Looks good! Just need to rebase to main and potentially harden against GitHub api failures.
Comment on lines
+525
to
+548
| api_url = f"https://api.github.com/repos/{owner}/{repo}/commits/{branch}?per_page=1&page=1" | ||
| response = requests.get(api_url) | ||
| if response.status_code == 200: | ||
| data = response.json() | ||
| return { | ||
| "sha": data["sha"][:7], | ||
| "date": data["commit"]["author"]["date"], | ||
| } | ||
|
|
||
| elif repository_type == "gitlab": | ||
| match = re.match(r"https://gitlab\.com/(.+?)/?$", repository) | ||
| if not match: | ||
| return None | ||
| project_path = match.group(1) | ||
|
|
||
| encoded_path = urllib.parse.quote(project_path, safe="") | ||
| api_url = f"https://gitlab.com/api/v4/projects/{encoded_path}/repository/commits/{branch}" | ||
| response = requests.get(api_url) | ||
| if response.status_code == 200: | ||
| data = response.json() | ||
| return { | ||
| "sha": data["short_id"], | ||
| "date": data["committed_date"], | ||
| } |
Contributor
There was a problem hiding this comment.
As in #26 Github may rate limit. Is it possible to use the same cloned repo from retrieve_plugin_markdown_files to extract the commit info?
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pixi.toml (1)
9-9: Run the suite, not one file.
test-unitis pinned tosource/test_collect_plugins.py, so any future unit tests added undersource/will be skipped locally and in CI until this task is updated manually. Point the task at the test directory/package instead.Proposed change
-test-unit = "pytest source/test_collect_plugins.py -v" +test-unit = "pytest source -v"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pixi.toml` at line 9, The test-unit task in pixi.toml is hardcoded to a single file; update the test-unit entry so it runs the whole test package/directory (e.g., change the value of test-unit to run "pytest source -v" or "pytest source/ -v") so all tests under source/ are executed locally and in CI; edit the test-unit key in pixi.toml (replace the current "pytest source/test_collect_plugins.py -v" value) to the directory-based pytest invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/collect_plugins.py`:
- Around line 507-509: Call to _build_snakemake_compat_index() should be wrapped
in a try/except so transient PyPI failures don’t abort catalog generation:
replace the direct call that assigns snakemake_compat_index with a guarded block
that catches exceptions (Exception), logs a warning, and sets
snakemake_compat_index to an empty index/structure (e.g., {}) so plugin
rendering can continue; ensure the variable name snakemake_compat_index is
preserved and any code that uses it can handle the empty fallback.
- Around line 551-560: In _get_plugin_git_info, validate the incoming repo_url
before attempting to clone: parse repo_url (e.g., with urllib.parse) and allow
only trusted schemes/hosts (for example scheme "https" and hosts matching
GitHub/GitLab patterns like "github.com" or "gitlab.com" or your org-specific
domains); also accept common packaging prefixes (e.g., "git+https://") after
normalizing. If the URL doesn't match the allowlist, skip the clone/docs
enrichment and return a safe default PluginGitInfo (or None) so the caller
continues without performing an outbound fetch. Apply this check at the top of
_get_plugin_git_info using the repo_url variable and keep the existing
branch/clone logic unchanged when the URL is allowed.
- Around line 570-576: The nested helper _show currently closes over loop
variables repo and branch which triggers Ruff B023; change _show to bind those
values as defaults (e.g. def _show(section: str, repo=repo, branch=branch) ->
Optional[str]) so each loop iteration captures the correct repo/branch, then use
that _show when building PluginDocs(intro=_show("intro"),
further=_show("further")). Ensure references are to the existing _show, repo,
branch, and PluginDocs symbols.
---
Nitpick comments:
In `@pixi.toml`:
- Line 9: The test-unit task in pixi.toml is hardcoded to a single file; update
the test-unit entry so it runs the whole test package/directory (e.g., change
the value of test-unit to run "pytest source -v" or "pytest source/ -v") so all
tests under source/ are executed locally and in CI; edit the test-unit key in
pixi.toml (replace the current "pytest source/test_collect_plugins.py -v" value)
to the directory-based pytest invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d29f359-9772-43eb-aff1-f998ae7ff1db
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.github/workflows/ci.yml.gitignorepixi.tomlsource/_static/custom.csssource/_templates/executor_plugin.rst.j2source/_templates/logger_plugin.rst.j2source/_templates/plugin_base.rst.j2source/_templates/report_plugin.rst.j2source/_templates/scheduler_plugin.rst.j2source/collect_plugins.pysource/test_collect_plugins.py
🚧 Files skipped from review as they are similar to previous changes (4)
- source/_templates/scheduler_plugin.rst.j2
- source/_static/custom.css
- source/_templates/report_plugin.rst.j2
- source/_templates/plugin_base.rst.j2
Comment on lines
507
to
509
| plugins = defaultdict(list) | ||
| snakemake_compat_index = _build_snakemake_compat_index() | ||
|
|
Contributor
There was a problem hiding this comment.
Don’t let the compatibility badge break the whole build.
_build_snakemake_compat_index() now runs before any plugin pages are rendered, and the initial fetch is not guarded. A transient PyPI failure here aborts catalog generation entirely, even though this metadata can safely degrade to “unknown”. Please fall back to an empty index and keep rendering the pages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@source/collect_plugins.py` around lines 507 - 509, Call to
_build_snakemake_compat_index() should be wrapped in a try/except so transient
PyPI failures don’t abort catalog generation: replace the direct call that
assigns snakemake_compat_index with a guarded block that catches exceptions
(Exception), logs a warning, and sets snakemake_compat_index to an empty
index/structure (e.g., {}) so plugin rendering can continue; ensure the variable
name snakemake_compat_index is preserved and any code that uses it can handle
the empty fallback.
Comment on lines
551
to
560
| def _get_plugin_git_info(repo_url: str, branches: Optional[List[str]] = None) -> PluginGitInfo: | ||
| """Clone the plugin repo once (bare) and return docs + commit info.""" | ||
|
|
||
| if branches is None: | ||
| branches = ["main", "master"] | ||
|
|
||
| def retrieve_plugin_markdown_files(repo_url: str, branches: [str], section: str): | ||
| """ | ||
| fetch the intro.md and further.md doc files provided by plugins | ||
| """ | ||
| docs_path = f"docs/{section}.md" | ||
| for branch in branches: | ||
| try: | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| repo = git.Repo.clone_from(repo_url, to_path=tmpdir, bare=True) |
Contributor
There was a problem hiding this comment.
Validate repository URLs before cloning them.
repo_url comes straight from package metadata and is passed to git clone unchanged. That makes the build perform arbitrary outbound fetches against whatever host/protocol a package advertises. Please gate this to the repository types you intend to trust (at least validated https GitHub/GitLab URLs), and skip commit/docs enrichment for everything else.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@source/collect_plugins.py` around lines 551 - 560, In _get_plugin_git_info,
validate the incoming repo_url before attempting to clone: parse repo_url (e.g.,
with urllib.parse) and allow only trusted schemes/hosts (for example scheme
"https" and hosts matching GitHub/GitLab patterns like "github.com" or
"gitlab.com" or your org-specific domains); also accept common packaging
prefixes (e.g., "git+https://") after normalizing. If the URL doesn't match the
allowlist, skip the clone/docs enrichment and return a safe default
PluginGitInfo (or None) so the caller continues without performing an outbound
fetch. Apply this check at the top of _get_plugin_git_info using the repo_url
variable and keep the existing branch/clone logic unchanged when the URL is
allowed.
Comment on lines
+570
to
+576
| def _show(section: str) -> Optional[str]: | ||
| try: | ||
| return repo.git.show(f"{branch}:docs/{section}.md") | ||
| except git.GitCommandError: | ||
| return None | ||
|
|
||
| docs = PluginDocs(intro=_show("intro"), further=_show("further")) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
funcs = []
for branch in ["main", "master"]:
def show(section):
return branch, section
funcs.append(show)
print(funcs[0]("intro"))
print(funcs[1]("intro"))
PYRepository: snakemake/snakemake-plugin-catalog
Length of output: 117
🏁 Script executed:
# First, check the file size and read the relevant lines
wc -l source/collect_plugins.pyRepository: snakemake/snakemake-plugin-catalog
Length of output: 107
🏁 Script executed:
# Read the code around lines 570-576 with context
sed -n '550,590p' source/collect_plugins.py | cat -nRepository: snakemake/snakemake-plugin-catalog
Length of output: 2004
🏁 Script executed:
# Search for the loop context and _show function
rg -B 20 "def _show" source/collect_plugins.py -A 10Repository: snakemake/snakemake-plugin-catalog
Length of output: 1324
Bind repo and branch into _show so QC passes.
Ruff flags B023 here because _show() closes over loop variables from for branch in branches:. The function is called immediately within the loop iteration, but the closure pattern still triggers linting failures. Add default parameters to bind the loop variables:
Proposed change
- def _show(section: str) -> Optional[str]:
+ def _show(
+ section: str,
+ repo=repo,
+ branch=branch,
+ ) -> Optional[str]:
try:
return repo.git.show(f"{branch}:docs/{section}.md")
except git.GitCommandError:
return None🧰 Tools
🪛 Ruff (0.15.5)
[warning] 572-572: Function definition does not bind loop variable repo
(B023)
[warning] 572-572: Function definition does not bind loop variable branch
(B023)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@source/collect_plugins.py` around lines 570 - 576, The nested helper _show
currently closes over loop variables repo and branch which triggers Ruff B023;
change _show to bind those values as defaults (e.g. def _show(section: str,
repo=repo, branch=branch) -> Optional[str]) so each loop iteration captures the
correct repo/branch, then use that _show when building
PluginDocs(intro=_show("intro"), further=_show("further")). Ensure references
are to the existing _show, repo, branch, and PluginDocs symbols.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
source/collect_plugins.py (2)
527-527:⚠️ Potential issue | 🟠 MajorGuard against transient PyPI failures.
_build_snakemake_compat_index()makes multiple PyPI API calls. A transient failure here will abort the entire catalog build, even though the Snakemake compatibility badge can safely degrade to "unknown".🛡️ Proposed fix to add error handling
plugins = defaultdict(list) - snakemake_compat_index = _build_snakemake_compat_index() + try: + snakemake_compat_index = _build_snakemake_compat_index() + except Exception as e: + print(f"Warning: Failed to build Snakemake compatibility index: {e}", file=sys.stderr) + snakemake_compat_index = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/collect_plugins.py` at line 527, Wrap the call to _build_snakemake_compat_index() in a try/except to guard against transient PyPI failures: call _build_snakemake_compat_index() inside a try, catch broad exceptions (Exception), log a warning that the Snakemake compatibility index failed to build (including the exception message) and set snakemake_compat_index to a safe degraded value (e.g. None or an empty dict) so the catalog build continues and the compatibility badge can be rendered as "unknown"; do not re-raise the exception. Ensure you update the same scope where snakemake_compat_index is used so downstream code handles the degraded value.
484-493:⚠️ Potential issue | 🟠 MajorExtract both lower and upper bounds from plugin interface specifiers.
The current logic only extracts
plugin_lowerbut ignoresplugin_upper. This can cause false negatives when a plugin specifies a range like>=2.0,<3.0that overlaps with a Snakemake requirement of>=2.5,<3.0. The check at line 505 (plugin_lower < snakemake_lower) would skip this as incompatible, even though the ranges overlap at[2.5, 3.0).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/collect_plugins.py` around lines 484 - 493, The code only sets plugin_lower from the parsed specifiers; update the loop that examines SpecifierSet(match.group(2)) to also compute plugin_upper: for each specifier s, if s.operator in (">=", ">") convert s.version to a Version and set plugin_lower = max(existing plugin_lower, v) (as already done), and if s.operator in ("<=", "<") convert s.version to Version and set plugin_upper = min(existing plugin_upper, v) (initialize plugin_upper to None and choose min when set); keep using _INTERFACE_PKG_RE, requires_dist and plugin_iface to locate the block, and after extracting both bounds adjust the later compatibility check (which currently compares plugin_lower with snakemake_lower) to test for interval overlap using plugin_lower/plugin_upper against snakemake_lower/snakemake_upper instead of only comparing lowers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@source/collect_plugins.py`:
- Line 527: Wrap the call to _build_snakemake_compat_index() in a try/except to
guard against transient PyPI failures: call _build_snakemake_compat_index()
inside a try, catch broad exceptions (Exception), log a warning that the
Snakemake compatibility index failed to build (including the exception message)
and set snakemake_compat_index to a safe degraded value (e.g. None or an empty
dict) so the catalog build continues and the compatibility badge can be rendered
as "unknown"; do not re-raise the exception. Ensure you update the same scope
where snakemake_compat_index is used so downstream code handles the degraded
value.
- Around line 484-493: The code only sets plugin_lower from the parsed
specifiers; update the loop that examines SpecifierSet(match.group(2)) to also
compute plugin_upper: for each specifier s, if s.operator in (">=", ">") convert
s.version to a Version and set plugin_lower = max(existing plugin_lower, v) (as
already done), and if s.operator in ("<=", "<") convert s.version to Version and
set plugin_upper = min(existing plugin_upper, v) (initialize plugin_upper to
None and choose min when set); keep using _INTERFACE_PKG_RE, requires_dist and
plugin_iface to locate the block, and after extracting both bounds adjust the
later compatibility check (which currently compares plugin_lower with
snakemake_lower) to test for interval overlap using plugin_lower/plugin_upper
against snakemake_lower/snakemake_upper instead of only comparing lowers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 488219f0-b709-4177-8396-e10975238b5b
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.github/workflows/ci.yml.gitignorepixi.tomlsource/_static/custom.csssource/_templates/executor_plugin.rst.j2source/_templates/logger_plugin.rst.j2source/_templates/plugin_base.rst.j2source/_templates/report_plugin.rst.j2source/_templates/scheduler_plugin.rst.j2source/collect_plugins.pysource/test_collect_plugins.py
🚧 Files skipped from review as they are similar to previous changes (6)
- source/_templates/plugin_base.rst.j2
- source/test_collect_plugins.py
- source/_static/custom.css
- source/_templates/executor_plugin.rst.j2
- .gitignore
- source/_templates/logger_plugin.rst.j2
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
source/collect_plugins.py (1)
610-613: Clone once, then probe branches locally.
clone_from()sits inside the branch loop, so repositories onmasterare fetched twice: once for the failedmainprobe and again formaster. Moving the clone outside the loop keeps this path closer to the intended single-clone design and trims network work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/collect_plugins.py` around lines 610 - 613, The clone is happening inside the for branch in branches loop (git.Repo.clone_from(repo_url, to_path=tmpdir, bare=True)), causing duplicate network work; move the TemporaryDirectory() and git.Repo.clone_from(repo_url, to_path=tmpdir, bare=True) outside the loop so you clone once into tmpdir/repo and then iterate branches to probe them locally (e.g., use the cloned repo object to check refs/fetch or checkout each branch), reusing repo and tmpdir variables rather than recloning per branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/collect_plugins.py`:
- Around line 260-282: The code currently treats any empty docs returned by
_get_plugin_git_info() as "no documentation found" and emits docs_warning, which
mislabels fetch/clone/default-branch failures as missing docs; update
_get_plugin_git_info to return an explicit error/status (e.g., None on fatal
error or a result object with an "error" flag/message) instead of silently
falling back to empty docs, and then in the caller (the block that assigns
git_info, docs_intro, docs_further and builds docs_warning) only emit the "No
documentation found" warning when git_info indicates a successful repo read with
genuinely empty docs (no intro and no further) — if git_info signals an error,
log or surface that git error separately (using commit_info/docs_* only when
git_info is successful) and do not produce the plugin-facing missing-docs
warning.
- Around line 382-400: The _commit_url function currently appends commit paths
directly to the repository string which can produce invalid URLs when repository
is a clone URL (e.g., ends with ".git" or has trailing slashes); modify
_commit_url to first normalize the repository by stripping a trailing ".git"
(case-insensitive) and any trailing slashes, then append "/commit/{commit_sha}"
for GitHub or "/-/commit/{commit_sha}" for GitLab; keep the same function
signature and behavior otherwise so other callers of _commit_url continue to
work.
---
Nitpick comments:
In `@source/collect_plugins.py`:
- Around line 610-613: The clone is happening inside the for branch in branches
loop (git.Repo.clone_from(repo_url, to_path=tmpdir, bare=True)), causing
duplicate network work; move the TemporaryDirectory() and
git.Repo.clone_from(repo_url, to_path=tmpdir, bare=True) outside the loop so you
clone once into tmpdir/repo and then iterate branches to probe them locally
(e.g., use the cloned repo object to check refs/fetch or checkout each branch),
reusing repo and tmpdir variables rather than recloning per branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a8749af-2be3-4e9a-bd37-385a72ec4bcc
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.github/workflows/ci.yml.gitignorepixi.tomlsource/_static/custom.csssource/_templates/executor_plugin.rst.j2source/_templates/logger_plugin.rst.j2source/_templates/plugin_base.rst.j2source/_templates/report_plugin.rst.j2source/_templates/scheduler_plugin.rst.j2source/collect_plugins.pysource/test_collect_plugins.py
🚧 Files skipped from review as they are similar to previous changes (7)
- source/_static/custom.css
- .gitignore
- source/_templates/logger_plugin.rst.j2
- source/_templates/plugin_base.rst.j2
- pixi.toml
- source/test_collect_plugins.py
- source/_templates/report_plugin.rst.j2
we display 3 additional badges: - last commit in git repo, with link. as a fallback if repo is not github/gitlab - last updated for the git repo (color-coded for freshness) - lower snakemake version that depends on the same version of the interface that the plugin depends on
johanneskoester
approved these changes
Mar 13, 2026
johanneskoester
requested changes
Mar 13, 2026
johanneskoester
approved these changes
Mar 13, 2026
btraven00
moved this from In review
to Approved+ready for merging
in Snakemake Hackathon 2026
github-project-automation
Bot
moved this from Approved+ready for merging
to Done
in Snakemake Hackathon 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was naively assuming that the major version of the plugin interface matches snakemake major version that is supported. johannes confirmed that in fact both
snakemakeand the plugin depend on a version of the interface.we can use the
pixiresolution, but that'll give us the upper bound for snakemake. It'd be nice to also have the lower bound.Summary by CodeRabbit
New Features
Documentation
Tests
Chores