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

disables dispatch for memory datastore #112

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

vroldanbet
Copy link
Contributor

fixes #111

users running the repository example where stuck because the SpiceDB instance spun up had Dispatch API server disabled, but actual dispatching enabled, so Check requests errored out.

This adds tests to verify the untested method ToEnvVarApplyConfiguration() and makes sure memory datastore does not set dispatchUpstreamAddr

@ensonic
Copy link
Contributor

ensonic commented Nov 25, 2022

I think you should also not announce the service:
https://github.com/authzed/spicedb-operator/blob/main/pkg/config/config.go#L522

I think also gateway needs some fix, because following the tutorial the service announces it, but it has no backend. I've filed a new bug for that (#113).

@vroldanbet
Copy link
Contributor Author

@ensonic thanks for opening the issue, that makes sense to me!

Comment on lines 1268 to 1270
parts := strings.Split(env, "=")
name := parts[0]
value := parts[1]
Copy link
Member

Choose a reason for hiding this comment

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

strings.Cut might be perfect here

@vroldanbet
Copy link
Contributor Author

@samkim mind having a look at the failing Ginko tests? 🙏🏻

@ensonic
Copy link
Contributor

ensonic commented Nov 29, 2022

I've patched in the CL (see #114) and now I get:

> zed permission check blog/post:1 read  blog/user:emilia --revision "${ZEDTOKEN}"
Error: rpc error: code = Unimplemented desc = unknown service dispatch.v1.DispatchService

@ensonic
Copy link
Contributor

ensonic commented Nov 29, 2022

I've patched in the CL (see #114) and now I get:

> zed permission check blog/post:1 read  blog/user:emilia --revision "${ZEDTOKEN}"
Error: rpc error: code = Unimplemented desc = unknown service dispatch.v1.DispatchService

My bad. When I build from the PR, I had to restart the spicedb-operator deployment (set image was not enough).

@vroldanbet vroldanbet force-pushed the disable-dispatch-memory-ds branch 2 times, most recently from 68a5069 to 230c186 Compare December 1, 2022 16:37
@vroldanbet vroldanbet requested review from a team and samkim December 1, 2022 16:38
fixes #111

users running the repository example where stuck
because the SpiceDB instance spun up had Dispatch API
server disabled, but actual dispatching enabled, so
Check requests errored out.

This adds tests to verify the untested method
ToEnvVarApplyConfiguration() and makes sure
memory datastore does not set dispatchUpstreamAddr

Co-authored-by: Sam Kim <sam@authzed.com>
Copy link
Member

@samkim samkim left a comment

Choose a reason for hiding this comment

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

LGTM

@vroldanbet vroldanbet merged commit 064f114 into main Dec 2, 2022
@vroldanbet vroldanbet deleted the disable-dispatch-memory-ds branch December 2, 2022 00:28
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

memory datastore is set with dispatch enabled by default
4 participants