<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 26, 2018 at 4:43 PM, Dennis Kliban <span dir="ltr"><<a href="mailto:dkliban@redhat.com" target="_blank">dkliban@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-">On Mon, Mar 26, 2018 at 3:38 PM, Austin Macdonald <span dir="ltr"><<a href="mailto:austin@redhat.com" target="_blank">austin@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Mon, Mar 26, 2018 at 2:30 PM, Dennis Kliban <span dir="ltr"><<a href="mailto:dkliban@redhat.com" target="_blank">dkliban@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div>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. <br></div></div></div></blockquote><div><br></div></span><div>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. </div></div></div></div></blockquote><div><br></div></span><div>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. <br></div></div></div></div></blockquote><div><br></div><div>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.</div><div><br></div><div>New Way: <a href="https://github.com/asmacdo/pulp_file/blob/8a8cbf81b6f0f1e382be7884f2b2406a06ea230b/pulp_file/app/serializers.py#L15-L21">https://github.com/asmacdo/pulp_file/blob/8a8cbf81b6f0f1e382be7884f2b2406a06ea230b/pulp_file/app/serializers.py#L15-L21</a></div><div>Old way:<a href="https://github.com/asmacdo/pulp_file/blob/8a8cbf81b6f0f1e382be7884f2b2406a06ea230b/pulp_file/app/serializers.py#L13-L30">https://github.com/asmacdo/pulp_file/blob/8a8cbf81b6f0f1e382be7884f2b2406a06ea230b/pulp_file/app/serializers.py#L13-L30</a></div><div><br></div><div>That "1 line of code" puts the plugin writer in contact with a very complex part of the pulpcore internals. `apply_async_with_reservation`: <a href="https://github.com/pulp/pulp/blob/e8f804dc6595732dad38ec478877d919d7d97063/pulpcore/pulpcore/tasking/tasks.py#L180">https://github.com/pulp/pulp/blob/e8f804dc6595732dad38ec478877d919d7d97063/pulpcore/pulpcore/tasking/tasks.py#L180</a> </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>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.  <br></div></div></blockquote><div><br></div></span><div>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/)</div></div></div></div></blockquote><div><br></div></span><div>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/<<wbr>uuid>/<br><br></div><div>If the above is true, that means the client has to be aware of '/api/v3/tasks/file/syncs/<<wbr>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. </div></div></div></div></blockquote><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>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.</blockquote><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>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.</blockquote><div><br></div><div>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".</div><div><br></div><div>Also, doing correctness enforcement in the client instead of the REST API seems very wrong.</div><div><br></div></span><span class="gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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.</div><div><br></div></div></div></div></blockquote><div><br></div></span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">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.</blockquote><div><br></div><div>Fair enough, but just because it isn't on the MVP doesn't mean this it should be completely ignored.</div></div></div></div></div></div></div></div>