-
Notifications
You must be signed in to change notification settings - Fork 10
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
Use a BatchManger explictly outside of hooks #110
Comments
Rethinking this, the above code would not really work. My main goal of this would be to use it server side to batch calls together so that they can easily share an const loader = new AppLoader({ app });
const actions = [
app.service('users').find({ loader }),
app.service('items').find({ loader })
];
const results = await Promise.all(actions); // Provided batch-service creates a loader and passes it
// to all of its batched calls
const actions = [
['users', 'find'],
['items', 'find'],
];
const results = await app.service('batch-service').create(actions); So I am just spitballing ideas to allow the developer to call the batch service in a more familiar way and outside of hooks. Maybe something like const result = await BatchManager.all((service) => {
return [
service('users').find(),
service('items').find(),
]
}) This is likely not even needed. I just wanted to capture my thoughts on it. But, I really like the idea of using this serverside. I can see some huge performance boosts when using resolvers. If all of the resolvers were batched, then they would go through that batch-service hooks first which could pass the same |
A simple solution to this on the server side would be to simply allow the create method to accept promises instead of just config. We can already use the service server side like app.service('batch').create({ calls: [
['users', 'find', {}],
['posts', 'find', {}]
]}) We could update this to also accept promises. They are functionally the same, but using promises is more natural app.service('batch').create({ calls: [
app.service('users').find(),
app.service('posts').find()
]}) This would be a simple change here: Line 20 in 4c924e1
if (typeof payload === 'function') {
return payload;
}
const [method, serviceName, ...args] = payload;
...rest That accomplishes the main goal on the server, which is to be able to use it with simpler syntax. |
This is a feature idea and not a bug
I know that I can use the
excludes
andtimeout
options to configure the batchClient to behave a certain way. But I have found some use cases where I think it would be nice to use some kind of static method to run some actions in a batch. For example,This could also be used server side. For example, the
Promise.all(actions)
does not allow the actions to share a dataloader from feathers-batchloader (unless you manually created one and passed to it, which is a perfectly viable option). UsingBatchManger.all(actions, managerConfig)
would go through the batch-service hooks and automatically share a dataloader (if setup, of course) and anything else that the batch-service may help out with.The text was updated successfully, but these errors were encountered: