vitalik / django-ninja

💨 Fast, Async-ready, Openapi, type hints based framework for building APIs
https://django-ninja.dev
MIT License
7.26k stars 431 forks source link

UUID params for non-Ninja endpoints get converted to string #280

Open srcreigh opened 3 years ago

srcreigh commented 3 years ago

Hello, thanks for your work on Ninja.

Today I debugged an issue where a DRF endpoint which calls int(self.kwargs['a_uuid_path_param']) started crashing. The cause is we added a new part of our API using ninja, when ninja gets imported code in ninja/signature/utils.py executes, which changes global path param settings so that uuids are parsed as strings, not UUIDs. Introduced here, https://github.com/vitalik/django-ninja/pull/187

I wonder if it's a goal for you that teams can incrementally migrate to Ninja from DRF with minimal code changes? This would be something to address if so, maybe for v1 as it may be a backwards incompatible change

I wonder if it makes sense to disable the string-UUIDs behaviour in non-Ninja routes - although that would be a breaking change so possibly in v1. or if there could be a "Adding Ninja to your existing DRF project" checklist in the docs somewhere which includes a note about the change in UUID parsing.

Thanks again 😁

vitalik commented 3 years ago

Hi @srcreigh

Not sure I understand this

do you have some code sample that I can use to reproduce the issue ?

srcreigh commented 3 years ago

No code at hand, let me try to explain again.

The issue is that importing ninja changes the behaviour of non-Ninja endpoints in the same app.

Imagine I have a 8 year old django app with 100 routes using DRF such as ListCreateApiView, some of them have UUID path parameters. Such as /tasks/<uuid:id>

I want to add 5 new routes using Ninja, however when I import ninja, the other endpoints break.. why? .. now my DRF handlers are receiving strings instead of UUIDs, and things like int(id) crash when they used to work.

The cause is this module-import-level config change which ninja executes:

https://github.com/stephenrauch/django-ninja/blob/master/ninja/signature/utils.py#L69-L81

Setting up a reproduce sample code would involve creating a django app with django-rest-framework with a uuid param in the route, and observing that when ninja is imported, it changes the type of the param received by the DRF endpoint

I would expect ninja to have no effect on other unrelated endpoints in the application, but due to the call to register_converter within ninja code, it does change the data other endpoints receive

vitalik commented 3 years ago

ok, got it

SmileyChris commented 2 years ago

I'm not sure why this converter is even a thing. Pydantic works fine with the UUID type, and if you're dealing with UUIDs then why would you not want to use the real type?