Repository: snowflakedb/gosnowflake
Pull request #1212 (2024-09-20 09:40:29)

[SNOW-1671768] Ignore max retries error when getting acelerate config
### Description

Ignore and log all errors thrown while getting accelerate configuration from bucket.

### Checklist
- [ ] Created tests which fail without the change (if possible)
- [ ] Extended the README / documentation, if necessary


sfc-gh-astachowski: @sfc-gh-hyu Ok, changed to ignore all errors in that part of the code, logging them all at warn level.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1212?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) Report
Attention: Patch coverage is `59.25926%` with `11 lines` in your changes missing coverage. Please review.
> Project coverage is 82.51%. Comparing base [(`099708d`)](https://app.codecov.io/gh/snowflakedb/gosnowflake/commit/099708d318689634a558f705ccc19b3b7b278972?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) to head [(`0464a02`)](https://app.codecov.io/gh/snowflakedb/gosnowflake/commit/0464a02de29492313694d18e55425ac21370fe0a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb).

| [Files with missing lines](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1212?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) | Patch % | Lines |
|---|---|---|
| [file\_transfer\_agent.go](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1212?src=pr&el=tree&filepath=file_transfer_agent.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb#diff-ZmlsZV90cmFuc2Zlcl9hZ2VudC5nbw==) | 59.25% | [11 Missing :warning: ](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #1212      +/-   ##
==========================================
- Coverage   82.52%   82.51%   -0.01%     
==========================================
  Files          54       54              
  Lines       12916    12909       -7     
==========================================
- Hits        10659    10652       -7     
  Misses       2257     2257              
```

| [Flag](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1212/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) | Coverage Δ | |
|---|---|---|
| [](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1212/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb) | `82.51% <59.25%> (?)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/snowflakedb/gosnowflake/pull/1212?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=snowflakedb).


Patch:
diff --git a/file_transfer_agent.go b/file_transfer_agent.go
index fe341cd..5fa4e6f 100644
--- a/file_transfer_agent.go
+++ b/file_transfer_agent.go
@@ -9,7 +9,6 @@ import (
 	"context"
 	"database/sql/driver"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"math"
@@ -24,7 +23,6 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go-v2/service/s3"
-	"github.com/aws/smithy-go"
 	"github.com/gabriel-vasile/mimetype"
 )
 
@@ -591,46 +589,47 @@ func (sfa *snowflakeFileTransferAgent) updateFileMetadataWithPresignedURL() erro
 	return nil
 }
 
