[Pulp-dev] Plugin relationship to tasks

Austin Macdonald austin at redhat.com
Mon Mar 26 22:02:26 UTC 2018


On Mon, Mar 26, 2018 at 4:43 PM, Dennis Kliban <dkliban at redhat.com> wrote:

> On Mon, Mar 26, 2018 at 3:38 PM, Austin Macdonald <austin at redhat.com>
> wrote:
>
>>
>>
>> On Mon, Mar 26, 2018 at 2:30 PM, Dennis Kliban <dkliban at redhat.com>
>> wrote:
>>
>>> This proposal does not make the plugin writer's job any easier. This
>>> simply changes where the plugin writer is providing validation logic, in a
>>> serializer vs. in a view.
>>>
>>
>> It does make the plugin writer's job easier *because* it changes how the
>> validation is provided, Plugin writers already have to understand how to
>> create models/serializers (and it is very simple) for all the other
>> objects. Seriallizer validation is much simpler than viewset validation
>> because serializers are literally designed to do this.
>>
>
> If we don't change anything, a plugin writer is responsible for creating a
> task that creates a Repository Version and a view which will perform
> validation and then dispatches a task. If we do make the proposed change,
> the plugin writer will still have to write the task code and then also a
> serializer that performs validation. The proposed change only removes the
> responsibility of dispatching a task which is 1 line of code.
>

The difference is not in the number of lines, it is in the complexity.
Validation with serializers is simpler, and more similar to other plugin
code.

New Way:
https://github.com/asmacdo/pulp_file/blob/8a8cbf81b6f0f1e382be7884f2b2406a06ea230b/pulp_file/app/serializers.py#L15-L21
Old way:
https://github.com/asmacdo/pulp_file/blob/8a8cbf81b6f0f1e382be7884f2b2406a06ea230b/pulp_file/app/serializers.py#L13-L30

That "1 line of code" puts the plugin writer in contact with a very complex
part of the pulpcore internals. `apply_async_with_reservation`:
https://github.com/pulp/pulp/blob/e8f804dc6595732dad38ec478877d919d7d97063/pulpcore/pulpcore/tasking/tasks.py#L180



> The other problem I see is that complexity for users is increased. Instead
>>> of having 1 resource for tracking task progress, there would be an infinite
>>> number of resources for tracking progress of tasks.
>>>
>>
>> In the proposal, Tasks are Master/Detail. The user doesn't have to
>> "track" it at all. They can still use v3/tasks/ to see all the tasks.
>> Retrieving tasks will either be the same (v3/tasks/) or easier if they know
>> the task they are looking for (v3/tasks/file/syncs/)
>>
>
> My understanding of the master/detail pattern we are using for our other
> resources is that each task's URI will look something like this:
> /api/v3/tasks/file/syncs/<uuid>/
>
> If the above is true, that means the client has to be aware of
> '/api/v3/tasks/file/syncs/<uuid>/' resource. The user would need to know
> how to interpret the response from each task type. There could be an
> infinite number of these tasks with many different arguments. So writing
> client code handling all those responses could become a nightmare. Right
> now the user just needs to know how to interpret the response from
> '/api/v3/tasks/<uuid>' for all tasks.
>

I might be missing your point on this one. If you are talking about polling
tasks, the href of a new Task is returned when the Task is created.The
client should use the href that is returned, never string formatting urls.
Even so, the Tasks in this proposal will be part of the API schema, so
clients (and docs users) know where the endpoints are and what the
arguments should be. This would actually be a big advantage over the way
things are now. AFAIK, v3/importers/1234/sync/ is invisible to clients, so
we replace one invisible endpoint with one visible endpoint. No complexity
added.

If the client doesn't know how to interpret extra fields (the Task
parameters), they don't need to. All tasks share the same base fields, and
those are the fields that need to be interpreted when the client is polling
a task.


> Value #1: We have to do something to address the problem that
> adding/removing content to a repository without plugin input is incorrect.
> This proposal is one possibility, but it isn't valid to compare the value
> against doing nothing. Instead, if you don't like this option, we need to
> compare it to other proposals for how to involve plugins in tasks.


> We don't have a problem. I agree that a user needs to know a whole lot of
> information to correctly create a working repository version for a complex
> content type such as a Docker manifest. Without all this information on
> hand, the user could easily create a broken repository version. However, a
> rich client could solve that problem. If the plugin writer wants to support
> simpler clients, she can provide an additional URL for handling the
> validation and additional logic for creating a repository version. We
> should probably have a recommended convention for plugin authors to use
> when adding additional URLs.


What I find troubling is that the most basic action (add/remove) suddenly
becomes complex. If using the file plugin POST to
v3/repository/1234/versions/. If using docker, POST to
v3/plugins/docker/add/. This is especially irritating because the first way
will still work for docker, but will break corrupt the repositories. So the
responsibility falls to users to know that this endpoint "works" but should
be avoided (for certain plugins). It will be surprising that one is a
"noun" and the other is an "action".

Also, doing correctness enforcement in the client instead of the REST API
seems very wrong.

Value #5: Task history becomes much more useful. Currently, the only data
> on a task that connects it to user-facing objects is "created_resource".
> This proposal will allow users to filter tasks by parameters. For example,
> they can filter sync tasks by "repository" or "importer". They can also use
> the detail endpoints (v3/tasks/file/ or v3/tasks/file/syncs/) to list tasks
> based on the plugin or action.
>
>
I don't think tracking history is one of the use cases we decided to
> support in the MVP. We should have a separate discussion on how we could
> accomplish this.


Fair enough, but just because it isn't on the MVP doesn't mean this it
should be completely ignored.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20180326/071d467c/attachment.htm>


More information about the Pulp-dev mailing list