Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add configurable read_delay_ms #1054

Merged
merged 1 commit into from
Mar 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions github/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type Config struct {
BaseURL string
Insecure bool
WriteDelay time.Duration
ReadDelay time.Duration
}

type Owner struct {
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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) {
Expand Down
15 changes: 15 additions & 0 deletions github/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -171,6 +177,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.",
}
}

Expand Down Expand Up @@ -247,12 +255,19 @@ func providerConfigure(p *schema.Provider) schema.ConfigureFunc {
}
log.Printf("[DEBUG] 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()
Expand Down
38 changes: 28 additions & 10 deletions github/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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))
Expand All @@ -125,14 +126,24 @@ 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
// optional functions that modify the RateLimitTransport struct itself. This
// 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)
Expand All @@ -148,6 +159,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) {
Expand Down
2 changes: 2 additions & 0 deletions website/docs/index.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down