Skip to content

Commit

Permalink
Move LRO failure detection to the implementing poller (Azure#17758)
Browse files Browse the repository at this point in the history
Previously, the Poller.Poll() method was determining when an LRO failed.
While this works for well-known states, it's not extensible.  Instead,
the underlying polling implementation should be responsible for
determining if it's reached a terminal state (failure or otherwise).
  • Loading branch information
jhendrixMSFT authored Apr 28, 2022
1 parent 297fbb1 commit 55c54b4
Show file tree
Hide file tree
Showing 14 changed files with 144 additions and 182 deletions.
16 changes: 8 additions & 8 deletions sdk/azcore/internal/pollers/armloc/loc.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,14 @@ func (p *Poller) URL() string {
return p.PollURL
}

// Done returns true if the LRO has reached a terminal state.
func (p *Poller) Done() bool {
return pollers.IsTerminalState(p.Status())
// State returns the current state of the LRO.
func (p *Poller) State() pollers.OperationState {
if p.CurState == pollers.StatusSucceeded {
return pollers.OperationStateSucceeded
} else if pollers.IsTerminalState(p.CurState) {
return pollers.OperationStateFailed
}
return pollers.OperationStateInProgress
}

// Update updates the Poller from the polling response.
Expand Down Expand Up @@ -96,8 +101,3 @@ func (p *Poller) Update(resp *http.Response) error {
func (p *Poller) FinalGetURL() string {
return ""
}

// Status returns the status of the LRO.
func (p *Poller) Status() string {
return p.CurState
}
19 changes: 7 additions & 12 deletions sdk/azcore/internal/pollers/armloc/loc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"
"testing"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared"
)

Expand Down Expand Up @@ -57,13 +58,10 @@ func TestNew(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if u := poller.FinalGetURL(); u != "" {
t.Fatal("expected empty final GET URL")
}
if s := poller.Status(); s != "InProgress" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if u := poller.URL(); u != fakePollingURL1 {
Expand All @@ -80,13 +78,13 @@ func TestNew(t *testing.T) {
if err := poller.Update(pollingResponse(http.StatusNoContent, http.NoBody)); err != nil {
t.Fatal(err)
}
if s := poller.Status(); s != "Succeeded" {
if s := poller.State(); s != pollers.OperationStateSucceeded {
t.Fatalf("unexpected status %s", s)
}
if err := poller.Update(pollingResponse(http.StatusConflict, http.NoBody)); err != nil {
t.Fatal(err)
}
if s := poller.Status(); s != "Failed" {
if s := poller.State(); s != pollers.OperationStateFailed {
t.Fatalf("unexpected status %s", s)
}
}
Expand All @@ -98,13 +96,10 @@ func TestUpdateWithProvState(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if u := poller.FinalGetURL(); u != "" {
t.Fatal("expected empty final GET URL")
}
if s := poller.Status(); s != "InProgress" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if u := poller.URL(); u != fakePollingURL1 {
Expand All @@ -121,13 +116,13 @@ func TestUpdateWithProvState(t *testing.T) {
if err := poller.Update(pollingResponse(http.StatusOK, strings.NewReader(`{ "properties": { "provisioningState": "Updating" } }`))); err != nil {
t.Fatal(err)
}
if s := poller.Status(); s != "Updating" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if err := poller.Update(pollingResponse(http.StatusOK, http.NoBody)); err != nil {
t.Fatal(err)
}
if s := poller.Status(); s != "Succeeded" {
if s := poller.State(); s != pollers.OperationStateSucceeded {
t.Fatalf("unexpected status %s", s)
}
}
16 changes: 8 additions & 8 deletions sdk/azcore/internal/pollers/async/async.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,14 @@ func New(resp *http.Response, finalState pollers.FinalStateVia, pollerID string)
return p, nil
}

// Done returns true if the LRO has reached a terminal state.
func (p *Poller) Done() bool {
return pollers.IsTerminalState(p.Status())
// State returns the current state of the LRO.
func (p *Poller) State() pollers.OperationState {
if p.CurState == pollers.StatusSucceeded {
return pollers.OperationStateSucceeded
} else if pollers.IsTerminalState(p.CurState) {
return pollers.OperationStateFailed
}
return pollers.OperationStateInProgress
}

// Update updates the Poller from the polling response.
Expand Down Expand Up @@ -125,8 +130,3 @@ func (p *Poller) FinalGetURL() string {
func (p *Poller) URL() string {
return p.AsyncURL
}

// Status returns the status of the LRO.
func (p *Poller) Status() string {
return p.CurState
}
31 changes: 10 additions & 21 deletions sdk/azcore/internal/pollers/async/async_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"
"testing"

"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/pollers"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared"
)

Expand Down Expand Up @@ -61,13 +62,10 @@ func TestNew(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if u := poller.FinalGetURL(); u != fakeResourceURL {
t.Fatalf("unexpected final get URL %s", u)
}
if s := poller.Status(); s != "Started" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if u := poller.URL(); u != fakePollingURL {
Expand All @@ -76,7 +74,7 @@ func TestNew(t *testing.T) {
if err := poller.Update(pollingResponse(strings.NewReader(`{ "status": "InProgress" }`))); err != nil {
t.Fatal(err)
}
if s := poller.Status(); s != "InProgress" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
}
Expand All @@ -88,10 +86,7 @@ func TestNewDeleteNoProvState(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if s := poller.Status(); s != "InProgress" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
}
Expand All @@ -105,10 +100,7 @@ func TestNewPutNoProvState(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if s := poller.Status(); s != "InProgress" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
}
Expand All @@ -125,8 +117,8 @@ func TestNewFinalGetLocation(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if u := poller.FinalGetURL(); u != locURL {
t.Fatalf("unexpected final get URL %s", u)
Expand All @@ -148,8 +140,8 @@ func TestNewFinalGetOrigin(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if u := poller.FinalGetURL(); u != fakeResourceURL {
t.Fatalf("unexpected final get URL %s", u)
Expand All @@ -168,10 +160,7 @@ func TestNewPutNoProvStateOnUpdate(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if s := poller.Status(); s != "InProgress" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if err := poller.Update(pollingResponse(strings.NewReader("{}"))); err == nil {
Expand Down
16 changes: 8 additions & 8 deletions sdk/azcore/internal/pollers/body/body.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,14 @@ func (p *Poller) URL() string {
return p.PollURL
}

// Done returns true if the LRO has reached a terminal state.
func (p *Poller) Done() bool {
return pollers.IsTerminalState(p.Status())
// State returns the current state of the LRO.
func (p *Poller) State() pollers.OperationState {
if p.CurState == pollers.StatusSucceeded {
return pollers.OperationStateSucceeded
} else if pollers.IsTerminalState(p.CurState) {
return pollers.OperationStateFailed
}
return pollers.OperationStateInProgress
}

// Update updates the Poller from the polling response.
Expand All @@ -102,8 +107,3 @@ func (p *Poller) Update(resp *http.Response) error {
func (*Poller) FinalGetURL() string {
return ""
}

// Status returns the status of the LRO.
func (p *Poller) Status() string {
return p.CurState
}
32 changes: 7 additions & 25 deletions sdk/azcore/internal/pollers/body/body_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,10 @@ func TestNew(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if u := poller.FinalGetURL(); u != "" {
t.Fatal("expected empty final GET URL")
}
if s := poller.Status(); s != "Started" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if u := poller.URL(); u != fakeResourceURL {
Expand All @@ -84,7 +81,7 @@ func TestNew(t *testing.T) {
if err := poller.Update(pollingResponse(http.StatusOK, strings.NewReader(`{ "properties": { "provisioningState": "InProgress" } }`))); err != nil {
t.Fatal(err)
}
if s := poller.Status(); s != "InProgress" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
}
Expand All @@ -97,13 +94,10 @@ func TestUpdateNoProvStateFail(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if u := poller.FinalGetURL(); u != "" {
t.Fatal("expected empty final GET URL")
}
if s := poller.Status(); s != "Started" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if u := poller.URL(); u != fakeResourceURL {
Expand All @@ -126,13 +120,10 @@ func TestUpdateNoProvStateSuccess(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if u := poller.FinalGetURL(); u != "" {
t.Fatal("expected empty final GET URL")
}
if s := poller.Status(); s != "Started" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if u := poller.URL(); u != fakeResourceURL {
Expand All @@ -152,13 +143,10 @@ func TestUpdateNoProvState204(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if poller.Done() {
t.Fatal("poller should not be done")
}
if u := poller.FinalGetURL(); u != "" {
t.Fatal("expected empty final GET URL")
}
if s := poller.Status(); s != "Started" {
if s := poller.State(); s != pollers.OperationStateInProgress {
t.Fatalf("unexpected status %s", s)
}
if u := poller.URL(); u != fakeResourceURL {
Expand All @@ -177,13 +165,10 @@ func TestNewNoInitialProvStateOK(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if !poller.Done() {
t.Fatal("poller not be done")
}
if u := poller.FinalGetURL(); u != "" {
t.Fatal("expected empty final GET URL")
}
if s := poller.Status(); s != "Succeeded" {
if s := poller.State(); s != pollers.OperationStateSucceeded {
t.Fatalf("unexpected status %s", s)
}
}
Expand All @@ -195,13 +180,10 @@ func TestNewNoInitialProvStateNC(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if !poller.Done() {
t.Fatal("poller not be done")
}
if u := poller.FinalGetURL(); u != "" {
t.Fatal("expected empty final GET URL")
}
if s := poller.Status(); s != "Succeeded" {
if s := poller.State(); s != pollers.OperationStateSucceeded {
t.Fatalf("unexpected status %s", s)
}
}
20 changes: 8 additions & 12 deletions sdk/azcore/internal/pollers/loc/loc.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,14 @@ func (p *Poller) URL() string {
return p.PollURL
}

func (p *Poller) Done() bool {
return pollers.IsTerminalState(p.Status())
func (p *Poller) State() pollers.OperationState {
if p.CurState == http.StatusAccepted {
return pollers.OperationStateInProgress
} else if p.CurState > 199 && p.CurState < 300 {
// any 2xx other than a 202 indicates success
return pollers.OperationStateSucceeded
}
return pollers.OperationStateFailed
}

func (p *Poller) Update(resp *http.Response) error {
Expand All @@ -72,13 +78,3 @@ func (p *Poller) Update(resp *http.Response) error {
func (p *Poller) FinalGetURL() string {
return p.FinalGET
}

func (p *Poller) Status() string {
if p.CurState == http.StatusAccepted {
return pollers.StatusInProgress
} else if p.CurState > 199 && p.CurState < 300 {
// any 2xx other than a 202 indicates success
return pollers.StatusSucceeded
}
return pollers.StatusFailed
}
Loading

0 comments on commit 55c54b4

Please sign in to comment.