Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add find_task_logs function #160

Closed
nadouani opened this issue Jun 4, 2020 · 7 comments · Fixed by #200
Closed

Add find_task_logs function #160

nadouani opened this issue Jun 4, 2020 · 7 comments · Fixed by #200
Assignees
Milestone

Comments

@nadouani
Copy link
Contributor

nadouani commented Jun 4, 2020

Request Type

Feature Request

Problem Description

This function allows searching for task logs

@nadouani nadouani added this to the 1.8.0 milestone Jun 4, 2020
@nadouani nadouani self-assigned this Jun 4, 2020
@jnahorny
Copy link

@nadouani I've found this issue when searching for a solution, after I've discovered get_task_logs() method only returns 10 results, with no option to change it.

I already wrote the version that behaves just like get_case_tasks() (allows you to pass params and additional query criteria).
I would happily sent it as a pull request, just want to discuss one thing -- name of the method.
I believe this should replace existing get_task_logs() implementation, for the sake of naming consistency.
I mean, if get_case_tasks() returns all the tasks, why does get_task_logs() not return all the logs? I'd say it's even more useful, as I think there's higher probability of having > 10 log entries in a task, than having > 10 tasks in the case. Isn't it?
Also, this woudn't break compatibility, because if you don't provide params, nor query, you'd call the method exactly the way it is now.

But, if you prefer other name (find_task_logs()?), and you want to keep existing get_task_logs() as is, I'm OK with that as well.

Thank you!

@jnahorny
Copy link

I had to delete the original PR, because I've messed something up when doing the rebase. This issue is now addressed in the following PR: #200

@nadouani
Copy link
Contributor Author

nadouani commented Nov 17, 2020

Hello @jnahorny #200 is perfect.

could you provide two functions:

  • find_tasks_logs(**attributes): generic function to search for logs
  • get_task_logs(task_id, **attributes) that calls find_tasks_logs by specifying the ParentId` criteria

If you don't have time, I can make it too, using your PR.

@jnahorny
Copy link

Hello @jnahorny #200 is perfect.

could you provide two functions:

  • find_tasks_logs(**attributes): generic function to search for logs
  • get_task_logs(task_id, **attributes) that calls find_tasks_logs by specifying the ParentId criteria

If you don't have time, I can make it too, using your PR.

@nadouani It's not that I don't have time, but I'm not sure how to do it. I guess it would be probably faster for you to do it, instead of explaining to me how to do it. But I'm open to both scenarios :)

@nadouani
Copy link
Contributor Author

Sounds good, I'll do it using your PR ;)

@jnahorny
Copy link

Sounds good, I'll do it using your PR ;)

Thank you! I can't wait for it to be in production!

@nadouani
Copy link
Contributor Author

@jnahorny take a look to b009365 to understand what I meant.

Thanks for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants