Skip to content

Conversation

ASLeonard
Copy link
Contributor

@ASLeonard ASLeonard commented Aug 15, 2025

Came across this while --touching many files, but it seems having something like max-jobs-per-timespan: "100/1m" in a profile will also limit the rate for files getting touched (which is a very different operation to actually submitting jobs).

Previously you would get many messages for

Job rate limit reached, waiting for free slots.

Whereas now you should only encounter that message if you are not in dryrun or touch mode.

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

Summary by CodeRabbit

  • Bug Fixes
    • Touch mode now bypasses job submission rate limiting, enabling faster execution when only updating timestamps.
    • Users with a configured submission cap will no longer experience artificial delays in touch runs; normal runs continue to honor rate limits, and dry-run behavior is unchanged.
    • Scheduling order and resource checks remain the same; only the rate limiting is skipped in touch mode, improving responsiveness without altering other execution semantics.

Copy link
Contributor

coderabbitai bot commented Aug 15, 2025

📝 Walkthrough

Walkthrough

Adjusted JobRateLimiter initialization in JobScheduler.init: the limiter is now created only when not in dry-run, not in touch mode, and max_jobs_per_timespan is set. This disables rate limiting during touch mode. Scheduling logic remains unchanged aside from bypassing rate limiting when no limiter is present.

Changes

Cohort / File(s) Summary
Scheduler rate limiting initialization
src/snakemake/scheduler.py
Instantiate JobRateLimiter only if not dry-run, not touch mode, and max_jobs_per_timespan configured; otherwise set to None, resulting in no rate limiting in touch mode. No public signatures changed.

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant Scheduler
  participant RateLimiter

  User->>Scheduler: Initialize JobScheduler
  alt not dry-run AND not touch AND max_jobs_per_timespan
    Scheduler->>RateLimiter: Create limiter
  else
    Scheduler->>Scheduler: limiter = None
  end

  User->>Scheduler: Schedule jobs
  alt limiter exists
    Scheduler->>RateLimiter: Check/Enforce rate limit
    RateLimiter-->>Scheduler: Allowed/Delayed
  else
    Scheduler->>Scheduler: Proceed without rate limiting
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

Suggested reviewers

  • johanneskoester

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ea7bdb2 and 8ef0cc3.

📒 Files selected for processing (1)
  • src/snakemake/scheduler.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

⚙️ CodeRabbit Configuration File

**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.

Files:

  • src/snakemake/scheduler.py
🧬 Code Graph Analysis (1)
src/snakemake/scheduler.py (2)
src/snakemake/workflow.py (2)
  • dryrun (400-404)
  • touch (407-412)
src/snakemake/io/__init__.py (2)
  • touch (779-801)
  • touch (1281-1282)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (56)
  • GitHub Check: tests (10, ubuntu-latest, py312)
  • GitHub Check: tests (9, windows-latest, py313)
  • GitHub Check: tests (9, windows-latest, py312)
  • GitHub Check: tests (10, ubuntu-latest, py311)
  • GitHub Check: tests (10, ubuntu-latest, py313)
  • GitHub Check: tests (10, windows-latest, py313)
  • GitHub Check: tests (10, windows-latest, py312)
  • GitHub Check: tests (9, ubuntu-latest, py311)
  • GitHub Check: tests (9, ubuntu-latest, py312)
  • GitHub Check: tests (9, ubuntu-latest, py313)
  • GitHub Check: tests (8, windows-latest, py313)
  • GitHub Check: tests (6, ubuntu-latest, py312)
  • GitHub Check: tests (8, windows-latest, py312)
  • GitHub Check: tests (8, ubuntu-latest, py313)
  • GitHub Check: tests (7, ubuntu-latest, py311)
  • GitHub Check: tests (6, windows-latest, py312)
  • GitHub Check: tests (7, windows-latest, py312)
  • GitHub Check: tests (7, windows-latest, py313)
  • GitHub Check: tests (6, windows-latest, py313)
  • GitHub Check: tests (8, ubuntu-latest, py312)
  • GitHub Check: tests (8, ubuntu-latest, py311)
  • GitHub Check: tests (7, ubuntu-latest, py313)
  • GitHub Check: tests (7, ubuntu-latest, py312)
  • GitHub Check: tests (5, ubuntu-latest, py313)
  • GitHub Check: tests (6, ubuntu-latest, py313)
  • GitHub Check: tests (5, windows-latest, py313)
  • GitHub Check: tests (6, ubuntu-latest, py311)
  • GitHub Check: tests (5, ubuntu-latest, py312)
  • GitHub Check: tests (5, macos-latest, py313)
  • GitHub Check: tests (5, windows-latest, py312)
  • GitHub Check: tests (5, ubuntu-latest, py311)
  • GitHub Check: tests (4, macos-latest, py313)
  • GitHub Check: tests (4, windows-latest, py312)
  • GitHub Check: tests (4, ubuntu-latest, py312)
  • GitHub Check: tests (4, windows-latest, py313)
  • GitHub Check: tests (4, ubuntu-latest, py311)
  • GitHub Check: tests (4, ubuntu-latest, py313)
  • GitHub Check: tests (3, windows-latest, py313)
  • GitHub Check: tests (3, ubuntu-latest, py312)
  • GitHub Check: tests (3, windows-latest, py312)
  • GitHub Check: tests (2, ubuntu-latest, py311)
  • GitHub Check: tests (3, ubuntu-latest, py313)
  • GitHub Check: tests (3, ubuntu-latest, py311)
  • GitHub Check: tests (2, macos-latest, py313)
  • GitHub Check: tests (2, windows-latest, py313)
  • GitHub Check: tests (2, windows-latest, py312)
  • GitHub Check: tests (2, ubuntu-latest, py312)
  • GitHub Check: tests (2, ubuntu-latest, py313)
  • GitHub Check: tests (1, macos-latest, py313)
  • GitHub Check: tests (1, windows-latest, py313)
  • GitHub Check: tests (1, ubuntu-latest, py313)
  • GitHub Check: tests (1, ubuntu-latest, py311)
  • GitHub Check: tests (1, macos-latest, py312)
  • GitHub Check: tests (1, windows-latest, py312)
  • GitHub Check: tests (1, ubuntu-latest, py312)
  • GitHub Check: apidocs
🔇 Additional comments (1)
src/snakemake/scheduler.py (1)

77-82: Disable rate limiting in touch/dry-run — verified, tests missing coverage

JobRateLimiter is only instantiated when not (dryrun or touch) and when max_jobs_per_timespan is set; scheduler.job_selector correctly bypasses rate limiting when the limiter is None. However, I could not find tests that combine max-jobs-per-timespan with dry-run or touch.

  • Key locations:
    • src/snakemake/scheduler.py:77-82 — job_rate_limiter initialization
    • src/snakemake/scheduler.py:500-540 — job_selector (bypasses limiter when None; uses get_free_jobs/register_jobs otherwise)
    • src/snakemake/scheduler.py:861+ — JobRateLimiter class (get_free_jobs / register_jobs / timespan)
    • tests/tests.py:1711-1712 — test_max_jobs_per_timespan exists, but no tests combining this with dry-run or touch

Relevant snippet (unchanged):

        self.job_rate_limiter = (
            JobRateLimiter(self.workflow.scheduling_settings.max_jobs_per_timespan)
            if not (self.dryrun or self.touch)
            and self.workflow.scheduling_settings.max_jobs_per_timespan
            else None
        )

Please add tests that run with --max-jobs-per-timespan together with --dry-run and --touch and assert that "Job rate limit reached, waiting for free slots." is not emitted in those modes.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@ASLeonard ASLeonard changed the title Don't rate limit jobs when touching fix: don't rate limit jobs when touching Aug 15, 2025
@johanneskoester johanneskoester merged commit 9c499e5 into snakemake:main Aug 18, 2025
80 of 81 checks passed
johanneskoester pushed a commit that referenced this pull request Aug 29, 2025
🤖 I have created a release *beep* *boop*
---


##
[9.10.0](v9.9.0...v9.10.0)
(2025-08-19)


### Features

* migrate to scheduler plugin interface and scheduler plugins
([#3676](#3676))
([26fcd38](26fcd38))


### Bug Fixes

* don't rate limit jobs when touching
([#3699](#3699))
([9c499e5](9c499e5))
* raise an error when different rules produce identical (temp) output
([#3667](#3667))
([f627176](f627176))
* silence messages on module load
([#3688](#3688))
([b13e9c8](b13e9c8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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