Project

Profile

Help

Story #5238

Story #5517: [EPIC] Automation Hub Release Blockers

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

Added by bmbouter 11 months ago. Updated 13 days ago.

Status:
POST
Priority:
Normal
Assignee:
Sprint/Milestone:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Platform Release:
Groomed:
No
Sprint Candidate:
No
Tags:
API Bindings
Sprint:
Sprint 76

Description

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.


Related issues

Related to Pulp - Issue #5311: Hitting /pulp/api/v3/ raises a 500 errorCLOSED - CURRENTRELEASE<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

History

#1 Updated by daviddavis 11 months ago

  • Sprint set to Sprint 57

#2 Updated by bmbouter 11 months ago

@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.

#3 Updated by bmbouter 11 months ago

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

  • use routers instead of urls.py for V3 viewsets
  • add the swagger_auto_schema to the ViewSet classes (one entry per method) similar to the diff below which uses these docs
  • ensure all ViewSet's have response serializers otherwise the response format's won't work.
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,

#4 Updated by osapryki 11 months ago

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.

#5 Updated by dkliban@redhat.com 11 months ago

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.

#6 Updated by bmbouter 11 months ago

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.

#7 Updated by bmbouter 11 months ago

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'

#8 Updated by bmbouter 11 months ago

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.

#9 Updated by bmbouter 11 months ago

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'

#10 Updated by rchan 11 months ago

  • Sprint changed from Sprint 57 to Sprint 58

#11 Updated by rchan 10 months ago

  • Status changed from ASSIGNED to NEW
  • Assignee deleted (bmbouter)
  • Sprint deleted (Sprint 58)

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

#12 Updated by bmbouter 9 months ago

  • Parent task set to #5517

#13 Updated by fao89 9 months ago

  • Related to Issue #5311: Hitting /pulp/api/v3/ raises a 500 error added

#14 Updated by fao89 4 months ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to fao89

#16 Updated by fao89 4 months ago

  • Sprint set to Sprint 68

#17 Updated by rchan 4 months ago

  • Sprint changed from Sprint 68 to Sprint 69

#18 Updated by rchan 3 months ago

  • Sprint changed from Sprint 69 to Sprint 70

#19 Updated by rchan 3 months ago

  • Sprint changed from Sprint 70 to Sprint 71

#20 Updated by rchan 2 months ago

  • Sprint changed from Sprint 71 to Sprint 72

#21 Updated by rchan about 2 months ago

  • Sprint changed from Sprint 72 to Sprint 73

#23 Updated by rchan about 1 month ago

  • Sprint changed from Sprint 73 to Sprint 74

#24 Updated by rchan 28 days ago

  • Sprint changed from Sprint 74 to Sprint 75

#25 Updated by rchan 13 days ago

  • Sprint changed from Sprint 75 to Sprint 76

Please register to edit this issue

Also available in: Atom PDF