Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
srcreigh opened this issue Nov 9, 2021 · 4 comments
Open

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

srcreigh opened this issue Nov 9, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@srcreigh
Copy link
Contributor

srcreigh commented Nov 9, 2021

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, #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
Copy link
Owner

vitalik commented Nov 10, 2021

Hi @srcreigh

Not sure I understand this

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

@srcreigh
Copy link
Contributor Author

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 vitalik added the bug Something isn't working label Nov 11, 2021
@vitalik
Copy link
Owner

vitalik commented Nov 11, 2021

ok, got it

@SmileyChris
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants