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

Repeated warnings view has no attribute 'get_serializer' #28

Closed
jayvdb opened this issue Apr 23, 2020 · 11 comments
Closed

Repeated warnings view has no attribute 'get_serializer' #28

jayvdb opened this issue Apr 23, 2020 · 11 comments
Labels
enhancement New feature or request

Comments

@jayvdb
Copy link
Contributor

jayvdb commented Apr 23, 2020

This is while processing django-oscar-api. Ideally the lack of a get_serializer is noticed once per view and only one warning is emitted.

WARNING #741: Exception raised while getting serializer from CheckoutView. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: type object 'CheckoutView' has no attribute 'get_serializer')
WARNING #742: Exception raised while getting serializer from CheckoutView. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: type object 'CheckoutView' has no attribute 'get_serializer')
WARNING #743: Exception raised while getting serializer from CheckoutView. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: type object 'CheckoutView' has no attribute 'get_serializer')
@tfranzel
Copy link
Owner

tfranzel commented Apr 23, 2020

warning are emitted for every operation (method+endpoint). no easy way around it. once the schema approaches a good state there should be rarely any warning.

i think i understand the problem with get_serializer. django-oscar-api is slightly abusing DRF by basing their views on APIView instead of GenericAPIView, which provides the serializer mechanics. then they set the serializer_class anyhow even though the base class does not provide mechanics for it.

i'll check if serializer retrieval can be a bit more forgiving.

@tfranzel tfranzel added the enhancement New feature or request label Apr 23, 2020
@tfranzel
Copy link
Owner

@jayvdb please try again. i added some educated guesses, which should at least solve some of the things django-oscar-api is doing.

@jayvdb
Copy link
Contributor Author

jayvdb commented Apr 24, 2020

Another 89 warnings removed, down to 810.
However now I see repeated

WARNING #511: Exception raised while getting serializer from CheckoutView. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: `PaymentMethodsSerializer` requires the request in the serializer context. Add `context={'request': request}` when instantiating the serializer.)
WARNING #512: Exception raised while getting serializer from CheckoutView. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: `PaymentMethodsSerializer` requires the request in the serializer context. Add `context={'request': request}` when instantiating the serializer.)
WARNING #513: Exception raised while getting serializer from CheckoutView. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: `PaymentMethodsSerializer` requires the request in the serializer context. Add `context={'request': request}` when instantiating the serializer.)
...
WARNING #769: Unable to guess serializer for LogoutView. This is graceful fallback handling for APIViews. Consider using GenericAPIView as view base class, if view is under your control. ignoring view for now. 
WARNING #770: Unable to guess serializer for LogoutView. This is graceful fallback handling for APIViews. Consider using GenericAPIView as view base class, if view is under your control. ignoring view for now. 
WARNING #771: Unable to guess serializer for LogoutView. This is graceful fallback handling for APIViews. Consider using GenericAPIView as view base class, if view is under your control. ignoring view for now.
...
WARNING #803: Unable to guess serializer for wrapped_target. This is graceful fallback handling for APIViews. Consider using GenericAPIView as view base class, if view is under your control. ignoring view for now. 
WARNING #804: Unable to guess serializer for wrapped_target. This is graceful fallback handling for APIViews. Consider using GenericAPIView as view base class, if view is under your control. ignoring view for now. 
WARNING #805: Unable to guess serializer for wrapped_target. This is graceful fallback handling for APIViews. Consider using GenericAPIView as view base class, if view is under your control. ignoring view for now. 

It should be possible to have a mapping/cache that allows only one warning per view, but you are in a better position to understand why that might not be possible.

@jayvdb
Copy link
Contributor Author

jayvdb commented Apr 24, 2020

wrapped_target was #30

If I dont use silk-profile, I get three warnings for url entry path('api-token-oauth/', get_user_jwt),

WARNING #4: could not resolve request body for POST /api-token-oauth/. defaulting to generic free-form object. (maybe annotate a Serializer class?)
WARNING #5: Unable to guess serializer for get_user_jwt. This is graceful fallback handling for APIViews. Consider using GenericAPIView as view base class, if view is under your control. ignoring view for now. 
WARNING #6: could not resolve "None" for POST /api-token-oauth/. Expected either a serializer or some supported override mechanism. defaulting to generic free-form object.
@api_view(['POST'])
@permission_classes((TokenHasReadWriteScope, ))
def get_user_jwt(request, *args, **kwargs):
    """
    This converts the access token by
    django-rest-framework-social-oauth2 to JWT.

    The request header should contain the token as:
    `Authentication: Bearer <access_token>`
    """
    user = request.user
    serializer = UserSerializer(user)
    return JsonResponse({'user': serializer.data,
                         'token': generate_jwt_token(serializer.data)},
                        status=200)

@jayvdb
Copy link
Contributor Author

jayvdb commented Apr 24, 2020

https://github.com/RealmTeam/django-rest-framework-social-oauth2/blob/f9de220606bd08981b9d81ab80dd69d70ceb1988/rest_framework_social_oauth2/views.py (current master) uses both api_view and APIView throughout. This effects all use of https://pypi.org/project/django-rest-framework-social-oauth2/ . Will a better OpenApiAuthenticationExtension implementation help avoid some of those? If so, it is worth a separate issue and become a priority.

@jayvdb
Copy link
Contributor Author

jayvdb commented Apr 24, 2020

rest framework generates WrappedAPIView, which is of type APIView.
I see this in a warning WARNING #37: could not derive type of path parameter "key" because <class 'rest_invitations.views.WrappedAPIView'> has no queryset. consider annotating the parameter type with @extend_schema. defaulting to "string". , which seems to come from
https://github.com/fmarco/django-rest-invitations/blob/0425086/rest_invitations/views.py

@jayvdb
Copy link
Contributor Author

jayvdb commented Apr 24, 2020

Some stats on the repeated nature of these warnings that I am seeing:

> wc -l all.txt
270 all.txt
> sort all.txt | uniq -c | wc -l
97

@jayvdb
Copy link
Contributor Author

jayvdb commented Apr 24, 2020

Here are the only remaining warnings which mention 'exception'. All of these are django-oscar-api

Exception raised while getting serializer from CheckoutView. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: `PaymentMethodsSerializer` requires the request in the serializer context. Add `context={'request': request}` when instantiating the serializer.)
Exception raised while getting serializer from PaymentMethodsView. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: `PaymentMethodsSerializer` requires the request in the serializer context. Add `context={'request': request}` when instantiating the serializer.)
Exception raised while getting serializer from PaymentStatesView. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: 'PaymentStatesView' should either include a `serializer_class` attribute, or override the `get_serializer_class()` method.)

The warning mentions "is get_queryset() not working without a request?" , and that is clear for at least two of them, and my guess is it also applies to the third.

@tfranzel
Copy link
Owner

i consider the duplicate warnings very low prio atm. good application for uniq -c | wc -l 😄

i'm evaluating whether i should introduce a "mocked" request to allow for simple processing. i did something similar for OAuth2 permission parsing.

depending on what those serializers do with the request, that may solve it or not. generally, there is a limit on how much opaque "magic" the introspection can overcome.

@tfranzel
Copy link
Owner

https://github.com/RealmTeam/django-rest-framework-social-oauth2/blob/f9de220606bd08981b9d81ab80dd69d70ceb1988/rest_framework_social_oauth2/views.py (current master) uses both api_view and APIView throughout. This effects all use of https://pypi.org/project/django-rest-framework-social-oauth2/ . Will a better OpenApiAuthenticationExtension implementation help avoid some of those? If so, it is worth a separate issue and become a priority.

no. setting the auth/permission on the schema operation has nothing to do with the parsing of the actual library endpoints for authentication.

rest framework generates WrappedAPIView, which is of type APIView.
I see this in a warning WARNING #37: could not derive type of path parameter "key" because <class 'rest_invitations.views.WrappedAPIView'> has no queryset. consider annotating the parameter type with @extend_schema. defaulting to "string". , which seems to come from
https://github.com/fmarco/django-rest-invitations/blob/0425086/rest_invitations/views.py

same issue. @api_view -> WrappedAPIView -> APIView which again does not hold any information on the model. no model = no way to pull the type of key from a model field.

maybe we need a way to override autodiscovery with manually given info. since lib code is not under your control, you cannot add @extend_schema. might be interesting to have a setting to globally override an encountered class. that would mean though that you have to provide your own "mock" serializers and views. that is a lot of trouble but prob less trouble than forking and patching the other libraries.

jayvdb added a commit to jayvdb/drf-spectacular that referenced this issue Apr 28, 2020
@tfranzel
Copy link
Owner

@jayvdb i took your advice to heart. duplicate warnings are not that nice. closing this issue as the main issue is resolved.

jayvdb added a commit to jayvdb/drf-spectacular that referenced this issue Apr 28, 2020
jayvdb added a commit to jayvdb/drf-spectacular that referenced this issue May 1, 2020
jayvdb added a commit to jayvdb/drf-spectacular that referenced this issue May 1, 2020
jayvdb added a commit to jayvdb/drf-spectacular that referenced this issue May 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants