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

Users should be able to configure API renderer_classes #6066

Closed
narun4sk opened this issue May 23, 2020 · 9 comments · Fixed by #9902
Closed

Users should be able to configure API renderer_classes #6066

narun4sk opened this issue May 23, 2020 · 9 comments · Fixed by #9902

Comments

@narun4sk
Copy link

narun4sk commented May 23, 2020

Issue Summary

Users should be able to configure API renderer_classes, currently these are hardcoded to [JSONRenderer, BrowsableAPIRenderer]:

renderer_classes = [JSONRenderer, BrowsableAPIRenderer]

Steps to Reproduce

Configure pages endpoint as usual, but do not include rest_framework in the INSTALLED_APPS. Try to open configured endpoint in the browser, the app will throw an exception:

Internal Server Error: /rest/p/
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py", line 145, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py", line 143, in _get_response
    response = response.render()
  File "/usr/local/lib/python3.8/site-packages/django/template/response.py", line 105, in render
    self.content = self.rendered_content
  File "/usr/local/lib/python3.8/site-packages/rest_framework/response.py", line 70, in rendered_content
    ret = renderer.render(self.data, accepted_media_type, context)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/renderers.py", line 723, in render
    template = loader.get_template(self.template)
  File "/usr/local/lib/python3.8/site-packages/django/template/loader.py", line 19, in get_template
    raise TemplateDoesNotExist(template_name, chain=chain)
django.template.exceptions.TemplateDoesNotExist: rest_framework/api.html

Technical details

  • Python Version: 3.8.2
  • Django Version: 3.0
  • Wagtail Version: 2.9

Currently I'm using this nasty trick:

from django.conf import settings
from wagtail.api.v2.views import PagesAPIViewSet
from wagtail.api.v2.router import WagtailAPIRouter

if not settings.DEBUG:
    from rest_framework.renderers import JSONRenderer

    class ProdPagesAPIViewSet(PagesAPIViewSet):
        renderer_classes = [JSONRenderer]

    PagesAPIViewSet = ProdPagesAPIViewSet

# Create the router. "wagtailapi" is the URL namespace
api_router = WagtailAPIRouter("wagtailapi")

api_router.register_endpoint("p", PagesAPIViewSet)
@marteinn
Copy link
Contributor

Thanks for filing an issue @narun4sk! How would you envision the api for setting “rendered_classes”? I think creating a subclass and overriding “rendered_classes” is the way you would do it in DRF as well.

@phptek
Copy link

phptek commented May 24, 2020

I've been looking to do something very similar myself, without copy-pasting all the Wagtail page-specific renderer logic into a subclass. Tried with JSON+HAL last week using https://github.com/seebass/drf-hal-json/ but just couldn't get it to work.

I'm going to try with JSON:API next, but imagine I will encounter the same underlying modification problems (See: https://github.com/django-json-api/django-rest-framework-json-api). Maybe some docs that describe how developers might go about modifying the Wagtail-specific JSON format? That would help heaps :-)

@narun4sk
Copy link
Author

@marteinn in DRF we have this luxury in settings like:

REST_FRAMEWORK = {
    "DEFAULT_RENDERER_CLASSES": [
        "rest_framework.renderers.JSONRenderer",
    ],
}

IMO it's not too difficult to implement the same idea, isn't it? 🤔

Something like:

WAGTAIL_API = {
    "DEFAULT_RENDERER_CLASSES": [
        "rest_framework.renderers.JSONRenderer",
        "rest_framework.renderers.BrowsableAPIRenderer",
    ],
}

Then if I don't like the defaults I would just set:

WAGTAIL_API["DEFAULT_RENDERER_CLASSES"] = ["rest_framework.renderers.JSONRenderer"]

@marteinn
Copy link
Contributor

Many thanks @narun4sk and @phptek. This is a design decision so I will raise the issue with the core team, will report back here when we have discussed.

@m-aciek
Copy link

m-aciek commented Apr 28, 2022

Hi, any updates on that?

It forces us to enable DRF browsable API, otherwise there's an error raised, see https://stackoverflow.com/questions/61961099/wagtail-api-v2-django-template-exceptions-templatedoesnotexist.

It's somewhat contrary to documentation which states

Optionally, you may also want to add rest_framework to INSTALLED_APPS. This would make the API browsable when viewed from a web browser but is not required for basic JSON-formatted output.

https://docs.wagtail.org/en/stable/advanced_topics/api/v2/configuration.html#enable-the-app

@mdfathi99
Copy link

mdfathi99 commented Sep 14, 2022

This helped me. Here is the simple code in api.py

from rest_framework.renderers import JSONRenderer

...

class ProdPagesAPIViewSet(PagesAPIViewSet):
    renderer_classes = [JSONRenderer]
    name = "pages"

api_router.register_endpoint("pages", ProdPagesAPIViewSet)

@bcdickinson
Copy link
Contributor

I've just hit this issue. I think as @m-aciek says, there's a documentation bug here. No code would need to change in Wagtail if we just documented that you need to subclass the various ViewSet subclasses to set your desired renderer classes, particularly if you want to remove the browsable API renderer.

@zerolab
Copy link
Contributor

zerolab commented Dec 15, 2022

I agree with this. Marked the issue as such. PRs welcome ;)

@salty-ivy
Copy link
Member

https://docs.wagtail.org/en/stable/advanced_topics/api/v2/configuration.html#
already states that we can subclass these APIViewSet classes
image

although it, doesn't show how so i would just add a code snippet to show that

lb- pushed a commit to salty-ivy/wagtail that referenced this issue Jan 19, 2023
- Update docs/advanced_topics/api/v2/configuration.md
- Fixes wagtail#6066
- Fix up spelling of customise (UK English not US)
@lb- lb- closed this as completed in #9902 Jan 19, 2023
lb- pushed a commit that referenced this issue Jan 19, 2023
- Update docs/advanced_topics/api/v2/configuration.md
- Fixes #6066
- Fix up spelling of customise (UK English not US)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants