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

KeyError: 'trace' #244

Closed
ndrbrt opened this issue Jan 3, 2021 · 10 comments
Closed

KeyError: 'trace' #244

ndrbrt opened this issue Jan 3, 2021 · 10 comments
Labels
fix confirmation pending issue has been fixed and confirmation from issue reporter is pending

Comments

@ndrbrt
Copy link

ndrbrt commented Jan 3, 2021

My environment:

  • macOS: 10.14.6
  • python: 3.8.2
  • django: 3.1.2
  • djangorestframework: 3.12.1
  • drf-spectacular: 0.12.0

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
Copy link
Owner

tfranzel commented Jan 3, 2021

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
Copy link
Author

ndrbrt commented Jan 3, 2021

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 added a commit that referenced this issue Jan 9, 2021
@tfranzel
Copy link
Owner

tfranzel commented Jan 9, 2021

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 tfranzel added the fix confirmation pending issue has been fixed and confirmation from issue reporter is pending label Jan 10, 2021
@tfranzel
Copy link
Owner

can you check this works with new release 0.13.0?

@ndrbrt
Copy link
Author

ndrbrt commented Jan 17, 2021

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

@mdellweg
Copy link

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
Copy link
Owner

tfranzel commented Mar 16, 2022

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 encode/django-rest-framework#4320 (comment)

CONNECT: not sure this even applies to DRF.

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

  • If there is no explicit head() method it gets routed to GET.
  • It only make sense to add this to the schema when this is explicitly defined.
  • Otherwise existing schemas are not to see any change from this addition.
  • It's a bit weird for ModelViewSets, since HEAD is possible for both list and retrieve

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
Copy link

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
Copy link
Owner

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix confirmation pending issue has been fixed and confirmation from issue reporter is pending
Projects
None yet
Development

No branches or pull requests

3 participants