Conversation
Allow users to configure polling interval per component via TaskConfig(poll_interval=timedelta(...)). Default is 5 seconds.
|
Warning Rate limit exceeded@jlowin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe change adds a per-component Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/fastmcp/server/tasks/subscriptions.py (2)
32-32: Default value inconsistency with TaskConfig.The default
poll_interval_ms=1000(1 second) differs fromTaskConfig.poll_intervalwhich defaults to 5 seconds (5000ms). Whilehandlers.pyalways passes an explicit value, aligning the fallback defaults would improve consistency and reduce confusion if these functions are ever called without the parameter.🔎 Proposed fix to align defaults
async def subscribe_to_task_updates( task_id: str, task_key: str, session: ServerSession, docket: Docket, - poll_interval_ms: int = 1000, + poll_interval_ms: int = 5000, ) -> None:
86-86: Same default inconsistency in helper functions.Both
_send_status_notificationand_send_progress_notificationalso default to 1000ms. If the suggestion above is adopted, update these as well for consistency.Also applies to: 158-158
docs/servers/tasks.mdx (1)
110-116: Code examples return immediately without async work.The examples show
quick_taskandslow_taskbut both return immediately without anyawaitor async operations. While syntactically valid, this doesn't demonstrate a realistic use case where poll intervals matter. Consider adding a brief async operation to make the examples more illustrative of when different poll intervals are useful.🔎 Proposed enhancement
# Poll every 2 seconds for a fast-completing task @mcp.tool(task=TaskConfig(mode="optional", poll_interval=timedelta(seconds=2))) async def quick_task() -> str: + await asyncio.sleep(5) # Completes in ~5 seconds return "Done quickly" # Poll every 30 seconds for a long-running task @mcp.tool(task=TaskConfig(mode="optional", poll_interval=timedelta(seconds=30))) async def slow_task() -> str: + await asyncio.sleep(300) # Completes in ~5 minutes return "Eventually done"This would also require adding
import asyncioat the top of the code block.src/fastmcp/server/tasks/protocol.py (2)
87-91: Consider handling malformed poll_interval data from Redis.If the Redis value contains non-numeric data (e.g., corruption or manual tampering),
int(poll_interval_bytes.decode("utf-8"))will raise aValueError. Since this is user-facing, a defensive fallback would prevent unexpected errors.🔎 Proposed defensive handling
- poll_interval_ms = ( - int(poll_interval_bytes.decode("utf-8")) - if poll_interval_bytes - else 5000 # Default to 5 seconds - ) + try: + poll_interval_ms = ( + int(poll_interval_bytes.decode("utf-8")) + if poll_interval_bytes + else 5000 # Default to 5 seconds + ) + except (ValueError, UnicodeDecodeError): + poll_interval_ms = 5000 # Default on parse error
367-371: Same defensive handling recommendation for cancel handler.Apply the same try/except pattern here to handle potential malformed data in Redis.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/server/tasks/test_task_config.pyis excluded by none and included by none
📒 Files selected for processing (5)
docs/servers/tasks.mdx(1 hunks)src/fastmcp/server/tasks/config.py(2 hunks)src/fastmcp/server/tasks/handlers.py(3 hunks)src/fastmcp/server/tasks/protocol.py(4 hunks)src/fastmcp/server/tasks/subscriptions.py(10 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bareexcept- be specific with exception types
Files:
src/fastmcp/server/tasks/protocol.pysrc/fastmcp/server/tasks/config.pysrc/fastmcp/server/tasks/subscriptions.pysrc/fastmcp/server/tasks/handlers.py
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/tasks.mdx
🧬 Code graph analysis (1)
src/fastmcp/server/tasks/protocol.py (2)
src/fastmcp/server/context.py (1)
session_id(360-409)src/fastmcp/server/server.py (1)
docket(382-387)
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (3)
src/fastmcp/server/tasks/config.py (1)
50-50: LGTM - Clean addition of poll_interval configuration.The new
poll_intervalfield is well-typed with a sensible default. Consider whether validation for non-positive values is needed (e.g.,timedelta(seconds=0)or negative) to prevent clients from being told to poll immediately in a tight loop, though this may be an edge case not worth guarding against if callers are trusted.src/fastmcp/server/tasks/handlers.py (2)
87-91: LGTM - Clean conversion and storage of poll_interval.The conversion from
timedeltato milliseconds viatotal_seconds() * 1000is correct, and the Redis storage uses consistent key naming and TTL with other task metadata. The value flows correctly through to both the subscription system and the CreateTaskResult response.
126-133: LGTM - Correct propagation to subscription system.The poll_interval_ms is correctly passed to
subscribe_to_task_updates, ensuring notifications include the configured interval.
TaskConfig now accepts a
poll_intervalparameter to control how frequently clients should poll for task status updates. This was previously hardcoded to 1 second across all components.Default is 5 seconds. The value is stored in Redis alongside other task metadata and used consistently across task creation, status queries, and notifications.