database: refactor database.go to not use globals for server config
The database package used global variables to configure the database. This
does not play well with the new configuration infrastructure.
Change-Id: Ia791ec0b8a5b6f550ee27638dfe361d5f29e00d1
Reviewed-on: https://go-review.googlesource.com/38396
Reviewed-by: Tuo Shan <shantuo@google.com>
diff --git a/database/database.go b/database/database.go
index 7a52ae8..c6fb1e4 100644
--- a/database/database.go
+++ b/database/database.go
@@ -80,51 +80,49 @@
func (p byPath) Less(i, j int) bool { return p[i].Path < p[j].Path }
func (p byPath) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
-// Configuration variables (and default values of flags.
-var (
- RedisServer = "redis://127.0.0.1:6379" // URL of Redis server
- RedisIdleTimeout = 250 * time.Second // Close Redis connections after remaining idle for this duration.
- RedisLog = false // Log database commands
-)
-
-func dialDb() (c redis.Conn, err error) {
- u, err := url.Parse(RedisServer)
- if err != nil {
- return nil, err
- }
-
- defer func() {
- if err != nil && c != nil {
- c.Close()
+// newDBDialer returns a function which returns connections to a redis
+// database at the given server uri. The server uri should be in the form of:
+// redis://user:pass@host:port
+func newDBDialer(server string, logConn bool) func() (c redis.Conn, err error) {
+ return func() (c redis.Conn, err error) {
+ u, err := url.Parse(server)
+ if err != nil {
+ return nil, err
}
- }()
- c, err = redis.Dial("tcp", u.Host)
- if err != nil {
- return
- }
+ defer func() {
+ if err != nil && c != nil {
+ c.Close()
+ }
+ }()
- if RedisLog {
- l := log.New(os.Stderr, "", log.LstdFlags)
- c = redis.NewLoggingConn(c, l, "")
- }
+ c, err = redis.Dial("tcp", u.Host)
+ if err != nil {
+ return c, err
+ }
- if u.User != nil {
- if pw, ok := u.User.Password(); ok {
- if _, err = c.Do("AUTH", pw); err != nil {
- return
+ if logConn {
+ l := log.New(os.Stderr, "", log.LstdFlags)
+ c = redis.NewLoggingConn(c, l, "")
+ }
+
+ if u.User != nil {
+ if pw, ok := u.User.Password(); ok {
+ if _, err = c.Do("AUTH", pw); err != nil {
+ return c, err
+ }
}
}
+ return c, err
}
- return
}
-// New creates a database configured from command line flags.
-func New() (*Database, error) {
+// New creates a gddo database
+func New(serverUri string, idleTimeout time.Duration, logConn bool) (*Database, error) {
pool := &redis.Pool{
- Dial: dialDb,
+ Dial: newDBDialer(serverUri, logConn),
MaxIdle: 10,
- IdleTimeout: RedisIdleTimeout,
+ IdleTimeout: idleTimeout,
}
c := pool.Get()
diff --git a/gddo-server/config.go b/gddo-server/config.go
index 00ed5f4..be929df 100644
--- a/gddo-server/config.go
+++ b/gddo-server/config.go
@@ -7,7 +7,6 @@
"strings"
"time"
- "github.com/golang/gddo/database"
"github.com/golang/gddo/log"
"github.com/spf13/pflag"
@@ -19,21 +18,31 @@
)
const (
- ConfigMaxAge = "max_age"
- ConfigGetTimeout = "get_timeout"
- ConfigRobotThreshold = "robot"
- ConfigAssetsDir = "assets"
- ConfigFirstGetTimeout = "first_get_timeout"
- ConfigBindAddress = "http"
+ // Server Config
ConfigProject = "project"
ConfigTrustProxyHeaders = "trust_proxy_headers"
- ConfigSidebar = "sidebar"
- ConfigDefaultGOOS = "default_goos"
- ConfigSourcegraphURL = "sourcegraph_url"
- ConfigGithubInterval = "github_interval"
- ConfigCrawlInterval = "crawl_interval"
- ConfigDialTimeout = "dial_timeout"
- ConfigRequestTimeout = "request_timeout"
+ ConfigBindAddress = "http"
+ ConfigAssetsDir = "assets"
+ ConfigRobotThreshold = "robot"
+
+ // Database Config
+ ConfigDBServer = "db-server"
+ ConfigDBIdleTimeout = "db-idle-timeout"
+ ConfigDBLog = "db-log"
+
+ // Display Config
+ ConfigSidebar = "sidebar"
+ ConfigSourcegraphURL = "sourcegraph_url"
+ ConfigDefaultGOOS = "default_goos"
+
+ // Crawl Config
+ ConfigMaxAge = "max_age"
+ ConfigGetTimeout = "get_timeout"
+ ConfigFirstGetTimeout = "first_get_timeout"
+ ConfigGithubInterval = "github_interval"
+ ConfigCrawlInterval = "crawl_interval"
+ ConfigDialTimeout = "dial_timeout"
+ ConfigRequestTimeout = "request_timeout"
)
// Initialize configuration
@@ -88,11 +97,9 @@
flags.Duration(ConfigCrawlInterval, 0, "Package updater sleeps for this duration between package updates. Zero disables updates.")
flags.Duration(ConfigDialTimeout, 5*time.Second, "Timeout for dialing an HTTP connection.")
flags.Duration(ConfigRequestTimeout, 20*time.Second, "Time out for roundtripping an HTTP request.")
-
- // TODO(stephenmw): pass these variables at database creation time.
- flags.StringVar(&database.RedisServer, "db-server", database.RedisServer, "URI of Redis server.")
- flags.DurationVar(&database.RedisIdleTimeout, "db-idle-timeout", database.RedisIdleTimeout, "Close Redis connections after remaining idle for this duration.")
- flags.BoolVar(&database.RedisLog, "db-log", database.RedisLog, "Log database commands")
+ flags.String(ConfigDBServer, "redis://127.0.0.1:6379", "URI of Redis server.")
+ flags.Duration(ConfigDBIdleTimeout, 250*time.Second, "Close Redis connections after remaining idle for this duration.")
+ flags.Bool(ConfigDBLog, false, "Log database commands")
return flags
}
diff --git a/gddo-server/main.go b/gddo-server/main.go
index 242531c..a6d9eac 100644
--- a/gddo-server/main.go
+++ b/gddo-server/main.go
@@ -907,7 +907,11 @@
}
var err error
- db, err = database.New()
+ db, err = database.New(
+ viper.GetString(ConfigDBServer),
+ viper.GetDuration(ConfigDBIdleTimeout),
+ viper.GetBool(ConfigDBLog),
+ )
if err != nil {
log.Fatalf("Error opening database: %v", err)
}
