Skip to content

Conversation

@rmosolgo
Copy link
Owner

@rmosolgo rmosolgo commented Jan 13, 2026

Implementing loads: has had a smell in it:

context.dataloader.run_isolated do

context.dataloader.run_isolated do

It's there because some code requires the argument values to be returned right away -- it doesn't use Dataloader (properly?).

This smell manifests the problem described in #5462 -- I added a spec for that issue in cf3a3f8

This problem also surfaced in #5479 because, when using Async, run_isolated isn't actually isolated: Async uses a top-level pool, so a pause in one place in the code can resume code elsewhere.

The smell was also called out in #5389 (comment).

When working on #5389, I didn't actually rework this code. But I think it's actually a good place to try out that new flow in an incremental way.

Fixes #5462

TODO:

  • Remove another Argument-related run_isolated call
  • What about the mutation-related run_isolated call?
  • Find a better solution than while ...; dataloader.yield; end -- some proper way for expressing or communicating dependencies in the code
  • Consider rooting out after_lazy usage in loading and authorization
  • Benchmark

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.

Add the benefit of dataloaders for mutation arguments

2 participants