Refactor the graph package to use errgroup

[jakedt]

Right now the graph package uses something that looks very similar to errgroups for running a bunch of checks in parallel, and was written naive of the existence of errgroups. I think we could make it cleaner and get rid of quite a bit of code by moving it over to using errgroups.

[tiev]

Hello,

I'm new to Golang and want to try it out by fixing an issue.

Authzed and SpiceDB (or Zanzibar) is a mind-blown way to solve the authorization problems, which bugs me for a long time.

May I proceed on solving this issue?

Can I confirm the scope of this is code in file internal/graph/check.go ?

[jakedt]

Hi @tiev ,

There would be changes in check.go, lookup.go, and expand.go. Roughly the idea is to stop using a wrapped type that contains both results and errors on the results channel, but instead use Errgroup to have two different control streams, one for the error case and one for the valid response case.

Hope this helps!

[josephschorr]

Reachable resources uses errgroup now, and lookup uses reachable resources.

This is still an opportunity for moving to errgroup in check and expand

[josephschorr]

We'd likely want to use https://github.com/authzed/spicedb/blob/main/internal/graph/taskrunner.go now

[Devanshu24]

Hi team! Is this something we still want to do? If yes, I would love to work on it. Also, if possible, it would be great if you could help with some starting guidance. Thanks!

[josephschorr]

Hi @Devanshu24, it will be a fairly involved change, but if you're interested in trying to do this, take a look in https://github.com/authzed/spicedb/blob/main/internal/graph/check.go and see how we changed it from using resolver functions into direct calls. The first step is applying the same work to https://github.com/authzed/spicedb/blob/main/internal/graph/expand.go