From 2042c904ac0b59ffeefb53b22d21225c56a5ca23 Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Fri, 13 Aug 2021 13:52:09 -0700 Subject: [PATCH] Removed and renamed some content (#15285) Unexported policyFunc Renamed Transport to Transporter Removed TransportFunc, Pager, and Poller interfaces Added responseError (removed from internal) Fixed logging type aliases Updated logging usage based on internal/log refactor Removed StatusCodesForRetry, ProgressReceiver, ErrNoMorePolicies Renamed LROPoller to Poller --- sdk/azcore/core.go | 61 +++-------------------- sdk/azcore/error.go | 41 ++++++++++----- sdk/azcore/example_test.go | 8 +-- sdk/azcore/go.mod | 4 +- sdk/azcore/go.sum | 5 +- sdk/azcore/log.go | 47 +++++------------ sdk/azcore/log_test.go | 44 ++++++++-------- sdk/azcore/policy_anonymous_credential.go | 2 +- sdk/azcore/policy_logging.go | 14 +++--- sdk/azcore/policy_logging_test.go | 4 +- sdk/azcore/policy_retry.go | 38 ++++++-------- sdk/azcore/policy_retry_test.go | 4 +- sdk/azcore/poller.go | 53 +++++++++----------- sdk/azcore/poller_test.go | 38 +++++++------- sdk/azcore/progress.go | 10 ++-- sdk/azcore/request.go | 2 +- sdk/azcore/request_test.go | 5 +- 17 files changed, 157 insertions(+), 223 deletions(-) diff --git a/sdk/azcore/core.go b/sdk/azcore/core.go index ff915e59eae3..4e5ed8405f25 100644 --- a/sdk/azcore/core.go +++ b/sdk/azcore/core.go @@ -6,7 +6,6 @@ package azcore import ( - "context" "errors" "io" "net/http" @@ -31,33 +30,24 @@ type Policy interface { Do(req *Request) (*Response, error) } -// PolicyFunc is a type that implements the Policy interface. +// policyFunc is a type that implements the Policy interface. // Use this type when implementing a stateless policy as a first-class function. -type PolicyFunc func(*Request) (*Response, error) +type policyFunc func(*Request) (*Response, error) // Do implements the Policy interface on PolicyFunc. -func (pf PolicyFunc) Do(req *Request) (*Response, error) { +func (pf policyFunc) Do(req *Request) (*Response, error) { return pf(req) } -// Transport represents an HTTP pipeline transport used to send HTTP requests and receive responses. -type Transport interface { +// Transporter represents an HTTP pipeline transport used to send HTTP requests and receive responses. +type Transporter interface { // Do sends the HTTP request and returns the HTTP response or error. Do(req *http.Request) (*http.Response, error) } -// TransportFunc is a type that implements the Transport interface. -// Use this type when implementing a stateless transport as a first-class function. -type TransportFunc func(*http.Request) (*http.Response, error) - -// Do implements the Transport interface on TransportFunc. -func (tf TransportFunc) Do(req *http.Request) (*http.Response, error) { - return tf(req) -} - // used to adapt a TransportPolicy to a Policy type transportPolicy struct { - trans Transport + trans Transporter } func (tp transportPolicy) Do(req *Request) (*Response, error) { @@ -80,12 +70,12 @@ type Pipeline struct { // NewPipeline creates a new Pipeline object from the specified Transport and Policies. // If no transport is provided then the default *http.Client transport will be used. -func NewPipeline(transport Transport, policies ...Policy) Pipeline { +func NewPipeline(transport Transporter, policies ...Policy) Pipeline { if transport == nil { transport = defaultHTTPClient } // transport policy must always be the last in the slice - policies = append(policies, PolicyFunc(httpHeaderPolicy), PolicyFunc(bodyDownloadPolicy), transportPolicy{trans: transport}) + policies = append(policies, policyFunc(httpHeaderPolicy), policyFunc(bodyDownloadPolicy), transportPolicy{trans: transport}) return Pipeline{ policies: policies, } @@ -121,41 +111,6 @@ func NopCloser(rs io.ReadSeeker) ReadSeekCloser { return nopCloser{rs} } -// Poller provides operations for checking the state of a long-running operation. -// An LRO can be in either a non-terminal or terminal state. A non-terminal state -// indicates the LRO is still in progress. A terminal state indicates the LRO has -// completed successfully, failed, or was cancelled. -type Poller interface { - // Done returns true if the LRO has reached a terminal state. - Done() bool - - // Poll fetches the latest state of the LRO. It returns an HTTP response or error. - // If the LRO has completed successfully, the poller's state is update and the HTTP - // response is returned. - // If the LRO has completed with failure or was cancelled, the poller's state is - // updated and the error is returned. - // If the LRO has not reached a terminal state, the poller's state is updated and - // the latest HTTP response is returned. - // If Poll fails, the poller's state is unmodified and the error is returned. - // Calling Poll on an LRO that has reached a terminal state will return the final - // HTTP response or error. - Poll(context.Context) (*http.Response, error) - - // ResumeToken returns a value representing the poller that can be used to resume - // the LRO at a later time. ResumeTokens are unique per service operation. - ResumeToken() (string, error) -} - -// Pager provides operations for iterating over paged responses. -type Pager interface { - // NextPage returns true if the pager advanced to the next page. - // Returns false if there are no more pages or an error occurred. - NextPage(context.Context) bool - - // Err returns the last error encountered while paging. - Err() error -} - // holds sentinel values used to send nulls var nullables map[reflect.Type]interface{} = map[reflect.Type]interface{}{} diff --git a/sdk/azcore/error.go b/sdk/azcore/error.go index ded724325077..97ac16831a50 100644 --- a/sdk/azcore/error.go +++ b/sdk/azcore/error.go @@ -6,15 +6,7 @@ package azcore import ( - "errors" "net/http" - - sdkruntime "github.com/Azure/azure-sdk-for-go/sdk/internal/runtime" -) - -var ( - // ErrNoMorePolicies is returned from Request.Next() if there are no more policies in the pipeline. - ErrNoMorePolicies = errors.New("no more policies") ) var ( @@ -31,9 +23,6 @@ type HTTPResponse interface { RawResponse() *http.Response } -// ensure our internal ResponseError type implements HTTPResponse -var _ HTTPResponse = (*sdkruntime.ResponseError)(nil) - // NonRetriableError represents a non-transient error. This works in // conjunction with the retry policy, indicating that the error condition // is idempotent, so no retries will be attempted. @@ -48,7 +37,33 @@ type NonRetriableError interface { // in this error type so that callers can access the underlying *http.Response as required. // DO NOT wrap failed HTTP requests that returned an error and no response with this type. func NewResponseError(inner error, resp *http.Response) error { - return sdkruntime.NewResponseError(inner, resp) + return &responseError{inner: inner, resp: resp} +} + +type responseError struct { + inner error + resp *http.Response +} + +// Error implements the error interface for type ResponseError. +func (e *responseError) Error() string { + return e.inner.Error() +} + +// Unwrap returns the inner error. +func (e *responseError) Unwrap() error { + return e.inner +} + +// RawResponse returns the HTTP response associated with this error. +func (e *responseError) RawResponse() *http.Response { + return e.resp +} + +// NonRetriable indicates this error is non-transient. +func (e *responseError) NonRetriable() { + // marker method } -var _ NonRetriableError = (*sdkruntime.ResponseError)(nil) +var _ HTTPResponse = (*responseError)(nil) +var _ NonRetriableError = (*responseError)(nil) diff --git a/sdk/azcore/example_test.go b/sdk/azcore/example_test.go index 8e34c5cec984..ae7ee4be91c0 100644 --- a/sdk/azcore/example_test.go +++ b/sdk/azcore/example_test.go @@ -49,15 +49,15 @@ func ExampleRequest_SetBody() { } // false positive by linter -func ExampleLogger_SetClassifications() { //nolint:govet +func ExampleLogSetClassifications() { //nolint:govet // only log HTTP requests and responses - azcore.SetClassifications(azcore.LogRequest, azcore.LogResponse) + azcore.LogSetClassifications(azcore.LogRequest, azcore.LogResponse) } // false positive by linter -func ExampleLogger_SetListener() { //nolint:govet +func ExampleLogSetListener() { //nolint:govet // a simple logger that writes to stdout - azcore.SetListener(func(cls azcore.LogClassification, msg string) { + azcore.LogSetListener(func(cls azcore.LogClassification, msg string) { fmt.Printf("%s: %s\n", cls, msg) }) } diff --git a/sdk/azcore/go.mod b/sdk/azcore/go.mod index d1352741ce9c..efc9975045f7 100644 --- a/sdk/azcore/go.mod +++ b/sdk/azcore/go.mod @@ -1,8 +1,8 @@ module github.com/Azure/azure-sdk-for-go/sdk/azcore require ( - github.com/Azure/azure-sdk-for-go/sdk/internal v0.5.2 - github.com/stretchr/testify v1.7.0 // indirect + github.com/Azure/azure-sdk-for-go/sdk/internal v0.6.0 + github.com/stretchr/testify v1.7.0 golang.org/x/net v0.0.0-20210610132358-84b48f89b13b ) diff --git a/sdk/azcore/go.sum b/sdk/azcore/go.sum index 3d9a653b9fb7..d099b19dca6d 100644 --- a/sdk/azcore/go.sum +++ b/sdk/azcore/go.sum @@ -1,5 +1,5 @@ -github.com/Azure/azure-sdk-for-go/sdk/internal v0.5.2 h1:E2xwjsWU81O/XuSaxAGa8Jmqz4Vm4NmrpMSO9/XevDg= -github.com/Azure/azure-sdk-for-go/sdk/internal v0.5.2/go.mod h1:Hl9Vte0DDolj9zqzmfnmY9/zfZbiT5KnvXqVwAvnR8Q= +github.com/Azure/azure-sdk-for-go/sdk/internal v0.6.0 h1:Lozt96x50m14Kb7U9FgYkI44AYXVa4lVhRF6exoLlqE= +github.com/Azure/azure-sdk-for-go/sdk/internal v0.6.0/go.mod h1:Hl9Vte0DDolj9zqzmfnmY9/zfZbiT5KnvXqVwAvnR8Q= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -18,6 +18,7 @@ golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9sn golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= diff --git a/sdk/azcore/log.go b/sdk/azcore/log.go index b08a47df7797..d599f2d704f5 100644 --- a/sdk/azcore/log.go +++ b/sdk/azcore/log.go @@ -6,62 +6,41 @@ package azcore import ( - "github.com/Azure/azure-sdk-for-go/sdk/internal/logger" + "github.com/Azure/azure-sdk-for-go/sdk/internal/log" ) -// LogClassification is used to group entries. Each group can be toggled on or off -type LogClassification logger.LogClassification +// LogClassification is used to group entries. Each group can be toggled on or off. +type LogClassification = log.Classification const ( // LogRequest entries contain information about HTTP requests. // This includes information like the URL, query parameters, and headers. - LogRequest LogClassification = "Request" + LogRequest = log.Request // LogResponse entries contain information about HTTP responses. // This includes information like the HTTP status code, headers, and request URL. - LogResponse LogClassification = "Response" + LogResponse = log.Response // LogRetryPolicy entries contain information specific to the retry policy in use. - LogRetryPolicy LogClassification = "RetryPolicy" + LogRetryPolicy = log.RetryPolicy // LogLongRunningOperation entries contain information specific to long-running operations. // This includes information like polling location, operation state and sleep intervals. - LogLongRunningOperation LogClassification = "LongRunningOperation" + LogLongRunningOperation = log.LongRunningOperation ) -// SetClassifications is used to control which classifications are written to +// LogSetClassifications is used to control which classifications are written to // the log. By default all log classifications are writen. -func SetClassifications(cls ...LogClassification) { - input := make([]logger.LogClassification, 0) - for _, l := range cls { - input = append(input, logger.LogClassification(l)) - } - logger.Log().SetClassifications(input...) -} - -// Listener is the function signature invoked when writing log entries. -// A Listener is required to perform its own synchronization if it's expected to be called -// from multiple Go routines -type Listener func(LogClassification, string) - -// transform to convert the azcore.Listener type into a usable one for internal.logger module -func transform(lst Listener) logger.Listener { - return func(l logger.LogClassification, msg string) { - azcoreL := LogClassification(l) - lst(azcoreL, msg) - } +func LogSetClassifications(cls ...LogClassification) { + log.SetClassifications(cls...) } // SetListener will set the Logger to write to the specified Listener. -func SetListener(lst Listener) { - if lst == nil { - logger.Log().SetListener(nil) - } else { - logger.Log().SetListener(transform(lst)) - } +func LogSetListener(lst func(log.Classification, string)) { + log.SetListener(lst) } // for testing purposes func resetClassifications() { - logger.Log().SetClassifications([]logger.LogClassification{}...) + log.TestResetClassifications() } diff --git a/sdk/azcore/log_test.go b/sdk/azcore/log_test.go index c3fce30c2b6a..509fdf79370e 100644 --- a/sdk/azcore/log_test.go +++ b/sdk/azcore/log_test.go @@ -10,47 +10,47 @@ import ( "net/http" "testing" - "github.com/Azure/azure-sdk-for-go/sdk/internal/logger" + "github.com/Azure/azure-sdk-for-go/sdk/internal/log" ) func TestLoggingDefault(t *testing.T) { // ensure logging with nil listener doesn't fail - SetListener(nil) - logger.Log().Write(logger.LogRequest, "this should work just fine") + LogSetListener(nil) + log.Write(log.Request, "this should work just fine") - log := map[LogClassification]string{} - SetListener(func(cls LogClassification, msg string) { - log[cls] = msg + testlog := map[LogClassification]string{} + LogSetListener(func(cls LogClassification, msg string) { + testlog[cls] = msg }) const req = "this is a request" - logger.Log().Write(logger.LogRequest, req) + log.Write(log.Request, req) const resp = "this is a response: %d" - logger.Log().Writef(logger.LogResponse, resp, http.StatusOK) - if l := len(log); l != 2 { + log.Writef(log.Response, resp, http.StatusOK) + if l := len(testlog); l != 2 { t.Fatalf("unexpected log entry count: %d", l) } - if log[LogRequest] != req { - t.Fatalf("unexpected log request: %s", log[LogRequest]) + if testlog[LogRequest] != req { + t.Fatalf("unexpected log request: %s", testlog[LogRequest]) } - if log[LogResponse] != fmt.Sprintf(resp, http.StatusOK) { - t.Fatalf("unexpected log response: %s", log[LogResponse]) + if testlog[LogResponse] != fmt.Sprintf(resp, http.StatusOK) { + t.Fatalf("unexpected log response: %s", testlog[LogResponse]) } } func TestLoggingClassification(t *testing.T) { - log := map[LogClassification]string{} - SetListener(func(cls LogClassification, msg string) { - log[cls] = msg + testlog := map[LogClassification]string{} + LogSetListener(func(cls LogClassification, msg string) { + testlog[cls] = msg }) - SetClassifications(LogRequest) + LogSetClassifications(LogRequest) defer resetClassifications() - logger.Log().Write(logger.LogResponse, "this shouldn't be in the log") - if s, ok := log[LogResponse]; ok { + log.Write(log.Response, "this shouldn't be in the log") + if s, ok := testlog[LogResponse]; ok { t.Fatalf("unexpected log entry %s", s) } const req = "this is a request" - logger.Log().Write(logger.LogRequest, req) - if log[LogRequest] != req { - t.Fatalf("unexpected log entry: %s", log[LogRequest]) + log.Write(log.Request, req) + if testlog[LogRequest] != req { + t.Fatalf("unexpected log entry: %s", testlog[LogRequest]) } } diff --git a/sdk/azcore/policy_anonymous_credential.go b/sdk/azcore/policy_anonymous_credential.go index 5c53c3cc0320..3f5bc5ceecd4 100644 --- a/sdk/azcore/policy_anonymous_credential.go +++ b/sdk/azcore/policy_anonymous_credential.go @@ -6,7 +6,7 @@ package azcore func anonCredAuthPolicyFunc(AuthenticationOptions) Policy { - return PolicyFunc(anonCredPolicyFunc) + return policyFunc(anonCredPolicyFunc) } func anonCredPolicyFunc(req *Request) (*Response, error) { diff --git a/sdk/azcore/policy_logging.go b/sdk/azcore/policy_logging.go index 41e4546f41d4..66e2fb8313b3 100644 --- a/sdk/azcore/policy_logging.go +++ b/sdk/azcore/policy_logging.go @@ -11,8 +11,8 @@ import ( "strings" "time" - "github.com/Azure/azure-sdk-for-go/sdk/internal/logger" - "github.com/Azure/azure-sdk-for-go/sdk/internal/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/internal/diag" + "github.com/Azure/azure-sdk-for-go/sdk/internal/log" ) // LogOptions configures the logging policy's behavior. @@ -52,7 +52,7 @@ func (p *logPolicy) Do(req *Request) (*Response, error) { req.SetOperationValue(opValues) // Log the outgoing request as informational - if logger.Log().Should(logger.LogRequest) { + if log.Should(log.Request) { b := &bytes.Buffer{} fmt.Fprintf(b, "==> OUTGOING REQUEST (Try=%d)\n", opValues.try) writeRequestWithResponse(b, req, nil, nil) @@ -60,7 +60,7 @@ func (p *logPolicy) Do(req *Request) (*Response, error) { if p.options.IncludeBody { err = req.writeBody(b) } - logger.Log().Write(logger.LogRequest, b.String()) + log.Write(log.Request, b.String()) if err != nil { return nil, err } @@ -73,7 +73,7 @@ func (p *logPolicy) Do(req *Request) (*Response, error) { tryDuration := tryEnd.Sub(tryStart) opDuration := tryEnd.Sub(opValues.start) - if logger.Log().Should(logger.LogResponse) { + if log.Should(log.Response) { // We're going to log this; build the string to log b := &bytes.Buffer{} fmt.Fprintf(b, "==> REQUEST/RESPONSE (Try=%d/%v, OpTime=%v) -- ", opValues.try, tryDuration, opDuration) @@ -86,11 +86,11 @@ func (p *logPolicy) Do(req *Request) (*Response, error) { writeRequestWithResponse(b, req, response, err) if err != nil { // skip frames runtime.Callers() and runtime.StackTrace() - b.WriteString(runtime.StackTrace(2, StackFrameCount)) + b.WriteString(diag.StackTrace(2, StackFrameCount)) } else if p.options.IncludeBody { err = response.writeBody(b) } - logger.Log().Write(logger.LogResponse, b.String()) + log.Write(log.Response, b.String()) } return response, err } diff --git a/sdk/azcore/policy_logging_test.go b/sdk/azcore/policy_logging_test.go index 4b364711ab56..6f60fd62b441 100644 --- a/sdk/azcore/policy_logging_test.go +++ b/sdk/azcore/policy_logging_test.go @@ -18,7 +18,7 @@ import ( func TestPolicyLoggingSuccess(t *testing.T) { log := map[LogClassification]string{} - SetListener(func(cls LogClassification, s string) { + LogSetListener(func(cls LogClassification, s string) { log[cls] = s }) srv, close := mock.NewServer() @@ -68,7 +68,7 @@ func TestPolicyLoggingSuccess(t *testing.T) { func TestPolicyLoggingError(t *testing.T) { log := map[LogClassification]string{} - SetListener(func(cls LogClassification, s string) { + LogSetListener(func(cls LogClassification, s string) { log[cls] = s }) srv, close := mock.NewServer() diff --git a/sdk/azcore/policy_retry.go b/sdk/azcore/policy_retry.go index fcd888771ca0..79a371b8c61b 100644 --- a/sdk/azcore/policy_retry.go +++ b/sdk/azcore/policy_retry.go @@ -14,7 +14,7 @@ import ( "net/http" "time" - "github.com/Azure/azure-sdk-for-go/sdk/internal/logger" + "github.com/Azure/azure-sdk-for-go/sdk/internal/log" ) const ( @@ -50,18 +50,6 @@ type RetryOptions struct { StatusCodes []int } -var ( - // StatusCodesForRetry is the default set of HTTP status code for which the policy will retry. - // Changing its value will affect future created clients that use the default values. - StatusCodesForRetry = []int{ - http.StatusRequestTimeout, // 408 - http.StatusInternalServerError, // 500 - http.StatusBadGateway, // 502 - http.StatusServiceUnavailable, // 503 - http.StatusGatewayTimeout, // 504 - } -) - // init sets any default values func (o *RetryOptions) init() { if o.MaxRetries == 0 { @@ -81,7 +69,13 @@ func (o *RetryOptions) init() { o.RetryDelay = 0 } if o.StatusCodes == nil { - o.StatusCodes = StatusCodesForRetry + o.StatusCodes = []int{ + http.StatusRequestTimeout, // 408 + http.StatusInternalServerError, // 500 + http.StatusBadGateway, // 502 + http.StatusServiceUnavailable, // 503 + http.StatusGatewayTimeout, // 504 + } } } @@ -151,7 +145,7 @@ func (p *retryPolicy) Do(req *Request) (resp *Response, err error) { try := int32(1) for { resp = nil // reset - logger.Log().Writef(logger.LogRetryPolicy, "\n=====> Try=%d %s %s", try, req.Method, req.URL.String()) + log.Writef(log.RetryPolicy, "\n=====> Try=%d %s %s", try, req.Method, req.URL.String()) // For each try, seek to the beginning of the Body stream. We do this even for the 1st try because // the stream may not be at offset 0 when we first get it and we want the same behavior for the @@ -171,9 +165,9 @@ func (p *retryPolicy) Do(req *Request) (resp *Response, err error) { tryCancel() } if err == nil { - logger.Log().Writef(logger.LogRetryPolicy, "response %d", resp.StatusCode) + log.Writef(log.RetryPolicy, "response %d", resp.StatusCode) } else { - logger.Log().Writef(logger.LogRetryPolicy, "error %v", err) + log.Writef(log.RetryPolicy, "error %v", err) } if err == nil && !resp.HasStatusCode(options.StatusCodes...) { @@ -182,7 +176,7 @@ func (p *retryPolicy) Do(req *Request) (resp *Response, err error) { } else if ctxErr := req.Context().Err(); ctxErr != nil { // don't retry if the parent context has been cancelled or its deadline exceeded err = ctxErr - logger.Log().Writef(logger.LogRetryPolicy, "abort due to %v", err) + log.Writef(log.RetryPolicy, "abort due to %v", err) return } @@ -190,13 +184,13 @@ func (p *retryPolicy) Do(req *Request) (resp *Response, err error) { var nre NonRetriableError if errors.As(err, &nre) { // the error says it's not retriable so don't retry - logger.Log().Writef(logger.LogRetryPolicy, "non-retriable error %T", nre) + log.Writef(log.RetryPolicy, "non-retriable error %T", nre) return } if try == options.MaxRetries+1 { // max number of tries has been reached, don't sleep again - logger.Log().Writef(logger.LogRetryPolicy, "MaxRetries %d exceeded", options.MaxRetries) + log.Writef(log.RetryPolicy, "MaxRetries %d exceeded", options.MaxRetries) return } @@ -208,13 +202,13 @@ func (p *retryPolicy) Do(req *Request) (resp *Response, err error) { if delay <= 0 { delay = options.calcDelay(try) } - logger.Log().Writef(logger.LogRetryPolicy, "End Try #%d, Delay=%v", try, delay) + log.Writef(log.RetryPolicy, "End Try #%d, Delay=%v", try, delay) select { case <-time.After(delay): try++ case <-req.Context().Done(): err = req.Context().Err() - logger.Log().Writef(logger.LogRetryPolicy, "abort due to %v", err) + log.Writef(log.RetryPolicy, "abort due to %v", err) return } } diff --git a/sdk/azcore/policy_retry_test.go b/sdk/azcore/policy_retry_test.go index 7266e66c4c37..d8de29aa37a5 100644 --- a/sdk/azcore/policy_retry_test.go +++ b/sdk/azcore/policy_retry_test.go @@ -91,7 +91,7 @@ func TestRetryPolicyFailOnStatusCodeRespBodyPreserved(t *testing.T) { srv.SetResponse(mock.WithStatusCode(http.StatusInternalServerError), mock.WithBody([]byte(respBody))) // add a per-request policy that reads and restores the request body. // this is to simulate how something like httputil.DumpRequest works. - pl := NewPipeline(srv, PolicyFunc(func(r *Request) (*Response, error) { + pl := NewPipeline(srv, policyFunc(func(r *Request) (*Response, error) { b, err := ioutil.ReadAll(r.Body) if err != nil { t.Fatal(err) @@ -612,7 +612,7 @@ func (r *rewindTrackingBody) Seek(offset int64, whence int) (int64, error) { // used to inject a nil response type nilRespInjector struct { - t Transport + t Transporter c int // the current request number r []int // the list of request numbers to return a nil response (one-based) } diff --git a/sdk/azcore/poller.go b/sdk/azcore/poller.go index 7947d65c11e9..185488e03a36 100644 --- a/sdk/azcore/poller.go +++ b/sdk/azcore/poller.go @@ -17,16 +17,13 @@ import ( "strings" "time" - "github.com/Azure/azure-sdk-for-go/sdk/internal/logger" + "github.com/Azure/azure-sdk-for-go/sdk/internal/log" ) -// ErrorUnmarshaller is the func to invoke when the endpoint returns an error response that requires unmarshalling. -type ErrorUnmarshaller func(*Response) error - -// NewLROPoller creates an LROPoller based on the provided initial response. -// pollerID - a unique identifier for an LRO. it's usually the client.Method string. +// NewPoller creates a Poller based on the provided initial response. +// pollerID - a unique identifier for an LRO, it's usually the client.Method string. // NOTE: this is only meant for internal use in generated code. -func NewLROPoller(pollerID string, resp *Response, pl Pipeline, eu ErrorUnmarshaller) (*LROPoller, error) { +func NewPoller(pollerID string, resp *Response, pl Pipeline, eu func(*Response) error) (*Poller, error) { // this is a back-stop in case the swagger is incorrect (i.e. missing one or more status codes for success). // ideally the codegen should return an error if the initial response failed and not even create a poller. if !lroStatusCodeValid(resp) { @@ -36,7 +33,7 @@ func NewLROPoller(pollerID string, resp *Response, pl Pipeline, eu ErrorUnmarsha loc := resp.Header.Get(headerLocation) // in the case of both headers, always prefer the operation-location header if opLoc != "" { - return &LROPoller{ + return &Poller{ lro: newOpPoller(pollerID, opLoc, loc, resp), pl: pl, eu: eu, @@ -44,20 +41,20 @@ func NewLROPoller(pollerID string, resp *Response, pl Pipeline, eu ErrorUnmarsha }, nil } if loc != "" { - return &LROPoller{ + return &Poller{ lro: newLocPoller(pollerID, loc, resp.StatusCode), pl: pl, eu: eu, resp: resp, }, nil } - return &LROPoller{lro: &nopPoller{}, resp: resp}, nil + return &Poller{lro: &nopPoller{}, resp: resp}, nil } -// NewLROPollerFromResumeToken creates an LROPoller from a resume token string. -// pollerID - a unique identifier for an LRO. it's usually the client.Method string. +// NewPollerFromResumeToken creates a Poller from a resume token string. +// pollerID - a unique identifier for an LRO, it's usually the client.Method string. // NOTE: this is only meant for internal use in generated code. -func NewLROPollerFromResumeToken(pollerID string, token string, pl Pipeline, eu ErrorUnmarshaller) (*LROPoller, error) { +func NewPollerFromResumeToken(pollerID string, token string, pl Pipeline, eu func(*Response) error) (*Poller, error) { // unmarshal into JSON object to determine the poller type obj := map[string]interface{}{} err := json.Unmarshal([]byte(token), &obj) @@ -94,21 +91,21 @@ func NewLROPollerFromResumeToken(pollerID string, token string, pl Pipeline, eu if err = json.Unmarshal([]byte(token), lro); err != nil { return nil, err } - return &LROPoller{lro: lro, pl: pl, eu: eu}, nil + return &Poller{lro: lro, pl: pl, eu: eu}, nil } -// LROPoller encapsulates state and logic for polling on long-running operations. +// Poller encapsulates state and logic for polling on long-running operations. // NOTE: this is only meant for internal use in generated code. -type LROPoller struct { +type Poller struct { lro lroPoller pl Pipeline - eu ErrorUnmarshaller + eu func(*Response) error resp *Response err error } // Done returns true if the LRO has reached a terminal state. -func (l *LROPoller) Done() bool { +func (l *Poller) Done() bool { if l.err != nil { return true } @@ -116,7 +113,7 @@ func (l *LROPoller) Done() bool { } // Poll sends a polling request to the polling endpoint and returns the response or error. -func (l *LROPoller) Poll(ctx context.Context) (*http.Response, error) { +func (l *Poller) Poll(ctx context.Context) (*http.Response, error) { if l.Done() { // the LRO has reached a terminal state, don't poll again if l.resp != nil { @@ -147,7 +144,7 @@ func (l *LROPoller) Poll(ctx context.Context) (*http.Response, error) { } // ResumeToken returns a token string that can be used to resume a poller that has not yet reached a terminal state. -func (l *LROPoller) ResumeToken() (string, error) { +func (l *Poller) ResumeToken() (string, error) { if l.Done() { return "", errors.New("cannot create a ResumeToken from a poller in a terminal state") } @@ -160,7 +157,7 @@ func (l *LROPoller) ResumeToken() (string, error) { // FinalResponse will perform a final GET request and return the final HTTP response for the polling // operation and unmarshall the content of the payload into the respType interface that is provided. -func (l *LROPoller) FinalResponse(ctx context.Context, respType interface{}) (*http.Response, error) { +func (l *Poller) FinalResponse(ctx context.Context, respType interface{}) (*http.Response, error) { if !l.Done() { return nil, errors.New("cannot return a final response from a poller in a non-terminal state") } @@ -200,15 +197,15 @@ func (l *LROPoller) FinalResponse(ctx context.Context, respType interface{}) (*h // 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. -func (l *LROPoller) PollUntilDone(ctx context.Context, freq time.Duration, respType interface{}) (*http.Response, error) { +func (l *Poller) PollUntilDone(ctx context.Context, freq time.Duration, respType interface{}) (*http.Response, error) { logPollUntilDoneExit := func(v interface{}) { - logger.Log().Writef(logger.LogLongRunningOperation, "END PollUntilDone() for %T: %v", l.lro, v) + log.Writef(log.LongRunningOperation, "END PollUntilDone() for %T: %v", l.lro, v) } - logger.Log().Writef(logger.LogLongRunningOperation, "BEGIN PollUntilDone() for %T", l.lro) + log.Writef(log.LongRunningOperation, "BEGIN PollUntilDone() for %T", l.lro) if l.resp != nil { // initial check for a retry-after header existing on the initial response if retryAfter := RetryAfter(l.resp.Response); retryAfter > 0 { - logger.Log().Writef(logger.LogLongRunningOperation, "initial Retry-After delay for %s", retryAfter.String()) + log.Writef(log.LongRunningOperation, "initial Retry-After delay for %s", retryAfter.String()) if err := delay(ctx, retryAfter); err != nil { logPollUntilDoneExit(err) return nil, err @@ -231,10 +228,10 @@ func (l *LROPoller) PollUntilDone(ctx context.Context, freq time.Duration, respT } d := freq if retryAfter := RetryAfter(resp); retryAfter > 0 { - logger.Log().Writef(logger.LogLongRunningOperation, "Retry-After delay for %s", retryAfter.String()) + log.Writef(log.LongRunningOperation, "Retry-After delay for %s", retryAfter.String()) d = retryAfter } else { - logger.Log().Writef(logger.LogLongRunningOperation, "delay for %s", d.String()) + log.Writef(log.LongRunningOperation, "delay for %s", d.String()) } if err = delay(ctx, d); err != nil { logPollUntilDoneExit(err) @@ -243,8 +240,6 @@ func (l *LROPoller) PollUntilDone(ctx context.Context, freq time.Duration, respT } } -var _ Poller = (*LROPoller)(nil) - // abstracts the differences between concrete poller types type lroPoller interface { Done() bool diff --git a/sdk/azcore/poller_test.go b/sdk/azcore/poller_test.go index c5d53d542ed9..cf35675bd3e2 100644 --- a/sdk/azcore/poller_test.go +++ b/sdk/azcore/poller_test.go @@ -37,8 +37,8 @@ type widget struct { Size int `json:"size"` } -func TestNewLROPollerFail(t *testing.T) { - p, err := NewLROPoller("fake.poller", &Response{ +func TestNewPollerFail(t *testing.T) { + p, err := NewPoller("fake.poller", &Response{ &http.Response{ StatusCode: http.StatusBadRequest, }, @@ -51,7 +51,7 @@ func TestNewLROPollerFail(t *testing.T) { } } -func TestNewLROPollerFromResumeTokenFail(t *testing.T) { +func TestNewPollerFromResumeTokenFail(t *testing.T) { tests := []struct { name string token string @@ -65,7 +65,7 @@ func TestNewLROPollerFromResumeTokenFail(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - p, err := NewLROPollerFromResumeToken("fake.poller", test.token, NewPipeline(nil), errUnmarshall) + p, err := NewPollerFromResumeToken("fake.poller", test.token, NewPipeline(nil), errUnmarshall) if err == nil { t.Fatal("unexpected nil error") } @@ -101,7 +101,7 @@ func TestOpPollerSimple(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -141,7 +141,7 @@ func TestOpPollerWithWidgetPUT(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -186,7 +186,7 @@ func TestOpPollerWithWidgetPOSTLocation(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -229,7 +229,7 @@ func TestOpPollerWithWidgetPOST(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -275,7 +275,7 @@ func TestOpPollerWithWidgetResourceLocation(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -317,7 +317,7 @@ func TestOpPollerWithResumeToken(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -342,7 +342,7 @@ func TestOpPollerWithResumeToken(t *testing.T) { if err != nil { t.Fatal(err) } - lro, err = NewLROPollerFromResumeToken("fake.poller", tk, pl, errUnmarshall) + lro, err = NewPollerFromResumeToken("fake.poller", tk, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -372,7 +372,7 @@ func TestLocPollerSimple(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -402,7 +402,7 @@ func TestLocPollerWithWidget(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -436,7 +436,7 @@ func TestLocPollerCancelled(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -473,7 +473,7 @@ func TestLocPollerWithError(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -510,7 +510,7 @@ func TestLocPollerWithResumeToken(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -535,7 +535,7 @@ func TestLocPollerWithResumeToken(t *testing.T) { if err != nil { t.Fatal(err) } - lro, err = NewLROPollerFromResumeToken("fake.poller", tk, pl, errUnmarshall) + lro, err = NewPollerFromResumeToken("fake.poller", tk, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -564,7 +564,7 @@ func TestLocPollerWithTimeout(t *testing.T) { }, } pl := NewPipeline(srv) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } @@ -586,7 +586,7 @@ func TestNopPoller(t *testing.T) { }, } pl := NewPipeline(nil) - lro, err := NewLROPoller("fake.poller", firstResp, pl, errUnmarshall) + lro, err := NewPoller("fake.poller", firstResp, pl, errUnmarshall) if err != nil { t.Fatal(err) } diff --git a/sdk/azcore/progress.go b/sdk/azcore/progress.go index 77b2d9707974..40e74a6bb3e0 100644 --- a/sdk/azcore/progress.go +++ b/sdk/azcore/progress.go @@ -9,19 +9,15 @@ import ( "io" ) -// ProgressReceiver defines the signature of a callback function invoked as progress is reported. -// Note that bytesTransferred resets to 0 if the stream is reset when retrying a network operation. -type ProgressReceiver func(bytesTransferred int64) - type progress struct { rc io.ReadCloser rsc ReadSeekCloser - pr ProgressReceiver + pr func(bytesTransferred int64) offset int64 } // NewRequestProgress adds progress reporting to an HTTP request's body stream. -func NewRequestProgress(body ReadSeekCloser, pr ProgressReceiver) ReadSeekCloser { +func NewRequestProgress(body ReadSeekCloser, pr func(bytesTransferred int64)) ReadSeekCloser { return &progress{ rc: body, rsc: body, @@ -31,7 +27,7 @@ func NewRequestProgress(body ReadSeekCloser, pr ProgressReceiver) ReadSeekCloser } // NewResponseProgress adds progress reporting to an HTTP response's body stream. -func NewResponseProgress(body io.ReadCloser, pr ProgressReceiver) io.ReadCloser { +func NewResponseProgress(body io.ReadCloser, pr func(bytesTransferred int64)) io.ReadCloser { return &progress{ rc: body, rsc: nil, diff --git a/sdk/azcore/request.go b/sdk/azcore/request.go index e4e0cc509280..ed50a6b98a98 100644 --- a/sdk/azcore/request.go +++ b/sdk/azcore/request.go @@ -116,7 +116,7 @@ func NewRequest(ctx context.Context, httpMethod string, endpoint string) (*Reque // To send a request through a pipeline call Pipeline.Do(). func (req *Request) Next() (*Response, error) { if len(req.policies) == 0 { - return nil, ErrNoMorePolicies + return nil, errors.New("no more policies") } nextPolicy := req.policies[0] nextReq := *req diff --git a/sdk/azcore/request_test.go b/sdk/azcore/request_test.go index 994d0a6132a1..88168051b36e 100644 --- a/sdk/azcore/request_test.go +++ b/sdk/azcore/request_test.go @@ -9,7 +9,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "io" "io/ioutil" "mime" @@ -61,8 +60,8 @@ func TestRequestEmptyPipeline(t *testing.T) { if resp != nil { t.Fatal("expected nil response") } - if !errors.Is(err, ErrNoMorePolicies) { - t.Fatalf("expected ErrNoMorePolicies, got %v", err) + if err == nil { + t.Fatal("unexpected nil error") } }