Gosnowflake unable to use because aws sdk update. #970

CCGV2 · 2023-11-18T05:08:33Z

gosnowflake unable to use because https://github.com/aws/aws-sdk-go-v2/releases/tag/release-2023-11-17, it changed s3 HeadObjectOutput.ContentLength from int64 to pointer int64, which make anything unable to build/test.

Please answer these questions before submitting your issue.
In order to accurately debug the issue this information is required. Thanks!

What version of GO driver are you using?
v1.7.0
What operating system and processor architecture are you using?
MacOS, 13.5.2 (22G91), M1 pro
What version of GO are you using?
go version go1.21.3 darwin/arm64

4.Server version:* E.g. 1.90.1
not important
5. What did you do?
just testing a unrelated test.

What did you expect to see?
No error

github.com/snowflakedb/gosnowflake

../../../../go/pkg/mod/github.com/snowflakedb/gosnowflake@v1.7.0/s3_storage_client.go:105:3: cannot use out.ContentLength (variable of type *int64) as int64 value in struct literal

MircoT · 2023-11-19T10:48:01Z

Hi! Same problem here. I use this simple workaround for the moment:

Remove the current dependency requirements
Set the same package versions of gosnowflake v1.7.0 to replace possible updates
Let go mod tidy do the dirty work for you

Package replacements:

replace (
	github.com/aws/aws-sdk-go-v2 => github.com/aws/aws-sdk-go-v2 v1.17.7
	github.com/aws/aws-sdk-go-v2/credentials => github.com/aws/aws-sdk-go-v2/credentials v1.13.18
	github.com/aws/aws-sdk-go-v2/feature/s3/manager => github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.59
	github.com/aws/aws-sdk-go-v2/service/s3 => github.com/aws/aws-sdk-go-v2/service/s3 v1.31.0
)

With that, I'm able to continue to work with the go client for snowflake. There will be a patch for such a problem?

sfc-gh-dszmolka · 2023-11-20T07:15:43Z

hi and thank you for reporting this issue and for sharing the workaround while the proper fix is in ! we're taking a look and i'll share progress on this thread as events unfold.

sfc-gh-dszmolka · 2023-11-20T11:45:47Z

--> current status:

fix underway on #971
edit: discussion still ongoing on the approach
edit2: merge held until January 2024 release, as December 2023 supposed to be free of breaking changes (like this would be). Explanation below
edit3: after @kenshaw contribution (thank you!) now it's possible to release the fix without upgrading our dependencies, thus targeting December release

sfc-gh-dszmolka · 2023-11-22T07:13:55Z

so we're trying to figure out what situation brought this issue for you folks and if anyone could describe the exact steps taken, that would be immensely helpful.

we observed that just by

go get github.com/snowflakedb/gosnowflake@v1.7.0

it doesn't update the AWS SDK.

did any of you by any chance upgrade the AWS SDK apart from the gosnowflake library, e.g. as required by your project ?
if anyone affected by this issue could bring clarity how it got introduced into your project, that would be really helpful, especially for us making a decision how to move forward. Thank you in advance! 🙏

vitaly-eureka-security · 2023-11-22T07:25:45Z

did any of you by any chance upgrade the AWS SDK apart from the gosnowflake library, e.g. as required by your project ? if anyone affected by this issue could bring clarity how it got introduced into your project, that would be really helpful, especially for us making a decision how to move forward. Thank you in advance! 🙏

In our case, the issue was surfaced when both Snowflake SDK and AWS SDK were updated to the latest.
AWS SDK have broken the compatibility in the latest versions around field nullability (we had to adjust our code base as well).

MircoT · 2023-11-22T08:27:28Z

so we're trying to figure out what situation brought this issue for you folks and if anyone could describe the exact steps taken, that would be immensely helpful.

we observed that just by
go get github.com/snowflakedb/gosnowflake@v1.7.0
it doesn't update the AWS SDK.

did any of you by any chance upgrade the AWS SDK apart from the gosnowflake library, e.g. as required by your project ? if anyone affected by this issue could bring clarity how it got introduced into your project, that would be really helpful, especially for us making a decision how to move forward. Thank you in advance! 🙏

In my case, I just made a go get -u to update the project dependencies.

sfc-gh-dszmolka · 2023-11-22T08:46:06Z

In my case, I just made a go get -u to update the project dependencies.

thank you ! does your project by any chance depend on github.com/aws/aws-sdk-go-v2/feature/s3/manager outside of gosnowflake ?

MircoT · 2023-11-22T09:36:11Z

In my case, I just made a go get -u to update the project dependencies.

