diff --git a/github/config.go b/github/config.go index e6c91af734..4eb2c76ef6 100644 --- a/github/config.go +++ b/github/config.go @@ -20,6 +20,7 @@ type Config struct { BaseURL string Insecure bool WriteDelay time.Duration + ReadDelay time.Duration } type Owner struct { @@ -31,10 +32,10 @@ type Owner struct { IsOrganization bool } -func RateLimitedHTTPClient(client *http.Client, writeDelay time.Duration) *http.Client { +func RateLimitedHTTPClient(client *http.Client, writeDelay time.Duration, readDelay time.Duration) *http.Client { client.Transport = NewEtagTransport(client.Transport) - client.Transport = NewRateLimitTransport(client.Transport, WithWriteDelay(writeDelay)) + client.Transport = NewRateLimitTransport(client.Transport, WithWriteDelay(writeDelay), WithReadDelay(readDelay)) client.Transport = logging.NewTransport("Github", client.Transport) return client @@ -48,7 +49,7 @@ func (c *Config) AuthenticatedHTTPClient() *http.Client { ) client := oauth2.NewClient(ctx, ts) - return RateLimitedHTTPClient(client, c.WriteDelay) + return RateLimitedHTTPClient(client, c.WriteDelay, c.ReadDelay) } func (c *Config) Anonymous() bool { @@ -57,7 +58,7 @@ func (c *Config) Anonymous() bool { func (c *Config) AnonymousHTTPClient() *http.Client { client := &http.Client{Transport: &http.Transport{}} - return RateLimitedHTTPClient(client, c.WriteDelay) + return RateLimitedHTTPClient(client, c.WriteDelay, c.ReadDelay) } func (c *Config) NewGraphQLClient(client *http.Client) (*githubv4.Client, error) { diff --git a/github/provider.go b/github/provider.go index d055100219..104125fae1 100644 --- a/github/provider.go +++ b/github/provider.go @@ -50,6 +50,12 @@ func Provider() terraform.ResourceProvider { Default: 1000, Description: descriptions["write_delay_ms"], }, + "read_delay_ms": { + Type: schema.TypeInt, + Optional: true, + Default: 0, + Description: descriptions["read_delay_ms"], + }, "app_auth": { Type: schema.TypeList, Optional: true, @@ -174,6 +180,8 @@ func init() { "app_auth.pem_file": "The GitHub App PEM file contents.", "write_delay_ms": "Amount of time in milliseconds to sleep in between writes to GitHub API. " + "Defaults to 1000ms or 1s if not set.", + "read_delay_ms": "Amount of time in milliseconds to sleep in between non-write requests to GitHub API. " + + "Defaults to 0ms if not set.", } } @@ -250,12 +258,19 @@ func providerConfigure(p *schema.Provider) schema.ConfigureFunc { } log.Printf("[INFO] Setting write_delay_ms to %d", writeDelay) + readDelay := d.Get("read_delay_ms").(int) + if readDelay < 0 { + return nil, fmt.Errorf("read_delay_ms must be greater than or equal to 0ms") + } + log.Printf("[DEBUG] Setting read_delay_ms to %d", readDelay) + config := Config{ Token: token, BaseURL: baseURL, Insecure: insecure, Owner: owner, WriteDelay: time.Duration(writeDelay) * time.Millisecond, + ReadDelay: time.Duration(readDelay) * time.Millisecond, } meta, err := config.Meta() diff --git a/github/transport.go b/github/transport.go index a0f77adc58..b0c5f8788e 100644 --- a/github/transport.go +++ b/github/transport.go @@ -49,8 +49,9 @@ func NewEtagTransport(rt http.RoundTripper) *etagTransport { // https://developer.github.com/v3/guides/best-practices-for-integrators/#dealing-with-abuse-rate-limits type RateLimitTransport struct { transport http.RoundTripper - delayNextRequest bool + nextRequestDelay time.Duration writeDelay time.Duration + readDelay time.Duration m sync.Mutex } @@ -61,14 +62,14 @@ func (rlt *RateLimitTransport) RoundTrip(req *http.Request) (*http.Response, err // and restoring bodies between retries below rlt.lock(req) - // If you're making a large number of POST, PATCH, PUT, or DELETE requests - // for a single user or client ID, wait at least one second between each request. - if rlt.delayNextRequest { - log.Printf("[DEBUG] Sleeping %s between write operations", rlt.writeDelay) - time.Sleep(rlt.writeDelay) + // Sleep for the delay that the last request defined. This delay might be different + // for read and write requests. See isWriteMethod for the distinction between them. + if rlt.nextRequestDelay > 0 { + log.Printf("[DEBUG] Sleeping %s between operations", rlt.nextRequestDelay) + time.Sleep(rlt.nextRequestDelay) } - rlt.delayNextRequest = isWriteMethod(req.Method) + rlt.nextRequestDelay = rlt.calculateNextDelay(req.Method) resp, err := rlt.transport.RoundTrip(req) if err != nil { @@ -89,7 +90,7 @@ func (rlt *RateLimitTransport) RoundTrip(req *http.Request) (*http.Response, err // When you have been limited, use the Retry-After response header to slow down. if arlErr, ok := ghErr.(*github.AbuseRateLimitError); ok { - rlt.delayNextRequest = false + rlt.nextRequestDelay = 0 retryAfter := arlErr.GetRetryAfter() log.Printf("[DEBUG] Abuse detection mechanism triggered, sleeping for %s before retrying", retryAfter) @@ -99,7 +100,7 @@ func (rlt *RateLimitTransport) RoundTrip(req *http.Request) (*http.Response, err } if rlErr, ok := ghErr.(*github.RateLimitError); ok { - rlt.delayNextRequest = false + rlt.nextRequestDelay = 0 retryAfter := time.Until(rlErr.Rate.Reset.Time) log.Printf("[DEBUG] Rate limit %d reached, sleeping for %s (until %s) before retrying", rlErr.Rate.Limit, retryAfter, time.Now().Add(retryAfter)) @@ -121,6 +122,15 @@ func (rlt *RateLimitTransport) unlock(req *http.Request) { rlt.m.Unlock() } +// calculateNextDelay returns a time.Duration specifying the backoff before the next request +// the actual value depends on the current method being a write or a read request +func (rlt *RateLimitTransport) calculateNextDelay(method string) time.Duration { + if isWriteMethod(method) { + return rlt.writeDelay + } + return rlt.readDelay +} + type RateLimitTransportOption func(*RateLimitTransport) // NewRateLimitTransport takes in an http.RoundTripper and a variadic list of @@ -128,7 +138,8 @@ type RateLimitTransportOption func(*RateLimitTransport) // may be used to alter the write delay in between requests, for example. func NewRateLimitTransport(rt http.RoundTripper, options ...RateLimitTransportOption) *RateLimitTransport { // Default to 1 second of write delay if none is provided - rlt := &RateLimitTransport{transport: rt, writeDelay: 1 * time.Second} + // Default to no read delay if none is provided + rlt := &RateLimitTransport{transport: rt, writeDelay: 1 * time.Second, readDelay: 0 * time.Second} for _, opt := range options { opt(rlt) @@ -144,6 +155,13 @@ func WithWriteDelay(d time.Duration) RateLimitTransportOption { } } +// WithReadDelay is used to set the delay between read requests +func WithReadDelay(d time.Duration) RateLimitTransportOption { + return func(rlt *RateLimitTransport) { + rlt.readDelay = d + } +} + // drainBody reads all of b to memory and then returns two equivalent // ReadClosers yielding the same bytes. func drainBody(b io.ReadCloser) (r1, r2 io.ReadCloser, err error) { diff --git a/website/docs/index.html.markdown b/website/docs/index.html.markdown index bd6d363c24..417eaa5352 100644 --- a/website/docs/index.html.markdown +++ b/website/docs/index.html.markdown @@ -109,6 +109,8 @@ The following arguments are supported in the `provider` block: * `write_delay_ms` - (Optional) The number of milliseconds to sleep in between write operations in order to satisfy the GitHub API rate limits. Defaults to 1000ms or 1 second if not provided. +* `read_delay_ms` - (Optional) The number of milliseconds to sleep in between non-write operations in order to satisfy the GitHub API rate limits. Defaults to 0ms. + Note: If you have a PEM file on disk, you can pass it in via `pem_file = file("path/to/file.pem")`. For backwards compatibility, if more than one of `owner`, `organization`,