Skip to content

Conversation

sansyrox
Copy link
Member

@sansyrox sansyrox commented Sep 12, 2025

Description

This PR fixes #

Summary

This PR does....

PR Checklist

Please ensure that:

  • The PR contains a descriptive title
  • The PR contains a descriptive summary of the changes
  • You build and test your changes before submitting a PR.
  • You have added relevant documentation
  • You have added relevant tests. We prefer integration tests wherever possible

Pre-Commit Instructions:

High-level PR Summary

This PR adds Pydantic support to the Robyn web framework, introducing a more modern, type-safe parameter parsing system. The implementation includes new parameter types (Query, Path, Header, Form) that enable automatic type conversion, validation, and better developer experience. The PR adds the necessary integration with Pydantic models for request body validation, comprehensive documentation in ADVANCED_PARAMS.md, example applications, and integration tests. The router has been modified to try advanced parameter parsing first before falling back to the original Robyn parameter handling logic, ensuring backward compatibility.

⏱️ Estimated Review Time: 1h 30m

💡 Review Order Suggestion
Order File Path
1 robyn/advanced_params.py
2 robyn/router.py
3 robyn/__init__.py
4 ADVANCED_PARAMS.md
5 examples/advanced_params_example.py
6 integration_tests/test_advanced_params.py
7 quick_start_example.py
8 pyproject.toml

Review by RecurseML

🔍 Review performed on 7bbfa0a..b15cabb

  Severity     Location     Issue     Delete  
Low quick_start_example.py:126 Redundant comment that restates the obvious
Low quick_start_example.py:130 Redundant comment that restates the obvious
✅ Files analyzed, no issues (5)

robyn/advanced_params.py
integration_tests/test_advanced_params.py
examples/advanced_params_example.py
robyn/__init__.py
robyn/router.py

Analyze latest changes

Need help? Join our Discord

Copy link

vercel bot commented Sep 12, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
robyn Error Error Sep 12, 2025 11:05pm

Copy link

codspeed-hq bot commented Sep 12, 2025

CodSpeed Performance Report

Merging #1247 will not alter performance

Comparing feat/pydantic-support (b15cabb) with main (a77063a)1

Summary

✅ 150 untouched

Footnotes

  1. No successful run was found on main (ffb7ebc) during the generation of this report, so a77063a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

# Mixed parameters
@app.put("/users/:user_id")
def update_user(
user: User, # Body
Copy link

Choose a reason for hiding this comment

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

This comment is redundant as it merely restates what is already obvious from the code. The parameter type User and its position as the first parameter in a route handler already indicate that it's a request body. According to the effective comments rule, comments should not restate what can be understood from the code itself.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.

Suggested change
user: User, # Body
user: User,

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

@app.put("/users/:user_id")
def update_user(
user: User, # Body
user_id: int = Path(...), # Path
Copy link

Choose a reason for hiding this comment

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

This comment is redundant as it merely restates what is already obvious from the code. The use of Path(...) already clearly indicates this is a path parameter. According to the effective comments rule, comments should not restate what can be understood from the code itself.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.

Suggested change
user_id: int = Path(...), # Path
user_id: int = Path(...),

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

def update_user(
user: User, # Body
user_id: int = Path(...), # Path
notify: bool = Query(True), # Query
Copy link

Choose a reason for hiding this comment

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

This comment is redundant as it merely restates what is already obvious from the code. The use of Query(True) already clearly indicates this is a query parameter. According to the effective comments rule, comments should not restate what can be understood from the code itself.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.

Suggested change
notify: bool = Query(True), # Query
notify: bool = Query(True),

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

user: User, # Body
user_id: int = Path(...), # Path
notify: bool = Query(True), # Query
x_source: Optional[str] = Header(None, alias="X-Source") # Header
Copy link

Choose a reason for hiding this comment

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

This comment is redundant as it merely restates what is already obvious from the code. The use of Header(None) already clearly indicates this is a header parameter. According to the effective comments rule, comments should not restate what can be understood from the code itself.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

@app.put("/items/:item_id")
def update_item(
user_data: CreateUserRequest, # Request body (JSON)
item_id: int = Path(..., description="Item ID"), # Path parameter
Copy link

Choose a reason for hiding this comment

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

This comment is redundant as it merely restates that this is a path parameter, which is already evident from the code using Path(...). The type annotation and description already make it clear this is a path parameter without needing an additional comment.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.

Suggested change
item_id: int = Path(..., description="Item ID"), # Path parameter
item_id: int = Path(..., description="Item ID"),

React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

notify: bool = Query(True, description="Send notifications"), # Query parameter
priority: int = Query(1, description="Priority level"), # Query parameter
x_source: Optional[str] = Header(None, alias="X-Source"), # Header parameter
x_trace: Optional[str] = Header(None, alias="X-Trace-ID"), # Header parameter
Copy link

Choose a reason for hiding this comment

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

This comment is redundant as it only states that this is a header parameter, which is already evident from the code using Header(). The type annotation and Header usage make this comment unnecessary.

📚 Relevant Docs

🔍 This comment matches your effective_comments.mdc rule.


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

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.

1 participant