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

get_queryset with an abstract model #1171

Open
alexandernst opened this issue Feb 8, 2024 · 13 comments
Open

get_queryset with an abstract model #1171

alexandernst opened this issue Feb 8, 2024 · 13 comments

Comments

@alexandernst
Copy link

Describe the bug
I have a use case in which I have a ListAPIView that connects to a 3rd party (Stripe) API, fetches data (invoices) and returns that data to the user. I have a serializer, but I don't have a model.

The entire code looks something like this:

class InvoicesList(generics.ListAPIView):
  serializer_class = InvoiceSerializer

  def get_queryset(self):
    if getattr(self, 'swagger_fake_view', False):
      return   # <----  ¿?¿?¿?¿?¿?¿?¿?¿?
  
    return StripeWrapper().get_invoices()

class InvoiceSerializer(serializers.Serializer):
  ...fields..
  ...fields...
  ...fields

class StripeWrapper():
  def get_invoices():
   return requests.get(......)

Since I don't have a model, drf-spectacular refuses to generate the proper openapi specs. It expects to receive an EmptyQuerySet (SomeModel.objects.none()), but I can't provide it any since I don't have an Invoice model. I could create an abstract model like this:

class Invoice(models.Model):
  class Meta:
    abstract = True

but I still won't be able to provide drf-spectacular with a Invoice.objects.none() since there is no manager in that class (and there can't be since it's abstract).

How should I proceed in this scenario? Is there some workaround?

To Reproduce
N/A

Expected behavior
N/A

@tfranzel
Copy link
Owner

tfranzel commented Feb 8, 2024

Hi, so 2 observations. I put your example in a test case and it works for me without error or warnings.

    if getattr(self, 'swagger_fake_view', False):
      return 

Should be enough. returning None will be handled gracefully internally. The given serializer will be used anyway since this call is merely to find potential path parameters (which there are none). Outside of spectacular, this if will never be entered, so no issue there for DRF.

We could, in case there are no path parameters, skip the get_queryset call. That would be a small fix, but I still don't understand where your code exactly breaks. What exactly is the problem? stacktrace?

@alexandernst
Copy link
Author

If I return None, I get this:

/code/business/apiviews/invoices.py: Warning [InvoicesL]: Failed to obtain model through view's queryset due to raised exception. Prevent this either by setting "queryset = Model.objects.none()" on the view, checking for "getattr(self, "swagger_fake_view", False)" in get_queryset() or by simply using @extend_schema. (Exception: 'NoneType' object has no attribute 'model')

@tfranzel
Copy link
Owner

tfranzel commented Feb 8, 2024

Something is weird here. That warning cannot be explained with your example.

get_view_model is only used twice. The first is with parameter extraction, but the warning is disabled for that usage. This exception is expected and does happen, however it is suppressed with that if statement.

The second usage is with django_filter. Are you using django-filter here too?

model = get_view_model(self.view, emit_warnings=False)

if emit_warnings:
warn(
f'Failed to obtain model through view\'s queryset due to raised exception. '


this testcase works as expected for me:

def test_foo(no_warnings):
    class InvoiceSerializer(serializers.Serializer):
        field = serializers.IntegerField()

    class InvoicesList(generics.ListAPIView):
        serializer_class = InvoiceSerializer

        def get_queryset(self):
            if getattr(self, 'swagger_fake_view', False):
                return
            raise RuntimeError("scheme gen should not get here")

    schema = generate_schema('/x/', view=InvoicesList)
    assert schema

@alexandernst
Copy link
Author

Does this help?

> /code/business/apiviews/invoices.py(28)get_queryset()
-> return None
(Pdb) bt
  /usr/local/lib/python3.10/threading.py(973)_bootstrap()
-> self._bootstrap_inner()
  /usr/local/lib/python3.10/threading.py(1016)_bootstrap_inner()
-> self.run()
  /usr/local/lib/python3.10/threading.py(953)run()
-> self._target(*self._args, **self._kwargs)
  /usr/local/lib/python3.10/socketserver.py(683)process_request_thread()
-> self.finish_request(request, client_address)
  /usr/local/lib/python3.10/socketserver.py(360)finish_request()
-> self.RequestHandlerClass(request, client_address, self)
  /usr/local/lib/python3.10/socketserver.py(747)__init__()
-> self.handle()
  /usr/local/lib/python3.10/site-packages/django/core/servers/basehttp.py(227)handle()
-> self.handle_one_request()
  /usr/local/lib/python3.10/site-packages/django/core/servers/basehttp.py(252)handle_one_request()
-> handler.run(self.server.get_app())
  /usr/local/lib/python3.10/wsgiref/handlers.py(137)run()
-> self.result = application(self.environ, self.start_response)
  /usr/local/lib/python3.10/site-packages/django/contrib/staticfiles/handlers.py(80)__call__()
-> return self.application(environ, start_response)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/wsgi.py(124)__call__()
-> response = self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/base.py(140)get_response()
-> response = self._middleware_chain(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/silk/middleware.py(72)__call__()
-> response = self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/corsheaders/middleware.py(56)__call__()
-> result = self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /code/ambiance/middleware.py(14)__call__()
-> response = self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/base.py(197)_get_response()
-> response = wrapped_callback(request, *callback_args, **callback_kwargs)
  /usr/local/lib/python3.10/site-packages/django/views/decorators/csrf.py(56)wrapper_view()
-> return view_func(*args, **kwargs)
  /usr/local/lib/python3.10/site-packages/django/views/generic/base.py(104)view()
-> return self.dispatch(request, *args, **kwargs)
  /usr/local/lib/python3.10/site-packages/rest_framework/views.py(506)dispatch()
-> response = handler(request, *args, **kwargs)
  /usr/local/lib/python3.10/site-packages/drf_spectacular/views.py(84)get()
-> return self._get_schema_response(request)
  /usr/local/lib/python3.10/site-packages/drf_spectacular/views.py(92)_get_schema_response()
-> data=generator.get_schema(request=request, public=self.serve_public),
  /usr/local/lib/python3.10/site-packages/drf_spectacular/generators.py(281)get_schema()
-> paths=self.parse(request, public),
  /usr/local/lib/python3.10/site-packages/drf_spectacular/generators.py(252)parse()
-> operation = view.schema.get_operation(
  /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(91)get_operation()
-> parameters = self._get_parameters()
  /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(258)_get_parameters()
-> **dict_helper(self._resolve_path_parameters(path_variables)),
  /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(483)_resolve_path_parameters()
-> model = get_view_model(self.view, emit_warnings=False)
  /usr/local/lib/python3.10/site-packages/drf_spectacular/plumbing.py(213)get_view_model()
-> return view.get_queryset().model
> /code/business/apiviews/invoices.py(28)get_queryset()
(Pdb) s
--Return--
> /code/business/apiviews/invoices.py(28)get_queryset()->None
-> return None
(Pdb) s
AttributeError: 'NoneType' object has no attribute 'model'
> /usr/local/lib/python3.10/site-packages/drf_spectacular/plumbing.py(213)get_view_model()
-> return view.get_queryset().model
(Pdb) ll
202     def get_view_model(view, emit_warnings=True):
203         """
204         obtain model from view via view's queryset. try safer view attribute first
205         before going through get_queryset(), which may perform arbitrary operations.
206         """
207         model = getattr(getattr(view, 'queryset', None), 'model', None)
208  
209         if model is not None:
210             return model
211  
212         try:
213  ->         return view.get_queryset().model
214         except Exception as exc:
215             if emit_warnings:
216                 warn(
217                     f'Failed to obtain model through view\'s queryset due to raised exception. '
218                     f'Prevent this either by setting "queryset = Model.objects.none()" on the '
219                     f'view, checking for "getattr(self, "swagger_fake_view", False)" in '
220                     f'get_queryset() or by simply using @extend_schema. (Exception: {exc})'
221                 )
(Pdb) process_request

@alexandernst
Copy link
Author

BTW, yes, I'm using django-filters

@tfranzel
Copy link
Owner

tfranzel commented Feb 8, 2024

your stacktrace cannot lead to that warning. emit_warnings=False does not allow it.

  /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(483)_resolve_path_parameters()
-> model = get_view_model(self.view, emit_warnings=False)

213  ->         return view.get_queryset().model
214         except Exception as exc:
215             if emit_warnings:
216                 warn(

The warning must come from your usage of django-filter.

Since I don't have a model, drf-spectacular refuses to generate the proper openapi specs.

what exactly do you mean with that? just a wrong schema or does it completely break with an exception?

@alexandernst
Copy link
Author

The warning must come from your usage of django-filter.

Yet the backtrace that I posted says otherwise, right?

You're checking

207         model = getattr(getattr(view, 'queryset', None), 'model', None)
208  
209         if model is not None:
210             return model

but I don't have queryset set to anything, which I believe will skip that if. Then you try return view.get_queryset().model, which is what is causing the Exception.

just a wrong schema or does it completely break with an exception?

It generates the rest of the endpoints, it just skips this particular endpoint.

@tfranzel
Copy link
Owner

tfranzel commented Feb 8, 2024

but I don't have queryset set to anything, which I believe will skip that if. Then you try return view.get_queryset().model, which is what is causing the Exception.

yes.
207 will be None.
209 will be then skipped
213 will raise the attribute error
214 will catch that exception
215 will skip the warning because -> model = get_view_model(self.view, emit_warnings=False) is how this is called in the stacktrace and if emit_warnings: thus cannot be entered like that.

this will all happen under /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(483)_resolve_path_parameters(), which has nothing to do with the serializer extraction.

Yet the backtrace that I posted says otherwise, right?

I'm just saying I cannot explain what you are observing with the code here. How can that warn() call happen with emit_warnings=False? It makes no sense to me.

@tfranzel
Copy link
Owner

tfranzel commented Feb 8, 2024

I should also add that test_foo test case above does not produce the warning and properly generates the Invoice component, which is correctly put in the operation.

So that test case exactly behaves as it is supposed to: no warning and the correct operation and component.

Even if I explain the warning away with django-filter (and thus a missing endpoint query parameter), it still does not explain the missing Invoice component as query param extraction also behaves independently.

@alexandernst
Copy link
Author

In contrib/django_filters.py:54, in get_schema_operation_parameters, you're calling get_view_model(auto_schema.view) without passing the second parameter (which is True by default), which is why the warning is getting printed.

@tfranzel
Copy link
Owner

tfranzel commented Feb 9, 2024

That is what I said before! get_view_model() is used twice and only once with the warning enabled (django-filter).

Your given stacktrace did not produce that warning because it was not in the django-filter code section, but rather in the path param extraction section, where the warning is turned off.

However, all of this does not explain why the Invoice serializer was not detected. That warning you got there from the django-filter plugin has nothing to do with the response serializer not showing up.

So, I cannot help you any further like that as I cannot reproduce your issue with the given information. The test case I have shown above behaves perfectly, so something else must be going on in your setup. You need to provide me a with a reproduction of your issue if you want futher assistance.

@alexandernst
Copy link
Author

Fair enough! I'll try to isolate the problem and provide a minimum reproducible demo.
In the meantime, do you think passing False as second parameter to that get_view_model call should be added?

@tfranzel
Copy link
Owner

Fair enough! I'll try to isolate the problem and provide a minimum reproducible demo.

yes, please do.

In the meantime, do you think passing False as second parameter to that get_view_model call should be added?

No I don't think so. It is deliberately made like that. The path parameter extraction has fallbacks that may work and so the warning may be unnessecary or misleading. For django-filter this method is all there is and so if there is no model available, the parameter cannot be properly extracted, thus the warning.

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