feat: split request params #952

VishnuSanal · 2024-08-26T06:27:42Z

Description

This PR fixes #850

Summary

This PR does split request params

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:

Ensure that you have run the pre-commit hooks on your PR.

vercel · 2024-08-26T06:27:46Z

@VishnuSanal is attempting to deploy a commit to the sparckles Team on Vercel.

A member of the Team first needs to authorize it.

for more information, see https://pre-commit.ci

This reverts commit 7429479

This reverts commit 15728ed.

…ions__'

VishnuSanal · 2024-08-28T10:11:34Z

@sansyrox, the tests pass here. can you PTAL at the interface + an initial review, if you will. :)

codspeed-hq · 2024-08-28T10:15:11Z

CodSpeed Performance Report

Merging #952 will not alter performance

_{Comparing VishnuSanal:split-request-params (acf76ca) with main (028f161)}

Summary

✅ 116 untouched benchmarks

🆕 29 new benchmarks

Benchmarks breakdown

	Benchmark	`main`	`VishnuSanal:split-request-params`	Change
🆕	`test_openapi_query_params`	N/A	6.4 ms	N/A
🆕	`test_split_request_params_get_body[split_request_typed-async]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_body[split_request_typed-sync]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_body[split_request_untyped-async]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_body[split_request_untyped-sync]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_combined[split_request_typed-async]`	N/A	5.1 ms	N/A
🆕	`test_split_request_params_get_combined[split_request_typed-sync]`	N/A	5.1 ms	N/A
🆕	`test_split_request_params_get_combined[split_request_untyped-async]`	N/A	5.1 ms	N/A
🆕	`test_split_request_params_get_combined[split_request_untyped-sync]`	N/A	5.1 ms	N/A
🆕	`test_split_request_params_get_combined_failure[async]`	N/A	5.1 ms	N/A
🆕	`test_split_request_params_get_combined_failure[sync]`	N/A	5.1 ms	N/A
🆕	`test_split_request_params_get_headers[split_request_typed-async]`	N/A	4.8 ms	N/A
🆕	`test_split_request_params_get_headers[split_request_typed-sync]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_headers[split_request_untyped-async]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_headers[split_request_untyped-sync]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_method[split_request_typed-async]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_method[split_request_typed-sync]`	N/A	4.8 ms	N/A
🆕	`test_split_request_params_get_method[split_request_untyped-async]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_method[split_request_untyped-sync]`	N/A	4.9 ms	N/A
🆕	`test_split_request_params_get_path_params[split_request_typed-async]`	N/A	4.9 ms	N/A
...	...	...	...	...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

sansyrox · 2024-08-28T10:40:58Z

Hey @VishnuSanal 👋

I really like the current implementation. Great job ✨ But I have a few follow ups. I will share them in a few hours

robyn/router.py

VishnuSanal · 2024-08-31T16:44:53Z

create custom type aliases & check them in the funciton signature

VishnuSanal · 2024-09-30T14:29:12Z

@sansyrox this is the swagger page for the following robyn app.

from typing import Optional

from robyn import Robyn
from robyn.types import Body, QueryParams, JSONResponse

app = Robyn(__file__)


class CreateItemBody(Body):
    name: Optional[str]
    description: str
    price: float
    tax: float


class CreateItemQueryParams(QueryParams):
    _id: int
    desc: Optional[str]


class CreateItemResponse(JSONResponse):
    body: CreateItemBody
    qp: CreateItemQueryParams


@app.post("/upload/file")
def create_item(body_item: CreateItemBody, query: CreateItemQueryParams) -> int:
    print(body_item, query)
    return 200


app.start()

integration_tests/base_routes.py

robyn/robyn.pyi

robyn/types.py

src/types/identity.rs

src/types/mod.rs

src/types/multimap.rs

unit_tests/test_request_object.py

VishnuSanal · 2024-10-02T18:10:01Z

@sansyrox PTAL :)

integration_tests/base_routes.py

sansyrox · 2024-10-06T12:59:52Z

robyn/robyn.pyi

@@ -89,7 +89,7 @@ class Url:
 class Identity:
    claims: dict[str, str]

-class QueryParams:
+class RustQueryParams:


This should be just QueryParams. No?

robyn/types.py

sansyrox · 2024-10-06T13:02:21Z

unit_tests/test_request_object.py

@@ -8,7 +8,7 @@ def test_request_object():
        path="/user",
    )
    request = Request(
-        query_params=QueryParams(),
+        query_params=RustQueryParams(),


No, this should be QueryParams. No part of the client facing code should require us to initialise RustQueryParams

sansyrox · 2024-10-06T13:02:37Z

robyn/types.py

+    """
+    A type alias for openapi response bodies
+    """
+
+    pass
+
+
+class Body:
+    """
+    A type alias for openapi request bodies
+    """
+
+    pass
+
+
+class QueryParams:
+    """
+    A type alias for query params type annotations
+    """
+


the docstrings still need work

sansyrox · 2024-10-06T13:03:55Z

robyn/types.py

+class QueryParams:
+    """
+    A type alias for query params type annotations
+    """
+


also you should implement the method definitions here. Associated with rust query params.

src/types/mod.rs

src/types/identity.rs

sansyrox · 2024-10-06T13:06:48Z

Hey @VishnuSanal , some more work is needed here. Also , could you have a look at the conflicts?

kliwongan and others added 2 commits July 13, 2024 20:51

Initial implementation

f53c1f5

chore: remove jsonify from base routes

1140f23

VishnuSanal and others added 10 commits August 26, 2024 11:58

Merge branch 'main' into split-request-params

8905384

[pre-commit.ci] auto fixes from pre-commit.com hooks

dec4ae2

for more information, see https://pre-commit.ci

[wip] pass request object along with the response body

15728ed

[wip] pass request object along with the response body

7429479

[wip] use only filtered_params in wrappedhandler

ee13c9c

[wip] use only filtered_params in wrappedhandler

4b5798b

Revert "[wip] pass request object along with the response body"

20dfe59

This reverts commit 7429479

Revert "[wip] pass request object along with the response body"

eba10a0

This reverts commit 15728ed.

update tests

8f575d1

fix: AttributeError: type object '_empty' has no attribute '__annotat…

dc93316

…ions__'

VishnuSanal changed the title ~~feat: [WIP] split request params~~ feat: split request params Aug 28, 2024

VishnuSanal marked this pull request as ready for review August 28, 2024 10:11

sansyrox reviewed Aug 28, 2024

View reviewed changes

robyn/router.py Outdated Show resolved Hide resolved

VishnuSanal added 3 commits August 29, 2024 15:33

add req as rquest

4a88712

map params based off of types

6c0a104

fix(ci): types for v3.8 _aarghhh..._

d3f39b0

VishnuSanal added 3 commits September 1, 2024 12:05

alias types

ac74906

raise syntax error for unexpected args

271ed0c

update tests

c03dcd6

VishnuSanal requested a review from sansyrox September 2, 2024 05:27

VishnuSanal added 3 commits September 2, 2024 10:58

update tests

138e9ae

fix(ci): dict -> Dict

d876825

fix tests

8fa6ea7

fix tests

7145dd3

VishnuSanal requested a review from sansyrox September 30, 2024 14:29