Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The object type returned by DataLoaderFactory.newDataLoaderWithTry().dispatcher is inconsistent with the specified generic type. #163

Open
nenryo opened this issue Jun 13, 2024 · 1 comment

Comments

@nenryo
Copy link

nenryo commented Jun 13, 2024

Describe the bug
I am new to this library. When learning this library, I found that the object type returned by DataLoaderFactory.newDataLoaderWithTry().dispatcher() is inconsistent with the specified generic type.

To Reproduce
1718273239014

@bbakerman
Copy link
Member

bbakerman commented Jun 14, 2024

You are right - is is inconsistent when the Try mechanism is used

Data loader is declared as DataLoader<K, V> and hence we get public CompletableFuture<List<V>> dispatch()

The Try wrappers are unwrapped by the engine code and hence the CompletableFuture<V> load(K key) values do come back as the expected V type but the dispatch gives back the raw values and does not unwrap them.

                        } else if (value instanceof Try) {
                            // we allow the batch loader to return a Try so we can better represent a computation
                            // that might have worked or not.
                            Try<V> tryValue = (Try<V>) value;
                            if (tryValue.isSuccess()) {
                                future.complete(tryValue.get());
                            } else {
                                stats.incrementLoadErrorCount(new IncrementLoadErrorCountStatisticsContext<>(key, callContext));
                                future.completeExceptionally(tryValue.getThrowable());
                                clearCacheKeys.add(keys.get(idx));
                            }

Perhaps it should unwrap them when giving his back - I would have to think on that more.

The other option would be to have something like DataLoader<K,V,BatchV> where BatchV is the generic type of value coming back from the BatchLoader but thats too viral a change really.

ps. We mostly expect people to use the values via load(k) calls rather than the values coming back via dispatch but you are right its inconsistent.

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

No branches or pull requests

2 participants