Skip to content
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

Simplify role fetching logic in query engine #282

Merged
merged 4 commits into from
Aug 22, 2024

Conversation

jnschaeffer
Copy link
Contributor

Prior implementations of the query engine fetched role information such as the owning resource ID directly from SpiceDB, as it was the only data store available. With the introduction of CRDB, that is no longer the case and the CRDB SQL table should be considered the authoritative source of most role data. This PR updates the query engine to fetch role resource owner ID and other data from the SQL DB whenever possible, getting rid of some obscure failure modes that occur when a role has no associated actions.

@jnschaeffer jnschaeffer requested review from a team as code owners August 22, 2024 20:09
Prior implementations of the query engine fetched role information
such as the owning resource ID directly from SpiceDB, as it was the
only data store available. With the introduction of CRDB, that is no
longer the case and the CRDB SQL table should be considered the
authoritative source of most role data. This commit updates the query
engine to fetch role resource owner ID and other data from the SQL DB
whenever possible, getting rid of some obscure failure modes that
occur when a role has no associated actions.

Signed-off-by: John Schaeffer <[email protected]>
As described.

Signed-off-by: John Schaeffer <[email protected]>
Copy link
Contributor

@bailinhe bailinhe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@jnschaeffer jnschaeffer merged commit 31bbd1c into infratographer:main Aug 22, 2024
4 checks passed
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.

2 participants