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

materialize-snowflake: make account field optional #1725

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

Alex-Bair
Copy link
Contributor

@Alex-Bair Alex-Bair commented Jul 16, 2024

Description:

Snowflake does not require an account query parameter in the connection string, so this change makes the account input optional. The go-snowflake driver's sf.DSN method for building the connection string can error out when account isn't provided, so instead we build the connection string ourselves.

Workflow steps:

(How does one use this feature, and how has it changed)

Documentation links affected:

The Snowflake materialization docs will need updated to reflect that account is not required.

Notes for reviewers:

Tested locally with:

  • both password & JWT authentication.
  • both flavors of Snowflake host URL formats
    • <orgname>-<account_name>.snowflakecomputing.com
    • <accountlocator>.<region>.<cloud>.snowflakecomputing.com

This change is Reviewable

@Alex-Bair Alex-Bair linked an issue Jul 16, 2024 that may be closed by this pull request
materialize-snowflake/config.go Outdated Show resolved Hide resolved
materialize-snowflake/pipe.go Outdated Show resolved Hide resolved
materialize-snowflake/config_test.go Outdated Show resolved Hide resolved
Snowflake does not require an account query parameter in the connection string, so this change makes the account field optional. The go-snowflake driver's sf.DSN method for building the connection string can error out when an account isn't provided, so instead we build the connection string ourselves.
Copy link
Member

@williamhbaker williamhbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Alex-Bair Alex-Bair merged commit 130affb into main Jul 17, 2024
48 of 50 checks passed
@Alex-Bair Alex-Bair deleted the bair/sf-account-optional branch July 17, 2024 19:37
Alex-Bair added a commit to estuary/flow that referenced this pull request Jul 17, 2024
Update documentation for estuary/connectors#1725. Additionally, I reordered the properties in the Endpoint listing to reflect how the order in the UI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

materialize-snowflake: make account input optional
2 participants