search

litestar-org / litestar

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

Refactor: Move optional dependencies to extras #185

Closed peterschutt closed 2 years ago

peterschutt commented 2 years ago

Starlite has several potentially optional dependencies: pydantic-factories for generating examples in OpenAPI schema, pyyaml for /openapi.yaml path and—of course—requests for testing. If we decide that requests is optional, we should consider to make pydantic-factories and pyyaml not required as well.

Originally posted by @vrslev in https://github.com/starlite-api/starlite/issues/174#issuecomment-1166215699

peterschutt commented 2 years ago

We could also add the template engine requirements to the list also - not that we include them already, but might be a good convenience pip install starlite[jinja2]

Goldziher commented 2 years ago

Sounds good. @vrslev would you like to take care of this one?

vrslev commented 2 years ago

I’m quite busy right now, sorry

peterschutt commented 2 years ago

No worries @vrslev - I just clicked on your comment in the PR and turned it into an issue so it wouldn't get lost. No need to get involved if you don't want to.

Bobronium commented 2 years ago

I can probably work on this too.

peterschutt commented 2 years ago

The fact that OpenAPIConfig.create_examples defaults False makes it a good choice for an optional extras. Its not a feature I knew I was missing and I get maximum recursion error on app startup if I turn it on.

schema/openapi.yaml works out of the box, so not so sure about pyyaml.

Template dependencies would be easy to have there as extras.

Maybe a 2.0 idea would be to have the base dependencies as slim as possible, and then have openapi-schema-pydantic, pydantic-factories and pyyaml tucked away in an openapi extras. In that case we could still have the default config there, but leave it up to the user to invoke it if they want, or supply their own.

Goldziher commented 2 years ago

@peterschutt -- recursion error? this is probably a bug. Can you create an issue for this so we track it?

peterschutt commented 2 years ago

@peterschutt -- recursion error? this is probably a bug. Can you create an issue for this so we track it?

OK, I've done that. I'm just picking my battles at the moment and didn't want to create the issue until I'd had a chance to try to figure it out. See #196

Goldziher commented 2 years ago

I dont think this issue is required. pydantic-factories is not an extra dependency since we use it in code in several places, and pyyaml shouldnt be an optional dep as well. Im closing this for now.