Skip to content
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

Deprecate db url (Do not merge) #5044

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 16 additions & 1 deletion services/horizon/internal/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ const (
StellarPubnet = "pubnet"
// StellarTestnet is a constant representing the Stellar test network
StellarTestnet = "testnet"

dbUrlDeprecationWarning = "DEPRECATION WARNING: --db-url is deprecated and will be removed in a future" +
" Horizon release. Please use the DATABASE_URL environment variable to configure Horizon's" +
" PostgreSQL database connection"
)

// validateBothOrNeither ensures that both options are provided, if either is provided.
Expand Down Expand Up @@ -132,7 +136,18 @@ func Flags() (*Config, support.ConfigOptions) {
ConfigKey: &config.DatabaseURL,
OptType: types.String,
Required: true,
Usage: "horizon postgres database to connect with",
Usage: dbUrlDeprecationWarning,
CustomSetValue: func(co *support.ConfigOption) error {
if val := viper.GetString(DatabaseURLFlagName); val != "" {
if co.IsSetViaCommandline() {
stdLog.Printf(dbUrlDeprecationWarning)

}
config.DatabaseURL = val
}
return nil
},
EnvOnly: false, // set this to true when removing this option from the commandline
},
&support.ConfigOption{
Name: "ro-database-url",
Expand Down
30 changes: 30 additions & 0 deletions services/horizon/internal/flags_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package horizon

import (
"bytes"
"fmt"
"io"
stdLog "log"
"testing"

"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -131,3 +135,29 @@ func Test_createCaptiveCoreConfig(t *testing.T) {
})
}
}

func TestDatabaseUrlDeprecationWarning(t *testing.T) {
origWriter := stdLog.Writer()
defer stdLog.SetOutput(origWriter)

config, flags := Flags()
horizonCmd := &cobra.Command{
Use: "horizon",
RunE: func(cmd *cobra.Command, args []string) error {
err := ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreConfig: true, AlwaysIngest: false})
return err
},
}

var writer io.Writer = &bytes.Buffer{}
stdLog.SetOutput(writer)
horizonCmd.SetArgs([]string{"--" + DatabaseURLFlagName + "=testUrl",
"--" + IngestFlagName + "=false",
"--" + EnableCaptiveCoreIngestionFlagName + "=false",
"--" + StellarCoreURLFlagName + "=testUrl",
})
_ = flags.Init(horizonCmd)
err := horizonCmd.Execute()
assert.NoError(t, err)
assert.Contains(t, writer.(*bytes.Buffer).String(), dbUrlDeprecationWarning)
}
21 changes: 16 additions & 5 deletions support/config/config_option.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ type ConfigOption struct {
ConfigKey interface{} // Pointer to the final key in the linked Config struct
flag *pflag.Flag // The persistent flag that the config option is attached to
Hidden bool // Indicates whether to hide the flag from --help output
EnvOnly bool // Indicates the option is supported via environment variable only (no commandline)
}

// Init handles initialisation steps, including configuring and binding the env variable name.
Expand All @@ -79,20 +80,25 @@ func (co *ConfigOption) Init(cmd *cobra.Command) error {
if co.EnvVar == "" {
co.EnvVar = strutils.KebabToConstantCase(co.Name)
}
// Initialise and bind the persistent flags
return co.setFlag(cmd)
if !co.EnvOnly {
// Initialise and bind the persistent flags
return co.setFlag(cmd)
}
return nil
}

// SetDeprecated Hides the deprecated flag from --help output
func (co *ConfigOption) SetDeprecated(cmd *cobra.Command) {
if co.Hidden {
if !co.EnvOnly && co.Hidden {
co.flag.Hidden = true
}
}

// Bind binds the config option to viper.
func (co *ConfigOption) Bind() {
viper.BindPFlag(co.Name, co.flag)
if !co.EnvOnly {
viper.BindPFlag(co.Name, co.flag)
}
viper.BindEnv(co.Name, co.EnvVar)
}

Expand Down Expand Up @@ -237,7 +243,12 @@ var envVars = parseEnvVars(os.Environ())
func IsExplicitlySet(co *ConfigOption) bool {
// co.flag.Changed is only set to true when the configuration is set via command line parameter.
// In the case where a variable is configured via environment variable we need to check envVars.
return co.flag.Changed || envVars[co.EnvVar]
return (co.flag != nil && co.flag.Changed) || envVars[co.EnvVar]
}

// IsSetViaCommandline returns true if the config option was set via commandline
func (co *ConfigOption) IsSetViaCommandline() bool {
return co.flag != nil && co.flag.Value.String() != ""
}

// SetOptionalString converts a command line uint to a *string where the nil
Expand Down