Vertx and Dataloader

[zenios]

When graphql is executed inside vertx we have the option to batch load with a callback function that will run inside vertx event-loop and hence mimic how the original data loader implementation works

The reason i would like to implement such functionality is not the performance of data loader as such but instead having the option to wrap data-loader inside a CompletableFuture.

This is very useful in cases you would like to rewrite all the data-fetchers (using transformation) and implement a rate limiter that will first do an async function to check if you are rate limited and then delegate to the original data-fetcher that might execute a dataloader.load

The issue i have faced with developing such implementation is the DataLoader class. Since its not an interface it is quite difficult to offer an alternative implementation

What do you think should be the way forward in order to make the DataLoader an interface with the least minimum breaking changes?

[bbakerman]

The origins of this code came from vertx.

https://github.com/engagingspaces/vertx-dataloader

It's too late to make DataLoader into an interface without a major major refactor. I cant see a way forward on that

[zenios]

I just had a look at the code

Even though vertx was used it was only used for its future and CompositeFuture capabilities and not for its eventloop

I have a reference implementation using vertx.runOnContext that behaves exactly like original implementation and works flawlessly

What if as a first step we convert the DataLoader into an interface with the same methods and the existing implementation as DataLoaderImpl.

All public static newDataLoaderXXX will be added to the interface that will in turn call the protected constructor of the implementation.

Finally all the dispatch methods will stay as is and any implementation that does not need them will return an empty CompletableFuture.

Thoughts?

[bbakerman]

The idea of breaking DataLoader into an interface is becoming more attractive. I inherited as a single class but it has grown in capability and it should have a been an interface in hindsight.

The issue i have faced with developing such implementation is the DataLoader class. Since its not an interface it is quite difficult to offer an alternative implementation

is this because you want all the normal DataLoaderRegistry / graphql-java field tracking but you want to have an alternative DataLoader implementation??

Do you even want the graphql-java field tracking for dispatch or would you use another mechanism (vertx event loop)??

if so would you expect the new DataLoader interface to have a "dispatch" method on it?

[zenios]

is this because you want all the normal DataLoaderRegistry

Exactly i just want to use the same functionality of getting the dataloader as the docs (environment.getDataLoader)

Do you even want the graphql-java field tracking for dispatch or would you use another mechanism (vertx event loop)??

Its not needed in my case actually my implementation right now returns an empty list

if so would you expect the new DataLoader interface to have a "dispatch" method on it?

To tell you the truth i don't mind if a dispatch method exists in the interface and documented that its there in case someone would like to implement a dataloader using field tracking

The ultimate goal is that such dataloader implementation will be pushed upstream to vertx and offered as an alternative method for batch loading.

[bbakerman]

The refactoring on this is too large - I had a interface branch and it just did not offer the return on investment and breaking change that I wanted. I have parked the idea of a DataLoader interface