Skip to content

Commit

Permalink
Fix lint issues
Browse files Browse the repository at this point in the history
  • Loading branch information
tstirrat15 committed Sep 12, 2024
1 parent 9c3b6b8 commit 8a1ae2c
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 41 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/bits-and-blooms/bitset v1.13.0 // indirect
github.com/bits-and-blooms/bloom/v3 v3.7.0 // indirect
github.com/ccoveille/go-safecast v1.1.0 // indirect
github.com/census-instrumentation/opencensus-proto v0.4.1 // indirect
github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,8 @@ github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl
github.com/boombuler/barcode v1.0.1/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8=
github.com/brianvoe/gofakeit/v6 v6.28.0 h1:Xib46XXuQfmlLS2EXRuJpqcw8St6qSZz75OUo0tgAW4=
github.com/brianvoe/gofakeit/v6 v6.28.0/go.mod h1:Xj58BMSnFqcn/fAQeSK+/PLtC5kSb7FJIq4JyGa8vEs=
github.com/ccoveille/go-safecast v1.1.0 h1:iHKNWaZm+OznO7Eh6EljXPjGfGQsSfa6/sxPlIEKO+g=
github.com/ccoveille/go-safecast v1.1.0/go.mod h1:QqwNjxQ7DAqY0C721OIO9InMk9zCwcsO7tnRuHytad8=
github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
Expand Down
47 changes: 35 additions & 12 deletions internal/cmd/restorer.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
"github.com/ccoveille/go-safecast"
"github.com/cenkalti/backoff/v4"
"github.com/mattn/go-isatty"
"github.com/rs/zerolog/log"
Expand Down Expand Up @@ -225,6 +226,13 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Experim
canceled, cancelErr := isCanceledError(ctx.Err(), err)
unknown := !retryable && !conflict && !canceled && err != nil

intExpectedLoaded, err := safecast.ToInt64(expectedLoaded)
if err != nil {
return err
}

intNumBatches := int64(len(batchesToBeCommitted))

switch {
case canceled:
r.bar.Describe("backup restore aborted")
Expand All @@ -235,24 +243,29 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Experim
case retryable && r.disableRetryErrors:
return err
case conflict && r.conflictStrategy == Skip:
r.skippedRels += int64(expectedLoaded)
r.skippedBatches += int64(len(batchesToBeCommitted))
r.duplicateBatches += int64(len(batchesToBeCommitted))
r.duplicateRels += int64(expectedLoaded)
r.skippedRels += intExpectedLoaded
r.skippedBatches += intNumBatches
r.duplicateBatches += intNumBatches
r.duplicateRels += intExpectedLoaded
r.bar.Describe("skipping conflicting batch")
case conflict && r.conflictStrategy == Touch:
r.bar.Describe("touching conflicting batch")
r.duplicateRels += int64(expectedLoaded)
r.duplicateBatches += int64(len(batchesToBeCommitted))
r.duplicateRels += intExpectedLoaded
r.duplicateBatches += intNumBatches
r.totalRetries++
numLoaded, retries, err = r.writeBatchesWithRetry(ctx, batchesToBeCommitted)
if err != nil {
return fmt.Errorf("failed to write retried batch: %w", err)
}

intNumLoaded, err := safecast.ToInt64(numLoaded)
if err != nil {
return err
}

retries++ // account for the initial attempt
r.writtenBatches += int64(len(batchesToBeCommitted))
r.writtenRels += int64(numLoaded)
r.writtenBatches += intNumBatches
r.writtenRels += intNumLoaded
case conflict && r.conflictStrategy == Fail:
r.bar.Describe("conflict detected, aborting restore")
return fmt.Errorf("duplicate relationships found")
Expand All @@ -264,17 +277,27 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Experim
return fmt.Errorf("failed to write retried batch: %w", err)
}

intNumLoaded, err := safecast.ToInt64(numLoaded)
if err != nil {
return err
}

retries++ // account for the initial attempt
r.writtenBatches += int64(len(batchesToBeCommitted))
r.writtenRels += int64(numLoaded)
r.writtenBatches += intNumBatches
r.writtenRels += intNumLoaded
default:
r.bar.Describe("restoring relationships from backup")
r.writtenBatches += int64(len(batchesToBeCommitted))
r.writtenBatches += intNumBatches
}

// it was a successful transaction commit without duplicates
if resp != nil {
r.writtenRels += int64(resp.NumLoaded)
intNumLoaded, err := safecast.ToInt64(resp.NumLoaded)
if err != nil {
return err
}

r.writtenRels += intNumLoaded
if expectedLoaded != resp.NumLoaded {
log.Warn().Uint64("loaded", resp.NumLoaded).Uint64("expected", expectedLoaded).Msg("unexpected number of relationships loaded")
}
Expand Down
46 changes: 25 additions & 21 deletions internal/cmd/restorer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
"github.com/authzed/spicedb/pkg/tuple"
"github.com/ccoveille/go-safecast"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -60,12 +61,13 @@ func TestRestorer(t *testing.T) {
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
require := require.New(t)
backupFileName := createTestBackup(t, testSchema, tt.relationships)
d, closer, err := decoderFromArgs(backupFileName)
require.NoError(t, err)
require.NoError(err)
t.Cleanup(func() {
require.NoError(t, closer.Close())
require.NoError(t, os.Remove(backupFileName))
require.NoError(closer.Close())
require.NoError(os.Remove(backupFileName))
})

expectedFilteredRels := make([]string, 0, len(tt.relationships))
Expand All @@ -79,7 +81,9 @@ func TestRestorer(t *testing.T) {

expectedBatches := len(expectedFilteredRels) / tt.batchSize
// there is always one extra commit, regardless there is or not a remainder batch
expectedCommits := expectedBatches/int(tt.batchesPerTransaction) + 1
batchesPerTransaction, err := safecast.ToInt(tt.batchesPerTransaction)
require.NoError(err)
expectedCommits := expectedBatches/batchesPerTransaction + 1
remainderBatch := false
if len(expectedFilteredRels)%tt.batchSize != 0 {
expectedBatches++
Expand Down Expand Up @@ -142,8 +146,8 @@ func TestRestorer(t *testing.T) {
expectedTouchedBatches := expectedRetries
expectedTouchedRels := expectedRetries * tt.batchSize
if tt.conflictStrategy == Touch {
expectedTouchedBatches += expectedConflicts * int(tt.batchesPerTransaction)
expectedTouchedRels += expectedConflicts * int(tt.batchesPerTransaction) * tt.batchSize
expectedTouchedBatches += expectedConflicts * batchesPerTransaction
expectedTouchedRels += expectedConflicts * batchesPerTransaction * tt.batchSize
}

expectedSkippedBatches := 0
Expand All @@ -156,28 +160,28 @@ func TestRestorer(t *testing.T) {
r := newRestorer(testSchema, d, c, tt.prefixFilter, tt.batchSize, tt.batchesPerTransaction, tt.conflictStrategy, tt.disableRetryErrors, 0*time.Second)
err = r.restoreFromDecoder(context.Background())
if expectsError != nil || (expectedConflicts > 0 && tt.conflictStrategy == Fail) {
require.ErrorIs(t, err, expectsError)
require.ErrorIs(err, expectsError)
return
}

require.NoError(t, err)
require.NoError(err)

// assert on mock stats
require.Equal(t, expectedBatches, c.receivedBatches, "unexpected number of received batches")
require.Equal(t, expectedCommits, c.receivedCommits, "unexpected number of batch commits")
require.Equal(t, len(expectedFilteredRels), c.receivedRels, "unexpected number of received relationships")
require.Equal(t, expectedTouchedBatches, c.touchedBatches, "unexpected number of touched batches")
require.Equal(t, expectedTouchedRels, c.touchedRels, "unexpected number of touched commits")
require.Equal(expectedBatches, c.receivedBatches, "unexpected number of received batches")
require.Equal(expectedCommits, c.receivedCommits, "unexpected number of batch commits")
require.Equal(len(expectedFilteredRels), c.receivedRels, "unexpected number of received relationships")
require.Equal(expectedTouchedBatches, c.touchedBatches, "unexpected number of touched batches")
require.Equal(expectedTouchedRels, c.touchedRels, "unexpected number of touched commits")

// assert on restorer stats
require.Equal(t, expectedWrittenRels, int(r.writtenRels), "unexpected number of written relationships")
require.Equal(t, expectedWrittenBatches, int(r.writtenBatches), "unexpected number of written relationships")
require.Equal(t, expectedSkippedBatches, int(r.skippedBatches), "unexpected number of conflicting batches skipped")
require.Equal(t, expectedSkippedRels, int(r.skippedRels), "unexpected number of conflicting relationships skipped")
require.Equal(t, uint(expectedConflicts)*tt.batchesPerTransaction, uint(r.duplicateBatches), "unexpected number of duplicate batches detected")
require.Equal(t, expectedConflicts*int(tt.batchesPerTransaction)*tt.batchSize, int(r.duplicateRels), "unexpected number of duplicate relationships detected")
require.Equal(t, int64(expectedRetries+expectedConflicts-expectedSkippedBatches), r.totalRetries, "unexpected number of retries")
require.Equal(t, len(tt.relationships)-len(expectedFilteredRels), int(r.filteredOutRels), "unexpected number of filtered out relationships")
require.Equal(expectedWrittenRels, int(r.writtenRels), "unexpected number of written relationships")
require.Equal(expectedWrittenBatches, int(r.writtenBatches), "unexpected number of written relationships")
require.Equal(expectedSkippedBatches, int(r.skippedBatches), "unexpected number of conflicting batches skipped")
require.Equal(expectedSkippedRels, int(r.skippedRels), "unexpected number of conflicting relationships skipped")
require.Equal(uint(expectedConflicts)*tt.batchesPerTransaction, uint(r.duplicateBatches), "unexpected number of duplicate batches detected")
require.Equal(expectedConflicts*batchesPerTransaction*tt.batchSize, int(r.duplicateRels), "unexpected number of duplicate relationships detected")
require.Equal(int64(expectedRetries+expectedConflicts-expectedSkippedBatches), r.totalRetries, "unexpected number of retries")
require.Equal(len(tt.relationships)-len(expectedFilteredRels), int(r.filteredOutRels), "unexpected number of filtered out relationships")
})
}
}
Expand Down
7 changes: 6 additions & 1 deletion internal/cmd/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/authzed/spicedb/pkg/schemadsl/compiler"
"github.com/authzed/spicedb/pkg/schemadsl/generator"
"github.com/authzed/spicedb/pkg/schemadsl/input"
"github.com/ccoveille/go-safecast"
"github.com/jzelinskie/cobrautil/v2"
"github.com/jzelinskie/stringz"
"github.com/rs/zerolog/log"
Expand Down Expand Up @@ -119,7 +120,11 @@ func schemaCopyCmdFunc(cmd *cobra.Command, args []string) error {
}

func schemaWriteCmdFunc(cmd *cobra.Command, args []string) error {
if len(args) == 0 && term.IsTerminal(int(os.Stdin.Fd())) {
intFd, err := safecast.ToInt(uint(os.Stdout.Fd()))
if err != nil {
return err
}
if len(args) == 0 && term.IsTerminal(intFd) {
return fmt.Errorf("must provide file path or contents via stdin")
}

Expand Down
8 changes: 6 additions & 2 deletions internal/cmd/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"
"strings"

"github.com/ccoveille/go-safecast"
"github.com/spf13/cobra"

v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
Expand Down Expand Up @@ -166,10 +167,13 @@ func ouputErrorWithSource(validateContents []byte, errWithSource spiceerrors.Err

func outputForLine(validateContents []byte, oneIndexedLineNumber uint64, sourceCodeString string, oneIndexedColumnPosition uint64) {
lines := strings.Split(string(validateContents), "\n")
errorLineNumber := int(oneIndexedLineNumber) - 1
// These should be fine to be zero if the cast fails.
intLineNumber, _ := safecast.ToInt(oneIndexedLineNumber)
intColumnPosition, _ := safecast.ToInt(oneIndexedColumnPosition)
errorLineNumber := intLineNumber - 1
for i := errorLineNumber - 3; i < errorLineNumber+3; i++ {
if i == errorLineNumber {
renderLine(lines, i, sourceCodeString, errorLineNumber, int(oneIndexedColumnPosition)-1)
renderLine(lines, i, sourceCodeString, errorLineNumber, intColumnPosition-1)
} else {
renderLine(lines, i, "", errorLineNumber, -1)
}
Expand Down
7 changes: 6 additions & 1 deletion internal/cmd/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/authzed/authzed-go/pkg/responsemeta"
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
"github.com/ccoveille/go-safecast"
"github.com/gookit/color"
"github.com/jzelinskie/cobrautil/v2"
"github.com/rs/zerolog/log"
Expand All @@ -20,7 +21,11 @@ import (
)

func versionCmdFunc(cmd *cobra.Command, _ []string) error {
if !term.IsTerminal(int(os.Stdout.Fd())) {
intFd, err := safecast.ToInt(uint(os.Stdout.Fd()))
if err != nil {
return err
}
if !term.IsTerminal(intFd) {
color.Disable()
}

Expand Down
4 changes: 2 additions & 2 deletions internal/grpcutil/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"golang.org/x/sync/semaphore"
)

func min(a int, b int) int {
func minimum(a int, b int) int {
if a <= b {
return a
}
Expand Down Expand Up @@ -60,7 +60,7 @@ func ConcurrentBatch(ctx context.Context, n int, batchSize int, maxWorkers int,
g.Go(func() error {
defer sem.Release(1)
start := batchNum * batchSize
end := min(start+batchSize, n)
end := minimum(start+batchSize, n)
return each(ctx, batchNum, start, end)
})
}
Expand Down
9 changes: 7 additions & 2 deletions internal/storage/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"

"github.com/99designs/keyring"
"github.com/ccoveille/go-safecast"
"github.com/jzelinskie/stringz"
"golang.org/x/term"

Expand All @@ -27,7 +28,7 @@ type Token struct {
}

func (t Token) Certificate() (cert []byte, ok bool) {
if t.CACert != nil && len(t.CACert) > 0 {
if len(t.CACert) > 0 {
return t.CACert, true
}
return nil, false
Expand Down Expand Up @@ -146,7 +147,11 @@ func fileExists(path string) (bool, error) {

func promptPassword(prompt string) (string, error) {
console.Printf(prompt)
b, err := term.ReadPassword(int(os.Stdin.Fd()))
intFd, err := safecast.ToInt(uint(os.Stdin.Fd()))
if err != nil {
return "", err
}
b, err := term.ReadPassword(intFd)
if err != nil {
return "", err
}
Expand Down

0 comments on commit 8a1ae2c

Please sign in to comment.