litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.34k stars 363 forks source link

Enhancement: Allow parameter of type `Iterable` to be processed as `list` #3631

Open rafalkrupinski opened 1 month ago

rafalkrupinski commented 1 month ago

Description

Litestar doesn't think it should process Iterable parameter as an array.

The original discussion was about default parameter value, but the problem is actually with how litestar handles Iterable itself

URL to code causing the issue

https://github.com/litestar-org/litestar/blob/8c4c15bb501879dabaecfbf0af541ac571c08cf3/litestar/_kwargs/parameter_definition.py#L67C9-L67C60

MCVE

@get('/sequence')
async def sequence(foo: Sequence[str]) -> Sequence[str]:
    return foo

@get('/iterable')
async def iterable(foo: Iterable[str]) -> Iterable[str]:
    return foo

app = Litestar(
    route_handlers=(
        sequence,
        iterable,
    )
)

Steps to reproduce

$ curl  'localhost:8000/sequence?foo=s1&foo=s2'
["s1","s2"]

$ curl 'localhost:8000/iterable?foo=s1&foo=s2'
s2

Screenshots

No response

Logs

No response

Litestar Version

2.9.1

Platform


[!NOTE]
While we are open for sponsoring on GitHub Sponsors and OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.

Fund with Polar

provinzkraut commented 1 month ago

Other than a theoretical, can you explain what use case this allows that's currently not possible?

Litestar will only ever provide parameters as concrete types, i.e. will always pass a list[str], never a Sequence[str], so I'm not sure what the benefit of allowing such annotations is, aside from catering to individual style preferences.

I'm asking because it is very much intentional to be restrictive about the typing, so as long as there's no functionality to be gained here, I'm wary of accepting such a feature request.

rafalkrupinski commented 1 month ago

Currently Set, Sequence and possibly other types are accepted as parameter types, so it's unintuitive for me as a user, that litestar doesn'tt accept Iterable. The change is for ergonomics.

For example you don't want to do this. Even if it worked with litestar, it's a mutable default parameter value.

param: list[str] = []

This works but has different semantics and generates different OpenAPI schema, and you need to handle None value manually, and you need another variable or assert to please mypy.

param: list[str]|None=None

you can do

param: Sequence[str]=()

but currently this fails

param: Iterable[str]=()

It's unintuitive that one abstract type works and another doesn't.

provinzkraut commented 1 month ago

Thanks for the explanation @rafalkrupinski, that seems reasonable!