Feature request for ltc delete-task & adding list of task in ltc list

[asifdxtreme]

As discussed in #124 I have done the code changes and here is quick view of my Implementation

root@ubuntu:~# ltc list
------------------------------= Apps =-------------------------------
App Name        Instances       DiskMB          MemoryMB        Route
lattice-app     0/1             0               128             lattice-app.192.168.11.11.xip.io, lattice-app-8080.192.168.11.11.xip.io => 8080

------------------------------= Tasks =------------------------------
Task Name       Cell ID         Status          Result          Failure Reason
ltc-EchoTask    lattice-cell-01 COMPLETED       N/A             failed to initialize container

root@ubuntu:~# ltc dt -h
NAME:
   delete-task, dt - Deletes the given task

USAGE:
   ltc delete-task TASK_NAME

root@ubuntu:~# ltc dt
Incorrect Usage: Please input a valid TASK_GUID
root@ubuntu:~# ltc delete-task ltc-EchoTask
Deleting the task ltc-EchoTask
OK
root@ubuntu:~# ltc delete-task InvalidTaskGuid
Deleting the task InvalidTaskGuid
Error Deleting the task InvalidTaskGuid
Failiure Reason :Task not found.

@davidwadden please review the code and give us the feedback if it require any changes.

Thanks
Asif

[cfdreddbot]

Hey asifhuawei!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/95418526.

[coveralls]

Coverage decreased (-0.12%) to 94.2% when pulling 086bca6 on asifhuawei:hwcf-issue-11 into b40b45a on cloudfoundry-incubator:develop.

[davidwadden]

we're looking at this now. one quick item:

• can you move app_examiner.ListTasks and app_examiner.TaskInfo over to task_examiner/task_examiner.go -- i realize this means the app_examiner_command_factory.go will now have a task_examiner import; we're okay with that for now.

[davidwadden]

and another note- if you want commits to get picked up in our tracker, i believe the story id's need to be surrounded with square brackets., i.e., [#12345678]

[coveralls]

Coverage increased (+0.2%) to 94.52% when pulling d6932ea on asifhuawei:hwcf-issue-11 into b40b45a on cloudfoundry-incubator:develop.

[asifdxtreme]

@davidwadden thanks for the feedback, we will make the changes and we will keep in mind about the story id's while raising the PR

[davidwadden]

another couple notes for delete-task:

1. rename the methods fromTaskDelete to DeleteTask; this applies to both MakeTaskDeleteCommand() as well as taskDelete()

2. Move the delete-task functionality from the task_examiner_command_factory.go to the task_runner_command_factory.go; the idea of these two (and the same is mostly true for apps), the examiner doesn't change the state of the system and the runner does.

[coveralls]

Coverage increased (+0.2%) to 94.52% when pulling d6932ea on asifhuawei:hwcf-issue-11 into b40b45a on cloudfoundry-incubator:develop.

[asifdxtreme]

Thanks @davidwadden for the review, we will move the functionality in the appropriate packages

[coveralls]

Coverage increased (+0.2%) to 94.53% when pulling 9c4fae2 on asifhuawei:hwcf-issue-11 into 405ad35 on cloudfoundry-incubator:develop.

[coveralls]

Coverage increased (+0.2%) to 94.53% when pulling 9c4fae2 on asifhuawei:hwcf-issue-11 into 405ad35 on cloudfoundry-incubator:develop.

[asifdxtreme]

Thanks @davidwadden for your valuable feedback, I have made the changes according to the review comments, please review the code.

Thanks
Asif

[coveralls]

Coverage increased (+0.2%) to 94.53% when pulling 0f577a1 on asifhuawei:hwcf-issue-11 into 405ad35 on cloudfoundry-incubator:develop.

[davidwadden]

Thanks a ton for turning this around on quick notice ! =)

[asifdxtreme]

Thanks @davidwadden for believing in us and merging this PR, it really encourage us to make more contributions to this project. I was going through the lattice tracker and came across this story #95422348 which looks similar to this pr, I am thinking of doing this as it is already been marked with "community" tag.

[davidwadden]

thank you, you guys have been a great help making lattice better

go ahead and work something up. we'll consider it

if you're feeling ambitious with the task thing, we have #95526084 - create integration tests for tasks

this assumes you've put together some kind of task json that actually can fire up and complete (i haven't spent much time with it myself) -- add another test case in to ltc test (integration_test_runner.go) that does something like:

1. ltc submit-task <sample_json>
2. ltc logs <task_guid>
3. polls ltc task for COMPLETED <task_guid>
4. ltc delete-task <task_guid>
5. verifies task is deleted

or start with some sub-set of that if you want some feedback

regards,
david

[asifdxtreme]

Sure @davidwadden I will try to complete both the stories by next week Tuesday

Thanks
Asif

[AmitRoushan]

Hi @davidwadden,

We have completed implementation of #95422348 and raised a PR for same.
We are working on integration tests for tasks.

Thanks for considering us for this task.

Thanks and Regards
@AmitRoushan

PS:Asif got held up in another internal task so I picked it.