Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ ARG BUILD_TYPE=release
RUN apt-get update && \
apt-get -y install \
cmake \
ninja-build \
python3 \
python3-pip \
python3-virtualenv && \
Expand Down
6 changes: 3 additions & 3 deletions infra/assets/tkvpp_config_standalone.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"loggingLevel": "debug"
},
"database": {
"path": "./var/tkvpp",
"path": "/var/tkvpp",
"walFilename": "wal",
"manifestFilenamePrefix": "manifest_",
"mode": "standalone"
Expand All @@ -28,10 +28,10 @@
"server": {
"transport": "grpc",
"host": "0.0.0.0",
"port": 8081,
"port": 9891,
"id": 1,
"peers": [
"0.0.0.0:8081"
"0.0.0.0:9891"
]
Comment on lines +31 to 35
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Peers should not use 0.0.0.0; make empty for standalone or use real endpoints

"0.0.0.0" is valid for binding but invalid as a remote peer address. In standalone mode, use an empty list; in clustered mode, list concrete host:port values for other nodes.

Apply:

-        "peers": [
-            "0.0.0.0:9891"
-        ]
+        "peers": []
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"port": 9891,
"id": 1,
"peers": [
"0.0.0.0:8081"
"0.0.0.0:9891"
]
"port": 9891,
"id": 1,
"peers": []
🤖 Prompt for AI Agents
In infra/assets/tkvpp_config_standalone.json around lines 31 to 35, the peers
array contains "0.0.0.0:9891" which is incorrect for remote peer addresses;
update the peers field to be an empty array for standalone mode (i.e., "peers":
[]) or replace with concrete host:port entries when configuring a cluster, and
ensure the bind address remains the correct separate setting if needed.

}
}
14 changes: 11 additions & 3 deletions infra/docker-compose.standalone.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
---
version: '3.8'
services:
app:
tkvpp-node-standalone:
image: tinykvpp-clang:latest
container_name: tkvpp-node-1
security_opt:
- seccomp:unconfined
container_name: tkvpp-node-standalone
command: ["/app/tkvpp", "-c", "/app/config.json"]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Don’t repeat the binary when image has ENTRYPOINT

With ENTRYPOINT ["/app/tkvpp"] in the image, your current command repeats the binary, leading to "/app/tkvpp /app/tkvpp -c ...". Pass only flags.

Apply:

-    command: ["/app/tkvpp", "-c", "/app/config.json"]
+    command: ["-c", "/app/config.json"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
command: ["/app/tkvpp", "-c", "/app/config.json"]
command: ["-c", "/app/config.json"]
🤖 Prompt for AI Agents
In infra/docker-compose.standalone.yaml around line 8 the docker-compose service
command redundantly repeats the image ENTRYPOINT ("/app/tkvpp"), resulting in
"/app/tkvpp /app/tkvpp -c ..."; change the command to pass only the
flags/arguments (e.g. "-c", "/app/config.json") so Docker uses the image
ENTRYPOINT with these flags rather than invoking the binary twice.

ports:
- "9891:9891"
volumes:
- ./assets/tkvpp_config_standalone.json:/app/config.json
- tkvpp-node-standalone:/var/tkvpp
volumes:
tkvpp-node-standalone:
2 changes: 1 addition & 1 deletion src/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ auto loadConfigJson(const std::string &configPath) -> json
std::fstream configStream(configPath, std::fstream::in);
if (!configStream.is_open())
{
throw std::runtime_error(fmt::format("Unable to open config file: %s", configPath));
throw std::runtime_error(fmt::format("Unable to open config file: {}", configPath));
}
return json::parse(configStream);
}
Expand Down
5 changes: 5 additions & 0 deletions tests/go/kvtest/cmd/kvtest/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package main

import ()
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix compile error: empty import block is invalid.

import () is a syntax error in Go (“expected import path, found ')'”). This file will not compile as-is.

If you intend to keep this entrypoint, wire it to Cobra and remove the empty import:

-package main
-
-import ()
-
-func main() {}
+package main
+
+import "github.com/lnikon/tinykvpp/tests/go/kvtest/cmd"
+
+func main() { cmd.Execute() }

Alternatively (recommended), delete this file entirely to avoid duplicate entrypoints (see next comment).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import ()
package main
import "github.com/lnikon/tinykvpp/tests/go/kvtest/cmd"
func main() { cmd.Execute() }
🤖 Prompt for AI Agents
In tests/go/kvtest/cmd/kvtest/main.go around line 3 there is an empty import
block (`import ()`) which causes a Go compile error; either delete this file
entirely (preferred to avoid duplicate entrypoints) or replace the empty import
with the actual imports and wire the file into Cobra by adding the needed
imports and invoking the root command (e.g., import the cobra/root command
package and call its Execute in main) so the file becomes a valid entrypoint.


func main() {}
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Avoid duplicate entrypoints; keep only one main.

You now have two mains:

  • tests/go/kvtest/main.go (calls cmd.Execute())
  • tests/go/kvtest/cmd/kvtest/main.go (this file)

Pick a single layout:

  • Option A (simplest): Keep tests/go/kvtest/main.go and delete tests/go/kvtest/cmd/kvtest/main.go.
  • Option B (conventional multi-command): Move root.go under tests/go/kvtest/cmd/kvtest (package cmd), keep this main to call cmd.Execute(), and remove tests/go/kvtest/main.go.

Having both will confuse consumers and CI; with the current syntax error, the module cannot be built.

Run the following to enumerate all main packages under kvtest and confirm you have only one after the change:


🏁 Script executed:

#!/bin/bash
# List all Go files that declare package main under tests/go/kvtest
rg -n '^package main\b' -g 'tests/go/kvtest/**/*.go' -S -C1

Length of output: 247


Remove the duplicate main entrypoint

The verification script still finds two package main declarations under tests/go/kvtest:

  • tests/go/kvtest/main.go
  • tests/go/kvtest/cmd/kvtest/main.go

You must choose one layout and delete the other to ensure there’s only a single executable entrypoint:

  • Option A (simplest):
    • Keep tests/go/kvtest/main.go (which calls cmd.Execute()).
    • Delete tests/go/kvtest/cmd/kvtest/main.go.

  • Option B (conventional multi-command):
    • Move root.go under tests/go/kvtest/cmd/kvtest (package cmd).
    • Keep tests/go/kvtest/cmd/kvtest/main.go to call cmd.Execute().
    • Delete tests/go/kvtest/main.go.

Maintaining both prevents the module from building and may confuse CI and downstream consumers. Please remove the unwanted main.go.

🤖 Prompt for AI Agents
In tests/go/kvtest/cmd/kvtest/main.go (lines 1-5) there's a duplicate package
main entrypoint; remove this file so only tests/go/kvtest/main.go remains as the
single executable entrypoint (Option A). After deletion, ensure
tests/go/kvtest/main.go calls cmd.Execute() and no other package/main files
exist under tests/go/kvtest; run `go build` or the test verifier to confirm the
module builds.

46 changes: 46 additions & 0 deletions tests/go/kvtest/cmd/root.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
Copyright © 2025 NAME HERE <EMAIL ADDRESS>
*/
package cmd

import (
"os"

"github.com/spf13/cobra"
)

// rootCmd represents the base command when called without any subcommands
var rootCmd = &cobra.Command{
Use: "kvtest",
Short: "A brief description of your application",
Long: `A longer description that spans multiple lines and likely contains
examples and usage of using your application. For example:

Cobra is a CLI library for Go that empowers applications.
This application is a tool to generate the needed files
to quickly create a Cobra application.`,
// Uncomment the following line if your bare application
// has an action associated with it:
// Run: func(cmd *cobra.Command, args []string) { },
}

// Execute adds all child commands to the root command and sets flags appropriately.
// This is called by main.main(). It only needs to happen once to the rootCmd.
func Execute() {
err := rootCmd.Execute()
if err != nil {
os.Exit(1)
}
}

func init() {
// Here you will define your flags and configuration settings.
// Cobra supports persistent flags, which, if defined here,
// will be global for your application.

// rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is $HOME/.kvtest.yaml)")

// Cobra also supports local flags, which will only run
// when this action is called directly.
rootCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle")
}
39 changes: 39 additions & 0 deletions tests/go/kvtest/go.sum
Original file line number Diff line number Diff line change
@@ -1,9 +1,45 @@
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/fsnotify/fsnotify v1.8.0 h1:dAwr6QBTBZIkG8roQaJjGof0pp0EeF+tNV7YBP3F/8M=
github.com/fsnotify/fsnotify v1.8.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0=
github.com/go-chi/httprate v0.15.0 h1:j54xcWV9KGmPf/X4H32/aTH+wBlrvxL7P+SdnRqxh5g=
github.com/go-chi/httprate v0.15.0/go.mod h1:rzGHhVrsBn3IMLYDOZQsSU4fJNWcjui4fWKJcCId1R4=
github.com/go-viper/mapstructure/v2 v2.2.1 h1:ZAaOCxANMuZx5RCeg0mBdEZk7DZasvvZIxtHqx8aGss=
github.com/go-viper/mapstructure/v2 v2.2.1/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM=
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/klauspost/cpuid/v2 v2.2.10 h1:tBs3QSyvjDyFTq3uoc/9xFpCuOsJQFNPiAhYdw2skhE=
github.com/klauspost/cpuid/v2 v2.2.10/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0=
github.com/pelletier/go-toml/v2 v2.2.3 h1:YmeHyLY8mFWbdkNWwpr+qIL2bEqT0o95WSdkNHvL12M=
github.com/pelletier/go-toml/v2 v2.2.3/go.mod h1:MfCQTFTvCcUyyvvwm1+G6H/jORL20Xlb6rzQu9GuUkc=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/sagikazarmark/locafero v0.7.0 h1:5MqpDsTGNDhY8sGp0Aowyf0qKsPrhewaLSsFaodPcyo=
github.com/sagikazarmark/locafero v0.7.0/go.mod h1:2za3Cg5rMaTMoG/2Ulr9AwtFaIppKXTRYnozin4aB5k=
github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo=
github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIKqDmF7Mt0=
github.com/spf13/afero v1.12.0 h1:UcOPyRBYczmFn6yvphxkn9ZEOY65cpwGKb5mL36mrqs=
github.com/spf13/afero v1.12.0/go.mod h1:ZTlWwG4/ahT8W7T0WQ5uYmjI9duaLQGy3Q2OAl4sk/4=
github.com/spf13/cast v1.7.1 h1:cuNEagBQEHWN1FnbGEjCXL2szYEXqfJPbP2HNUaca9Y=
github.com/spf13/cast v1.7.1/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo=
github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo=
github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0=
github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/spf13/pflag v1.0.7 h1:vN6T9TfwStFPFM5XzjsvmzZkLuaLX+HS+0SeFLRgU6M=
github.com/spf13/pflag v1.0.7/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/spf13/viper v1.20.1 h1:ZMi+z/lvLyPSCoNtFCpqjy0S4kPbirhpTMwl8BkW9X4=
github.com/spf13/viper v1.20.1/go.mod h1:P9Mdzt1zoHIG8m2eZQinpiBjo6kCmZSKBClNNqjJvu4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8=
github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU=
github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0=
github.com/zeebo/xxh3 v1.0.2/go.mod h1:5NWz9Sef7zIDm2JHfFlcQvNekmcEl9ekUZQQKCYaDcA=
go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE=
go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI=
go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ=
golang.org/x/net v0.40.0 h1:79Xs7wF06Gbdcg4kdCCIQArK11Z1hr5POQ6+fIYHNuY=
golang.org/x/net v0.40.0/go.mod h1:y0hY0exeL2Pku80/zKK7tpntoX23cqL3Oa6njdgRtds=
golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc=
Expand All @@ -18,3 +54,6 @@ google.golang.org/grpc v1.74.2 h1:WoosgB65DlWVC9FqI82dGsZhWFNBSLjQ84bjROOpMu4=
google.golang.org/grpc v1.74.2/go.mod h1:CtQ+BGjaAIXHs/5YS3i473GqwBBa1zGQNevxdeBEXrM=
google.golang.org/protobuf v1.36.6 h1:z1NpPI8ku2WgiWnf+t9wTPsn6eP1L7ksHUlkfLvd9xY=
google.golang.org/protobuf v1.36.6/go.mod h1:jduwjTPXsFjZGTmRluh+L6NjiWu7pchiJ2/5YcXBHnY=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
13 changes: 7 additions & 6 deletions tests/go/kvtest/main.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package kvtest
/*
Copyright © 2025 NAME HERE <EMAIL ADDRESS>

import (
v1 "github.com/lnikon/tinykvpp/tests/go/kvtest/proto"
"google.golang.org/grpc"
)
*/
package main

import "github.com/lnikon/tinykvpp/tests/go/kvtest/cmd"

func main() {
let req := v1.GetRequest{}
cmd.Execute()
}
122 changes: 122 additions & 0 deletions tests/go/kvtest/pkg/adapters/tinykvpp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package adapters

import (
"context"
"fmt"
"time"

"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"

"github.com/lnikon/tinykvpp/tests/go/kvtest/pkg/core"
pb "github.com/lnikon/tinykvpp/tests/go/kvtest/proto/tinykvpp/v1"
)
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Import path for pb must match proto go_package

Once you fix go_package in tinykvpp_service.proto, ensure this import matches it. See the proto review for two consistent options.

Run this quick check in the repo to catch mismatches:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "Proto go_package values:"
rg -nP 'option\s+go_package\s*=\s*"([^"]+)"' tests/go/kvtest/proto -n
echo
echo "pb imports in adapters:"
rg -nP '"github\.com/lnikon/tinykvpp/[^"]+/proto/tinykvpp/v1"' tests/go/kvtest/pkg -n

Length of output: 542


Adjust the pb import to match the go_package in your proto

The go_package in tinykvpp_service.proto is set to:

option go_package = "github.com/lnikon/tinykvpp/protos/tinykvpp/v1";

—but in tests/go/kvtest/pkg/adapters/tinykvpp.go you’re importing:

pb "github.com/lnikon/tinykvpp/tests/go/kvtest/proto/tinykvpp/v1"

These must be identical. Update the import to:

pb "github.com/lnikon/tinykvpp/protos/tinykvpp/v1"

so that it matches the go_package option exactly.

Locations to update:

  • tests/go/kvtest/pkg/adapters/tinykvpp.go: line 12

Diff example:

-   pb "github.com/lnikon/tinykvpp/tests/go/kvtest/proto/tinykvpp/v1"
+   pb "github.com/lnikon/tinykvpp/protos/tinykvpp/v1"
🤖 Prompt for AI Agents
In tests/go/kvtest/pkg/adapters/tinykvpp.go around lines 11 to 13, the protobuf
import alias `pb` points to the wrong module path (tests/go/kvtest/proto/...)
which does not match the `go_package` declared in the proto; update the `pb`
import to the exact go_package path
"github.com/lnikon/tinykvpp/protos/tinykvpp/v1" so the import matches the proto
option exactly and resolves package mismatches.


type TinyKVPPConfig struct {
Address string `yaml:"address"`
ConnectionTimeout time.Duration `yaml:"connection_timeout"`
RequestTimeout time.Duration `yaml:"request_timeout"`
MaxRetries int `yaml:"max_retries"`
}

type TinyKVPPAdapter struct {
config TinyKVPPConfig
conn *grpc.ClientConn
client pb.TinyKVPPServiceClient
}

var _ core.KVStoreInterface = (*TinyKVPPAdapter)(nil)

func NewTinyKVPPAdapter(config core.Config) *TinyKVPPAdapter {
return &TinyKVPPAdapter{}
}

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Constructor ignores its config; either use it or drop the parameter

Right now NewTinyKVPPAdapter takes a config but discards it. This is a footgun.

-func NewTinyKVPPAdapter(config core.Config) *TinyKVPPAdapter {
-	return &TinyKVPPAdapter{}
-}
+func NewTinyKVPPAdapter(config core.Config) *TinyKVPPAdapter {
+	a := &TinyKVPPAdapter{}
+	if cfg, ok := config.(TinyKVPPConfig); ok {
+		a.config = cfg
+	}
+	return a
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func NewTinyKVPPAdapter(config core.Config) *TinyKVPPAdapter {
return &TinyKVPPAdapter{}
}
func NewTinyKVPPAdapter(config core.Config) *TinyKVPPAdapter {
a := &TinyKVPPAdapter{}
if cfg, ok := config.(TinyKVPPConfig); ok {
a.config = cfg
}
return a
}
🤖 Prompt for AI Agents
In tests/go/kvtest/pkg/adapters/tinykvpp.go around lines 30 to 33, the
NewTinyKVPPAdapter constructor accepts a core.Config but ignores it; either
persist the config on the adapter or remove the unused parameter. Fix option A:
add a config field to TinyKVPPAdapter (e.g., cfg core.Config), set it in the
constructor by returning &TinyKVPPAdapter{cfg: config}, and update usages to
access adapter.cfg as needed. Fix option B: if the adapter does not need config,
remove the parameter from NewTinyKVPPAdapter and update all call sites and tests
to call the zero-arg constructor. Ensure imports and tests compile after the
change.

func (a *TinyKVPPAdapter) Connect(config core.Config) error {
tinyConfig, ok := config.(TinyKVPPConfig)
if !ok {
return fmt.Errorf("invalid config type for TinyKVPP adapter")
}

a.config = tinyConfig

conn, err := grpc.NewClient(
a.config.Address, grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
return fmt.Errorf("failed to connect to %s: %w", a.config.Address, err)
}

a.conn = conn
a.client = pb.NewTinyKVPPServiceClient(conn)

return a.HealthCheck(context.Background())
}

func (a *TinyKVPPAdapter) Close() error {
if a.conn != nil {
return a.conn.Close()
}
return nil
}

func (a *TinyKVPPAdapter) Get(ctx context.Context, key []byte) ([]byte, error) {
if a.conn == nil {
return nil, fmt.Errorf("not connected to TinyKVPP")
}

// Apply requet timeout
ctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
defer cancel()

req := &pb.GetRequest{Key: string(key)}
resp, err := a.client.Get(ctx, req)
if err != nil {
return nil, fmt.Errorf("grpc get failed for key %s: %v", key, err)
}

if !resp.Found {
return nil, &core.KVError{Op: "get", Err: fmt.Errorf("key not found: %s", string(key))}
}

return []byte(resp.Value), nil
}
Comment on lines +61 to +81
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Get: fix typo, use request timeout, and return sentinel for not found

  • Typo: “requet” -> “request”.
  • Prefer returning a sentinel error (e.g., core.ErrKeyNotFound) so callers can do errors.Is.
  • Minor: wrap transport error with %w.
 func (a *TinyKVPPAdapter) Get(ctx context.Context, key []byte) ([]byte, error) {
   if a.conn == nil {
     return nil, fmt.Errorf("not connected to TinyKVPP")
   }

-  // Apply requet timeout
+  // Apply request timeout
   ctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
   defer cancel()

   req := &pb.GetRequest{Key: string(key)}
   resp, err := a.client.Get(ctx, req)
   if err != nil {
-    return nil, fmt.Errorf("grpc get failed for key %s: %v", key, err)
+    return nil, fmt.Errorf("grpc get failed for key %s: %w", key, err)
   }

   if !resp.Found {
-    return nil, &core.KVError{Op: "get", Err: fmt.Errorf("key not found: %s", string(key))}
+    return nil, core.ErrKeyNotFound
   }

   return []byte(resp.Value), nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (a *TinyKVPPAdapter) Get(ctx context.Context, key []byte) ([]byte, error) {
if a.conn == nil {
return nil, fmt.Errorf("not connected to TinyKVPP")
}
// Apply requet timeout
ctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
defer cancel()
req := &pb.GetRequest{Key: string(key)}
resp, err := a.client.Get(ctx, req)
if err != nil {
return nil, fmt.Errorf("grpc get failed for key %s: %v", key, err)
}
if !resp.Found {
return nil, &core.KVError{Op: "get", Err: fmt.Errorf("key not found: %s", string(key))}
}
return []byte(resp.Value), nil
}
func (a *TinyKVPPAdapter) Get(ctx context.Context, key []byte) ([]byte, error) {
if a.conn == nil {
return nil, fmt.Errorf("not connected to TinyKVPP")
}
// Apply request timeout
ctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
defer cancel()
req := &pb.GetRequest{Key: string(key)}
resp, err := a.client.Get(ctx, req)
if err != nil {
return nil, fmt.Errorf("grpc get failed for key %s: %w", key, err)
}
if !resp.Found {
return nil, core.ErrKeyNotFound
}
return []byte(resp.Value), nil
}
🤖 Prompt for AI Agents
In tests/go/kvtest/pkg/adapters/tinykvpp.go around lines 61 to 81, fix the
comment typo and improve error handling: correct "requet" to "request"; when
returning a not-found result, return the package sentinel error
(core.ErrKeyNotFound) or wrap it with fmt.Errorf("%w", core.ErrKeyNotFound)
instead of constructing a new core.KVError so callers can use errors.Is; and
wrap the gRPC transport error using the %w verb (fmt.Errorf("grpc get failed for
key %s: %w", string(key), err)) so the underlying error is preserved for
inspection.


func (a *TinyKVPPAdapter) Put(ctx context.Context, key, value []byte) error {
if a.conn == nil {
return fmt.Errorf("not connected to TinyKVPP")
}

// Apply requet timeout
ctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
defer cancel()

req := &pb.PutRequest{Key: string(key), Value: string(value)}
resp, err := a.client.Put(ctx, req)
if err != nil {
return fmt.Errorf("grpc put failed for key %s: %v\n", key, err)
}

if len(resp.Error) > 0 {
return &core.KVError{Op: "put", Err: fmt.Errorf("put failed: %s", resp.Error)}
}

if !resp.Success {
return &core.KVError{Op: "put", Err: fmt.Errorf("put operation unsuccessful")}
}

return nil
}
Comment on lines 83 to 107
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Put: fix typo, consistent error wrapping, and avoid trailing newline in errors

  • Typo: “requet” -> “request”.
  • Use %w to preserve cause.
  • Don’t append newline in error messages.
 func (a *TinyKVPPAdapter) Put(ctx context.Context, key, value []byte) error {
   if a.conn == nil {
     return fmt.Errorf("not connected to TinyKVPP")
   }

-  // Apply requet timeout
+  // Apply request timeout
   ctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
   defer cancel()

   req := &pb.PutRequest{Key: string(key), Value: string(value)}
   resp, err := a.client.Put(ctx, req)
   if err != nil {
-    return fmt.Errorf("grpc put failed for key %s: %v\n", key, err)
+    return fmt.Errorf("grpc put failed for key %s: %w", key, err)
   }

-  if len(resp.Error) > 0 {
-    return &core.KVError{Op: "put", Err: fmt.Errorf("put failed: %s", resp.Error)}
+  if len(resp.Error) > 0 { // if you keep the string field
+    return &core.KVError{Op: "put", Err: fmt.Errorf("%s", resp.Error)}
   }

   if !resp.Success {
     return &core.KVError{Op: "put", Err: fmt.Errorf("put operation unsuccessful")}
   }

   return nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (a *TinyKVPPAdapter) Put(ctx context.Context, key, value []byte) error {
if a.conn == nil {
return fmt.Errorf("not connected to TinyKVPP")
}
// Apply requet timeout
ctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
defer cancel()
req := &pb.PutRequest{Key: string(key), Value: string(value)}
resp, err := a.client.Put(ctx, req)
if err != nil {
return fmt.Errorf("grpc put failed for key %s: %v\n", key, err)
}
if len(resp.Error) > 0 {
return &core.KVError{Op: "put", Err: fmt.Errorf("put failed: %s", resp.Error)}
}
if !resp.Success {
return &core.KVError{Op: "put", Err: fmt.Errorf("put operation unsuccessful")}
}
return nil
}
func (a *TinyKVPPAdapter) Put(ctx context.Context, key, value []byte) error {
if a.conn == nil {
return fmt.Errorf("not connected to TinyKVPP")
}
// Apply request timeout
ctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
defer cancel()
req := &pb.PutRequest{Key: string(key), Value: string(value)}
resp, err := a.client.Put(ctx, req)
if err != nil {
return fmt.Errorf("grpc put failed for key %s: %w", key, err)
}
if len(resp.Error) > 0 { // if you keep the string field
return &core.KVError{Op: "put", Err: fmt.Errorf("%s", resp.Error)}
}
if !resp.Success {
return &core.KVError{Op: "put", Err: fmt.Errorf("put operation unsuccessful")}
}
return nil
}
🤖 Prompt for AI Agents
In tests/go/kvtest/pkg/adapters/tinykvpp.go around lines 83-107, fix the typo in
the inline comment ("requet" -> "request"), stop appending a newline to the gRPC
error message, and wrap the underlying gRPC error with %w to preserve the causal
error (use fmt.Errorf("grpc put failed for key %s: %w", string(key), err)). Keep
other KVError returns as-is but ensure any fmt.Errorf used for wrapping uses %w
consistently.


func (a *TinyKVPPAdapter) HealthCheck(ctx context.Context) error {
if a.conn == nil {
return fmt.Errorf("not connected")
}

testKey := "__health_check__"
a.client.Get(ctx, &pb.GetRequest{Key: testKey})

return nil
}
Comment on lines +109 to +118
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

HealthCheck should validate RPC reachability and honor timeouts

Currently it ignores the RPC result, so Connect always returns nil if dialing succeeds. Propagate RPC error to surface server unavailability.

 func (a *TinyKVPPAdapter) HealthCheck(ctx context.Context) error {
   if a.conn == nil {
     return fmt.Errorf("not connected")
   }

-  testKey := "__health_check__"
-  a.client.Get(ctx, &pb.GetRequest{Key: testKey})
-
-  return nil
+  // Short probe
+  if a.config.RequestTimeout <= 0 {
+    a.config.RequestTimeout = 1 * time.Second
+  }
+  cctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
+  defer cancel()
+  _, err := a.client.Get(cctx, &pb.GetRequest{Key: "__health_check__"})
+  // Presence isn’t required; only transport success matters.
+  if err != nil {
+    return fmt.Errorf("health check RPC failed: %w", err)
+  }
+  return nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (a *TinyKVPPAdapter) HealthCheck(ctx context.Context) error {
if a.conn == nil {
return fmt.Errorf("not connected")
}
testKey := "__health_check__"
a.client.Get(ctx, &pb.GetRequest{Key: testKey})
return nil
}
func (a *TinyKVPPAdapter) HealthCheck(ctx context.Context) error {
if a.conn == nil {
return fmt.Errorf("not connected")
}
// Short probe
if a.config.RequestTimeout <= 0 {
a.config.RequestTimeout = 1 * time.Second
}
cctx, cancel := context.WithTimeout(ctx, a.config.RequestTimeout)
defer cancel()
_, err := a.client.Get(cctx, &pb.GetRequest{Key: "__health_check__"})
// Presence isn’t required; only transport success matters.
if err != nil {
return fmt.Errorf("health check RPC failed: %w", err)
}
return nil
}
🤖 Prompt for AI Agents
In tests/go/kvtest/pkg/adapters/tinykvpp.go around lines 109-118, the
HealthCheck currently ignores the RPC result so dialing success always makes
Connect appear healthy; capture and propagate the error from a.client.Get(ctx,
&pb.GetRequest{Key: testKey}) and return it (so RPC failures surface to callers)
and ensure the call uses the provided ctx so any deadlines/timeouts are honored;
keep the existing conn nil check and return fmt.Errorf("not connected") when
appropriate.


func (a *TinyKVPPAdapter) Name() string {
return "TinyKVPP"
}
Loading