Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Commit

Permalink
Kelp UI: Fix filepaths for windows by introducing the kelpos.OsPath s…
Browse files Browse the repository at this point in the history
…truct (#400)

* 1 - add OSPath struct with test

* 2 - cleaner interface and factory structure for ospath

* 3a - partial replacement of paths to use ospath

* 3b - more partial replacement of paths to use ospath

* 3c - some more partial replacement of paths to use ospath

* 3d - fix compile errors in ospath refactoring

* 4 - update configsDir usage in gui/backend

* 5 - fix start_bot.go

* 6 - remove support for OSPath.String() because of ambiguity

* 7 - change panic in ospath.String() to log.Fatal call

* 8 - fix log calls to ospath by replacing Stringer function with AsString()

* 9 - added support for relative paths to ospath + test

* 10 - start_bot.go uses relative paths to start off bot in the gui

* 11 - added comment on choice to use relative paths when starting bots from the GUI
  • Loading branch information
nikhilsaraf authored Apr 8, 2020
1 parent 8729754 commit e6d89f7
Show file tree
Hide file tree
Showing 14 changed files with 334 additions and 147 deletions.
169 changes: 68 additions & 101 deletions cmd/server_amd64.go

Large diffs are not rendered by default.

34 changes: 14 additions & 20 deletions gui/backend/api_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,17 @@ import (
"log"
"net/http"
"os"
"path/filepath"

"github.com/stellar/go/clients/horizonclient"
"github.com/stellar/kelp/support/kelpos"
)

// APIServer is an instance of the API service
type APIServer struct {
dirPath string
binPath string
configsDir string
logsDir string
basepath *kelpos.OSPath
kelpBinPath *kelpos.OSPath
configsDir *kelpos.OSPath
logsDir *kelpos.OSPath
kos *kelpos.KelpOS
horizonTestnetURI string
horizonPubnetURI string
Expand All @@ -34,7 +33,7 @@ type APIServer struct {
// MakeAPIServer is a factory method
func MakeAPIServer(
kos *kelpos.KelpOS,
currentDirUnix string,
basepath *kelpos.OSPath,
horizonTestnetURI string,
apiTestNet *horizonclient.Client,
horizonPubnetURI string,
Expand All @@ -43,23 +42,18 @@ func MakeAPIServer(
noHeaders bool,
quitFn func(),
) (*APIServer, error) {
binPath, e := filepath.Abs(os.Args[0])
if e != nil {
return nil, fmt.Errorf("could not get binPath of currently running binary: %s", e)
}

dirPath := currentDirUnix
configsDir := filepath.Join(currentDirUnix, "ops", "configs")
logsDir := filepath.Join(currentDirUnix, "ops", "logs")
kelpBinPath := basepath.Join(os.Args[0])
configsDir := basepath.Join("ops", "configs")
logsDir := basepath.Join("ops", "logs")

optionsMetadata, e := loadOptionsMetadata()
if e != nil {
return nil, fmt.Errorf("error while loading options metadata when making APIServer: %s", e)
}

return &APIServer{
dirPath: dirPath,
binPath: binPath,
basepath: basepath,
kelpBinPath: kelpBinPath,
configsDir: configsDir,
logsDir: logsDir,
kos: kos,
Expand Down Expand Up @@ -125,24 +119,24 @@ func (s *APIServer) writeJsonWithLog(w http.ResponseWriter, v interface{}, doLog
}

func (s *APIServer) runKelpCommandBlocking(namespace string, cmd string) ([]byte, error) {
cmdString := fmt.Sprintf("%s %s", s.binPath, cmd)
cmdString := fmt.Sprintf("%s %s", s.kelpBinPath.Unix(), cmd)
return s.kos.Blocking(namespace, cmdString)
}

func (s *APIServer) runKelpCommandBackground(namespace string, cmd string) (*kelpos.Process, error) {
cmdString := fmt.Sprintf("%s %s", s.binPath, cmd)
cmdString := fmt.Sprintf("%s %s", s.kelpBinPath.Unix(), cmd)
return s.kos.Background(namespace, cmdString)
}

func (s *APIServer) setupOpsDirectory() error {
e := s.kos.Mkdir(s.configsDir)
if e != nil {
return fmt.Errorf("error setting up configs directory: %s\n", e)
return fmt.Errorf("error setting up configs directory (%s): %s\n", s.configsDir, e)
}

e = s.kos.Mkdir(s.logsDir)
if e != nil {
return fmt.Errorf("error setting up logs directory: %s\n", e)
return fmt.Errorf("error setting up logs directory (%s): %s\n", s.logsDir, e)
}

return nil
Expand Down
12 changes: 6 additions & 6 deletions gui/backend/autogenerate_bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,18 @@ func (s *APIServer) autogenerateBot(w http.ResponseWriter, r *http.Request) {

filenamePair := bot.Filenames()
sampleTrader := s.makeSampleTrader(kp.Seed())
traderFilePath := fmt.Sprintf("%s/%s", s.configsDir, filenamePair.Trader)
log.Printf("writing autogenerated bot config to file: %s\n", traderFilePath)
e = toml.WriteFile(traderFilePath, sampleTrader)
traderFilePath := s.configsDir.Join(filenamePair.Trader)
log.Printf("writing autogenerated bot config to file: %s\n", traderFilePath.AsString())
e = toml.WriteFile(traderFilePath.Native(), sampleTrader)
if e != nil {
s.writeError(w, fmt.Sprintf("error writing trader toml file: %s\n", e))
return
}

sampleBuysell := makeSampleBuysell()
strategyFilePath := fmt.Sprintf("%s/%s", s.configsDir, filenamePair.Strategy)
log.Printf("writing autogenerated strategy config to file: %s\n", strategyFilePath)
e = toml.WriteFile(strategyFilePath, sampleBuysell)
strategyFilePath := s.configsDir.Join(filenamePair.Strategy)
log.Printf("writing autogenerated strategy config to file: %s\n", strategyFilePath.AsString())
e = toml.WriteFile(strategyFilePath.Native(), sampleBuysell)
if e != nil {
s.writeError(w, fmt.Sprintf("error writing strategy toml file: %s\n", e))
return
Expand Down
3 changes: 2 additions & 1 deletion gui/backend/delete_bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ func (s *APIServer) deleteBot(w http.ResponseWriter, r *http.Request) {

// delete configs
botPrefix := model2.GetPrefix(botName)
_, e = s.kos.Blocking("rm", fmt.Sprintf("rm %s/%s*", s.configsDir, botPrefix))
botConfigPath := s.configsDir.Join(botPrefix)
_, e = s.kos.Blocking("rm", fmt.Sprintf("rm %s*", botConfigPath.Unix()))
if e != nil {
s.writeError(w, fmt.Sprintf("error running rm command for bot configs: %s\n", e))
return
Expand Down
2 changes: 1 addition & 1 deletion gui/backend/generate_bot_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (s *APIServer) doGenerateBotName() (string, error) {
}

func (s *APIServer) prefixExists(prefix string) (bool, error) {
command := fmt.Sprintf("ls %s | grep %s", s.configsDir, prefix)
command := fmt.Sprintf("ls %s | grep %s", s.configsDir.Unix(), prefix)
_, e := s.kos.Blocking("prefix", command)
if e != nil {
if strings.Contains(e.Error(), "exit status 1") {
Expand Down
8 changes: 4 additions & 4 deletions gui/backend/get_bot_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@ func (s *APIServer) getBotConfig(w http.ResponseWriter, r *http.Request) {
}

filenamePair := model2.GetBotFilenames(botName, "buysell")
traderFilePath := fmt.Sprintf("%s/%s", s.configsDir, filenamePair.Trader)
traderFilePath := s.configsDir.Join(filenamePair.Trader)
var botConfig trader.BotConfig
e = config.Read(traderFilePath, &botConfig)
e = config.Read(traderFilePath.Native(), &botConfig)
if e != nil {
s.writeErrorJson(w, fmt.Sprintf("cannot read bot config at path '%s': %s\n", traderFilePath, e))
return
}
strategyFilePath := fmt.Sprintf("%s/%s", s.configsDir, filenamePair.Strategy)
strategyFilePath := s.configsDir.Join(filenamePair.Strategy)
var buysellConfig plugins.BuySellConfig
e = config.Read(strategyFilePath, &buysellConfig)
e = config.Read(strategyFilePath.Native(), &buysellConfig)
if e != nil {
s.writeErrorJson(w, fmt.Sprintf("cannot read strategy config at path '%s': %s\n", strategyFilePath, e))
return
Expand Down
4 changes: 2 additions & 2 deletions gui/backend/get_bot_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ func (s *APIServer) runGetBotInfoDirect(w http.ResponseWriter, botName string) {
}

filenamePair := model2.GetBotFilenames(botName, buysell)
traderFilePath := fmt.Sprintf("%s/%s", s.configsDir, filenamePair.Trader)
traderFilePath := s.configsDir.Join(filenamePair.Trader)
var botConfig trader.BotConfig
e = config.Read(traderFilePath, &botConfig)
e = config.Read(traderFilePath.Native(), &botConfig)
if e != nil {
s.writeErrorJson(w, fmt.Sprintf("cannot read bot config at path '%s': %s\n", traderFilePath, e))
return
Expand Down
2 changes: 1 addition & 1 deletion gui/backend/list_bots.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

func (s *APIServer) listBots(w http.ResponseWriter, r *http.Request) {
log.Printf("listing bots\n")
resultBytes, e := s.kos.Blocking("ls", fmt.Sprintf("ls %s | sort", s.configsDir))
resultBytes, e := s.kos.Blocking("ls", fmt.Sprintf("ls %s | sort", s.configsDir.Unix()))
if e != nil {
s.writeErrorJson(w, fmt.Sprintf("error when listing bots: %s\n", e))
return
Expand Down
28 changes: 27 additions & 1 deletion gui/backend/start_bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,33 @@ func (s *APIServer) startBot(w http.ResponseWriter, r *http.Request) {
func (s *APIServer) doStartBot(botName string, strategy string, iterations *uint8, maybeFinishCallback func()) error {
filenamePair := model2.GetBotFilenames(botName, strategy)
logPrefix := model2.GetLogPrefix(botName, strategy)
command := fmt.Sprintf("trade -c %s/%s -s %s -f %s/%s -l %s/%s --ui", s.configsDir, filenamePair.Trader, strategy, s.configsDir, filenamePair.Strategy, s.logsDir, logPrefix)

// use relative paths here so it works under windows. In windows we use unix paths to reference the config files since it is
// started under the linux subsystem, but it is a windows binary so uses the windows naming scheme (C:\ etc.). Therefore we need
// to either find a regex replacement to convert from unix to windows (/mnt/c -> C:\) or we can use relative paths which we did.
// Note that /mnt/c is unlikely to be valid in windows (but is valid in the linux subsystem) since it's usually prefixed by the
// volume (C:\ etc.), which is why relative paths works so well here as it avoids this confusion.
traderRelativeConfigPath, e := s.configsDir.Join(filenamePair.Trader).RelFromPath(s.basepath)
if e != nil {
return fmt.Errorf("unable to get relative path of trader config file from basepath: %s", e)
}

stratRelativeConfigPath, e := s.configsDir.Join(filenamePair.Strategy).RelFromPath(s.basepath)
if e != nil {
return fmt.Errorf("unable to get relative path of strategy config file from basepath: %s", e)
}

logRelativePrefix, e := s.logsDir.Join(logPrefix).RelFromPath(s.basepath)
if e != nil {
return fmt.Errorf("unable to get relative log prefix from basepath: %s", e)
}

command := fmt.Sprintf("trade -c %s -s %s -f %s -l %s --ui",
traderRelativeConfigPath.Unix(),
strategy,
stratRelativeConfigPath.Unix(),
logRelativePrefix.Unix(),
)
if iterations != nil {
command = fmt.Sprintf("%s --iter %d", command, *iterations)
}
Expand Down
12 changes: 6 additions & 6 deletions gui/backend/upsert_bot_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,19 @@ func (s *APIServer) upsertBotConfig(w http.ResponseWriter, r *http.Request) {
}

filenamePair := model2.GetBotFilenames(req.Name, req.Strategy)
traderFilePath := fmt.Sprintf("%s/%s", s.configsDir, filenamePair.Trader)
traderFilePath := s.configsDir.Join(filenamePair.Trader)
botConfig := req.TraderConfig
log.Printf("upsert bot config to file: %s\n", traderFilePath)
e = toml.WriteFile(traderFilePath, &botConfig)
log.Printf("upsert bot config to file: %s\n", traderFilePath.AsString())
e = toml.WriteFile(traderFilePath.Native(), &botConfig)
if e != nil {
s.writeErrorJson(w, fmt.Sprintf("error writing trader botConfig toml file for bot '%s': %s", req.Name, e))
return
}

strategyFilePath := fmt.Sprintf("%s/%s", s.configsDir, filenamePair.Strategy)
strategyFilePath := s.configsDir.Join(filenamePair.Strategy)
strategyConfig := req.StrategyConfig
log.Printf("upsert strategy config to file: %s\n", strategyFilePath)
e = toml.WriteFile(strategyFilePath, &strategyConfig)
log.Printf("upsert strategy config to file: %s\n", strategyFilePath.AsString())
e = toml.WriteFile(strategyFilePath.Native(), &strategyConfig)
if e != nil {
s.writeErrorJson(w, fmt.Sprintf("error writing strategy toml file for bot '%s': %s", req.Name, e))
return
Expand Down
4 changes: 2 additions & 2 deletions scripts/ccxt_bin_gen/ccxt_bin_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func checkPkgTool(kos *kelpos.KelpOS) {

func downloadCcxtSource(kos *kelpos.KelpOS, downloadDir string) {
fmt.Printf("making directory where we can download ccxt file %s ... ", downloadDir)
e := kos.Mkdir(downloadDir)
_, e := kos.Blocking("mkdir", fmt.Sprintf("mkdir -p %s", downloadDir))
if e != nil {
log.Fatal(errors.Wrap(e, "could not make directory for downloadDir "+downloadDir))
}
Expand Down Expand Up @@ -147,7 +147,7 @@ func copyDependencyFiles(kos *kelpos.KelpOS, outDir string, pkgCmdOutput string)

func mkDir(kos *kelpos.KelpOS, zipDir string) {
fmt.Printf("making directory %s ... ", zipDir)
e := kos.Mkdir(zipDir)
_, e := kos.Blocking("mkdir", fmt.Sprintf("mkdir -p %s", zipDir))
if e != nil {
log.Fatal(errors.Wrap(e, "unable to make directory "+zipDir))
}
Expand Down
131 changes: 131 additions & 0 deletions support/kelpos/ospath.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package kelpos

import (
"fmt"
"log"
"os"
"path/filepath"
"runtime/debug"
"strings"
)

// OSPath encapsulates the pair of the native path (i.e. windows or unix) and the unix path
// this allows certain commands which are unix-specific to have access to the path instead of running transformations
type OSPath struct {
native string
unix string
isRel bool
}

// String() is the Stringer method which is unsupprted
func (o *OSPath) String() string {
log.Fatalf("String method is unsupported because the usage is ambiguous for this struct, use .Unix(), .Native(), or .AsString() instead, trace:\n%s", string(debug.Stack()))
return ""
}

// AsString produces a string representation and we intentionally don't use the Stringer API because this can mistakenly
// be used in place of a string path which will produce hidden runtime errors which is dangerous
func (o *OSPath) AsString() string {
return fmt.Sprintf("OSPath[native=%s, unix=%s, isRel=%v]", o.native, o.unix, o.isRel)
}

// makeOSPath is an internal helper that enforced always using toUnixFilepath on the unix path
func makeOSPath(native string, unix string, isRel bool) *OSPath {
return &OSPath{
native: filepath.Clean(native),
unix: toUnixFilepath(filepath.Clean(unix)),
isRel: isRel,
}
}

// MakeOsPathBase is a factory method for the OSPath struct based on the current binary's directory
func MakeOsPathBase() (*OSPath, error) {
currentDirUnslashed, e := getCurrentDirUnix()
if e != nil {
return nil, fmt.Errorf("could not get current directory: %s", e)
}
binaryDirectoryNative, e := getBinaryDirectoryNative()
if e != nil {
return nil, fmt.Errorf("could not get binary directory: %s", e)
}
ospath := makeOSPath(binaryDirectoryNative, currentDirUnslashed, false)

if filepath.Base(ospath.Native()) != filepath.Base(ospath.Unix()) {
return nil, fmt.Errorf("ran from directory (%s) but need to run from the same directory as the location of the binary (%s), cd over to the location of the binary", ospath.Unix(), ospath.Native())
}

return ospath, nil
}

// Native returns the native representation of the path as a string
func (o *OSPath) Native() string {
return o.native
}

// Unix returns the unix representation of the path as a string
func (o *OSPath) Unix() string {
return o.unix
}

// IsRelative returns true if this is a relative path, otherwise false
func (o *OSPath) IsRelative() bool {
return o.isRel
}

// Join makes a new OSPath struct by modifying the internal path representations together
func (o *OSPath) Join(elem ...string) *OSPath {
nativePaths := []string{o.native}
nativePaths = append(nativePaths, elem...)

unixPaths := []string{o.unix}
unixPaths = append(unixPaths, elem...)

return makeOSPath(
filepath.Join(nativePaths...),
filepath.Join(unixPaths...),
o.isRel,
)
}

// RelFromBase returns a *OSPath that is relative to the basepath
func (o *OSPath) RelFromBase() (*OSPath, error) {
basepath, e := MakeOsPathBase()
if e != nil {
return nil, fmt.Errorf("unable to fetch ospathbase: %s", e)
}

return o.RelFromPath(basepath)
}

// RelFromPath returns a *OSPath that is relative from the provided path
func (o *OSPath) RelFromPath(basepath *OSPath) (*OSPath, error) {
newRelNative, e := filepath.Rel(basepath.Native(), o.Native())
if e != nil {
return nil, fmt.Errorf("unable to make relative native path from basepath: %s", e)
}

newRelUnix, e := filepath.Rel(basepath.Unix(), o.Unix())
if e != nil {
return nil, fmt.Errorf("unable to make relative unix path from basepath: %s", e)
}

// set this to be a relative path
return makeOSPath(newRelNative, newRelUnix, true), nil
}

func getCurrentDirUnix() (string, error) {
kos := GetKelpOS()
outputBytes, e := kos.Blocking("pwd", "pwd")
if e != nil {
return "", fmt.Errorf("could not fetch current directory: %s", e)
}
return strings.TrimSpace(string(outputBytes)), nil
}

func getBinaryDirectoryNative() (string, error) {
return filepath.Abs(filepath.Dir(os.Args[0]))
}

func toUnixFilepath(path string) string {
return filepath.ToSlash(path)
}
Loading

0 comments on commit e6d89f7

Please sign in to comment.