thank you ! does your project by any chance depend on github.com/aws/aws-sdk-go-v2/feature/s3/manager outside of gosnowflake ?

No, it doesn't. The main require was github.com/snowflakedb/gosnowflake, I haven't the aws sdk used in the code.

sfc-gh-dszmolka · 2023-11-22T09:48:16Z

makes sense, thank you! per go help get:

The -u flag instructs get to update modules providing dependencies
of packages named on the command line to use >> newer minor << or patch
releases when available.

which I guess what forced the newer AWS SDK to be downloaded instead of the version specified in the go.mod by gosnowflake (v1.11.59) since AWS decided to release this breaking change with a MINOR version bump only (v1.14.0) instead of MAJOR --> it was allowed by the -u flag probably

this is a very good insight, thank you !

sfc-gh-dszmolka · 2023-11-22T13:04:13Z

Again thank you all of you who provided your inputs for us to better understand how this breaks in your environment, without gosnowflake upgrading any of the dependencies !

After a long discussion and considering:

our upcoming (December) release is supposed to be free from breaking changes
upgrading relevant gosnowflake dependencies (SNOW-974548 Upgrade AWS SDK #971) would be a breaking change for all of our users and customers, even for those who did not upgrade aws-sdk-go-v2/feature/s3/manager & co and currently not affected

we decided to hold this PR and postpone it into the January release.

We're aware this might not be an ideal situation, but considering all of the factors and the effects, this is what we went with.

For those of you who are affected by AWS releasing the breaking change under a MINOR version change and thus aws-sdk-go-v2/feature/s3/manager v1.14.0 finding its way into your project in some way, please use @MircoT 's workaround.

This thread will be kept updated with the progress, which is unlikely to happen until January release I'm afraid.

kenshaw · 2023-11-26T23:41:59Z

Was going to open a separate ticket, but usql will need this issue fixed, as other database drivers that use the AWS SDK will eventually uncover this. The fix is simple:

diff --git a/s3_storage_client.go b/s3_storage_client.go
index 82638e7..c8066de 100644
--- a/s3_storage_client.go
+++ b/s3_storage_client.go
@@ -100,9 +100,13 @@ func (util *snowflakeS3Client) getFileHeader(meta *fileMetadata, filename string
                        out.Metadata[amzMatdesc],
                }
        }
+       var contentLength int64
+       if out.ContentLength != nil {
+               contentLength = *out.ContentLength
+       }
        return &fileHeader{
                out.Metadata[sfcDigest],
-               out.ContentLength,
+               contentLength,
                &encMeta,
        }, nil
 }

I would open a PR for it, but I don't have the ability to run all the unit tests for this driver.

kenshaw · 2023-11-26T23:47:34Z

As I realized this wouldn't be backwards compatible, here's a quick version that would work for either version of the SDK:

diff --git a/s3_storage_client.go b/s3_storage_client.go
index 82638e7..f67287c 100644
--- a/s3_storage_client.go
+++ b/s3_storage_client.go
@@ -102,7 +102,7 @@ func (util *snowflakeS3Client) getFileHeader(meta *fileMetadata, filename string
        }
        return &fileHeader{
                out.Metadata[sfcDigest],
-               out.ContentLength,
+               convContentLength(out.ContentLength),
                &encMeta,
        }, nil
 }
@@ -281,3 +281,15 @@ func (util *snowflakeS3Client) getS3Object(meta *fileMetadata, filename string)
                Key:    &s3path,
        }, nil
 }
+
+func convContentLength(v interface{}) int64 {
+       switch x := v.(type) {
+       case int64:
+               return x
+       case *int64:
+               if x != nil {
+                       return *x
+               }
+       }
+       return 0
+}

sfc-gh-dszmolka · 2023-11-27T07:45:52Z

nice idea, thank you @kenshaw !

sfc-gh-dszmolka · 2023-11-28T06:17:04Z

so thanks to your above contribution, we can implement and release the fix after all most likely in the December release without upgrading the dependencies - which release should be upcoming in the near future. will keep this thread posted

kenshaw · 2023-11-28T11:26:57Z

@sfc-gh-dszmolka Glad this worked for you!

dustin-decker · 2023-11-29T03:49:31Z

New workaround until a release is cut

// remove when https://github.com/snowflakedb/gosnowflake/issues/970 is resolved
replace github.com/snowflakedb/gosnowflake => github.com/snowflakedb/gosnowflake v1.7.1-0.20231128064355-f33dfc7eb567

gtomitsuka · 2023-12-05T21:20:18Z

That latest workaround causes my build to crash w/o any context (Process finished with the exit code 137 (interrupted by signal 9: SIGKILL)), has anyone else seen this?

I also tried to use the latest version v1.7.1-0.20231205121005-b8e673614933 with the same outcome

sfc-gh-pfus · 2023-12-06T08:54:55Z

Hi @gtomitsuka ! Is there a chance of some repro? All our tests pass either on older version of AWS SDK (master) or after bumping library after the breaking change (https://github.com/snowflakedb/gosnowflake/actions/runs/7111880779)

gtomitsuka · 2023-12-06T16:23:25Z

Nevermind, it's unrelated; I tried with the original fix on v1.7.0 (replace aws-sdk-go-v2 -> ...) and it still didn't work.

sfc-gh-dszmolka · 2023-12-07T08:53:13Z

released with 1.7.1, closing

CCGV2 added the bug Erroneous or unexpected behaviour label Nov 18, 2023

sfc-gh-dszmolka assigned sfc-gh-pfus Nov 20, 2023

sfc-gh-dszmolka added the status-in_progress Issue is worked on by the driver team label Nov 20, 2023

sfc-gh-dszmolka assigned sfc-gh-triage-snowpark-ecosystem-dl and unassigned sfc-gh-pfus Nov 20, 2023

sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review and removed status-in_progress Issue is worked on by the driver team labels Nov 20, 2023

sfc-gh-dszmolka mentioned this issue Nov 22, 2023

s3_storage_client.go: cannot use out.ContentLength (variable of type *int64) as int64 value in struct literal #973

Closed

sfc-gh-dszmolka added status-in_progress Issue is worked on by the driver team and removed status-pr_pending_merge A PR is made and is under review labels Nov 22, 2023

alexlokshin-czi mentioned this issue Nov 27, 2023

[bot] Upgrade AWS Mocks chanzuckerberg/go-misc#919

Closed

sfc-gh-dszmolka added status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-in_progress Issue is worked on by the driver team labels Nov 28, 2023

codingllama mentioned this issue Nov 28, 2023

Bump the go group with 19 updates gravitational/teleport#34973

Closed

sfc-gh-dszmolka closed this as completed Dec 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gosnowflake unable to use because aws sdk update. #970

Gosnowflake unable to use because aws sdk update. #970

CCGV2 commented Nov 18, 2023

MircoT commented Nov 19, 2023

sfc-gh-dszmolka commented Nov 20, 2023

sfc-gh-dszmolka commented Nov 20, 2023 •

edited

Loading

sfc-gh-dszmolka commented Nov 22, 2023

vitaly-eureka-security commented Nov 22, 2023

MircoT commented Nov 22, 2023

sfc-gh-dszmolka commented Nov 22, 2023

MircoT commented Nov 22, 2023

sfc-gh-dszmolka commented Nov 22, 2023

sfc-gh-dszmolka commented Nov 22, 2023

kenshaw commented Nov 26, 2023

kenshaw commented Nov 26, 2023

sfc-gh-dszmolka commented Nov 27, 2023

sfc-gh-dszmolka commented Nov 28, 2023

kenshaw commented Nov 28, 2023

dustin-decker commented Nov 29, 2023

gtomitsuka commented Dec 5, 2023

sfc-gh-pfus commented Dec 6, 2023

gtomitsuka commented Dec 6, 2023

sfc-gh-dszmolka commented Dec 7, 2023

Gosnowflake unable to use because aws sdk update. #970

Gosnowflake unable to use because aws sdk update. #970

Comments

CCGV2 commented Nov 18, 2023

github.com/snowflakedb/gosnowflake

MircoT commented Nov 19, 2023

sfc-gh-dszmolka commented Nov 20, 2023

sfc-gh-dszmolka commented Nov 20, 2023 • edited Loading

sfc-gh-dszmolka commented Nov 22, 2023

vitaly-eureka-security commented Nov 22, 2023

MircoT commented Nov 22, 2023

sfc-gh-dszmolka commented Nov 22, 2023

MircoT commented Nov 22, 2023

sfc-gh-dszmolka commented Nov 22, 2023

sfc-gh-dszmolka commented Nov 22, 2023

kenshaw commented Nov 26, 2023

kenshaw commented Nov 26, 2023

sfc-gh-dszmolka commented Nov 27, 2023

sfc-gh-dszmolka commented Nov 28, 2023

kenshaw commented Nov 28, 2023

dustin-decker commented Nov 29, 2023

gtomitsuka commented Dec 5, 2023

sfc-gh-pfus commented Dec 6, 2023

gtomitsuka commented Dec 6, 2023

sfc-gh-dszmolka commented Dec 7, 2023

sfc-gh-dszmolka commented Nov 20, 2023 •

edited

Loading