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

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 * **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): class AsyncTask(object): def __init__(self, id, target_uri, fn, objstore, kill_cb=None, opaque=None): 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'. 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/

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.
* **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.
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.
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/
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

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/
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Paulo Ricardo Paz Vital Linux Technology Center, IBM Systems http://www.ibm.com/linux/ltc/

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/
_______________________________________________
-- Paulo Ricardo Paz Vital Linux Technology Center, IBM Systems http://www.ibm.com/linux/ltc/

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/
_______________________________________________

Ok, the thing is harder then expected. To implement the proposed solution, it's necessary some changes in AsyncTask calls and control. So, after some discussions on scrum meetings (on our IRC channel) and other discussions about the topic, was agreed that we need keep a reference of each instance/object of AsyncTask created and to prevent some issues, like memory leak and increase size database, changes are necessary in how AsyncTask information is stored. A new issue (https://github.com/kimchi-project/wok/issues/158) was created to solve this points, first. Basically, no changes in API are required but only how tasks are added and AsyncTask stores/gets the information of a task. A dictionary will be created to control the AsyncTasks objects and once a task is finished (with success or not) this entry will be deleted, and the usage of objectstore will be removed. Probably, few changes in all plugins that are using add_task() from wok.utils will be necessary. But all updates will be submitted to ML, if necessary. Patches related to issue #158 will be submitted by tomorrow. Thanks and best regards, Paulo. On Aug 15 09:58AM, Daniel Henrique Barboza wrote:
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/
_______________________________________________
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Paulo Ricardo Paz Vital Linux Technology Center, IBM Systems http://www.ibm.com/linux/ltc/
participants (3)
-
Aline Manera
-
Daniel Henrique Barboza
-
Paulo Ricardo Paz Vital