[Kimchi-devel] [Wok] [RFC] Issue #122: Make AsyncTask stoppable.

[Daniel Henrique Barboza]

I have the same reservations as Aline regarding the API change. You'll
need to check every single task API call of all plug-ins (Gingerbase Ginger
Gingers390x and Kimchi) to be sure that a plug-in isn't using the kill_cb
parameter by accident.


Other than that, I am OK with this proposal.


Daniel

On 08/15/2016 09:48 AM, Paulo Ricardo Paz Vital wrote:
> Refreshing the memory and waiting for new comments. If no new comments, I'll
> submit a patch with this proposal until the end of this week.
>
> Best regards, Paulo.
>
> On Jun 09 01:07PM, Paulo Ricardo Paz Vital wrote:
>> On Jun 08 03:38PM, Aline Manera wrote:
>>> Hi Paulo,
>>>
>>> On 06/06/2016 11:21 AM, Paulo Ricardo Paz Vital wrote:
>>>> This is an RFC for "Make AsyncTask stoppable" in Wok (and any other plugin). The
>>>> main idea is give to user a way to stop a Task that is still running, cleaning
>>>> all references and processes and setting the Task status to "killed".
>>>>
>>>> To do this, the Task API need to be modified, by implementing the DELETE method
>>>> in Task Resource. Said that, the API will look like:
>>>>
>>>> ### Resource: Task
>>>>
>>>> **URI:** /tasks/*:id*
>>>>
>>>> A task represents an asynchronous operation that is being performed by the
>>>> server.
>>>>
>>>> **Methods:**
>>>>
>>>> * **GET**: Retrieve the full description of the Task
>>>>       * id: The Task ID is used to identify this Task in the API.
>>>>       * status: The current status of the Task
>>>>           * running: The task is running
>>>>           * finished: The task has finished successfully
>>>>           * failed: The task failed
>>>> +        * killed: The task was killed by user
>>>>       * message: Human-readable details about the Task status
>>>>       * target_uri: Resource URI related to the Task
>>>> +* **DELETE**: Delete the Task
>>> It is good to mention that this action will put the Task in the 'killed'
>>> status.
>>>
>> Ok.
>>
>>>> * **POST**: *See Task Actions*
>>>>
>>>> **Actions (POST):**
>>>>
>>>> *No actions defined*
>>>>
>>>> Since an AsyncTask is basic a thread running in the system and this thread can
>>>> execute a pure Python method or a background command (by using run_command()
>>>> from wok.utils), the developer must pass as argument to the add_task() a method
>>>> to be executed by the DELETE operation, called here as 'kill_cb'. With this
>>>> idea, the add_task() method and also the AsyncTask constructor will be like:
>>>>
>>>> def add_task(target_uri, fn, objstore, kill_cb=None, opaque=None):
>>> I am not sure it is a good idea to change the parameters order.
>>> Some plugins may have using the opaque parameter as the third value so it
>>> will cause issues.
>>> So unless, you check all references to make sure it will not be a problem,
>>> I'd recommend to move the kill_cb as the last parameter.
>>>
>> I already was expecting this, so this is not a problem to me to check and change
>> the sequence of parameters. Will not do this only if the modifications are too
>> many to do ;-)
>>
>>>> class AsyncTask(object):
>>>>       def __init__(self, id, target_uri, fn, objstore, kill_cb=None, opaque=None):
>>> Same I commented above.
>>>
>>>> If none kill_cb is passed, the task will not be able to stopped and an error (or
>>>> warning) message will be raised to user if DELETE operation is executed.
>>>> Otherwise, the kill_cb method will be executed by kill() method (responsible to
>>>> execute the DELETE operation) of AsyncTask class and its status set to 'killed'.
>>> Go for it! =)
>>>
>>> Don't forget to add tests to test/exemplify how it will really work.
>>>
>> For sure.
>>
>>>> If you have any comments or suggestions, please feel free to reply to ML.
>>>>
>>>> Thanks and bestr regards,
>>>> --
>>>> Paulo Ricardo Paz Vital
>>>> Linux Technology Center, IBM Systems
>>>> http://www.ibm.com/linux/ltc/
>>>>
>>>> _______________________________________________