Add batching support

[patrick91]

trafficstars

Needs validation and configuration options

[codecov[bot]]

Codecov Report

Merging #2698 (9d62fce) into main (efcd602) will increase coverage by 0.01%. The diff coverage is 96.66%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2698      +/-   ##
==========================================
+ Coverage   96.11%   96.12%   +0.01%     
==========================================
  Files         211      211              
  Lines        9232     9264      +32     
  Branches     1489     1500      +11     
==========================================
+ Hits         8873     8905      +32     
  Misses        229      229              
  Partials      130      130              

[luisbc92]

Hi!

I'm currently running my project on top of your feature branch as I transitioned from Graphene over to Strawberry before learning that batching was not supported.

Everything works great! I have yet to run into any issues. Is there a particular reason why integration into main has stalled?

Thank you!

[patrick91]

@luisbc92 just time! thanks for the ping, I'll add to my list of things to do in the next few days 😊

[luisbc92]

Wow! Fastest response I've ever gotten. You should get a trophy.

Thank you for your work!

[patrick91]

@luisbc92 @Speedy1991 @mikoda1995 do you have any opinions on the configuration options? /cc @bellini666 @erikwrede

[erikwrede]

I prefer an explicit BatchingConfig.enable_batching over not schema.batching_config <=> batching disabled.

Will do full review later

[bellini666]

I prefer an explicit BatchingConfig.enable_batching over not schema.batching_config <=> batching disabled.

Will do full review later

I agree with @erikwrede here. Just maybe BatchingConfig.enabled since batching is already implied.

Also, maybe use a TypedDict instead of a dataclass in here? In general I prefer dataclasses, but typeddict would allow to pass the configuration without having to import BatchingConfig and still be properly typed. No strong opinions about this, just a thought that I always have when doing stuff like that =P

Regarding everything else, I took a quick look and it seems fine! :)

[patrick91]

Also, maybe use a TypedDict instead of a dataclass in here? In general I prefer dataclasses, but typeddict would allow to pass the configuration without having to import BatchingConfig and still be properly typed. No strong opinions about this, just a thought that I always have when doing stuff like that =P

I think that would be quite nice, we should do it for the config class too, maybe in another PR (with deprecation and codemod I guess :) )

[codspeed-hq[bot]]

CodSpeed Performance Report

Merging #2698 will not alter performance

_{Comparing feature/batching-2023 (9d62fce) with main (efcd602)}

Summary

✅ 1 untouched benchmarks

[cpd67]

Hi! :smiley: This would be an awesome feature to have soon, as we're currently thinking of transitioning our project over to using Strawberry and batching is a feature we use a lot. If there's anything I could do to help, let me know and i'd be happy to assist!