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

Warnings/errors should include a hint about which serializer #314

Closed
spookylukey opened this issue Feb 23, 2021 · 1 comment
Closed

Warnings/errors should include a hint about which serializer #314

spookylukey opened this issue Feb 23, 2021 · 1 comment

Comments

@spookylukey
Copy link
Contributor

I currently have a rather large list of warnings/errors when running ./manage.py spectacular. Some of them look like this:

Warning #2: unable to resolve type hint for function "_get_pk_val". consider using a type hint or @extend_schema_field. defaulting to string.

This is not easy to track down, especially as _get_pk_val doesn't appear anywhere in my code base. Even for names that do appear it can be hard to track down when you have lots of serializers. It would be really helpful if there were some more hints on this front.

Thanks for the great package BTW.

spookylukey added a commit to spookylukey/drf-spectacular that referenced this issue Mar 2, 2021
…in warnings.

Two techniques for doing this:

1. If we already have a view/viewset object in our local scope,
   we can add it into the message

2. For other deeply nested places from which we call `error()` or `warn()`,
   we instead use a stack of traces that get added as prefices
   to the message (rather than having to pass this context information
   through many layers of code).

This also fixes a set of consistency issues with warning/error messages:

- Always start message with `{serializer/view name}: ` if possible. This means
  that method 1 and 2 above produce the same format of message.
- First sentence shouldn't have a capital letter
  (the message is printed after colon from the prefix)
- Subsequent sentences should have a capital letter.
spookylukey added a commit to spookylukey/drf-spectacular that referenced this issue Mar 8, 2021
…in warnings.

The main technique for doing this in deeply nested places from which we call
`error()` or `warn()`, is to a stack of traces that get added as prefices to the
message (rather than having to pass this context information through many layers
of code).

This also fixes a set of consistency issues with warning/error messages:

- Always start message with `{serializer/view name}: ` if possible.
  This meant removing this info from the body of the message
  where we are now adding it as a prefix.
- First sentence shouldn't have a capital letter
  (the message is printed after colon from the prefix)
- Subsequent sentences should have a capital letter.
spookylukey added a commit to spookylukey/drf-spectacular that referenced this issue Mar 8, 2021
…in warnings.

The main technique for doing this in deeply nested places from which we call
`error()` or `warn()`, is to a stack of traces that get added as prefices to the
message (rather than having to pass this context information through many layers
of code).

This also fixes a set of consistency issues with warning/error messages:

- Always start message with `{serializer/view name}: ` if possible.
  This meant removing this info from the body of the message
  where we are now adding it as a prefix.
- First sentence shouldn't have a capital letter
  (the message is printed after colon from the prefix)
- Subsequent sentences should have a capital letter.
tfranzel added a commit that referenced this issue Mar 9, 2021
…ngs_alt1

Fixed issue #314 - include information about view/serializer in warnings
@tfranzel
Copy link
Owner

tfranzel commented Mar 9, 2021

fixed by #327

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants