Skip to content

fix(java-sdk): fix followDependenciesExecution return type#128

Merged
Skraye merged 1 commit intokestra-io:mainfrom
ICIJ:fix(java-sdk)/follow-dependencies-execution
Nov 20, 2025
Merged

fix(java-sdk): fix followDependenciesExecution return type#128
Skraye merged 1 commit intokestra-io:mainfrom
ICIJ:fix(java-sdk)/follow-dependencies-execution

Conversation

@ClemDoum
Copy link
Copy Markdown
Contributor

What changes are being made and why?


The /api/v1/{tenant}/executions/{executionId}/follow-dependencies API routes returns a stream of ExecutionStatusEvent yet the client code deserialize the event stream as a stream of Execution which ends up in a stream of empty objects when calling KestraClient().builder().executions().followDependenciesExecution(executionId, MAIN_TENANT, false, true) (because property names don't match)

How the changes have been QAed?


Before:

KestraClient().builder().executions().followDependenciesExecution(executionId, MAIN_TENANT, false, true).toStream().toList()

return a list of empty Execution objects (null properties every everywhere

After:

KestraClient().builder().executions().followDependenciesExecution(executionId, MAIN_TENANT, false, true).toStream().toList()

should return a list of valid ExecutionStatusEvent (I wasn't able to test properly as I wasn't able to setup unit test properly)

@github-project-automation github-project-automation Bot moved this to To review in Pull Requests Nov 18, 2025
@ClemDoum ClemDoum changed the title fix(java-sdk): fix followExecution and followDependenciesExecution return types fix(java-sdk): fix followDependenciesExecution return type Nov 18, 2025
@MilosPaunovic MilosPaunovic added area/backend Needs backend code changes kind/external Pull requests raised by community contributors labels Nov 18, 2025
@MilosPaunovic MilosPaunovic requested a review from Skraye November 18, 2025 09:54
@Skraye Skraye self-assigned this Nov 19, 2025
@Skraye
Copy link
Copy Markdown
Member

Skraye commented Nov 19, 2025

Hello, I think we will need an issue and a reproducer because I dont think your changes are correct.
Below, the ExecutionStatusEvent that you did use :
image
as you can see, only the executionId is populated, everything else is null

On the other side, with the current Execution, every properties that should be filled are correctly populated.
image

If you want to do further test on your side, you can use the beginning of the run-tests.sh script to publish a local version of the lib locally and use it on your own project.

Side note for information: Our SDK is auto-generated, so please avoid modifying source files as changes will be overwritten. Specifically, methods using SSE are overridden during generation (see reference here), but we can handle that specific modification internally.

Thanks for the contribution!

@ClemDoum
Copy link
Copy Markdown
Contributor Author

ClemDoum commented Nov 19, 2025

Hi @Skraye, I might be using a different setup than your.
I'm running kestra open source v1.1.4 standalone server.

Then I slightly modified the ExecutionsApiTest.followDependenciesExecution like this (for the purpose of reproducing):

@Test
    public void followDependenciesExecution() throws Exception {
        Execution execution = createdExecution(LOG_FLOW, StateType.SUCCESS);
        String executionId = execution.getId();

        CountDownLatch completionLatch = new CountDownLatch(1);

        kestraClient().executions().followDependenciesExecution(executionId, MAIN_TENANT, false, true)
            .toStream()
            .forEach(e -> assertNotNull(e.getId()));

        kestraClient().executions().followDependenciesExecution(executionId, MAIN_TENANT, false, true)
                .doOnNext(event -> {
                    assertThat(event.getFlowId()).isEqualTo(execution.getFlowId());
                })
                .doFinally(signalType -> {
                    completionLatch.countDown();
                })
                .subscribe();

        boolean completed = completionLatch.await(30, TimeUnit.SECONDS);

        assertThat(completed).isTrue();
    }

I do get the following assertion error:

expected: not <null>
org.opentest4j.AssertionFailedError: expected: not <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertNotNull.failNull(AssertNotNull.java:49)
	at org.junit.jupiter.api.AssertNotNull.assertNotNull(AssertNotNull.java:35)
	at org.junit.jupiter.api.AssertNotNull.assertNotNull(AssertNotNull.java:30)
	at org.junit.jupiter.api.Assertions.assertNotNull(Assertions.java:301)
	at io.kestra.example.ExecutionsApiTest.lambda$followDependenciesExecution$47(ExecutionsApiTest.java:1056)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)

What I don't understand is that the base test doest work event without my addition. If i switch the return type to ExecutionStatusEvent then everything works as expected.

Thank your for pointing out the overwriting issue with the template !

@ClemDoum ClemDoum force-pushed the fix(java-sdk)/follow-dependencies-execution branch from 264ae3e to 00d1c57 Compare November 20, 2025 08:56
@Skraye
Copy link
Copy Markdown
Member

Skraye commented Nov 20, 2025

Checked again, see my error, and you are right!
I'm merging your pull request and will do the adjustement on another commit, thank a lot for your contribution!

Copy link
Copy Markdown
Member

@Skraye Skraye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Skraye
Copy link
Copy Markdown
Member

Skraye commented Nov 20, 2025

@ClemDoum can you rebase your branch so I can merge it ? I dont have permission to commit on your branch

@ClemDoum ClemDoum force-pushed the fix(java-sdk)/follow-dependencies-execution branch from 00d1c57 to ca9e40a Compare November 20, 2025 16:05
@ClemDoum
Copy link
Copy Markdown
Contributor Author

rebase done @Skraye

@Skraye Skraye merged commit 3984410 into kestra-io:main Nov 20, 2025
@github-project-automation github-project-automation Bot moved this from To review to Done in Pull Requests Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/backend Needs backend code changes kind/external Pull requests raised by community contributors

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants