Skip to content
This repository was archived by the owner on Jan 16, 2021. It is now read-only.

Commit 1e2b1a1

Browse files
zombiezenzombiezen
committed
gddo-server: replace globals with struct
This is intended to simplify the number of stages of initialization in gddo-server. Before this CL, globals and the config may or may not be correctly initialized at various points of executing methods during main(). While this largely does not come up because the handlers are not used until appengine.Main, it can be problematic for background tasks. After this CL, handlers (being methods on the server struct) can depend on server being completely initialized, and the server can depend on the config being completely initialized. Change-Id: I1a83a2337a2cdb4d98ec00b26d5e0498ed7e4edd Reviewed-on: https://go-review.googlesource.com/67290 Reviewed-by: Tuo Shan <[email protected]>
1 parent d0b0199 commit 1e2b1a1

File tree

6 files changed

+234
-225
lines changed

6 files changed

+234
-225
lines changed

‎gddo-server/background.go‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,64 +11,62 @@ import (
1111
"log"
1212
"time"
1313

14-
"github.com/spf13/viper"
15-
1614
"github.com/golang/gddo/gosrc"
1715
)
1816

19-
func doCrawl(ctx context.Context) error {
17+
func (s *server) doCrawl(ctx context.Context) error {
2018
// Look for new package to crawl.
21-
importPath, hasSubdirs, err := db.PopNewCrawl()
19+
importPath, hasSubdirs, err := s.db.PopNewCrawl()
2220
if err != nil {
2321
log.Printf("db.PopNewCrawl() returned error %v", err)
2422
return nil
2523
}
2624
if importPath != "" {
27-
if pdoc, err := crawlDoc(ctx, "new", importPath, nil, hasSubdirs, time.Time{}); pdoc == nil && err == nil {
28-
if err := db.AddBadCrawl(importPath); err != nil {
25+
if pdoc, err := s.crawlDoc(ctx, "new", importPath, nil, hasSubdirs, time.Time{}); pdoc == nil && err == nil {
26+
if err := s.db.AddBadCrawl(importPath); err != nil {
2927
log.Printf("ERROR db.AddBadCrawl(%q): %v", importPath, err)
3028
}
3129
}
3230
return nil
3331
}
3432

3533
// Crawl existing doc.
36-
pdoc, pkgs, nextCrawl, err := db.Get(ctx, "-")
34+
pdoc, pkgs, nextCrawl, err := s.db.Get(ctx, "-")
3735
if err != nil {
3836
log.Printf("db.Get(\"-\") returned error %v", err)
3937
return nil
4038
}
4139
if pdoc == nil || nextCrawl.After(time.Now()) {
4240
return nil
4341
}
44-
if _, err = crawlDoc(ctx, "crawl", pdoc.ImportPath, pdoc, len(pkgs) > 0, nextCrawl); err != nil {
42+
if _, err = s.crawlDoc(ctx, "crawl", pdoc.ImportPath, pdoc, len(pkgs) > 0, nextCrawl); err != nil {
4543
// Touch package so that crawl advances to next package.
46-
if err := db.SetNextCrawl(pdoc.ImportPath, time.Now().Add(viper.GetDuration(ConfigMaxAge)/3)); err != nil {
44+
if err := s.db.SetNextCrawl(pdoc.ImportPath, time.Now().Add(s.v.GetDuration(ConfigMaxAge)/3)); err != nil {
4745
log.Printf("ERROR db.SetNextCrawl(%q): %v", pdoc.ImportPath, err)
4846
}
4947
}
5048
return nil
5149
}
5250

53-
func readGitHubUpdates(ctx context.Context) error {
51+
func (s *server) readGitHubUpdates(ctx context.Context) error {
5452
const key = "gitHubUpdates"
5553
var last string
56-
if err := db.GetGob(key, &last); err != nil {
54+
if err := s.db.GetGob(key, &last); err != nil {
5755
return err
5856
}
59-
last, names, err := gosrc.GetGitHubUpdates(ctx, httpClient, last)
57+
last, names, err := gosrc.GetGitHubUpdates(ctx, s.httpClient, last)
6058
if err != nil {
6159
return err
6260
}
6361

6462
for _, name := range names {
6563
log.Printf("bump crawl github.com/%s", name)
66-
if err := db.BumpCrawl("github.com/" + name); err != nil {
64+
if err := s.db.BumpCrawl("github.com/" + name); err != nil {
6765
log.Println("ERROR force crawl:", err)
6866
}
6967
}
7068

71-
if err := db.PutGob(key, last); err != nil {
69+
if err := s.db.PutGob(key, last); err != nil {
7270
return err
7371
}
7472
return nil

‎gddo-server/config.go‎

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,17 @@ const (
5252
ConfigMemcacheAddr = "memcache_addr"
5353
)
5454

55-
// Initialize configuration
56-
func init() {
57-
ctx := context.Background()
58-
55+
func loadConfig(ctx context.Context, args []string) (*viper.Viper, error) {
56+
v := viper.New()
5957
// Gather information from execution environment.
6058
if os.Getenv(gaeProjectEnvVar) != "" {
61-
viper.Set("on_appengine", true)
59+
v.Set("on_appengine", true)
6260
} else {
63-
viper.Set("on_appengine", false)
61+
v.Set("on_appengine", false)
6462
}
6563
if metadata.OnGCE() {
66-
gceProjectAttributeDefault(ctx, viper.GetViper(), ConfigGAAccount, "ga-account")
67-
gceProjectAttributeDefault(ctx, viper.GetViper(), ConfigGCELogName, "gce-log-name")
64+
gceProjectAttributeDefault(ctx, v, ConfigGAAccount, "ga-account")
65+
gceProjectAttributeDefault(ctx, v, ConfigGCELogName, "gce-log-name")
6866
if id, err := metadata.ProjectID(); err != nil {
6967
log.Warn(ctx, "failed to retrieve project ID", "error", err)
7068
} else {
@@ -74,25 +72,30 @@ func init() {
7472

7573
// Setup command line flags
7674
flags := buildFlags()
77-
flags.Parse(os.Args)
78-
if err := viper.BindPFlags(flags); err != nil {
79-
panic(err)
75+
if err := flags.Parse(args); err != nil {
76+
return nil, err
77+
}
78+
if err := v.BindPFlags(flags); err != nil {
79+
return nil, err
8080
}
8181

82-
// Also fetch from enviorment
83-
viper.SetEnvPrefix("gddo")
84-
viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
85-
viper.AutomaticEnv()
86-
viper.BindEnv(ConfigProject, gaeProjectEnvVar)
87-
viper.BindEnv(ConfigGAAccount, gaAccountEnvVar)
82+
// Also fetch from environment
83+
v.SetEnvPrefix("gddo")
84+
v.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
85+
v.AutomaticEnv()
86+
v.BindEnv(ConfigProject, gaeProjectEnvVar)
87+
v.BindEnv(ConfigGAAccount, gaAccountEnvVar)
8888

8989
// Read from config.
90-
readViperConfig(ctx)
90+
if err := readViperConfig(ctx, v); err != nil {
91+
return nil, err
92+
}
9193

9294
// Set defaults based on other configs
93-
setDefaults()
95+
setDefaults(v)
9496

95-
log.Info(ctx, "config values loaded", "values", viper.AllSettings())
97+
log.Info(ctx, "config values loaded", "values", v.AllSettings())
98+
return v, nil
9699
}
97100

98101
func gceProjectAttributeDefault(ctx context.Context, v *viper.Viper, cfg, attr string) {
@@ -109,17 +112,17 @@ func gceProjectAttributeDefault(ctx context.Context, v *viper.Viper, cfg, attr s
109112
// setDefaults sets defaults for configuration options that depend on other
110113
// configuration options. This allows for smart defaults but allows for
111114
// overrides.
112-
func setDefaults() {
115+
func setDefaults(v *viper.Viper) {
113116
// ConfigGAERemoteAPI is based on project.
114-
project := viper.GetString(ConfigProject)
117+
project := v.GetString(ConfigProject)
115118
if project != "" {
116119
defaultEndpoint := fmt.Sprintf("serviceproxy-dot-%s.appspot.com", project)
117-
viper.SetDefault(ConfigGAERemoteAPI, defaultEndpoint)
120+
v.SetDefault(ConfigGAERemoteAPI, defaultEndpoint)
118121
}
119122
}
120123

121124
func buildFlags() *pflag.FlagSet {
122-
flags := pflag.NewFlagSet("default", pflag.ExitOnError)
125+
flags := pflag.NewFlagSet("default", pflag.ContinueOnError)
123126

124127
flags.StringP("config", "c", "", "path to motd config file")
125128
flags.String(ConfigProject, "", "Google Cloud Platform project used for Google services")
@@ -147,31 +150,31 @@ func buildFlags() *pflag.FlagSet {
147150
return flags
148151
}
149152

150-
// readViperConfig finds and then parses a config file. It will log.Fatal if the
151-
// config file was specified or could not parse. Otherwise it will only warn
152-
// that it failed to load the config.
153-
func readViperConfig(ctx context.Context) {
154-
viper.AddConfigPath(".")
155-
viper.AddConfigPath("/etc")
156-
viper.SetConfigName("gddo")
157-
if viper.GetString("config") != "" {
158-
viper.SetConfigFile(viper.GetString("config"))
153+
// readViperConfig finds and then parses a config file. It will return
154+
// an error if the config file was specified or could not parse.
155+
// Otherwise it will only warn that it failed to load the config.
156+
func readViperConfig(ctx context.Context, v *viper.Viper) error {
157+
v.AddConfigPath(".")
158+
v.AddConfigPath("/etc")
159+
v.SetConfigName("gddo")
160+
if v.GetString("config") != "" {
161+
v.SetConfigFile(v.GetString("config"))
159162
}
160163

161-
if err := viper.ReadInConfig(); err != nil {
164+
if err := v.ReadInConfig(); err != nil {
162165
// If a config exists but could not be parsed, we should bail.
163166
if _, ok := err.(viper.ConfigParseError); ok {
164-
log.Fatal(ctx, "failed to parse config", "error", err)
167+
return fmt.Errorf("parse config: %v", err)
165168
}
166169

167170
// If the user specified a config file location in flags or env and
168171
// we failed to load it, we should bail. If not, it is just a warning.
169-
if viper.GetString("config") != "" {
170-
log.Fatal(ctx, "failed to load configuration file", "error", err)
171-
} else {
172-
log.Warn(ctx, "failed to load configuration file", "error", err)
172+
if v.GetString("config") != "" {
173+
return fmt.Errorf("load config: %v", err)
173174
}
174-
} else {
175-
log.Info(ctx, "loaded configuration file successfully", "path", viper.ConfigFileUsed())
175+
log.Warn(ctx, "failed to load configuration file", "error", err)
176+
return nil
176177
}
178+
log.Info(ctx, "loaded configuration file successfully", "path", v.ConfigFileUsed())
179+
return nil
177180
}

‎gddo-server/crawl.go‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ import (
1414
"strings"
1515
"time"
1616

17-
"github.com/spf13/viper"
18-
1917
"github.com/golang/gddo/doc"
2018
"github.com/golang/gddo/gosrc"
2119
)
@@ -25,7 +23,7 @@ var (
2523
)
2624

2725
// crawlDoc fetches the package documentation from the VCS and updates the database.
28-
func crawlDoc(ctx context.Context, source string, importPath string, pdoc *doc.Package, hasSubdirs bool, nextCrawl time.Time) (*doc.Package, error) {
26+
func (s *server) crawlDoc(ctx context.Context, source string, importPath string, pdoc *doc.Package, hasSubdirs bool, nextCrawl time.Time) (*doc.Package, error) {
2927
message := []interface{}{source}
3028
defer func() {
3129
message = append(message, importPath)
@@ -51,15 +49,15 @@ func crawlDoc(ctx context.Context, source string, importPath string, pdoc *doc.P
5149
// Old import path for Go sub-repository.
5250
pdoc = nil
5351
err = gosrc.NotFoundError{Message: "old Go sub-repo", Redirect: "golang.org/x/" + importPath[len("code.google.com/p/go."):]}
54-
} else if blocked, e := db.IsBlocked(importPath); blocked && e == nil {
52+
} else if blocked, e := s.db.IsBlocked(importPath); blocked && e == nil {
5553
pdoc = nil
5654
err = gosrc.NotFoundError{Message: "blocked."}
5755
} else if testdataPat.MatchString(importPath) {
5856
pdoc = nil
5957
err = gosrc.NotFoundError{Message: "testdata."}
6058
} else {
6159
var pdocNew *doc.Package
62-
pdocNew, err = doc.Get(ctx, httpClient, importPath, etag)
60+
pdocNew, err = doc.Get(ctx, s.httpClient, importPath, etag)
6361
message = append(message, "fetch:", int64(time.Since(start)/time.Millisecond))
6462
if err == nil && pdocNew.Name == "" && !hasSubdirs {
6563
for _, e := range pdocNew.Errors {
@@ -72,7 +70,7 @@ func crawlDoc(ctx context.Context, source string, importPath string, pdoc *doc.P
7270
}
7371
}
7472

75-
maxAge := viper.GetDuration(ConfigMaxAge)
73+
maxAge := s.v.GetDuration(ConfigMaxAge)
7674
nextCrawl = start.Add(maxAge)
7775
switch {
7876
case strings.HasPrefix(importPath, "github.com/") || (pdoc != nil && len(pdoc.Errors) > 0):
@@ -84,31 +82,31 @@ func crawlDoc(ctx context.Context, source string, importPath string, pdoc *doc.P
8482

8583
if err == nil {
8684
message = append(message, "put:", pdoc.Etag)
87-
if err := put(ctx, pdoc, nextCrawl); err != nil {
85+
if err := s.put(ctx, pdoc, nextCrawl); err != nil {
8886
log.Println(err)
8987
}
9088
return pdoc, nil
9189
} else if e, ok := err.(gosrc.NotModifiedError); ok {
92-
if pdoc.Status == gosrc.Active && !isActivePkg(importPath, e.Status) {
90+
if pdoc.Status == gosrc.Active && !s.isActivePkg(importPath, e.Status) {
9391
if e.Status == gosrc.NoRecentCommits {
9492
e.Status = gosrc.Inactive
9593
}
9694
message = append(message, "archive", e)
9795
pdoc.Status = e.Status
98-
if err := db.Put(ctx, pdoc, nextCrawl, false); err != nil {
96+
if err := s.db.Put(ctx, pdoc, nextCrawl, false); err != nil {
9997
log.Printf("ERROR db.Put(%q): %v", importPath, err)
10098
}
10199
} else {
102100
// Touch the package without updating and move on to next one.
103101
message = append(message, "touch")
104-
if err := db.SetNextCrawl(importPath, nextCrawl); err != nil {
102+
if err := s.db.SetNextCrawl(importPath, nextCrawl); err != nil {
105103
log.Printf("ERROR db.SetNextCrawl(%q): %v", importPath, err)
106104
}
107105
}
108106
return pdoc, nil
109107
} else if e, ok := err.(gosrc.NotFoundError); ok {
110108
message = append(message, "notfound:", e)
111-
if err := db.Delete(ctx, importPath); err != nil {
109+
if err := s.db.Delete(ctx, importPath); err != nil {
112110
log.Printf("ERROR db.Delete(%q): %v", importPath, err)
113111
}
114112
return nil, e
@@ -118,26 +116,26 @@ func crawlDoc(ctx context.Context, source string, importPath string, pdoc *doc.P
118116
}
119117
}
120118

121-
func put(ctx context.Context, pdoc *doc.Package, nextCrawl time.Time) error {
119+
func (s *server) put(ctx context.Context, pdoc *doc.Package, nextCrawl time.Time) error {
122120
if pdoc.Status == gosrc.NoRecentCommits &&
123-
isActivePkg(pdoc.ImportPath, gosrc.NoRecentCommits) {
121+
s.isActivePkg(pdoc.ImportPath, gosrc.NoRecentCommits) {
124122
pdoc.Status = gosrc.Active
125123
}
126-
if err := db.Put(ctx, pdoc, nextCrawl, false); err != nil {
124+
if err := s.db.Put(ctx, pdoc, nextCrawl, false); err != nil {
127125
return fmt.Errorf("ERROR db.Put(%q): %v", pdoc.ImportPath, err)
128126
}
129127
return nil
130128
}
131129

132130
// isActivePkg reports whether a package is considered active,
133131
// either because its directory is active or because it is imported by another package.
134-
func isActivePkg(pkg string, status gosrc.DirectoryStatus) bool {
132+
func (s *server) isActivePkg(pkg string, status gosrc.DirectoryStatus) bool {
135133
switch status {
136134
case gosrc.Active:
137135
return true
138136
case gosrc.NoRecentCommits:
139137
// It should be inactive only if it has no imports as well.
140-
n, err := db.ImporterCount(pkg)
138+
n, err := s.db.ImporterCount(pkg)
141139
if err != nil {
142140
log.Printf("ERROR db.ImporterCount(%q): %v", pkg, err)
143141
}

0 commit comments

Comments
 (0)