fao89 / pulp_ansible

A Pulp plugin that manages Ansible content, i.e. roles, collections
https://pulp-ansible.readthedocs.io/en/latest/
GNU General Public License v2.0
0 stars 0 forks source link

As a user, the Galaxy V3 APIs has working Python bindings #9

Open fao89 opened 2 years ago

fao89 commented 2 years ago

Author: @bmbouter (bmbouter)

Redmine Issue: 5238, https://pulp.plan.io/issues/5238


The viewset's aren't correctly represented (or at all) in the Python bindings. Here's what needs to be done:

1. Add swagger tags to those viewsets
2. Verify they are all present and organized in /pulp/api/v3/docs/
3. Verify the endpoints are all usable in the bindings.

fao89 commented 2 years ago

From: @bmbouter (bmbouter) Date: 2019-08-08T21:39:56Z


@dkliban thanks for the testing and pointers on the issues we're facing here. I'll try to summarize the two root causes we've learned so far:

The schema generator that Pulp uses is incompatible with the view as_view() used in urls.py here Specifically the issue is that the schema generated expects to be handling ViewSets that are registered using routers. If you try to hand it a single view as in what as_view() produces when the schema is being generated you'll get a 500 error due to this incompatibility.

This Pulp schema generator and as_view() incompatibility could be fixed (we prototyped one 2 months ago AIUI), but for the extra code and cases the schema generator needs to handle due to that "fix" it's a lot easier to use routes and viewsets instead. The idea is that instead of limiting the view methods with urls.py and as_view(), use mixins onto the ViewSet model and have DRF handle it by the List, Create, Update, etc mixins it inherits from. Then use the DRF router config on the viewset to register the route instead of urls.py. This way the mixins are in control of the supported methods, and the swagger-auto-schema decorations on the viewset will work this time because the schema generator will now work with a ViewSet instead of a single View.

There is a second issue also, we noticed some of the bindings won't look right due to a lack of Serialzers on them. I haven't looked at how many ViewSets need them added, but there is at least one missing from here Without that the response format can't be known by the bindings.

fao89 commented 2 years ago

From: @bmbouter (bmbouter) Date: 2019-08-08T21:55:53Z


Just for clarity here's a redux of the next steps I'm planning. All of them involve updating the V3 viewsets:

diff --git a/pulp_ansible/app/galaxy/v3/views.py b/pulp_ansible/app/galaxy/v3/views.py
index 527fb02..987a0bd 100644
--- a/pulp_ansible/app/galaxy/v3/views.py
+++ b/pulp_ansible/app/galaxy/v3/views.py
@@ -1,6 +1,8 @@
 import semantic_version

 from django.shortcuts import get_object_or_404
+from django.utils.decorators import method_decorator
+from drf_yasg.utils import swagger_auto_schema
 from rest_framework import viewsets
 from rest_framework import mixins

@@ -38,6 +40,14 @@ class AnsibleDistributionMixin:
         return context

+@method_decorator(name="retrieve", decorator=swagger_auto_schema(
+    operation_description="Retrieve a single Collection.",
+    tag="Galaxy V3 API",
+))
+@method_decorator(name="list", decorator=swagger_auto_schema(
+    operation_description="List all Collections.",
+    tag="Galaxy V3 API",
+))
 class CollectionViewSet(
     ExceptionHandlerMixin,
     AnsibleDistributionMixin,
fao89 commented 2 years ago

From: osapryki (osapryki) Date: 2019-08-09T13:33:01Z


It doesn't seem to work, because looks like pulp dispatches viewsets by model:

https://github.com/pulp/pulpcore/blob/master/pulpcore/app/apps.py#L114

If that's true it leads to the following downsides:

1. Any two or more viewsets that use queryset based on the same model can't co-exist.
2. Any viewset to be registered in schema generator has to have queryset attribute.

fao89 commented 2 years ago

From: @dkliban (dkliban@redhat.com) Date: 2019-08-09T13:59:20Z


1. I don't think there is such a limitation.
2. Yes, the OpenAPI schema generator expects a queryset to be registered with a viewset. The Model associated with the ViewSet is used to determine which resource is being operated on.

fao89 commented 2 years ago

From: @bmbouter (bmbouter) Date: 2019-08-09T14:07:54Z


osapryki wrote:

It doesn't seem to work, because looks like pulp dispatches viewsets by model:

https://github.com/pulp/pulpcore/blob/master/pulpcore/app/apps.py#L114

I don't think there is a requirement to use NamedModelViewSet to make both the viewset and its swagger bindings work well w/ Pulp. For example look at the viewset of the collection uploader: https://github.com/pulp/pulp_ansible/blob/master/pulp_ansible/app/viewsets.py#L220-L267 That is vanilla DRF and Swagger, and it shows up in the docs and swagger bindings here: https://pulp-ansible.readthedocs.io/en/latest/restapi.html#tag/ansible:-collections

With \^ info @osapryki can you re-express your concern?

If that's true it leads to the following downsides:

1. Any two or more viewsets that use queryset based on the same model can't co-exist.
2. Any viewset to be registered in schema generator has to have queryset attribute.

fao89 commented 2 years ago

From: @bmbouter (bmbouter) Date: 2019-08-09T14:28:38Z


The viewsets currently merged produce a 500 error when generating the schema at all. With the current HEAD of master ( be07c925c889a8e8149f4d49aef755b1090a9081 ) a call to :24817/pulp/api/v3/ gives you the following traceback:

Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]: pulp: django.request:ERROR: Internal Server Error: /pulp/api/v3/
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]: Traceback (most recent call last):
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = get_response(request)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = self.process_exception_by_middleware(e, request)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = wrapped_callback(request, *callback_args, **callback_kwargs)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     return view_func(*args, **kwargs)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/django/views/generic/base.py", line 71, in view
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     return self.dispatch(request, *args, **kwargs)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/views.py", line 497, in dispatch
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = self.handle_exception(exc)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/schemas/views.py", line 48, in handle_exception
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     return super().handle_exception(exc)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/views.py", line 457, in handle_exception
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     self.raise_uncaught_exception(exc)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/views.py", line 468, in raise_uncaught_exception
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     raise exc
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/views.py", line 494, in dispatch
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = handler(request, *args, **kwargs)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/schemas/views.py", line 37, in get
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     schema = self.schema_generator.get_schema(request, self.public)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/schemas/openapi.py", line 64, in get_schema
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     paths = self.get_paths(None if public else request)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/schemas/openapi.py", line 47, in get_paths
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     operation = view.schema.get_operation(path, method)
Aug 09 14:27:00 pulp3-source-fedora29.localhost.example.com gunicorn[675]: AttributeError: 'DefaultSchema' object has no attribute 'get_operation'
fao89 commented 2 years ago

From: @bmbouter (bmbouter) Date: 2019-08-09T14:31:21Z


Here's a test script I use to populate all data so when I port the URLs I can be sure the response format's don't change and everything works.

fao89 commented 2 years ago

From: @bmbouter (bmbouter) Date: 2019-08-09T14:42:59Z


Also requesting data from these viewsets yields 500's so I can't see the existing response behaviors before I port it.

Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]: pulp: django.request:ERROR: Internal Server Error: /pulp_ansible/galaxy/foo/api/v3/collections/
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]: Traceback (most recent call last):
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = get_response(request)
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = self.process_exception_by_middleware(e, request)
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = wrapped_callback(request, *callback_args, **callback_kwargs)
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     return view_func(*args, **kwargs)
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/viewsets.py", line 114, in view
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     return self.dispatch(request, *args, **kwargs)
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/views.py", line 497, in dispatch
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = self.handle_exception(exc)
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/views.py", line 457, in handle_exception
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     self.raise_uncaught_exception(exc)
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/views.py", line 468, in raise_uncaught_exception
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     raise exc
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/views.py", line 494, in dispatch
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     response = handler(request, *args, **kwargs)
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/usr/local/lib/pulp/lib64/python3.7/site-packages/rest_framework/mixins.py", line 38, in list
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     queryset = self.filter_queryset(self.get_queryset())
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/home/vagrant/devel/pulp_ansible/pulp_ansible/app/galaxy/v3/views.py", line 61, in get_queryset
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     distro_content = self.get_distro_content(self.kwargs["path"])
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:   File "/home/vagrant/devel/pulp_ansible/pulp_ansible/app/galaxy/v3/views.py", line 32, in get_distro_content
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]:     return RepositoryVersion.latest(distro.repository).content
Aug 09 14:40:50 pulp3-source-fedora29.localhost.example.com gunicorn[675]: AttributeError: 'NoneType' object has no attribute 'content'
fao89 commented 2 years ago

From: @RCMariko (rchan) Date: 2019-09-13T18:26:29Z


Haven't gotten to this yet and need to address blocking issues, so removing from sprint & assignee.

fao89 commented 2 years ago

From: @fao89 (fao89) Date: 2020-03-12T00:31:53Z


https://github.com/pulp/pulpcore/pull/577 https://github.com/pulp/pulp_ansible/pull/298

fao89 commented 2 years ago

From: pulpbot (pulpbot) Date: 2020-05-15T21:31:23Z


PR: https://github.com/pulp/pulp_ansible/pull/298