Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-1855886 gracefully ignore when default region us-west-2 is used in DSN or NewConnector #1278

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

sfc-gh-dszmolka
Copy link
Contributor

Description

us-west-2 region is the default region in Snowflake, and a special one. We already had checks for the case where someone accidentally configures it in Region, but for Account we did not have any, which led to an Account (mis)configured as myaccount.us-west-2 instead of omitting the region (as is the default) and using myaccount, not being able to connect to Snowflake and instead failing with HTTP404.

This change attempts to step over such simple misconfiguration gracefully and

  • ignores the extra .us-west-2 from Account instead of allowing the connection to proceed, which then surely will fail on the Snowflake backend
  • logs a line about the misconfiguration on Info level, giving it a chance for the user to notice and correct it

Checklist

  • Created tests which fail without the change (if possible)
  • Extended the README / documentation, if necessary

@sfc-gh-dszmolka sfc-gh-dszmolka requested a review from a team as a code owner December 16, 2024 11:41
dsn.go Outdated Show resolved Hide resolved
dsn_test.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.

Project coverage is 82.20%. Comparing base (45b3d45) to head (afc60ef).

Files with missing lines Patch % Lines
dsn.go 63.63% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1278      +/-   ##
==========================================
- Coverage   82.21%   82.20%   -0.02%     
==========================================
  Files          55       55              
  Lines       13537    13547      +10     
==========================================
+ Hits        11130    11136       +6     
- Misses       2407     2411       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sfc-gh-dszmolka sfc-gh-dszmolka requested a review from a team December 17, 2024 13:54
@sfc-gh-dszmolka sfc-gh-dszmolka merged commit 7d34091 into master Dec 18, 2024
37 of 39 checks passed
@sfc-gh-dszmolka sfc-gh-dszmolka deleted the SNOW-1855886-ignore-us-west-2-in-account branch December 18, 2024 09:53
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants