Skip to content

Commit

Permalink
Set floor of one second for polling delay
Browse files Browse the repository at this point in the history 
To avoid excessive polling.
Add 429 to list of retriable HTTP status codes.
  • Loading branch information
@jhendrixMSFT
jhendrixMSFT committed Oct 25, 2021
1 parent 84308db commit f9bc80b
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 27 deletions.
1 change: 1 addition & 0 deletions sdk/azcore/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* Added string typdef `arm.Endpoint` to provide a hint toward expected ARM client endpoints
* `azcore.ClientOptions` contains common pipeline configuration settings
* Added support for multi-tenant authorization in `arm/runtime`
* Require one second minimum when calling `PollUntilDone()`

### Bug Fixes
* Fixed a potential panic when creating the default Transporter.
Expand Down
12 changes: 6 additions & 6 deletions sdk/azcore/arm/runtime/poller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func TestNewPollerAsync(t *testing.T) {
t.Fatal(err)
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -147,7 +147,7 @@ func TestNewPollerBody(t *testing.T) {
t.Fatal(err)
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -184,7 +184,7 @@ func TestNewPollerLoc(t *testing.T) {
t.Fatal(err)
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -215,7 +215,7 @@ func TestNewPollerInitialRetryAfter(t *testing.T) {
t.Fatalf("unexpected poller type %s", pt.String())
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -273,7 +273,7 @@ func TestNewPollerFailedWithError(t *testing.T) {
t.Fatalf("unexpected poller type %s", pt.String())
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err == nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -309,7 +309,7 @@ func TestNewPollerSuccessNoContent(t *testing.T) {
t.Fatal(err)
}
var result mockType
_, err = poller.PollUntilDone(context.Background(), 10*time.Millisecond, &result)
_, err = poller.PollUntilDone(context.Background(), time.Second, &result)
if err != nil {
t.Fatal(err)
}
Expand Down
6 changes: 4 additions & 2 deletions sdk/azcore/internal/pollers/poller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,11 @@ func (l *Poller) FinalResponse(ctx context.Context, respType interface{}) (*http
// PollUntilDone will handle the entire span of the polling operation until a terminal state is reached,
// then return the final HTTP response for the polling operation and unmarshal the content of the payload
// into the respType interface that is provided.
// freq - the time to wait between polling intervals if the endpoint doesn't send a Retry-After header.
// A good starting value is 30 seconds. Note that some resources might benefit from a different value.
// freq - the time to wait between intervals in absence of a Retry-After header. Minimum is one second.
func (l *Poller) PollUntilDone(ctx context.Context, freq time.Duration, respType interface{}) (*http.Response, error) {
if freq < time.Second {
return nil, errors.New("polling frequency minimum is one second")
}
start := time.Now()
logPollUntilDoneExit := func(v interface{}) {
log.Writef(log.EventLRO, "END PollUntilDone() for %T: %v, total time: %s", l.lro, v, time.Since(start))
Expand Down
15 changes: 11 additions & 4 deletions sdk/azcore/internal/pollers/poller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,13 @@ func TestNewPoller(t *testing.T) {
t.Fatal("unexpected empty resume token")
}
resp, err = p.PollUntilDone(context.Background(), 1*time.Millisecond, nil)
if err == nil {
t.Fatal("unexpected nil error")
}
if resp != nil {
t.Fatal("expected nil response")
}
resp, err = p.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -167,7 +174,7 @@ func TestNewPollerWithFinalGET(t *testing.T) {
Shape string `json:"shape"`
}
var w widget
resp, err := p.PollUntilDone(context.Background(), 1*time.Millisecond, &w)
resp, err := p.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -198,7 +205,7 @@ func TestNewPollerFail1(t *testing.T) {
p := NewPoller(&fakePoller{Ep: srv.URL()}, firstResp, pl, func(*http.Response) error {
return errors.New("failed")
})
resp, err := p.PollUntilDone(context.Background(), 1*time.Millisecond, nil)
resp, err := p.PollUntilDone(context.Background(), time.Second, nil)
if err == nil {
t.Fatal("unexpected nil error")
} else if s := err.Error(); s != "failed" {
Expand All @@ -221,7 +228,7 @@ func TestNewPollerFail2(t *testing.T) {
p := NewPoller(&fakePoller{Ep: srv.URL()}, firstResp, pl, func(*http.Response) error {
return errors.New("failed")
})
resp, err := p.PollUntilDone(context.Background(), 1*time.Millisecond, nil)
resp, err := p.PollUntilDone(context.Background(), time.Second, nil)
if err == nil {
t.Fatal("unexpected nil error")
} else if s := err.Error(); s != "failed" {
Expand All @@ -244,7 +251,7 @@ func TestNewPollerError(t *testing.T) {
p := NewPoller(&fakePoller{Ep: srv.URL()}, firstResp, pl, func(*http.Response) error {
return errors.New("failed")
})
resp, err := p.PollUntilDone(context.Background(), 1*time.Millisecond, nil)
resp, err := p.PollUntilDone(context.Background(), time.Second, nil)
if err == nil {
t.Fatal("unexpected nil error")
} else if s := err.Error(); s != "fatal" {
Expand Down
1 change: 1 addition & 0 deletions sdk/azcore/runtime/policy_retry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func setDefaults(o *policy.RetryOptions) {
if o.StatusCodes == nil {
o.StatusCodes = []int{
http.StatusRequestTimeout, // 408
http.StatusTooManyRequests, // 429
http.StatusInternalServerError, // 500
http.StatusBadGateway, // 502
http.StatusServiceUnavailable, // 503
Expand Down
30 changes: 15 additions & 15 deletions sdk/azcore/runtime/poller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func TestLocPollerSimple(t *testing.T) {
if !closed() {
t.Fatal("initial response body wasn't closed")
}
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err := lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -141,7 +141,7 @@ func TestLocPollerWithWidget(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -178,7 +178,7 @@ func TestLocPollerCancelled(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err == nil {
t.Fatal("unexpected nil error")
}
Expand Down Expand Up @@ -218,7 +218,7 @@ func TestLocPollerWithError(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err == nil {
t.Fatal("unexpected nil error")
}
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestLocPollerWithResumeToken(t *testing.T) {
if err != nil {
t.Fatal(err)
}
resp, err = lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err = lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand All @@ -295,7 +295,7 @@ func TestLocPollerWithTimeout(t *testing.T) {
srv, close := mock.NewServer()
srv.AppendResponse(mock.WithStatusCode(http.StatusAccepted))
srv.AppendResponse(mock.WithStatusCode(http.StatusAccepted))
srv.AppendResponse(mock.WithSlowResponse(2 * time.Second))
srv.AppendResponse(mock.WithSlowResponse(5 * time.Second))
defer close()

firstResp := &http.Response{
Expand All @@ -314,8 +314,8 @@ func TestLocPollerWithTimeout(t *testing.T) {
if !closed() {
t.Fatal("initial response body wasn't closed")
}
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
resp, err := lro.PollUntilDone(ctx, 5*time.Millisecond, nil)
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
resp, err := lro.PollUntilDone(ctx, time.Second, nil)
cancel()
if err == nil {
t.Fatal("unexpected nil error")
Expand Down Expand Up @@ -357,7 +357,7 @@ func TestOpPollerSimple(t *testing.T) {
if !closed() {
t.Fatal("initial response body wasn't closed")
}
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err := lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -401,7 +401,7 @@ func TestOpPollerWithWidgetPUT(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -449,7 +449,7 @@ func TestOpPollerWithWidgetPOSTLocation(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -495,7 +495,7 @@ func TestOpPollerWithWidgetPOST(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -544,7 +544,7 @@ func TestOpPollerWithWidgetResourceLocation(t *testing.T) {
t.Fatal("initial response body wasn't closed")
}
var w widget
resp, err := lro.PollUntilDone(context.Background(), 5*time.Millisecond, &w)
resp, err := lro.PollUntilDone(context.Background(), time.Second, &w)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -613,7 +613,7 @@ func TestOpPollerWithResumeToken(t *testing.T) {
if err != nil {
t.Fatal(err)
}
resp, err = lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err = lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -656,7 +656,7 @@ func TestNopPoller(t *testing.T) {
if resp != firstResp {
t.Fatal("unexpected response")
}
resp, err = lro.PollUntilDone(context.Background(), 5*time.Millisecond, nil)
resp, err = lro.PollUntilDone(context.Background(), time.Second, nil)
if err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit f9bc80b

Please sign in to comment.