Skip to content

Conversation

@viqtor
Copy link
Contributor

@viqtor viqtor commented Aug 20, 2023

resolves #636

@viqtor viqtor force-pushed the consumer-refresh-errors branch from d9dad86 to 0ea3c95 Compare August 21, 2023 06:33
@Sunjeet
Copy link
Contributor

Sunjeet commented Sep 8, 2023

Thanks for this contribution. Changing the return type of the triggerAsyncRefresh method from void to CompletableFuture<Void> would cause binary backwards incompatibility. Is there a reason to not simply use the synchronous flavor triggerRefresh/triggerRefreshTo in your application?

@Sunjeet Sunjeet self-requested a review September 8, 2023 19:59
@viqtor
Copy link
Contributor Author

viqtor commented Sep 9, 2023

Is there a reason to not simply use the synchronous flavor triggerRefresh/triggerRefreshTo in your application?

We have been following the reference implementation. We have since switched to the sync alternative since dicsovering this issue but it may lead to false assumptions when you pass in a refreshExectutor to your consumer that isn't used.

I just noticed that the same method is also used during the getInitialLoad() which wont trip the catch block.

Would an alternative solution be to deprecate these methods and add new ones with the new return type?

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.

errors during consumer async refresh are hidden

2 participants