Back port of Task Thin Execution List and List By Name #6062

corneil · 2024-11-11T14:50:14Z

Updates api docs for tasks/thinexecutions/name and all HATEOAS links. Updates related tests for links. Improved link handling and tests in TaskTemplate.
Original PR #5994
Issues #5991, #5973

Fixes #5995

onobc

Thanks for the back port @corneil . I have some questions. I notice there is also quite a few files modified in the source that is not in this PR. Please describe the discrepancies.

Thanks

...test/java/org/springframework/cloud/dataflow/server/rest/documentation/ApiDocumentation.java

spring-cloud-dataflow-docs/src/main/asciidoc/api-guide.adoc

...w-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskTemplate.java

...-core/src/main/java/org/springframework/cloud/dataflow/server/controller/RootController.java

.../java/org/springframework/cloud/dataflow/server/controller/TaskExecutionControllerTests.java

src/scripts/build-docs.sh

corneil · 2024-11-11T16:13:26Z

I have resolved comments. There are a few elements I'll port to main to get parity.

...va/org/springframework/cloud/dataflow/server/config/DataFlowControllerAutoConfiguration.java

Updated task thin executions link handing and added extra test for parity with 2.11.x See spring-cloud#6062

Updated task thin executions link handing and added extra test for parity with 2.11.x. Update minimum supported version of client. See spring-cloud#6062

onobc · 2024-11-12T16:50:01Z

...w-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskTemplate.java

-		this.thinExecutionsByNameLink = resources.getLink(THIN_EXECUTIONS_BY_NAME_RELATION).orElse(null);
-		this.executionLaunchLink = resources.getLink(EXECUTION_LAUNCH_RELATION).orElse(null);
-		this.executionsCurrentLink = resources.getLink(EXECUTIONS_CURRENT_RELATION).orElse(null);
+		if(VersionUtils.isDataFlowServerVersionGreaterThanOrEqualToRequiredVersion(version, VALIDATION_THIN_TASK_VERSION) && resources.getLink(THIN_EXECUTIONS_RELATION).isPresent()) {


I would consider moving this logic into a well-named helper method to avoid the repeat checks as its hard to read currently.

Removed the resources.getLink(THIN_EXECUTIONS_RELATION).isPresent() from if

onobc · 2024-11-12T16:53:22Z

...w-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskTemplate.java

@@ -165,12 +181,7 @@ public class TaskTemplate implements TaskOperations {
 			this.executionLaunchLink = null;
 		}

-		if (VersionUtils.isDataFlowServerVersionGreaterThanOrEqualToRequiredVersion(version, EXECUTIONS_CURRENT_RELATION_VERSION)) {


Why did this move up? If its not required to be at the top, please keep it in place so as to avoid confusing during the review.

I fixed the order to match the previous version.

I would prefer change history over matching order in a different branch. Please undo the change.

Will a squash into on commit not fix this so that the diff is smaller because the order is that same as before the PR branch?

onobc · 2024-11-12T16:54:29Z

...w-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskTemplate.java

@@ -283,18 +294,26 @@ public TaskExecutionResource.Page executionList() {
 	}

 	@Override
-	public PagedModel<TaskExecutionThinResource> thinExecutionList() {
+	public TaskExecutionThinResource.Page thinExecutionList() {


Signature change? Seems breaking.

The result is the same type.

How is that?

spring-cloud-dataflow/spring-cloud-dataflow-rest-resource/src/main/java/org/springframework/cloud/dataflow/rest/resource/TaskExecutionThinResource.java

Line 185 in 464868b

public static class Page extends PagedModel<TaskExecutionThinResource> {

Well if they are the same let's leave it as is since this is a simple backport on what will be a patch release. What is the value in changing it?

The other uses on tasks/execution uses the .Page I noticed when comparing the various tasks/executions and tasks/thinexecutions controller methods.

This is a lot cleaner as a result return restTemplate.getForObject(executionsLink.getHref(), TaskExecutionThinResource.Page.class);

onobc · 2024-11-12T16:54:57Z

src/scripts/build-docs.sh

@@ -4,7 +4,7 @@ if [ -z "$BASH_VERSION" ]; then
    exit 1
 fi
 SCDIR=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
-SCDIR=$(realpath $SCDIR)
-pushd "$SCDIR" > /dev/null || exit
+SCDIR=$(realpath "$SCDIR/../..")


I still don't understand what this has to do w/ thin executions

I'll move this to a separate PR

...w-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskTemplate.java

onobc · 2024-11-13T15:15:52Z

...w-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskTemplate.java

@@ -66,16 +65,17 @@ public class TaskTemplate implements TaskOperations {

 	private static final String EXECUTIONS_CURRENT_RELATION_VERSION = "1.7.0";

-	private static final String VALIDATION_RELATION_VERSION = "1.7.0";


Why is this deleted? Please add a comment.

I restored that.

I don't see it.

Updated task thin executions link handing and added extra test for parity with 2.11.x. Update minimum supported version of client. Added test for invalid old version. See spring-cloud#6062

onobc · 2024-11-14T13:55:02Z

...w-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskTemplate.java

@@ -134,51 +136,51 @@ public class TaskTemplate implements TaskOperations {
 				EXECUTION_RELATION_BY_NAME,
 				EXECUTIONS_INFO_RELATION,
 				PLATFORM_LIST_RELATION,
-				RETRIEVE_LOG
+				RETRIEVE_LOG,
+				VALIDATION_REL


What is this now?

This was rolled back in the version I want to push.

Updated task thin executions link handing and added extra test for parity with 2.11.x. Update minimum supported version of client. Added test for invalid old version. See spring-cloud#6062

Updated task thin executions link handing and added extra test for parity with 2.11.x See spring-cloud#6062 Update Task Thin Executions list function for parity with 2.11.x Updated task thin executions link handing and added extra test for parity with 2.11.x. Update minimum supported version of client. See spring-cloud#6062 Update Task Thin Executions list function for parity with 2.11.x Updated task thin executions link handing and added extra test for parity with 2.11.x. Update minimum supported version of client. See spring-cloud#6062 Update Task Thin Executions list function for parity with 2.11.x Updated task thin executions link handing and added extra test for parity with 2.11.x. Update minimum supported version of client. See spring-cloud#6062 Revised TaskTemplate to retain order of link init in the constructor. Update Task Thin Executions list function for parity with 2.11.x Updated task thin executions link handing and added extra test for parity with 2.11.x. Update minimum supported version of client. Added test for invalid old version. See spring-cloud#6062

corneil added the review/at-least-one Only one reviewer is required to merge this PR. label Nov 11, 2024

corneil requested review from cppwfs and onobc November 11, 2024 14:50

corneil force-pushed the 2.11.x branch from 2d6b7a3 to ad21f25 Compare November 11, 2024 14:52

onobc added the type/backport Is a issue to track backport, use with branch/xxx label Nov 11, 2024

onobc self-assigned this Nov 11, 2024

onobc requested changes Nov 11, 2024

View reviewed changes

onobc reviewed Nov 11, 2024

View reviewed changes

...va/org/springframework/cloud/dataflow/server/config/DataFlowControllerAutoConfiguration.java Show resolved Hide resolved

corneil added a commit to corneil/spring-cloud-dataflow that referenced this pull request Nov 12, 2024

Update Task Thin Executions list function for parity with 2.11.x

d0fc313

Updated task thin executions link handing and added extra test for parity with 2.11.x See spring-cloud#6062

corneil added a commit to corneil/spring-cloud-dataflow that referenced this pull request Nov 12, 2024

Update Task Thin Executions list function for parity with 2.11.x

364414b

Updated task thin executions link handing and added extra test for parity with 2.11.x See spring-cloud#6062

corneil mentioned this pull request Nov 12, 2024

Update Task Thin Executions list function for parity with 2.11.x #6063

Closed

corneil requested a review from onobc November 12, 2024 12:45

onobc reviewed Nov 12, 2024

View reviewed changes

corneil force-pushed the corneil/gh-5995 branch from 8118ada to ea8bfd3 Compare November 13, 2024 08:43

onobc reviewed Nov 13, 2024

View reviewed changes

...w-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskTemplate.java Show resolved Hide resolved

onobc reviewed Nov 13, 2024

View reviewed changes

onobc reviewed Nov 14, 2024

View reviewed changes

corneil requested a review from onobc November 14, 2024 13:56

Update Task Thin Executions list function for parity with 2.11.x

198bf9d

Updated task thin executions link handing and added extra test for parity with 2.11.x. Update minimum supported version of client. Added test for invalid old version. See spring-cloud#6062

corneil force-pushed the corneil/gh-5995 branch from ea8bfd3 to 198bf9d Compare November 14, 2024 14:11

onobc merged commit d7ff312 into spring-cloud:2.11.x Nov 14, 2024
2 of 3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Back port of Task Thin Execution List and List By Name #6062

Back port of Task Thin Execution List and List By Name #6062

corneil commented Nov 11, 2024 •

edited

Loading

onobc left a comment

corneil commented Nov 11, 2024

onobc Nov 12, 2024

corneil Nov 13, 2024

onobc Nov 12, 2024

corneil Nov 13, 2024

onobc Nov 13, 2024

corneil Nov 14, 2024

onobc Nov 12, 2024

corneil Nov 13, 2024

onobc Nov 13, 2024

corneil Nov 14, 2024

onobc Nov 14, 2024

corneil Nov 14, 2024

corneil Nov 14, 2024

onobc Nov 12, 2024

corneil Nov 13, 2024

onobc Nov 13, 2024

corneil Nov 14, 2024

onobc Nov 14, 2024

onobc Nov 14, 2024

corneil Nov 14, 2024

		@@ -66,16 +65,17 @@ public class TaskTemplate implements TaskOperations {

		private static final String EXECUTIONS_CURRENT_RELATION_VERSION = "1.7.0";

		private static final String VALIDATION_RELATION_VERSION = "1.7.0";

Back port of Task Thin Execution List and List By Name #6062

Back port of Task Thin Execution List and List By Name #6062

Conversation

corneil commented Nov 11, 2024 • edited Loading

onobc left a comment

Choose a reason for hiding this comment

corneil commented Nov 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

corneil commented Nov 11, 2024 •

edited

Loading