Skip to content

Conversation

@andimarek
Copy link
Member

This simplifies the engine tracking:

We really only need to take care of CF that are externally contributed to the engine and the sync code in start. This means we don't need most call/run code, but rather need to handle the CF returned by DataFetcher or instrumentation correctly.

Also: this changes the code to have zero impact when no EngineStateObserver is registered.

@andimarek andimarek added this to the 23.0 breaking changes milestone Apr 5, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2025

Test Results

  313 files    313 suites   53s ⏱️
3 588 tests 3 583 ✅ 5 💤 0 ❌
3 677 runs  3 672 ✅ 5 💤 0 ❌

Results for commit 59195c1.

♻️ This comment has been updated with latest results.

// allow instrumentation to tweak the result
executionResult = executionResult.thenCompose(result -> instrumentation.instrumentExecutionResult(result, instrumentationParameters, instrumentationState));
return executionResult;
return instrumentation.instrumentExecutionResult(abortException.toExecutionResult(), instrumentationParameters, instrumentationState);
Copy link
Member

Choose a reason for hiding this comment

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

yeah that was weird code

Copy link
Member

@bbakerman bbakerman left a comment

Choose a reason for hiding this comment

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

I approve generally but some code to be cleaned up

@dondonz dondonz merged commit 154bb89 into master Apr 7, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants