Skip to content

Commit 015a7ba

Browse files
Thomas StrombergThomas Stromberg
authored andcommitted
code cleanup
1 parent 51f14e5 commit 015a7ba

File tree

8 files changed

+210
-257
lines changed

8 files changed

+210
-257
lines changed

persist/cloudrun/cloudrun.go

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package cloudrun
55

66
import (
77
"context"
8-
"log/slog"
98
"os"
109

1110
"github.com/codeGROOVE-dev/bdcache"
@@ -21,27 +20,15 @@ import (
2120
// The cacheID is used as the database name for Datastore or subdirectory for local files.
2221
// This function always succeeds by falling back to local files if Datastore is unavailable.
2322
func New[K comparable, V any](ctx context.Context, cacheID string) (bdcache.PersistenceLayer[K, V], error) {
24-
// Check if running in Cloud Run
23+
// Try Datastore in Cloud Run environments
2524
if os.Getenv("K_SERVICE") != "" {
26-
slog.Debug("detected cloud run environment", "service", os.Getenv("K_SERVICE"))
27-
28-
// Try Datastore first in Cloud Run
2925
p, err := datastore.New[K, V](ctx, cacheID)
3026
if err == nil {
31-
slog.Info("using datastore persistence", "cacheID", cacheID)
3227
return p, nil
3328
}
34-
slog.Warn("datastore unavailable in cloud run, falling back to local files",
35-
"error", err,
36-
"cacheID", cacheID)
29+
// Datastore unavailable, fall through to local files
3730
}
3831

3932
// Fall back to local files (either not in Cloud Run, or Datastore failed)
40-
p, err := localfs.New[K, V](cacheID, "")
41-
if err != nil {
42-
return nil, err
43-
}
44-
45-
slog.Info("using local file persistence", "cacheID", cacheID)
46-
return p, nil
33+
return localfs.New[K, V](cacheID, "")
4734
}

persist/datastore/datastore.go

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"encoding/json"
88
"errors"
99
"fmt"
10-
"log/slog"
1110
"time"
1211

1312
"github.com/codeGROOVE-dev/bdcache"
@@ -28,11 +27,11 @@ type persister[K comparable, V any] struct {
2827
// ValidateKey checks if a key is valid for Datastore persistence.
2928
// Datastore has stricter key length limits than files.
3029
func (*persister[K, V]) ValidateKey(key K) error {
31-
keyStr := fmt.Sprintf("%v", key)
32-
if len(keyStr) > maxDatastoreKeyLen {
33-
return fmt.Errorf("key too long: %d bytes (max %d for datastore)", len(keyStr), maxDatastoreKeyLen)
30+
s := fmt.Sprintf("%v", key)
31+
if len(s) > maxDatastoreKeyLen {
32+
return fmt.Errorf("key too long: %d bytes (max %d for datastore)", len(s), maxDatastoreKeyLen)
3433
}
35-
if keyStr == "" {
34+
if s == "" {
3635
return errors.New("key cannot be empty")
3736
}
3837
return nil
@@ -67,8 +66,6 @@ func New[K comparable, V any](ctx context.Context, cacheID string) (bdcache.Pers
6766
// Verify connectivity (assert readiness)
6867
// Note: ds9 doesn't expose Ping, but client creation validates connectivity
6968

70-
slog.Debug("initialized datastore persistence", "database", cacheID, "kind", datastoreKind)
71-
7269
return &persister[K, V]{
7370
client: client,
7471
kind: datastoreKind,
@@ -78,8 +75,7 @@ func New[K comparable, V any](ctx context.Context, cacheID string) (bdcache.Pers
7875
// makeKey creates a Datastore key from a cache key.
7976
// We use the string representation directly as the key name.
8077
func (p *persister[K, V]) makeKey(key K) *ds.Key {
81-
keyStr := fmt.Sprintf("%v", key)
82-
return ds.NameKey(p.kind, keyStr, nil)
78+
return ds.NameKey(p.kind, fmt.Sprintf("%v", key), nil)
8379
}
8480

8581
// Load retrieves a value from Datastore.
@@ -168,10 +164,7 @@ func (p *persister[K, V]) LoadRecent(ctx context.Context, limit int) (entries <-
168164
}
169165

170166
iter := p.client.Run(ctx, query)
171-
172167
now := time.Now()
173-
loaded := 0
174-
expired := 0
175168

176169
for {
177170
var entry datastoreEntry
@@ -194,7 +187,6 @@ func (p *persister[K, V]) LoadRecent(ctx context.Context, limit int) (entries <-
194187

195188
// Skip expired entries - cleanup is handled by native TTL or periodic Cleanup() calls
196189
if !entry.Expiry.IsZero() && now.After(entry.Expiry) {
197-
expired++
198190
continue
199191
}
200192

@@ -207,10 +199,6 @@ func (p *persister[K, V]) LoadRecent(ctx context.Context, limit int) (entries <-
207199
// If Sscanf fails, try direct type assertion for string keys
208200
sk, ok := any(ks).(K)
209201
if !ok {
210-
slog.Warn("failed to parse key from datastore",
211-
"keyStr", ks,
212-
"expectedType", fmt.Sprintf("%T", key),
213-
"error", err)
214202
continue
215203
}
216204
key = sk
@@ -219,18 +207,11 @@ func (p *persister[K, V]) LoadRecent(ctx context.Context, limit int) (entries <-
219207
// Decode value from base64
220208
vb, err := base64.StdEncoding.DecodeString(entry.Value)
221209
if err != nil {
222-
slog.Warn("failed to decode value from datastore",
223-
"key", ks,
224-
"error", err)
225210
continue
226211
}
227212

228213
var value V
229214
if err := json.Unmarshal(vb, &value); err != nil {
230-
slog.Warn("failed to unmarshal value from datastore",
231-
"key", ks,
232-
"valueLength", len(vb),
233-
"error", err)
234215
continue
235216
}
236217

@@ -240,10 +221,7 @@ func (p *persister[K, V]) LoadRecent(ctx context.Context, limit int) (entries <-
240221
Expiry: entry.Expiry,
241222
UpdatedAt: entry.UpdatedAt,
242223
}
243-
loaded++
244224
}
245-
246-
slog.Info("loaded cache entries from datastore", "loaded", loaded, "expired", expired)
247225
}()
248226

249227
return entryCh, errCh
@@ -277,7 +255,6 @@ func (p *persister[K, V]) Cleanup(ctx context.Context, maxAge time.Duration) (in
277255
return 0, fmt.Errorf("delete expired entries: %w", err)
278256
}
279257

280-
slog.Info("cleaned up expired entries", "count", len(keys), "kind", p.kind)
281258
return len(keys), nil
282259
}
283260

@@ -302,7 +279,6 @@ func (p *persister[K, V]) Flush(ctx context.Context) (int, error) {
302279
return 0, fmt.Errorf("delete all entries: %w", err)
303280
}
304281

305-
slog.Info("flushed datastore cache", "count", len(keys), "kind", p.kind)
306282
return len(keys), nil
307283
}
308284

persist/localfs/integration_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -985,7 +985,7 @@ func TestFilePersist_Flush_RemovesFiles(t *testing.T) {
985985
}()
986986

987987
ctx := context.Background()
988-
cacheDir := fp.(*persister[string, int]).Dir
988+
cacheDir := fp.(*persister[string, int]).Dir //nolint:errcheck // Test code - panic is acceptable if type assertion fails
989989

990990
// Store multiple entries
991991
for i := range 10 {
@@ -997,9 +997,10 @@ func TestFilePersist_Flush_RemovesFiles(t *testing.T) {
997997
// Count .gob files on disk before flush
998998
countGobFiles := func() int {
999999
count := 0
1000+
//nolint:errcheck // WalkDir errors are handled by returning nil to continue walking
10001001
_ = filepath.WalkDir(cacheDir, func(path string, d os.DirEntry, err error) error {
10011002
if err != nil {
1002-
return nil
1003+
return nil //nolint:nilerr // Intentionally continue walking on errors
10031004
}
10041005
if !d.IsDir() && filepath.Ext(path) == ".gob" {
10051006
count++

0 commit comments

Comments
 (0)