Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7k
Schemas & client libraries.#4179
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
rest_framework/routers.py Outdated
| renderer_classes=view_renderers | ||
| defget(self,request,*args,**kwargs): | ||
| ifrequest.accepted_renderer.format=='corejson': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is there any reason why this couldn't be brought into theCoreJSONRenderer?
lovelydinosaurJun 15, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The problem is less what to do withCoreJSONRenderer and more about making sure that we preserve the behavior of that view for anything thatisn't a schema renderer.
(Also I'd consider that part still slightly in flux - clearly needs refinement)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
making sure that we preserve the behavior of that view for anything that isn't a schema renderer
Right, my main concern with that line (which I'm sure will probably change another few times before this is merged) is that it's hard-codingcorejson as the only renderer with schema support.
Also I'd consider that part still slightly in flux - clearly needs refinement
Definitely understandable.
| returnencode_multipart(self.BOUNDARY,data) | ||
| classCoreJSONRenderer(BaseRenderer): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There was a move in 3.1 to remove features which had third-party dependencies from the core and put them into third-party packages that were still highly visible. Seedjango-rest-framework-xml,django-rest-framework-yaml, anddjango-rest-framework-oauth for examples.
Is that still something we are pushing for?
lovelydinosaurJun 15, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Kinda. I seecoreapi as a foundational thing here, so it's a bit different.
The various types of schema and docs that you can use it to generate will be third party, yup.
So eg we can have third party packages for schema formats: Swagger / API Blueprint / JSON Hyperschema, for docs templates driven by a coreapi.Document object, and for various hypermedia styles.
Usingcoreapi ensures that we're able to provide a common interface for all of those, so I don't have any great issues with pulling it in. If it's in core, then we're making the promise that it's an interface that is available to third-party devs, which should help drive folks making schema renderers / hypermedia renderers and docs renderers. Having said that, wecould push for it to be third-party, if we wanted to, so I might be open to discussion.
docs/api-guide/schemas.md Outdated
| router = DefaultRouter(schema_title='Server Monitoring API') | ||
| The schema will be included in by the root URL, `/`, and presented to clients |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"included in by" seems confusing. Maybe take out the "by"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Good catch, that's a typo.
lovelydinosaur commentedJul 4, 2016
Remaining issues have now been split out into individual tickets. (Ensure it will be released prior to DjangoCon US, and give at least one clear working day to resolve any ciritical issues, post-release) |
marcgibbons commentedJul 4, 2016
💯 |
lovelydinosaur commentedJul 4, 2016
Yeah. Indeed. |
| returntemplate_render(template,context) | ||
| defget_fields(self,view): | ||
| filter_class=getattr(view,'filter_class',None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Wouldn't it make more sense to useself.get_filter_class here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Possibly, tho we don't actually have aqueryset/model available to us at this point, so couldn't dynamically generate the filter class if none was set explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What are the problems with just callingview.get_queryset()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sure, that'd probably work okay.
DRF 3.4.7 is the same version run by edx/credentials. Release notes are athttp://www.django-rest-framework.org/topics/release-notes/#34x-series. Highlights include: support for Django 1.10 (encode/django-rest-framework#4158 - sigh), support for schema generation (encode/django-rest-framework#4179 - required for newer versions of django-rest-swagger), and a change that prevents paginated views from re-running queries when count queries return 0 (encode/django-rest-framework#4201 - this explains the expected query count changes made in tests).LEARNER-1590
DRF 3.4.7 is the same version run by edx/credentials. Release notes are athttp://www.django-rest-framework.org/topics/release-notes/#34x-series.Highlights include: - [Support for Django 1.10](encode/django-rest-framework#4158) - sigh - [Support for schema generation](encode/django-rest-framework#4179) - required for newer versions of django-rest-swagger - A [change](encode/django-rest-framework#4201) that prevents paginated views from re-running queries when count queries return 0 - this explains the expected query count changes made in testsLEARNER-1590
DRF 3.4.7 is the same version run by edx/credentials. Release notes are athttp://www.django-rest-framework.org/topics/release-notes/#34x-series. Highlights include:1. [Support for Django 1.10](encode/django-rest-framework#4158) - sigh2. [Support for schema generation](encode/django-rest-framework#4179) - required for newer versions of django-rest-swagger3. A change that [prevents paginated views from re-running queries](encode/django-rest-framework#4201) when count queries return 0 - this explains the expected query count changes made in testsLEARNER-1590
DRF 3.4.7 is the same version run by edx/credentials. Release notes are athttp://www.django-rest-framework.org/topics/release-notes/#34x-series. Highlights include:1. [Support for Django 1.10](encode/django-rest-framework#4158) - sigh2. [Support for schema generation](encode/django-rest-framework#4179) - required for newer versions of django-rest-swagger3. A change that [prevents paginated views from re-running queries](encode/django-rest-framework#4201) when count queries return 0 - this explains the expected query count changes made in testsLEARNER-1590
swehba commentedNov 19, 2018
@tomchristie FYI, the links in your original comment at the top of this thread are broken. |
Uh oh!
There was an error while loading.Please reload this page.
Support for schema generation & dynamic client libraries.
See thenew tutorial section,schema documentation, andapi client documentation.
Needed to complete tutorial 7:
--auth basicsupport to Core API.Other work as part of this, prior to 3.4.0 release.
Document,ErrorandLinkin JSON renderer?ListSerializerand other non-form.Moved to separate tickets:
SchemaGeneratorcoreapiinto separate packages. (HTML, OpenAPI, HyperSchema, HAL...)Deferred:
Handle binary downloads.Not strictly required for 3.4UseNot strictly required for 3.4idnotpkin schema representations or change serializer representations?Add to quickstartNot strictly required for 3.4