+type s3BucketAccelerateConfigGetter interface {
+	GetBucketAccelerateConfiguration(ctx context.Context, params *s3.GetBucketAccelerateConfigurationInput, optFns ...func(*s3.Options)) (*s3.GetBucketAccelerateConfigurationOutput, error)
+}
+
+type s3ClientCreator interface {
+	extractBucketNameAndPath(location string) (*s3Location, error)
+	createClient(info *execResponseStageInfo, useAccelerateEndpoint bool) (cloudClient, error)
+}
+
+func (sfa *snowflakeFileTransferAgent) transferAccelerateConfigWithUtil(s3Util s3ClientCreator) error {
+	s3Loc, err := s3Util.extractBucketNameAndPath(sfa.stageInfo.Location)
+	if err != nil {
+		return err
+	}
+	s3Cli, err := s3Util.createClient(sfa.stageInfo, false)
+	if err != nil {
+		return err
+	}
+	client, ok := s3Cli.(s3BucketAccelerateConfigGetter)
+	if !ok {
+		return (&SnowflakeError{
+			Number:   ErrFailedToConvertToS3Client,
+			SQLState: sfa.data.SQLState,
+			QueryID:  sfa.data.QueryID,
+			Message:  errMsgFailedToConvertToS3Client,
+		}).exceptionTelemetry(sfa.sc)
+	}
+	ret, err := client.GetBucketAccelerateConfiguration(context.Background(), &s3.GetBucketAccelerateConfigurationInput{
+		Bucket: &s3Loc.bucketName,
+	})
+	sfa.useAccelerateEndpoint = ret != nil && ret.Status == "Enabled"
+	if err != nil {
+		logger.WithContext(sfa.sc.ctx).Warnln("An error occurred when getting accelerate config:", err)
+	}
+	return nil
+}
+
 func (sfa *snowflakeFileTransferAgent) transferAccelerateConfig() error {
 	if sfa.stageLocationType == s3Client {
 		s3Util := new(snowflakeS3Client)
-		s3Loc, err := s3Util.extractBucketNameAndPath(sfa.stageInfo.Location)
-		if err != nil {
-			return err
-		}
-		s3Cli, err := s3Util.createClient(sfa.stageInfo, false)
-		if err != nil {
-			return err
-		}
-		client, ok := s3Cli.(*s3.Client)
-		if !ok {
-			return (&SnowflakeError{
-				Number:   ErrFailedToConvertToS3Client,
-				SQLState: sfa.data.SQLState,
-				QueryID:  sfa.data.QueryID,
-				Message:  errMsgFailedToConvertToS3Client,
-			}).exceptionTelemetry(sfa.sc)
-		}
-		ret, err := client.GetBucketAccelerateConfiguration(context.Background(), &s3.GetBucketAccelerateConfigurationInput{
-			Bucket: &s3Loc.bucketName,
-		})
-		sfa.useAccelerateEndpoint = ret != nil && ret.Status == "Enabled"
-		if err != nil {
-			var ae smithy.APIError
-			if errors.As(err, &ae) {
-				if ae.ErrorCode() == "AccessDenied" {
-					return nil
-				} else if ae.ErrorCode() == "MethodNotAllowed" {
-					return nil
-				} else if strings.EqualFold(ae.ErrorCode(), "UnsupportedArgument") {
-					// In AWS China and US Gov partitions, Transfer Acceleration is not supported
-					// https://docs.amazonaws.cn/en_us/aws/latest/userguide/s3.html#feature-diff
-					// https://docs.aws.amazon.com/govcloud-us/latest/UserGuide/govcloud-s3.html
-					return nil
-				}
-			}
-			return err
-		}
+		return sfa.transferAccelerateConfigWithUtil(s3Util)
 	}
 	return nil
 }


Test patch:
diff --git a/file_transfer_agent_test.go b/file_transfer_agent_test.go
index 3ffd6a8..69a10b3 100644
--- a/file_transfer_agent_test.go
+++ b/file_transfer_agent_test.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"io"
 	"net/url"
 	"os"
@@ -49,6 +50,135 @@ func TestGetBucketAccelerateConfiguration(t *testing.T) {
 	})
 }
 
+type s3ClientCreatorMock struct {
+	extract func(string) (*s3Location, error)
+	create  func(info *execResponseStageInfo, useAccelerateEndpoint bool) (cloudClient, error)
+}
+
+func (mock *s3ClientCreatorMock) extractBucketNameAndPath(location string) (*s3Location, error) {
+	return mock.extract(location)
+}
+
+func (mock *s3ClientCreatorMock) createClient(info *execResponseStageInfo, useAccelerateEndpoint bool) (cloudClient, error) {
+	return mock.create(info, useAccelerateEndpoint)
+}
+
+type s3BucketAccelerateConfigGetterMock struct {
+	err error
+}
+
+func (mock *s3BucketAccelerateConfigGetterMock) GetBucketAccelerateConfiguration(ctx context.Context, params *s3.GetBucketAccelerateConfigurationInput, optFns ...func(*s3.Options)) (*s3.GetBucketAccelerateConfigurationOutput, error) {
+	return nil, mock.err
+}
+
+func TestGetBucketAccelerateConfigurationTooManyRetries(t *testing.T) {
+	runSnowflakeConnTest(t, func(sct *SCTest) {
+		buf := &bytes.Buffer{}
+		logger.SetOutput(buf)
+		err := logger.SetLogLevel("warn")
+		if err != nil {
+			return
+		}
+		sfa := &snowflakeFileTransferAgent{
+			ctx:         context.Background(),
+			sc:          sct.sc,
+			commandType: uploadCommand,
+			srcFiles:    make([]string, 0),
+			data: &execResponseData{
+				SrcLocations: make([]string, 0),
+			},
+			stageInfo: &execResponseStageInfo{
+				Location: "test",
+			},
+		}
+		err = sfa.transferAccelerateConfigWithUtil(&s3ClientCreatorMock{
+			extract: func(s string) (*s3Location, error) {
+				return &s3Location{bucketName: "test", s3Path: "test"}, nil
+			},
+			create: func(info *execResponseStageInfo, useAccelerateEndpoint bool) (cloudClient, error) {
+				return &s3BucketAccelerateConfigGetterMock{err: errors.New("testing")}, nil
+			},
+		})
+		assertNilE(t, err)
+		assertStringContainsE(t, buf.String(), "msg=\"An error occurred when getting accelerate config: testing\"")
+	})
+}
+
+func TestGetBucketAccelerateConfigurationFailedExtractBucketNameAndPath(t *testing.T) {
+	runSnowflakeConnTest(t, func(sct *SCTest) {
+		sfa := &snowflakeFileTransferAgent{
+			ctx:         context.Background(),
+			sc:          sct.sc,
+			commandType: uploadCommand,
+			srcFiles:    make([]string, 0),
+			data: &execResponseData{
+				SrcLocations: make([]string, 0),
+			},
+			stageInfo: &execResponseStageInfo{
+				Location: "test",
+			},
+		}
+		err := sfa.transferAccelerateConfigWithUtil(&s3ClientCreatorMock{
+			extract: func(s string) (*s3Location, error) {
+				return nil, errors.New("failed extraction")
+			},
+		})
+		assertNotNilE(t, err)
+	})
+}
+
+func TestGetBucketAccelerateConfigurationFailedCreateClient(t *testing.T) {
+	runSnowflakeConnTest(t, func(sct *SCTest) {
+		sfa := &snowflakeFileTransferAgent{
+			ctx:         context.Background(),
+			sc:          sct.sc,
+			commandType: uploadCommand,
+			srcFiles:    make([]string, 0),
+			data: &execResponseData{
+				SrcLocations: make([]string, 0),
+			},
+			stageInfo: &execResponseStageInfo{
+				Location: "test",
+			},
+		}
+		err := sfa.transferAccelerateConfigWithUtil(&s3ClientCreatorMock{
+			extract: func(s string) (*s3Location, error) {
+				return &s3Location{bucketName: "test", s3Path: "test"}, nil
+			},
+			create: func(info *execResponseStageInfo, useAccelerateEndpoint bool) (cloudClient, error) {
+				return nil, errors.New("failed creation")
+			},
+		})
+		assertNotNilE(t, err)
+	})
+}
+
+func TestGetBucketAccelerateConfigurationInvalidClient(t *testing.T) {
+	runSnowflakeConnTest(t, func(sct *SCTest) {
+		sfa := &snowflakeFileTransferAgent{
+			ctx:         context.Background(),
+			sc:          sct.sc,
+			commandType: uploadCommand,
+			srcFiles:    make([]string, 0),
+			data: &execResponseData{
+				SrcLocations: make([]string, 0),
+			},
+			stageInfo: &execResponseStageInfo{
+				Location: "test",
+			},
+		}
+		err := sfa.transferAccelerateConfigWithUtil(&s3ClientCreatorMock{
+			extract: func(s string) (*s3Location, error) {
+				return &s3Location{bucketName: "test", s3Path: "test"}, nil
+			},
+			create: func(info *execResponseStageInfo, useAccelerateEndpoint bool) (cloudClient, error) {
+				return 1, nil
+			},
+		})
+		assertNotNilE(t, err)
+	})
+}
+
 func TestUnitDownloadWithInvalidLocalPath(t *testing.T) {
 	tmpDir, err := os.MkdirTemp("", "data")
 	if err != nil {