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

django-mongoengine basic compatibility #200

Closed
last-partizan opened this issue Nov 14, 2020 · 4 comments
Closed

django-mongoengine basic compatibility #200

last-partizan opened this issue Nov 14, 2020 · 4 comments

Comments

@last-partizan
Copy link

Hi, i'm trying to use drf-spectacular with django-mongoengine, and this line of code is getting in the way:

assert isinstance(model_field, models.Field)

I must write ugly monkey-patching to get around of this.

Could you please change this check, to something like looks_like(model_field)?

Something like hasattr(model_field, "get_internal_type") would do, if this check is really required. But, for me it works fine without it. Maybe even just delete that line?

I can write PR if you suggest changes. Or maybe you can suggest less ugly way to fool isinstance.

@tfranzel
Copy link
Owner

hi @last-partizan,

yes that's an ugly hack for sure. 😄 so initially the purpose of this line was to make sure the assumptions hold and there is no crazy usage with hard to understand stacktraces. at the time i was not aware of alternate model implementations. yes i think this could be changed with little impact. the question i ask myself is whether this one line will be sufficient to support MongoEngine.

I'm not sure if you noticed the MongoEngine PR we currently have: #181 . It is still missing tests, for which i provided a sample in the comments. Though, I have not heard back from the PR author @ngocngoan.

According to this PR it looks like other things need to (slightly) change too. I see you are the maintainer of MongoEngine, so i guess you have a lot more insight into what would be required for good support on our side. What do you think?

@last-partizan
Copy link
Author

@tfranzel no, i didn't noticed that PR. Looks nice.

But, that's for raw mongoengine and djangorestframework-mongoengine and i don't familiar with it enough.

I am maintaining django-mongoengine, and it is slightly different beast, it tries to imitate django behaviours on top of mongoengine, so it can work with forms, views and django admin.

I'm using raw Serializers, with custom save/update methods, so for my case, just changing that line would be enough. And adding some methods like get_internal_type for fields in django-mongoengine, which i already done.

@last-partizan
Copy link
Author

No need for any changes, i subclassed AutoSchema instead of my ugly hack.

MongoEngine/django-mongoengine@2ef9529

@tfranzel
Copy link
Owner

i'm glad the PR pointed you in the right direction. if you have trouble down the line, we can certainly can come back and do the change.

last-partizan added a commit to last-partizan/drf-spectacular that referenced this issue Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants