-
Notifications
You must be signed in to change notification settings - Fork 22
Feat/rapid docker #183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feat/rapid docker #183
Conversation
WalkthroughAdds a Docker-based service implementation for the Relayer, enables relayer subcommands (start/stop/restart/log) and removes the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ServiceFactory
participant DockerService
User->>CLI: run `relayer start` / `stop` / `restart` / `log`
CLI->>ServiceFactory: NewService(Relayer)
ServiceFactory->>DockerService: instantiate NewDocker(Relayer)
CLI->>DockerService: Start / Stop / Restart / Log
DockerService->>Docker: invoke docker CLI (create/run/stop/log)
Docker-->>DockerService: result / logs
DockerService-->>CLI: success / error / streamed logs
CLI-->>User: output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (2)
service/types.go (1)
39-41: Enable relayer InitCommand — consistent with workflowReturning
"relayer init"is correct for user guidance and error messages.Consider adding a short comment in
GetBinaryName/GetServiceSlugnoting that relayer is intentionally Docker-managed to prevent misuse by future callers.cmd/relayer.go (1)
101-116: Remove dead, commented-out flag codeThe
update-clientflag handling is fully commented. Keeping large commented blocks clutters the file and confuses future readers.
Please delete these lines or re-introduce the feature behind a follow-up PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cmd/flags.go(1 hunks)cmd/relayer.go(3 hunks)cmd/validate.go(1 hunks)models/relayer/init.go(2 hunks)service/docker.go(1 hunks)service/service.go(1 hunks)service/types.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
cmd/validate.go (1)
io/filesystem.go (1)
FileOrFolderExists(17-20)
service/service.go (2)
service/types.go (1)
Relayer(13-13)service/docker.go (1)
NewDocker(16-20)
cmd/relayer.go (4)
cmd/helper_text.go (1)
RelayerHelperText(19-19)service/types.go (1)
Relayer(13-13)cmd/flags.go (2)
FlagDetach(7-7)FlagN(5-5)service/service.go (1)
NewService(24-37)
models/relayer/init.go (4)
ui/loading.go (1)
NonRetryableErrorLoading(145-147)service/service.go (1)
NewService(24-37)service/types.go (1)
Relayer(13-13)common/constants.go (1)
RelayerDirectory(27-27)
service/docker.go (2)
service/types.go (7)
CommandName(5-5)Minitia(10-10)UpgradableInitia(8-8)NonUpgradableInitia(9-9)OPinitExecutor(11-11)OPinitChallenger(12-12)Relayer(13-13)common/constants.go (1)
RelayerDirectory(27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Integration Tests on Ubuntu
🔇 Additional comments (2)
service/service.go (1)
25-27: Relayer → Docker factory override looks goodRouting
RelayertoNewDockerearly is appropriate and keeps OS-specific branches intact for others.service/types.go (1)
24-26: Enable relayer PrettyName — consistent with CLIIncluding
RelayerinGetPrettyName()is correct and aligns with the new command.
cmd/flags.go
Outdated
| FlagPollingInterval = "polling-interval" | ||
|
|
||
| FlagUpdateClient = "update-client" | ||
| // FlagUpdateClient = "update-client" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Remove dead flag or mark deprecation explicitly
Commenting out FlagUpdateClient is fine short-term. Prefer removing the constant (and any usages) or adding a deprecation note to avoid confusion.
Run to verify there are no remaining references before deletion:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --glob '!.git' -e 'FlagUpdateClient|update-client' || trueLength of output: 534
Remove all commented references to FlagUpdateClient
Since the flag is no longer used, delete the constant and its commented-out usages to prevent confusion.
• cmd/flags.go, line 15: drop
// FlagUpdateClient = "update-client"• cmd/relayer.go, lines 101, 114, 133: remove the commented blocks
// updateClient, err := cmd.Flags().GetString(FlagUpdateClient)
// return fmt.Errorf("invalid update-client flag value: %q, expected 'true' or 'false'", updateClient)
// startCmd.Flags().String(FlagUpdateClient, "true", "Update light clients with new header information before starting the relayer (can be 'true' or 'false')")🤖 Prompt for AI Agents
In cmd/flags.go at line 15, remove the commented-out constant declaration for
FlagUpdateClient entirely. Additionally, in cmd/relayer.go at lines 101, 114,
and 133, delete all commented-out code blocks referencing FlagUpdateClient,
including flag retrieval, error handling, and flag definition comments, to clean
up unused code and avoid confusion.
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| detach, err := cmd.Flags().GetBool(FlagDetach) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // updateClient, err := cmd.Flags().GetString(FlagUpdateClient) | ||
| // if err != nil { | ||
| // return err | ||
| // } | ||
|
|
||
| // switch updateClient { | ||
| // case "true": | ||
| // err = relayer.UpdateClientFromConfig() | ||
| // if err != nil { | ||
| // return err | ||
| // } | ||
| // case "false": | ||
| // default: | ||
| // return fmt.Errorf("invalid update-client flag value: %q, expected 'true' or 'false'", updateClient) | ||
| // } | ||
|
|
||
| s, err := service.NewService(service.Relayer) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err = s.Start(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if detach { | ||
| return nil | ||
| } | ||
|
|
||
| return s.Log(100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
--detach flag semantics are misleading
Docker.Start() always runs docker run -d, so the relayer container is detached regardless of the flag.
The flag only toggles log streaming, not container detachment. Consider either:
- Renaming the flag to
--follow-logs(default false), or - Respecting
--detach=falseby omitting-dinDocker.Start()when logs should be streamed.
This avoids surprising behaviour for users who expect the container to run in the foreground when --detach is not supplied.
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "=== func NewService definition ==="
rg -n "func NewService" -g '*.go' -A5 -B5 || true
echo
echo "=== Service struct definition ==="
rg -n "type Service" -g '*.go' -A5 -B5 || true
echo
echo "=== Service.Start method implementation ==="
rg -n "func (s .*Service) Start" -g '*.go' -A20 -B5 || true
echo
echo "=== Docker.Start method implementation ==="
rg -n "func (?:\\*?Docker) Start" -g '*.go' -A20 -B5 || trueLength of output: 1510
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "=== Finding NewDocker definition ==="
rg -n "NewDocker" -g '*.go' -A5 -B5 || true
echo
echo "=== Searching for '-d' flag usage in service package ==="
rg -n "\"-d\"" -g 'service/*.go' -A3 -B3 || true
echo
echo "=== Searching for 'docker run' invocation ==="
rg -n "docker run" -g 'service/*.go' -A3 -B3 || trueLength of output: 1637
Clarify --detach flag behavior
The --detach flag in cmd/relayer.go currently only controls whether logs are streamed (s.Log(100)), but service/docker.go always invokes docker run -d. As a result, users who omit --detach still get a detached container, leading to confusing semantics.
• In cmd/relayer.go (around lines 95–130):
– detach, _ := cmd.Flags().GetBool(FlagDetach) only gates s.Log(100); it doesn’t influence container detachment.
• In service/docker.go (in func (d *Docker) Start):
– The args slice unconditionally includes "-d", so docker run always detaches.
To resolve this mismatch, choose one of the following refactorings:
- Rename
--detachto--follow-logs(default false), since it only toggles log following. - Make
--detach=falseomit-din the DockerStartimplementation, so the container stays attached when logs are streamed.
Aligning the CLI flag with actual container behavior will avoid surprising users.
🤖 Prompt for AI Agents
In cmd/relayer.go around lines 95 to 130, the --detach flag currently only
controls whether logs are streamed but does not affect container detachment,
which is always detached due to the unconditional "-d" flag in
service/docker.go's Docker Start function. To fix this, either rename the CLI
flag from --detach to --follow-logs to reflect its actual behavior of toggling
log streaming, or modify the Docker Start function to conditionally include the
"-d" flag based on the --detach flag value so that when --detach is false, the
container runs attached and logs can be streamed directly. This change will
align the CLI flag semantics with the container's actual run mode and avoid user
confusion.
| // TODO: validate docker? | ||
| if serviceFile != "" { | ||
| if !weaveio.FileOrFolderExists(serviceFile) { | ||
| return fmt.Errorf("service file %s not found", serviceFile) | ||
| } | ||
|
|
||
| serviceBinary, serviceHome, err := service.GetServiceBinaryAndHome() | ||
| if err != nil { | ||
| return fmt.Errorf("could not determine %v binary and home directory: %w", prettyName, err) | ||
| } | ||
|
|
||
| if !weaveio.FileOrFolderExists(serviceHome) { | ||
| return fmt.Errorf("home directory %s not found", serviceHome) | ||
| } | ||
|
|
||
| if !weaveio.FileOrFolderExists(serviceBinary) { | ||
| return fmt.Errorf("%s binary not found at %s", prettyName, serviceBinary) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix false positives for Docker-backed services by adding Docker-specific validation
When serviceFile == "" (Docker), the function skips all checks and returns nil, incorrectly treating the service as initiated. Add Docker validations: ensure Docker CLI exists, relayer home exists, and config.json is present.
Apply within this hunk:
- // TODO: validate docker?
- if serviceFile != "" {
+ // Docker-backed services may not have a service file; validate accordingly.
+ if serviceFile != "" {
if !weaveio.FileOrFolderExists(serviceFile) {
return fmt.Errorf("service file %s not found", serviceFile)
}
serviceBinary, serviceHome, err := service.GetServiceBinaryAndHome()
if err != nil {
return fmt.Errorf("could not determine %v binary and home directory: %w", prettyName, err)
}
if !weaveio.FileOrFolderExists(serviceHome) {
return fmt.Errorf("home directory %s not found", serviceHome)
}
if !weaveio.FileOrFolderExists(serviceBinary) {
return fmt.Errorf("%s binary not found at %s", prettyName, serviceBinary)
}
+ } else {
+ // Docker-based validation
+ if _, err := exec.LookPath("docker"); err != nil {
+ return fmt.Errorf("docker CLI not found: %w", err)
+ }
+ userHome, err := os.UserHomeDir()
+ if err != nil {
+ return fmt.Errorf("failed to get user home directory: %w", err)
+ }
+ relayerHome := filepath.Join(userHome, common.RelayerDirectory)
+ if !weaveio.FileOrFolderExists(relayerHome) {
+ return fmt.Errorf("relayer home directory %s not found", relayerHome)
+ }
+ cfg := filepath.Join(relayerHome, "config.json")
+ if !weaveio.FileOrFolderExists(cfg) {
+ return fmt.Errorf("missing relayer config at %s", cfg)
+ }
}Also update imports:
// cmd/validate.go
import (
"fmt"
"os"
"os/exec"
"path/filepath"
"github.com/spf13/cobra"
"github.com/initia-labs/weave/common"
weaveio "github.com/initia-labs/weave/io"
"github.com/initia-labs/weave/service"
)🤖 Prompt for AI Agents
In cmd/validate.go around lines 30 to 48, the current validation skips checks
when serviceFile is empty, causing false positives for Docker-backed services.
Add validation for Docker by checking if the Docker CLI executable exists using
exec.LookPath, verify the relayer home directory exists, and confirm the
presence of config.json in the relayer home. Also, update the imports to include
"os", "os/exec", and "path/filepath" as specified to support these checks.
| // Get the user's home directory | ||
| userHome, err := os.UserHomeDir() | ||
| if err != nil { | ||
| return ui.NonRetryableErrorLoading{Err: fmt.Errorf("failed to get user home directory: %v", err)} | ||
| } | ||
|
|
||
| srv, err := service.NewService(service.Relayer) | ||
| if err != nil { | ||
| return ui.NonRetryableErrorLoading{Err: fmt.Errorf("failed to initialize service: %v", err)} | ||
| } | ||
|
|
||
| // TODO: fix version | ||
| if err = srv.Create("", filepath.Join(userHome, common.RelayerDirectory)); err != nil { | ||
| return ui.NonRetryableErrorLoading{Err: fmt.Errorf("failed to create service: %v", err)} | ||
| } | ||
|
|
||
| // prune existing logs, ignore error | ||
| _ = srv.PruneLogs() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Pass a pinned Docker image tag to Create; align success message with Docker workflow
srv.Create("", ...)uses an empty version; this is brittle. Pass a pinned image tag (configurable), or at minimum"latest"as an interim, and wire a TODO to move to a config-driven version.- Consider updating the post-setup success instructions to reflect Docker-based start/stop via
weave relayerrather than manualnpm start.
Apply minimal fix for the version parameter:
- // TODO: fix version
- if err = srv.Create("", filepath.Join(userHome, common.RelayerDirectory)); err != nil {
+ // TODO: replace default tag with a pinned, config-driven version (e.g., from build info or config)
+ const defaultRelayerTag = "latest"
+ if err = srv.Create(defaultRelayerTag, filepath.Join(userHome, common.RelayerDirectory)); err != nil {
return ui.NonRetryableErrorLoading{Err: fmt.Errorf("failed to create service: %v", err)}
}Additionally, consider updating the success message to reflect Docker management:
// models/relayer/init.go
func getRelayerSetSuccessMessage() string {
userHome, _ := os.UserHomeDir()
relayerHome := filepath.Join(userHome, common.RelayerDirectory)
s := "\n" + styles.RenderPrompt("Rapid relayer config is generated successfully!", []string{}, styles.Completed)
s += "\n" + styles.RenderPrompt(fmt.Sprintf("Config file is saved at %s/config.json. You can modify it as needed.", relayerHome), []string{}, styles.Information)
s += "\n" + styles.RenderPrompt("To start relaying with Docker:", []string{}, styles.Empty)
s += "\n" + styles.RenderPrompt("1. weave relayer start", []string{}, styles.Empty)
s += "\n" + styles.RenderPrompt("2. weave relayer log", []string{}, styles.Empty)
return s + "\n"
}🤖 Prompt for AI Agents
In models/relayer/init.go around lines 2524 to 2542, the call to srv.Create
currently passes an empty string for the version parameter, which is brittle.
Update this call to pass a pinned Docker image tag such as "latest" as a minimal
fix, and add a TODO comment to make this configurable in the future.
Additionally, update the success message function to reflect Docker-based
start/stop commands by replacing manual npm instructions with "weave relayer
start" and "weave relayer log" commands as shown in the provided example.
| func (d *Docker) Create(version, appHome string) error { | ||
| // Pull the appropriate image based on command type and version | ||
| imageName, err := d.getImageName("main") | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get image name: %v", err) | ||
| } | ||
|
|
||
| pullCmd := exec.Command("docker", "pull", imageName) | ||
| if output, err := pullCmd.CombinedOutput(); err != nil { | ||
| return fmt.Errorf("failed to pull docker image: %v, output: %s", err, string(output)) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version argument ignored
Create() receives version but immediately hard-codes "main" when calling getImageName, rendering the parameter useless and preventing users from pinning images.
Same issue appears in Start().
- imageName, err := d.getImageName("main")
+ imageName, err := d.getImageName(version)Apply the same fix in Start().
📝 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.
| func (d *Docker) Create(version, appHome string) error { | |
| // Pull the appropriate image based on command type and version | |
| imageName, err := d.getImageName("main") | |
| if err != nil { | |
| return fmt.Errorf("failed to get image name: %v", err) | |
| } | |
| pullCmd := exec.Command("docker", "pull", imageName) | |
| if output, err := pullCmd.CombinedOutput(); err != nil { | |
| return fmt.Errorf("failed to pull docker image: %v, output: %s", err, string(output)) | |
| } | |
| return nil | |
| } | |
| func (d *Docker) Create(version, appHome string) error { | |
| // Pull the appropriate image based on command type and version | |
| - imageName, err := d.getImageName("main") | |
| + imageName, err := d.getImageName(version) | |
| if err != nil { | |
| return fmt.Errorf("failed to get image name: %v", err) | |
| } | |
| pullCmd := exec.Command("docker", "pull", imageName) | |
| if output, err := pullCmd.CombinedOutput(); err != nil { | |
| return fmt.Errorf("failed to pull docker image: %v, output: %s", err, string(output)) | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In service/docker.go around lines 22 to 35, the Create() method receives a
version argument but ignores it by hardcoding "main" when calling getImageName,
which prevents users from specifying image versions. To fix this, replace the
hardcoded "main" with the version parameter in the getImageName call. Then,
apply the same change in the Start() method to ensure consistency in version
handling.
| args := []string{ | ||
| "run", | ||
| "-d", | ||
| "--name", serviceName, | ||
| "--restart", "unless-stopped", | ||
| "--network", "host", | ||
| // TODO: do we need this? | ||
| "-v", fmt.Sprintf("%s:/app/data", appHome), | ||
| } | ||
| args = append(args, optionalArgs...) | ||
|
|
||
| // Add volume mount | ||
| if volumes, err := d.getVolumeMount(); err != nil { | ||
| return err | ||
| } else { | ||
| args = append(args, volumes...) | ||
| } | ||
|
|
||
| // Add port mappings | ||
| if ports, err := d.getPortMappings(); err != nil { | ||
| return err | ||
| } else { | ||
| args = append(args, ports...) | ||
| } | ||
|
|
||
| // Add the image name | ||
| if imageName, err := d.getImageName("main"); err != nil { | ||
| return err | ||
| } else { | ||
| args = append(args, imageName) | ||
| } | ||
|
|
||
| // Add command arguments | ||
| if cmdArgs, err := d.getCommandArgs(); err != nil { | ||
| return err | ||
| } else { | ||
| args = append(args, cmdArgs...) | ||
| } | ||
|
|
||
| cmd := exec.Command("docker", args...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple runtime issues in docker run assembly
-
--network hosttogether with explicit-pmappings – the port mapping flags are ignored on host network and create confusion. -
Host-network is not available on Docker Desktop (macOS/Windows).
service.NewServiceselects Docker unconditionally, soweave relayer startwill fail on those platforms. -
-v <path>:/app/datamounts a host path pointing inside Docker’s volume directory (/var/lib/docker/volumes/...).
That path may not exist yet and breaks portability.
Prefer-v <volumeName>:/app/dataor--mount source=<volumeName>,target=/app/data.
Recommend:
- "--network", "host",
- "-v", fmt.Sprintf("%s:/app/data", appHome),
+ "--network", "bridge",
+ "-v", fmt.Sprintf("%s:/app/data", volumeName), // ensure volume exists via `docker volume create`and skip getPortMappings() when host networking is chosen.
Failing to address these points will lead to start-up errors on non-Linux hosts and silent port-exposure bugs on Linux.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In service/docker.go between lines 49 and 88, fix the docker run command
assembly by skipping the addition of port mappings when the host network mode is
used, since port mappings are ignored in that mode. Also, modify the volume
mount to use a named Docker volume or the --mount syntax instead of mounting a
host path inside Docker's volume directory to improve portability and avoid
missing path errors. Additionally, consider adding platform checks to avoid
using host networking on Docker Desktop for macOS/Windows, as it is unsupported
there, to prevent startup failures.
| func (d *Docker) PruneLogs() error { | ||
| serviceName, err := d.GetServiceName() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| cmd := exec.Command("docker", "logs", "--truncate", "0", serviceName) | ||
| return cmd.Run() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Invalid log-pruning command
docker logs has no --truncate flag; this call will error.
Common approaches are:
docker logs --since 1s <container>
docker ps --filter … | xargs -I{} sh -c 'truncate -s 0 $(docker inspect ...)'
Please replace with a supported method or drop the helper.
🤖 Prompt for AI Agents
In service/docker.go around lines 246 to 254, the PruneLogs function uses an
invalid docker command with a non-existent --truncate flag on docker logs, which
causes errors. Replace this command with a supported approach such as using
"docker logs --since 1s <container>" to effectively prune logs or implement a
method that truncates the log file by inspecting the container's log path and
truncating it directly. Alternatively, if pruning logs is not feasible, consider
removing this helper function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @models/relayer/init.go:
- Around line 2536-2539: The call to srv.Create currently passes an empty
version string which is brittle; change the version argument from "" to "latest"
in the srv.Create call (the call that uses userHome and common.RelayerDirectory)
and add a TODO comment to make the image tag configurable (e.g., via build info
or config) so it can be replaced later with a pinned tag or config value; keep
the error handling as-is (wrapping the error in ui.NonRetryableErrorLoading).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
models/relayer/init.go
🧰 Additional context used
🧬 Code graph analysis (1)
models/relayer/init.go (4)
ui/loading.go (1)
NonRetryableErrorLoading(145-147)service/service.go (1)
NewService(24-37)service/types.go (1)
Relayer(13-13)common/constants.go (1)
RelayerDirectory(27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Integration Tests on macOS
| // TODO: fix version | ||
| if err = srv.Create("", filepath.Join(userHome, common.RelayerDirectory)); err != nil { | ||
| return ui.NonRetryableErrorLoading{Err: fmt.Errorf("failed to create service: %v", err)} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass a pinned Docker image tag instead of empty string for version parameter.
The empty version string passed to srv.Create is brittle and may result in unpredictable behavior. As a minimal fix, pass "latest" and add a TODO to make this configurable (e.g., from build info or config).
🔎 Proposed fix
- // TODO: fix version
- if err = srv.Create("", filepath.Join(userHome, common.RelayerDirectory)); err != nil {
+ // TODO: replace default tag with a pinned, config-driven version (e.g., from build info or config)
+ const defaultRelayerTag = "latest"
+ if err = srv.Create(defaultRelayerTag, filepath.Join(userHome, common.RelayerDirectory)); err != nil {
return ui.NonRetryableErrorLoading{Err: fmt.Errorf("failed to create service: %v", err)}
}📝 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.
| // TODO: fix version | |
| if err = srv.Create("", filepath.Join(userHome, common.RelayerDirectory)); err != nil { | |
| return ui.NonRetryableErrorLoading{Err: fmt.Errorf("failed to create service: %v", err)} | |
| } | |
| // TODO: replace default tag with a pinned, config-driven version (e.g., from build info or config) | |
| const defaultRelayerTag = "latest" | |
| if err = srv.Create(defaultRelayerTag, filepath.Join(userHome, common.RelayerDirectory)); err != nil { | |
| return ui.NonRetryableErrorLoading{Err: fmt.Errorf("failed to create service: %v", err)} | |
| } |
🤖 Prompt for AI Agents
In @models/relayer/init.go around lines 2536-2539, The call to srv.Create
currently passes an empty version string which is brittle; change the version
argument from "" to "latest" in the srv.Create call (the call that uses userHome
and common.RelayerDirectory) and add a TODO comment to make the image tag
configurable (e.g., via build info or config) so it can be replaced later with a
pinned tag or config value; keep the error handling as-is (wrapping the error in
ui.NonRetryableErrorLoading).
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
I have...
!in the type prefix if API or client breaking changeReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
Improvements
Behavior
✏️ Tip: You can customize this high-level summary in your review settings.