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

add django rest framework mongoengine support #181

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add django rest framework mongoengine support #181

wants to merge 1 commit into from

Conversation

ngocngoan
Copy link

  • Add class MongoEngineAutoSchema, which inherits from class AutoSchema and is specified at the entry DEFAULT_SCHEMA_CLASS in the configuration
REST_FRAMEWORK = {
    ...
    "DEFAULT_SCHEMA_CLASS": "drf_spectacular.openapi.MongoEngineAutoSchema",
    ...
}
  • Add new function get_mongoengine_extended_doc_excludes that compatible with mongoengine, specify this function in the config
SPECTACULAR_SETTINGS = {
    "GET_LIB_DOC_EXCLUDES": 'drf_spectacular.plumbing.get_mongoengine_extended_doc_excludes',
}

@tfranzel
Copy link
Owner

thanks for the PR. that is awesome! couple of things before i can really review it.

  • this breaks everyone not using mongoengine, which is a no-go. code must always work for regular DRF users.
  • all this functionality is contrib and therefore goes into contrib/django_rest_framework_mongoengine.py
  • no added deps into main.txt, these are optionals and please only a minimal set. it would install mongoengine for every spectacular user.
  • unit tests missing. please refer to other contrib packages for how the tests work here. it does not have to be fancy but something that at least tests basic functionality.

@ngocngoan
Copy link
Author

ngocngoan commented Oct 28, 2020

I will consider your terms. When the code is completed, I will notify you.

@codecov
Copy link

codecov bot commented Oct 28, 2020

Codecov Report

Attention: Patch coverage is 0% with 29 lines in your changes missing coverage. Please review.

Project coverage is 96.84%. Comparing base (1a49ddf) to head (bb3c6f2).
Report is 842 commits behind head on master.

Files Patch % Lines
...cular/contrib/django_rest_framework_mongoengine.py 0.00% 29 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #181      +/-   ##
==========================================
- Coverage   97.60%   96.84%   -0.77%     
==========================================
  Files          49       50       +1     
  Lines        3672     3703      +31     
==========================================
+ Hits         3584     3586       +2     
- Misses         88      117      +29     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@tfranzel tfranzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ngocngoan thanks! that looks so much better!

in order to merge this, we need to test the functionality. here is the test script i created when you first opened the issue, in order to check if this is even remotely working:
https://gist.github.com/tfranzel/c11382ea8e3801b701de0f4ef73fe0d5

feel free to adapt. basic functionality should be included.

django-filter>=2.3.0
pymongo>=3.11.0
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pymongo is not required as it is a dependency of mongoengine

mongomock will likely also come in handy for testing

@tfranzel tfranzel added the help wanted Extra attention is needed label Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants