get_schema_fields: include exception in warning

[blueyed]

No description provided.

[rpkilby]

Just my two cents... but I'm -1 on this. Warnings do not usually have their traceback embedded in the message, and it seems unnecessary to do so here.

• Warnings can be promoted to errors with python -W error, which will then provide the traceback.
• Users can explicitly filter for the warning and get the exception from the warning.
• Users can override warnings.showwarning (1, 2) to log the traceback without having to promote the warnings to exceptions.

[blueyed]

It helped my use case, which is pytest displaying the warning in our test suite:

old:

======= warnings summary ========
project/app/tests/test_api_docs.py::test_docs_media_types_and_index
  …/project/.venv/lib/python3.6/site-packages/django_filters/rest_framework/backends.py:93: UserWarning: <class 'refreshtoken.views.RefreshTokenViewSet'> is not compatible with schema generation
    "{} is not compatible with schema generation".format(view.__class__)

new (based on 1.1.1):

======= warnings summary ========
project/app/tests/test_api_docs.py::test_docs_media_types_and_index
  …/Vcs/django-filter/django_filters/rest_framework/backends.py:94: UserWarning: <class 'refreshtoken.views.RefreshTokenViewSet'> is not compatible with schema generation. Got exception when calling get_filter_class: TypeError("int() argument must be a string, a bytes-like object or a number, not 'AnonymousUser'",)
    view.__class__, exc)

1. The problem with -W error is that it triggers other issues (and stops there), e.g. Importing rfc3987 causes DeprecationWarning since python 3.6 dgerber/rfc3987#14 or ImportWarning: can't resolve package from __spec__ or __package__, falling back on __name__ and __path__ etc.

2. The filtering with -W is not very user friendly (i.e. easy to get right).

3. might be nice to have pytest include the traceback already, but that would not have the real exception there then (from a quick glance at the snippets you've linked to).

After all, it might be better to use a logger instead (warning level), and then use exc_info=True to include the traceback there?

Despite seeing this in tests, you would otherwise see it in logs, where logging behaves better typically than warnings - except for only being displayed once etc.

[rpkilby]

might be nice to have pytest include the traceback already, but that would not have the real exception there then (from a quick glance at the snippets you've linked to).

The warning should have an attached traceback, which should have the original exception context. This does seem like it could be a flag/option for pytest.

After all, it might be better to use a logger instead (warning level), and then use exc_info=True to include the traceback there?

In my mind, logging is intended more for events in the system, whereas warnings are more used for notifying the programmer of potential issues in their code. eg, "hey, this service took too long to response - you may want to check this out." vs "hey, you're using a deprecated API, use this instead."

Despite seeing this in tests, you would otherwise see it in logs, where logging behaves better typically than warnings - except for only being displayed once etc.

You may be interested in logging.captureWarnings.

[blueyed]

Ok, makes sense.
Thanks for your feedback, closing.