search

spring-cloud / spring-cloud-dataflow

A microservices-based Streaming and Batch data processing in Cloud Foundry and Kubernetes
https://dataflow.spring.io
Apache License 2.0
1.11k stars 579 forks source link

Getting a Page of TaskExecutionResources Using the DataFlowTemplate's taskOperations. #4051

Closed JustinLiAtSBU closed 4 years ago

JustinLiAtSBU commented 4 years ago

Problem description: If I wanted to implement some pagination for TaskExecutionResources, I have to use the methods in the DataFlowTemplate, such as executionListByTaskName().getContent(). Then I would use Page, Pageable, and PageImpl to create a page out of the Collection that was returned from the function. However, since PageImpl requires a list, I would have to convert the collection into an ArrayList. This is very inefficient and could get extremely slow if there was a lot of task executions. What's more inefficient is that what I have to do to get a page is to

  1. Pull ALL of the task executions from the database
  2. Convert a collection into an array list
  3. Getting a sublist of them to return as a "page" However, this seems to be the only way of implementing pagination.

Solution description: A solution that I would like to see is another function in the DataFlowTemplate that is similar to DataFlowTemplate.taskOperations().executionListByTaskName(taskName). However, this new function would have additional parameters for page size and page number, or a Pageable object. The goal of this new function would be to allow the user to easily and efficiently implement pagination on TaskExecutionResources so that they can be displayed on a dashboard or something.

Description of alternatives: My alternative solution was described above.

Additional context: If anything I wrote was unclear, I also made a StackOverflow post here that might describe this better.

sabbyanandan commented 4 years ago

@JustinLiAtSBU: Thanks for reporting! All this sounds like a reasonable enhancement request. If you have cycles, please feel free to take a stab at it and start the dialog with the pull request — we could collaborate to get this improved.

JustinLiAtSBU commented 4 years ago

Hi Sabby. To be completely honest I’m not familiar with the source code of spring, but I’ll take a look at the code and see if there’s anything I can do. I have just cloned it on my local.

On Fri, Jul 17, 2020 at 4:08 PM Sabby Anandan notifications@github.com wrote:

@JustinLiAtSBU https://github.com/JustinLiAtSBU: Thanks for reporting! All this sounds like a reasonable enhancement request. If you have cycles, please feel free to take a stab at it and start the dialog with the pull request — we could collaborate to get this improved.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spring-cloud/spring-cloud-dataflow/issues/4051#issuecomment-660315049, or unsubscribe https://github.com/notifications/unsubscribe-auth/AM33TF5VKWB2GZ533HDDKG3R4CVVTANCNFSM4O4LPWPA .

sabbyanandan commented 4 years ago

@JustinLiAtSBU: Thanks for considering. I am going to tag @cppwfs and @ilayaperumalg to help guide you with the relevant changes.

JustinLiAtSBU commented 4 years ago

@sabbyanandan Great. Thank you.

cppwfs commented 4 years ago

Hello @JustinLiAtSBU , There are 3 classes that require an update:

  1. You need to add a new function to the TaskOperations for the executionListByTaskName found here: https://github.com/spring-cloud/spring-cloud-dataflow/blob/master/spring-cloud-dataflow-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskOperations.java#L109-L115 that would pass in page sizes and page.
  2. Add the implementation to the TaskTemplate for the executionListByTaskName https://github.com/spring-cloud/spring-cloud-dataflow/blob/master/spring-cloud-dataflow-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskOperations.java#L109-L115 but with the parameters required.
  3. Update the tests : https://github.com/spring-cloud/spring-cloud-dataflow/blob/master/spring-cloud-dataflow-rest-client/src/test/java/org/springframework/cloud/dataflow/rest/client/TaskTemplateTests.java

If you can submit a PR for this that would be awesome!
If not you can still hit the RESTful api directly as discussed here: https://docs.spring.io/spring-cloud-dataflow/docs/current/reference/htmlsingle/#api-guide-resources-task-executions-list-by-name

JustinLiAtSBU commented 4 years ago

Thanks for sharing this. I’m going to start working on this once I get off work.

On Tue, Jul 21, 2020 at 10:01 AM Glenn Renfro notifications@github.com wrote:

Hello @JustinLiAtSBU https://github.com/JustinLiAtSBU , There are 3 class that require an update:

If you can submit a PR for this that would be awesome! If not you can still hit the RESTful api directly as discussed here: https://docs.spring.io/spring-cloud-dataflow/docs/current/reference/htmlsingle/#api-guide-resources-task-executions-list-by-name

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spring-cloud/spring-cloud-dataflow/issues/4051#issuecomment-661879849, or unsubscribe https://github.com/notifications/unsubscribe-auth/AM33TF4FBKCYJ3TXJHFVLYLR4WNUFANCNFSM4O4LPWPA .

siddhantsorann commented 4 years ago

@JustinLiAtSBU Are you working on this? I don't mind giving this a shot. @cppwfs Does this only need the changes you specified above? A new method to get executionListByTaskName as a Pageable object?

siddhantsorann commented 4 years ago

@sabbyanandan Can I start work on this ticket? Seems doable.

sabbyanandan commented 4 years ago

Yes, please! Any help is appreciated.

siddhantsorann commented 4 years ago

Hey @sabbyanandan @cppwfs I have raised a draft PR for this issue, could you please check if I'm on the right track there? Might not have exactly understood the issue. Can't find where the exposed method in DataFlowTemplate will be used. Thanks.