[wip] SpiceDB reconnect behavior #18570
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
roboquat
added
do-not-merge/work-in-progress
do-not-merge/hold
size/S
size/M
and removed
size/S
labels
geropl
force-pushed
the
gpl/fga-reconnect
branch
from
9e07410 to
a9ec85f
Compare
|
New and updated dependencies detected. Learn more about Socket for GitHub ↗︎
|
geropl
force-pushed
the
gpl/fga-reconnect
branch
2 times, most recently
from
261329a to
6ed8284
Compare
geropl
force-pushed
the
gpl/fga-reconnect
branch
from
8ef2955 to
f3fe53e
Compare
geropl
commented
Aug 24, 2023
| @@ -26,10 +28,6 @@ export class SpiceDBAuthorizer { | |||
| userId: string; | |||
| }, | |||
| ): Promise<boolean> { | |||
| if (!this.client) { | |||
| return true; | |||
There was a problem hiding this comment.
This was really confusing in testing 🙈
And as we now require SpiceDB in every environment, we should remove it alltogether.
geropl
commented
Aug 24, 2023
| .toDynamicValue((ctx) => { | ||
| const config = spiceDBConfigFromEnv(); | ||
| if (!config) { | ||
| throw new Error("[spicedb] Missing configuration expected in env vars!"); |
There was a problem hiding this comment.
We error now if the configuration is incomplete.
geropl
force-pushed
the
gpl/fga-reconnect
branch
from
f3fe53e to
6a91d0c
Compare
|
Rebasing the PR 🙄 |
…ng config + client call metrics)
geropl
force-pushed
the
gpl/fga-reconnect
branch
from
6a91d0c to
4f0751f
Compare
|
|
||
| export function createInitializingAuthorizer(spiceDbAuthorizer: SpiceDBAuthorizer): Authorizer { | ||
| const target = new Authorizer(spiceDbAuthorizer); | ||
| const initialized = (async () => { | ||
| await target.addInstallationAdminRole(BUILTIN_INSTLLATION_ADMIN_USER_ID); | ||
| await target.addUser(BUILTIN_INSTLLATION_ADMIN_USER_ID); | ||
| })(); | ||
| })().catch((err) => log.error("Failed to initialize authorizer", err)); |
There was a problem hiding this comment.
this would hide the error and only log. Is that intentional?
| const timer = spicedbClientLatency.startTimer(); | ||
| let error: Error | undefined; | ||
| try { | ||
| const existing = await this.client.readRelationships(v1.ReadRelationshipsRequest.create(req)); | ||
| const client = this.clientProvider.getClient(); |
There was a problem hiding this comment.
we could extract this into
private get client() {
return this.clientProvider.getClient();
}
svenefftinge
approved these changes
Aug 24, 2023
|
/unhold |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Before,
serverwaited at least 120s before establishing a gRPC connection to SpiceDB again.This PR:
0.12.1which exposes gRPC Client- and ChannelOptionsdeadlineof8sto each call against SpiceDB to ensure we are not starving30sℹ️ SpiceDB config is required now, and
serverfails to start if there is no config in the env vars.Summary generated by Copilot
🤖 Generated by Copilot at 57c33a4
Add performance logging to SpiceDB client calls in
spicedb-authorizer.ts. This helps to measure and optimize the latency of authorization checks and updates.Related Issue(s)
Fixes EXP-379
How to test
kubectl delete pod spicedb-...Documentation
Preview status
Gitpod was successfully deployed to your preview environment.
Build Options
Build
Run the build with werft instead of GHA
Run Leeway with
--dont-testPublish
Installer
Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
If enabled this will build
install/previewIf enabled this will create the environment on GCE infra
Valid options are
all,workspace,webapp,ide,jetbrains,vscode,ssh. If enabled,with-previewandwith-large-vmwill be enabled./hold