Added RFC 6570 complaint form style query expansion as optional param… #427
Conversation
There was a problem hiding this comment.
Thank you for working on this!
This PR makes good progress toward RFC 6570 support by adding form-style query expansion.
There are a few suggestions:
- Type Conversion for Query Parameters. Query parameters from URLs are always strings, but the code doesn't convert them to match function parameter
types.
Example of the problem:
@server.resource("resource://items/{category}{?limit}")
def get_items(category: str, limit: int = 10) -> str:
# When called with resource://items/books?limit=20
# limit will be string "20" not int 20
return f"Items in {category}, limit: {limit} (type: {type(limit)})"
Suggested fix: Add type conversion based on function annotations in
- Handle Edge Cases in Query Parsing. The current implementation might not handle edge cases properly:
- Empty query values (?param=)
- URL-encoded values (?name=John%20Doe)
-
Add Tests for Type Conversion Scenarios
-
Consider Documenting Query Parameter Behavior
|
@ihrpr thanks for the feedback during the fn call with params result = self.fn(**params) can i use call_fn_with_arg_validation for type validation and conversion and should i fallback to default values for optional parameters if validation fails for any one of them |
|
@ihrpr the pydantic validate_call already takes care of the compatible type conversion based on annontated parameters for query parameters and raises validation error for incompatible types added a decorator which check if the validation error is raised by optional parameters and ignores them so they fall back to default values to keep it compliant added relevant tests covering these cases for the cases in query parsing the empty values and url encoded parameters are handled by parse_qs added relevant tests covering these |
Add RFC 6570 support for optional parameters as form-style query expansions
Motivation and Context
Adds optional parameter support with RFC 6570 compliance as form-style query expansions. This change addresses issue #378, .
Breaking Changes
None
Types of changes
Checklist