tfranzel / drf-spectacular

Sane and flexible OpenAPI 3 schema generation for Django REST framework.
https://drf-spectacular.readthedocs.io
BSD 3-Clause "New" or "Revised" License
2.38k stars 264 forks source link

KeyError: 'trace' #244

Closed ndrbrt closed 3 years ago

ndrbrt commented 3 years ago

My environment:

I got the following error:

$ poetry run ./mysite/manage.py spectacular --file schema.yml

Traceback (most recent call last):
  File "./mysite/manage.py", line 15, in <module>
    execute_from_command_line(sys.argv)
  File "<path-to-my-venv>/lib/python3.8/site-packages/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "<path-to-my-venv>/lib/python3.8/site-packages/django/core/management/__init__.py", line 395, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "<path-to-my-venv>/lib/python3.8/site-packages/django/core/management/base.py", line 330, in run_from_argv
    self.execute(*args, **cmd_options)
  File "<path-to-my-venv>/lib/python3.8/site-packages/django/core/management/base.py", line 371, in execute
    output = self.handle(*args, **options)
  File "<path-to-my-venv>/lib/python3.8/site-packages/drf_spectacular/management/commands/spectacular.py", line 50, in handle
    schema = generator.get_schema(request=None, public=True)
  File "<path-to-my-venv>/lib/python3.8/site-packages/drf_spectacular/generators.py", line 188, in get_schema
    paths=self.parse(request, public),
  File "<path-to-my-venv>/lib/python3.8/site-packages/drf_spectacular/generators.py", line 165, in parse
    operation = view.schema.get_operation(path, path_regex, method, self.registry)
  File "<path-to-my-venv>/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 55, in get_operation
    operation['operationId'] = self.get_operation_id()
  File "<path-to-my-venv>/lib/python3.8/site-packages/drf_spectacular/openapi.py", line 273, in get_operation_id
    action = self.method_mapping[self.method.lower()]
KeyError: 'trace'
tfranzel commented 3 years ago

hi @ndrbrt, i'm a bit surprised how you got there. http verbs TRACE and OPTIONS are not explicitly supported. a quick test i did just ignored the def trace() on a viewset. can you comment on how the view is constructed?

ndrbrt commented 3 years ago

Ok, I kinda figured out the problem. I have some urls which map to a view called not_found, which explicitly returns a 404 (I had to disable some urls from a package the brute force way).

Now, that view looks like this:

from rest_framework.decorators import api_view
from django.http import Http404

from .constants import HTTP_METHODS

@api_view(HTTP_METHODS)
def not_found(request, *args, **kwargs):
    raise Http404

And those HTTP_METHODS constant is:

"""
HTTP Methods list, as reported on
https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods
"""
HTTP_METHODS = [
    'GET',
    'HEAD',
    'POST',
    'PUT',
    'DELETE',
    'CONNECT',
    'OPTIONS',
    'TRACE',
    'PATCH'
]

As I expected, after commenting the unsupported methods, it starts working.

In the end, I should probably solve this by just rewrite that view as a class, but maybe this is an edge case that's worth covering?

Thank you in advance.

tfranzel commented 3 years ago

even though quite uncommon we should not run into these kind of issues but rather just ignore those endpoints.

that fix should do it.

tfranzel commented 3 years ago

can you check this works with new release 0.13.0?

ndrbrt commented 3 years ago

I just tested it out and can confirm it's now working with 0.13.0. Thank you!

mdellweg commented 2 years ago

According to https://swagger.io/specification/#path-item-object, all the http verbs are supported in the OpenAPI schema. Personally, i would really like to see the HEAD verb there.

tfranzel commented 2 years ago

OPTIONS: not available as view method as it is handled like e.g. permission_classes (DEFAULT_METADATA_CLASS/SimpleMetadata). This is so fringe that I won't invest time here due to the very limited benefit.

TRACE: rejected by upstream https://github.com/encode/django-rest-framework/issues/4320#issuecomment-235856970

CONNECT: not sure this even applies to DRF.

HEAD: The only extra HTTP method that might make sense.

I came up with a test but since HEAD handling is a little bit different from the rest, this is not a straightforward as it might sound. I will try something out, but I'm only able to invest a limited amount of time here.

mdellweg commented 2 years ago

Right: CONNECT is probably only interesting to proxies.

What i am missing is the head method generated by openapi-generator, and i am wondering if the api spec would need to advertise that.

tfranzel commented 2 years ago

well that is another good but orthogonal question but I would guess it needs to be included in the schema. some targets to openapi-generator might generate it in any case but I wouldn't bet on it. It's certainly in the realm of possible.

from a schema perspective it makes sense to include it if desired, but as I mentioned DRF handles HEAD a bit different which makes this a non-trivial change to spectacular (given above requirements).

mdellweg commented 2 years ago

Let me add HEAD specifically as a wish in a new issue then. Thank you for considering it.