Skip to content

Commit

Permalink
Set the default data directory correctly (#1249)
Browse files Browse the repository at this point in the history
We now use cwd/juno as the data directory.

Previously, we set the default to the empty string "" and manually set
the default directory in `node.New`. This meant the incorrect default
was printed in two cases:

1. `--help`
2. When printing the config on boot.

Finally, this centralizes all of our config management to the
`cmd/juno` package; if `juno.NewCmd.Execute` returns without an error,
the config is structurally valid.
  • Loading branch information
joshklop authored Sep 26, 2023
1 parent 5a6b72e commit c9e9b01
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 164 deletions.
18 changes: 17 additions & 1 deletion cmd/juno/juno.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"os"
"os/signal"
"path/filepath"
"syscall"
"time"

Expand Down Expand Up @@ -60,7 +61,6 @@ const (
defaultHTTPPort = 6060
defaultWS = false
defaultWSPort = 6061
defaultDBPath = ""
defaultEthNode = ""
defaultPprof = false
defaultPprofPort = 6062
Expand Down Expand Up @@ -148,10 +148,17 @@ func NewCmd(config *node.Config, run func(*cobra.Command, []string) error) *cobr
}

var cfgFile string
var cwdErr error

// PreRunE populates the configuration struct from the Cobra flags and Viper configuration.
// This is called in step 3 of the process described above.
junoCmd.PreRunE = func(cmd *cobra.Command, _ []string) error {
// If we couldn't find the current working directory and the database path is empty,
// return the error.
if cwdErr != nil && config.DatabasePath == "" {
return fmt.Errorf("find current working directory: %v", cwdErr)
}

v := viper.New()
if cfgFile != "" {
v.SetConfigType("yaml")
Expand All @@ -171,6 +178,15 @@ func NewCmd(config *node.Config, run func(*cobra.Command, []string) error) *cobr
mapstructure.TextUnmarshallerHookFunc(), mapstructure.StringToTimeDurationHookFunc())))
}

var defaultDBPath string
defaultDBPath, cwdErr = os.Getwd()
// Use empty string if we can't get the working directory.
// We don't want to return an error here since that would make `--help` fail.
// If the error is non-nil and a db path is not provided by the user, we'll return it in PreRunE.
if cwdErr == nil {
defaultDBPath = filepath.Join(defaultDBPath, "juno")
}

// For testing purposes, these variables cannot be declared outside the function because Cobra
// may mutate their values.
defaultLogLevel := utils.INFO
Expand Down
7 changes: 6 additions & 1 deletion cmd/juno/juno_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main_test
import (
"context"
"os"
"path/filepath"
"testing"
"time"

Expand All @@ -15,6 +16,9 @@ import (
)

func TestConfigPrecedence(t *testing.T) {
pwd, err := os.Getwd()
require.NoError(t, err)

// The purpose of these tests are to ensure the precedence of our config
// values is respected. Since viper offers this feature, it would be
// redundant to enumerate all combinations. Thus, only a select few are
Expand All @@ -27,7 +31,7 @@ func TestConfigPrecedence(t *testing.T) {
defaultHTTPPort := uint16(6060)
defaultWS := false
defaultWSPort := uint16(6061)
defaultDBPath := ""
defaultDBPath := filepath.Join(pwd, "juno")
defaultNetwork := utils.MAINNET
defaultPprof := false
defaultPprofPort := uint16(6062)
Expand Down Expand Up @@ -122,6 +126,7 @@ func TestConfigPrecedence(t *testing.T) {
Pprof: defaultPprof,
PprofHost: defaultHost,
PprofPort: defaultPprofPort,
DatabasePath: defaultDBPath,
},
},
"config file with all settings but without any other flags": {
Expand Down
8 changes: 0 additions & 8 deletions node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"net/url"
"os"
"path/filepath"
"reflect"
"runtime"
"time"
Expand Down Expand Up @@ -85,13 +84,6 @@ type Node struct {
// New sets the config and logger to the StarknetNode.
// Any errors while parsing the config on creating logger will be returned.
func New(cfg *Config, version string) (*Node, error) { //nolint:gocyclo,funlen
if cfg.DatabasePath == "" {
dirPrefix, err := utils.DefaultDataDir()
if err != nil {
return nil, err
}
cfg.DatabasePath = filepath.Join(dirPrefix, cfg.Network.String())
}
log, err := utils.NewZapLogger(cfg.LogLevel, cfg.Colour)
if err != nil {
return nil, err
Expand Down
23 changes: 0 additions & 23 deletions node/node_test.go
Original file line number Diff line number Diff line change
@@ -1,37 +1,14 @@
package node_test

import (
"path/filepath"
"testing"
"time"

"github.com/NethermindEth/juno/node"
"github.com/NethermindEth/juno/utils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestDefaultDbPath(t *testing.T) {
defaultDataDir, err := utils.DefaultDataDir()
require.NoError(t, err)

networks := []utils.Network{utils.GOERLI, utils.MAINNET, utils.GOERLI2, utils.INTEGRATION}

for _, n := range networks {
t.Run(n.String(), func(t *testing.T) {
cfg := &node.Config{Network: n, DatabasePath: ""}
expectedCfg := node.Config{
Network: n,
DatabasePath: filepath.Join(defaultDataDir, n.String()),
}
snNode, err := node.New(cfg, "1.2.3")
require.NoError(t, err)

assert.Equal(t, expectedCfg, snNode.Config())
})
}
}

// Create a new node with all services enabled.
func TestNewNode(t *testing.T) {
config := &node.Config{
Expand Down
50 changes: 0 additions & 50 deletions utils/datadir.go

This file was deleted.

81 changes: 0 additions & 81 deletions utils/datadir_test.go

This file was deleted.

0 comments on commit c9e9b01

Please sign in to comment.