From ca88bb4adada4c7a4344d8750051a770014d202c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn?= Date: Mon, 3 Dec 2018 18:07:18 +0000 Subject: [PATCH] Remove timeouts in lookout-sdk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Martín --- cmd/lookoutd/common.go | 9 +++++++++ provider/github/client.go | 7 ++----- server/server.go | 26 ++++++++++++++------------ service/bblfsh/bblfsh.go | 27 ++++++++++++++++----------- service/git/syncer.go | 15 ++++++++------- 5 files changed, 49 insertions(+), 35 deletions(-) diff --git a/cmd/lookoutd/common.go b/cmd/lookoutd/common.go index de6507fd..294fd208 100644 --- a/cmd/lookoutd/common.go +++ b/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) } diff --git a/provider/github/client.go b/provider/github/client.go index 408ed3ff..a4d7c6b4 100644 --- a/provider/github/client.go +++ b/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, diff --git a/server/server.go b/server/server.go index 80a3d72f..f38dcc20 100644 --- a/server/server.go +++ b/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 diff --git a/service/bblfsh/bblfsh.go b/service/bblfsh/bblfsh.go index d21b65bf..4c45d5bb 100644 --- a/service/bblfsh/bblfsh.go +++ b/service/bblfsh/bblfsh.go @@ -15,9 +15,10 @@ 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 } @@ -25,16 +26,13 @@ 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 diff --git a/service/git/syncer.go b/service/git/syncer.go index 051f692a..090040a3 100644 --- a/service/git/syncer.go +++ b/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: