Skip to content

Conversation

@gwct
Copy link

@gwct gwct commented Oct 29, 2025

Summary by CodeRabbit

  • New Features

    • Automatic partition selection for Cannon cluster at Harvard
    • Enhanced resource validation and management
    • New testing documentation and workflows
  • Bug Fixes

    • Logo verification in release announcements
  • Documentation

    • Comprehensive Cannon cluster integration guide
    • Installation instructions via PyPI and Bioconda
    • Profile configuration and resource specification guide
  • Chores

    • CI/CD automation for release management
    • GitHub Actions workflows for upstream sync checks
    • Project rebranding to Cannon executor

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This PR rebrands the Snakemake executor plugin from generic SLURM to Cannon-specific infrastructure at Harvard University. Changes include project metadata updates, new automation workflows for upstream release tracking and version management, enhanced resource validation logic, comprehensive documentation rewrite, and expanded test coverage with Cannon-specific profiles.

Changes

Cohort / File(s) Summary
GitHub Workflows & Automation
.github/workflows/check-upstream-release.yml, .github/workflows/conventional-prs.yml, .github/workflows/pypi-release.yml, .github/workflows/announce-release.yml.disabled, .github/workflows/bump-and-tag.yml.disabled, .github/workflows/check-upstream-release-and-sync.yml.disabled, .github/workflows/release.yml.disabled
New workflows for upstream release detection, notification, version bumping, and PyPI publishing. Conventional PR workflow enhanced with semantic PR types validation.
GitHub Configuration
.github/ISSUE_TEMPLATE/config.yml, .github/pr-types.yml, .github/releases.yml
Issue template, PR type definitions, and release notes generation configuration enabling automated changelog.
Release Automation Scripts
.github/scripts/check_upstream_release.py, .github/scripts/merge_test.sh
Python script to detect upstream releases and Bash script for 3-way merge testing against upstream.
Core Executor Implementation
snakemake_executor_plugin_cannon/__init__.py, snakemake_executor_plugin_cannon/cannon.py, snakemake_executor_plugin_cannon/validation.py, snakemake_executor_plugin_cannon/submit_string.py, snakemake_executor_plugin_cannon/utils.py
Major refactoring: new Cannon resource management module, SLURM extra validation, partition selection logic, MPI validation, and memory/GPU parsing utilities.
Efficiency Reporting
snakemake_executor_plugin_cannon/efficiency_report.py
Enhanced time parsing to support SLURM sacct formats with day components and fractional seconds.
Documentation
README.md, docs/intro.md, docs/further.md, docs/profile.md, docs/tests.md
Complete rebranding to Cannon; added installation, profile setup, resource specification, and testing guidance; removed Gitpod references.
Test Suite & Configuration
tests/tests.py, tests/Snakefile, tests/cannon-test-profile/config.yaml
New test workflow, Cannon-specific profile with resource presets, and expanded test coverage for time parsing and SLURM extra validation.
Project Metadata
pyproject.toml, CHANGELOG.md, .gitignore
Renamed package to snakemake-executor-plugin-cannon, bumped version to 1.9.2.post0, updated authors/URLs to Harvard Informatics, added test output ignore patterns.

Sequence Diagram(s)

sequenceDiagram
    participant Scheduler
    participant Executor as Cannon Executor<br/>(__init__.py)
    participant CANNON as CANNON<br/>(cannon.py)
    participant Validation as Validation<br/>(validation.py)
    participant Job
    
    Scheduler->>Executor: Submit job with resources
    
    rect rgb(220, 240, 250)
    Note over Executor,CANNON: Resource Normalization
    Executor->>CANNON: normalize_mem(job)
    CANNON-->>Executor: mem_mb (normalized)
    Executor->>CANNON: parse_num_gpus(job)
    CANNON-->>Executor: gpu_count
    end
    
    rect rgb(240, 250, 220)
    Note over Executor,CANNON: Partition Selection
    Executor->>CANNON: get_cannon_partitions()
    CANNON-->>Executor: partition_specs
    alt GPU requested
        Executor->>Executor: Select GPU partition
    else Based on resources
        Executor->>Executor: Select by mem/cpu/runtime
    end
    end
    
    rect rgb(250, 220, 220)
    Note over Executor,Validation: Validation
    Executor->>Validation: validate_slurm_extra(job)
    Validation-->>Executor: Valid or WorkflowError
    Executor->>CANNON: check_resources(partition)
    CANNON-->>Executor: OK or WorkflowError
    end
    
    Executor-->>Scheduler: Submit string with partition
Loading
sequenceDiagram
    participant Upstream as Upstream<br/>snakemake-executor-plugin-slurm
    participant Schedule as GitHub<br/>Scheduler
    participant CheckJob as check-release<br/>Job
    participant Notify as notify<br/>Job
    participant Fork as Fork<br/>Repository
    
    Schedule->>CheckJob: Trigger (daily / manual)
    
    rect rgb(220, 240, 250)
    Note over CheckJob,Upstream: Check Phase
    CheckJob->>Upstream: Fetch latest release (API)
    Upstream-->>CheckJob: Release version
    CheckJob->>CheckJob: Compare with local version
    CheckJob-->>Schedule: Exit 42 (sync-needed=true)<br/>or Exit 0
    end
    
    alt sync-needed = true
        Schedule->>Notify: Run notify job
        rect rgb(240, 250, 220)
        Note over Notify,Fork: Notification Phase
        Notify->>Fork: Check for existing sync PR
        Fork-->>Notify: PR exists? or not found
        alt No existing PR
            Notify->>Fork: Create issue with sync instructions
        else PR exists
            Notify->>Notify: Skip (commented-out)
        end
        end
    else sync-needed = false
        Note over Schedule: No action needed
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

  • Partition selection logic (get_partition_arg) contains multiple conditional branches with complex resource heuristics requiring careful validation against Cannon specifications.
  • New cannon.py module introduces table-driven resource management with resource validation, memory/GPU parsing, and error handling across multiple functions.
  • Validation module (validation.py) requires review of regex patterns and forbidden SLURM options coverage.
  • Workflow automation (check-upstream-release, bump-and-tag workflows) includes conditional job triggers and version computation logic that needs verification against intended release process.
  • Time parsing refactor (efficiency_report.py) replaces simple logic with multi-format strptime loop; verify all sacct formats are correctly handled.
  • Broad scope spanning executor logic, scripts, workflows, documentation, and tests increases review surface area.

Areas requiring extra attention:

  • Partition selection heuristics in __init__.py get_partition_arg() method—verify decision tree matches Cannon cluster topology
  • Resource validation rules in cannon.py check_resources() against actual Cannon partition limits
  • SLURM extra forbidden options regex patterns completeness in validation.py
  • MPI task validation logic in submit_string.py for mutual exclusivity and bounds checking
  • Time format parsing exhaustiveness in efficiency_report.py against real sacct output samples

Possibly related PRs

Suggested reviewers

  • cmeesters
  • johanneskoester

Poem

🐰 A rabbit's verse on Cannon dreams,
From SLURM to Harvard's gleaming scheme,
Resources flow through paths so bright,
Partitions chosen just just right—
With workflows humming, tests run true,
The plugin dreams... now Cannon too!

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c980d1f and e480357.

⛔ Files ignored due to path filters (1)
  • tests/rulegraph.png is excluded by !**/*.png
📒 Files selected for processing (29)
  • .github/ISSUE_TEMPLATE/config.yml (1 hunks)
  • .github/pr-types.yml (1 hunks)
  • .github/releases.yml (1 hunks)
  • .github/scripts/check_upstream_release.py (1 hunks)
  • .github/scripts/merge_test.sh (1 hunks)
  • .github/workflows/announce-release.yml.disabled (1 hunks)
  • .github/workflows/bump-and-tag.yml.disabled (1 hunks)
  • .github/workflows/check-upstream-release-and-sync.yml.disabled (1 hunks)
  • .github/workflows/check-upstream-release.yml (1 hunks)
  • .github/workflows/conventional-prs.yml (2 hunks)
  • .github/workflows/pypi-release.yml (1 hunks)
  • .github/workflows/release.yml.disabled (1 hunks)
  • .gitignore (1 hunks)
  • CHANGELOG.md (1 hunks)
  • README.md (1 hunks)
  • docs/further.md (1 hunks)
  • docs/intro.md (1 hunks)
  • docs/profile.md (1 hunks)
  • docs/tests.md (1 hunks)
  • pyproject.toml (1 hunks)
  • snakemake_executor_plugin_cannon/__init__.py (7 hunks)
  • snakemake_executor_plugin_cannon/cannon.py (1 hunks)
  • snakemake_executor_plugin_cannon/efficiency_report.py (1 hunks)
  • snakemake_executor_plugin_cannon/submit_string.py (2 hunks)
  • snakemake_executor_plugin_cannon/utils.py (1 hunks)
  • snakemake_executor_plugin_cannon/validation.py (1 hunks)
  • tests/Snakefile (1 hunks)
  • tests/cannon-test-profile/config.yaml (1 hunks)
  • tests/tests.py (4 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gwct gwct changed the title Update 1.9.2 feat: Update 1.9.2 Oct 29, 2025
@gwct gwct closed this Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants