How should we handle viewsets that have to have different logic in pulp ansible and galaxy ng?

[newswangerd]

One of the goals of the API Refactor is for galaxy_ng to be able to directly import the URLs from pulp_ansible and provide the same set of APIs as pulp_ansible with no viewset subclassing or duplication. Right now there are two endpoints that are heavily modified by galaxy_ng which can't be imported from pulp ansible.

Collection Upload Viewset

The galaxy ng version of the upload viewset currently does the following:

• Requires that the distribution path that the collection is being uploaded into is named inbound-{namespace} where the namespace has to match the namespace of the collection being uploaded
• Checks that the user uploading the collection has upload permissions on the namespace
• Performs the collection import
• Once the import is finished, the collection is moved into the staging repository if auto approve collections is turned off and the published repository if it's turned on

There's a lot of logic here that is specific to repositories that are only created by galaxy_ng and are required by the workflows that galaxy_ng expects.

Collection Move Viewset

This viewset doesn't exist in pulp_ansible yet, but the version in galaxy ng currently curates the synclists when it's running on console.redhat.com and the current plan is for this to launch a task to sign collections as well.

Solution

At the moment the best we can do is override the viewsets in galaxy ng, which runs against the goals of the API refactor (remove duplicate code and viewset subclassing). Other suggestions on how to solve this problem are appreciated.

[rochacbruno]

One option is to use a configurable middleware such as this one https://github.com/IV1T3/django-middleware-fileuploadvalidation

A middleware would go between any file upload to perform additional tasks.

Another way is to use hooks or signals

But to me seems like the best options is what we already have in place which is subclass and overload.

[bmbouter]

I need to think more about solutions, but part of my assumption is that we can't continue subclassing at all because pulp_ansible can't be safely used as a library. I'll try to think it over and look at some other options.

[rochacbruno]

FYK: The issue #837 is adding more of that kind of logic

[rochacbruno]

Config based dependency injection (a.k.a hooks) seems to be an option, if done with the proper level of validation it is safe.

Settings based

This is what django does when we include string based objects to its INSTALLED_APPS, MIDDLEWARES etc..

Pulp_ansible would set hooks on the needed parts of the workflow, e.g: On the CollectionVersionUpload viewset there will be pre_request_hook, pre_validation_hook, post_... the exact hooks can be defined on-demand as long as we come with a standard for its naming and message passing.

Entry point based

This is how Pytest solves this kind of problems, the hooks keeps the same, maybe the settings also, but we require an additional validation that the handler of the hook would be registered as a valid python entrypoint.

The advantage of this approach is that it is easily interchangeable and managing versions is also flexible, one can install pip install galaxy-pulp-ansible-hooks==1.0.0 and it registers the needed handlers, or we can have it on galaxy_ng setup.py.

Other advantage is that Python already offers the tooling to discover the entry points registered.

Signals

Django uses it a lot on its testing machinery, I know there are some concerns on using it for production, we are already relying on signals for some of our models.

Basically the hooks would be simply signals that are send and on galaxy_ng side we connect to those signals and handle it.

For uploading process, validations etc that is a very common approach on Django community.