Merge pull request #380 from carlosms/no-timeouts-sdk

Remove timeouts in lookout-sdk

cmd/lookoutd/common.go

@@ -109,6 +109,15 @@ func (c *lookoutdCommand) initConfig() (Config, error) {
 		return conf, fmt.Errorf("Can't open configuration file: %s", err)
 	}
 
+	// Set default timeouts
+	conf.Timeout = TimeoutConfig{
+		AnalyzerReview: 10 * time.Minute,
+		AnalyzerPush:   60 * time.Minute,
+		GithubRequest:  time.Minute,
+		GitFetch:       20 * time.Minute,
+		BblfshParse:    2 * time.Minute,
+	}
+
 	if err := yaml.Unmarshal([]byte(configData), &conf); err != nil {
 		return conf, fmt.Errorf("Can't parse configuration file: %s", err)
 	}

provider/github/client.go

@@ -240,7 +240,8 @@ type Client struct {
 	gitAuth          gitAuthFn
 }
 
-// NewClient creates new Client
+// NewClient creates new Client.
+// A timeout of zero means no timeout.
 func NewClient(
 	t http.RoundTripper,
 	cache *cache.ValidableCache,
@@ -266,10 +267,6 @@ func NewClient(
 		}
 	}
 
-	if timeout == 0 {
-		timeout = 30 * time.Second
-	}
-
 	return &Client{
 		Client: github.NewClient(&http.Client{
 			Transport: limitRT,

server/server.go

@@ -40,7 +40,8 @@ type Server struct {
 	analyzerPushTimeout   time.Duration
 }
 
-// NewServer creates new Server
+// NewServer creates a new Server with the given configuration. If the Timeouts
+// are zero it means no timeout.
 func NewServer(
 	p lookout.Poster,
 	fileGetter lookout.FileGetter,
@@ -50,13 +51,6 @@ func NewServer(
 	reviewTimeout time.Duration,
 	pushTimeout time.Duration,
 ) *Server {
-	if reviewTimeout == 0 {
-		reviewTimeout = 5 * time.Minute
-	}
-	if pushTimeout == 0 {
-		pushTimeout = 30 * time.Minute
-	}
-
 	return &Server{p, fileGetter, analyzers, eventOp, commentOp, reviewTimeout, pushTimeout}
 }
 
@@ -142,8 +136,12 @@ func (s *Server) HandleReview(ctx context.Context, e *lookout.ReviewEvent, safeP
 			e.Configuration = *st
 		}
 
-		ctx, cancel := context.WithTimeout(ctx, s.analyzerReviewTimeout)
-		defer cancel()
+		if s.analyzerReviewTimeout > 0 {
+			var cancel context.CancelFunc
+			ctx, cancel = context.WithTimeout(ctx, s.analyzerReviewTimeout)
+			defer cancel()
+		}
+
 		resp, err := a.NotifyReviewEvent(ctx, e)
 		if err != nil {
 			return nil, err
@@ -190,8 +188,12 @@ func (s *Server) HandlePush(ctx context.Context, e *lookout.PushEvent, safePosti
 			e.Configuration = *st
 		}
 
-		ctx, cancel := context.WithTimeout(ctx, s.analyzerPushTimeout)
-		defer cancel()
+		if s.analyzerPushTimeout > 0 {
+			var cancel context.CancelFunc
+			ctx, cancel = context.WithTimeout(ctx, s.analyzerPushTimeout)
+			defer cancel()
+		}
+
 		resp, err := a.NotifyPushEvent(ctx, e)
 		if err != nil {
 			return nil, err

service/bblfsh/bblfsh.go

@@ -15,26 +15,24 @@ import (
 
 // Service implements data service interface which adds UAST to the responses
 type Service struct {
-	changes      lookout.ChangeGetter
-	files        lookout.FileGetter
-	client       protocol.ProtocolServiceClient
+	changes lookout.ChangeGetter
+	files   lookout.FileGetter
+	client  protocol.ProtocolServiceClient
+	// parseTimeout of zero means no timeout.
 	parseTimeout time.Duration
 }
 
 var _ lookout.ChangeGetter = &Service{}
 var _ lookout.FileGetter = &Service{}
 
 // NewService creates new bblfsh Service
+// A parseTimeout of zero means no timeout.
 func NewService(
 	changes lookout.ChangeGetter,
 	files lookout.FileGetter,
 	conn *grpc.ClientConn,
 	parseTimeout time.Duration,
 ) *Service {
-	if parseTimeout == 0 {
-		parseTimeout = 60 * time.Second
-	}
-
 	return &Service{
 		changes:      changes,
 		files:        files,
@@ -96,8 +94,9 @@ func (s *Service) GetFiles(ctx context.Context, req *lookout.FilesRequest) (look
 }
 
 type BaseScanner struct {
-	ctx          context.Context
-	client       protocol.ProtocolServiceClient
+	ctx    context.Context
+	client protocol.ProtocolServiceClient
+	// parseTimeout of zero means no timeout.
 	parseTimeout time.Duration
 	err          error
 }
@@ -129,8 +128,14 @@ func (s *BaseScanner) parseFile(f *lookout.File) (*uast.Node, error) {
 		Encoding: protocol.UTF8,
 		Language: strings.ToLower(f.Language),
 	}
-	ctx, cancel := context.WithTimeout(s.ctx, s.parseTimeout)
-	defer cancel()
+
+	ctx := s.ctx
+	if s.parseTimeout > 0 {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, s.parseTimeout)
+		defer cancel()
+	}
+
 	resp, err := s.client.Parse(ctx, req)
 	if err != nil {
 		return nil, err

service/git/syncer.go

@@ -25,6 +25,7 @@ type Syncer struct {
 	l *Library
 
 	authProvider AuthProvider
+	// fetchTimeout of zero means no timeout.
 	fetchTimeout time.Duration
 }
 
@@ -34,12 +35,9 @@ type AuthProvider interface {
 	GitAuth(ctx context.Context, repoInfo *lookout.RepositoryInfo) transport.AuthMethod
 }
 
-// NewSyncer returns a Syncer for the given Library. authProvider can be nil
+// NewSyncer returns a Syncer for the given Library. authProvider can be nil.
+// A fetchTimeout of zero means no timeout.
 func NewSyncer(l *Library, authProvider AuthProvider, fetchTimeout time.Duration) *Syncer {
-	if fetchTimeout == 0 {
-		fetchTimeout = 10 * time.Minute
-	}
-
 	return &Syncer{l: l, authProvider: authProvider, fetchTimeout: fetchTimeout}
 }
 
@@ -108,8 +106,11 @@ func (s *Syncer) fetch(ctx context.Context, repoInfo *lookout.RepositoryInfo, r
 	mutex.Lock()
 	defer mutex.Unlock()
 
-	ctx, cancel := context.WithTimeout(ctx, s.fetchTimeout)
-	defer cancel()
+	if s.fetchTimeout > 0 {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, s.fetchTimeout)
+		defer cancel()
+	}
 	err = r.FetchContext(ctx, opts)
 	switch err {
 	case git.NoErrAlreadyUpToDate: