Those sound like great improvements! For both fatal and non-fatal errors, in the case where the error raised is a PulpCodedException, it would be valuable to capture the extra user-facing data in a standard structure so it could be rendered in a user-friendly way.
In the diff, I see a mix of using the words "failed" and "errored". Assuming there's not an intentional difference, perhaps we should take this opportunity to standardize on one. "Errored" is a bit awkward to read and more awkward to say, so I'd lean toward going back to the pulp 2 state of just "error", or go with "failed". But my reasoning is only aesthetic, so really any of the choices would be fine. Michael On Thu, Sep 15, 2016 at 5:46 PM, Brian Bouterse <[email protected]> wrote: > In porting the tasking system to work with the new models I have made some > adjustments that I think are improvements. Please send feedback or leave it > directly on the PR. I plan to turn this into documentation at some point as > a separate effort. This applies to methods we will mark in the plugin API > such as sync(), publish() which are run inside of a pulp task dedicated to > calling into the plugin code. > > Here is a recap so everyone is aware and can give feedback: > > == Improvements over Fatal vs Non Fatal Exceptions in Tasking System == > - The tasking system allows non_fatal_errors to be recorded as the task > runs. This is different than before which required any error recording to > be done using the task result. @dkliban pointed out that recording as you > go is better in case you later experience an unexpected, fatal exception. > See the new plugin controller here[0]. > > - Fatal exceptions are any exception raised during Task execution. This is > roughly what we did before but we did a poor job of documenting it. > > - Fatal exceptions are recorded with the exception traceback in the > 'result' attribute. Previously this was recorded in the 'errors' attribute > which caused mixing/overwriting problems between fatal and non-fatal > exceptions. In this new code, if you experience a fatal error the result > *is* your exception. See the implementation here [1] > > - The errors field is renamed to non_fatal_errors to clearly indicate its > semantic purpose to collect non_fatal_errors[2]. It is still a JSON field > but it is now structured due to the use of the plugin controller [0]. It > probably could be more structured, or less. Input would be great here. > > == Less work for plugin writers == > - spawned_tasks are now populated as you dispatch tasks from within a > task. Previously it was the plugin writer's job to name any spawned tasks. > This is one more responsibility moved to platform. See the implementation > wherever this method is used [3]. If you want to dispatch tasks and not > have them in the spawned_tasks list making it inherit from PulpTask[4] > directly will do that. > > == Less code to Maintain == > - We no longer support sigterm handlers and are going to assume that a > task can be killed at any moment. We deprecated this part of the old Plugin > API on the 2.y line even. On 2.y, the tasking system used to have code > which would "install" a sigterm handler callback for that specific task. > That installation code is now deleted [5]. > > - The Task.results field is a JSON field and it stores anything returned > by the task [6]. We used to maintain an object called TaskResult which no > longer has value with all the changes ^. Plugin writers can just return a > JSON serializable python object as a task result. > > [0]: https://github.com/pulp/pulp/pull/2752/files#diff-d378ced2dc > 71e64a3ff4985690c331b4R5 > [1]: https://github.com/pulp/pulp/pull/2752/files#diff-a5f1119f12 > 29212c0027870093b9356cR418 > [2]: https://github.com/pulp/pulp/pull/2752/files#diff-f2e147f346 > d6cb888f808c77a58e84bdR134 > [3]: https://github.com/pulp/pulp/pull/2752/files#diff-a5f1119f12 > 29212c0027870093b9356cR426 > [4]: https://github.com/pulp/pulp/pull/2752/files#diff-a5f1119f12 > 29212c0027870093b9356cR33 > [5]: https://github.com/pulp/pulp/pull/2752/files#diff-abee42f9c3 > a1b498c07cb513ca7cb974L629 > [6]: https://github.com/pulp/pulp/pull/2752/files#diff-a5f1119f12 > 29212c0027870093b9356cR366 > > -Brian > > _______________________________________________ > Pulp-dev mailing list > [email protected] > https://www.redhat.com/mailman/listinfo/pulp-dev >
_______________________________________________ Pulp-dev mailing list [email protected] https://www.redhat.com/mailman/listinfo/pulp-dev
