From 6f0d6c2bf6d9d19c93c8757a87162bf1ce5e2bb3 Mon Sep 17 00:00:00 2001 From: Bailin He <15058035+bailinhe@users.noreply.github.com> Date: Fri, 19 Apr 2024 17:02:19 -0400 Subject: [PATCH] Apply suggestions from code review Co-authored-by: John Schaeffer Signed-off-by: Bailin He <15058035+bailinhe@users.noreply.github.com> --- internal/query/errors.go | 4 ++-- internal/query/roles_v2.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/query/errors.go b/internal/query/errors.go index 21b27e1d4..6f21394e4 100644 --- a/internal/query/errors.go +++ b/internal/query/errors.go @@ -31,8 +31,8 @@ var ( // ErrResourceNotFound represents an error when no matching resource was found ErrResourceNotFound = errors.New("resource not found") - // ErrRoleBindingNotFound represents an error when no matching role-binding was found - ErrRoleBindingNotFound = errors.New("role-binding not found") + // ErrRoleBindingNotFound represents an error when no matching role binding was found + ErrRoleBindingNotFound = errors.New("role binding not found") // ErrRoleHasTooManyResources represents an error which a role has too many resources ErrRoleHasTooManyResources = errors.New("role has too many resources") diff --git a/internal/query/roles_v2.go b/internal/query/roles_v2.go index dd5fb5cfb..d386c3d5e 100644 --- a/internal/query/roles_v2.go +++ b/internal/query/roles_v2.go @@ -179,7 +179,7 @@ func (e *engine) GetRoleV2(ctx context.Context, role types.Resource) (types.Role ctx, span := e.tracer.Start( ctx, "engine.GetRoleV2", - trace.WithAttributes(attribute.Stringer("role", role.ID)), + trace.WithAttributes(attribute.Stringer("permissions.role_id", role.ID)), ) defer span.End() @@ -284,7 +284,7 @@ func (e *engine) UpdateRoleV2(ctx context.Context, actor, roleResource types.Res return role, nil } - // 1. update role in permission-api DB + // 1. update role in permissions-api DB dbRole, err := e.store.UpdateRole(dbCtx, actor.ID, role.ID, newName) if err != nil { span.RecordError(err) @@ -295,7 +295,7 @@ func (e *engine) UpdateRoleV2(ctx context.Context, actor, roleResource types.Res return types.Role{}, err } - // 2. update permissions relationships in spice db + // 2. update permissions relationships in SpiceDB updates := []*pb.RelationshipUpdate{} roleRef := resourceToSpiceDBRef(e.namespace, roleResource) @@ -321,7 +321,7 @@ func (e *engine) UpdateRoleV2(ctx context.Context, actor, roleResource types.Res ) } - // 2.c write updates to spicedb + // 2.c write updates to SpiceDB request := &pb.WriteRelationshipsRequest{Updates: updates} if _, err := e.client.WriteRelationships(ctx, request); err != nil { @@ -339,10 +339,10 @@ func (e *engine) UpdateRoleV2(ctx context.Context, actor, roleResource types.Res logRollbackErr(e.logger, e.store.RollbackContext(dbCtx)) - // At this point, spicedb changes have already been applied. + // At this point, SpiceDB changes have already been applied. // Attempting to rollback could result in failures that could result in the same situation. // - // TODO: add spicedb rollback logic along with rollback failure scenarios. + // TODO: add SpiceDB rollback logic along with rollback failure scenarios. return types.Role{}, err }