-
Notifications
You must be signed in to change notification settings - Fork 28
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
Doc: Sink to Snowflake #2115
Doc: Sink to Snowflake #2115
Conversation
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave some comments. Others LGTM, thanks.
Co-authored-by: Shanlan(Lily) <[email protected]>
Co-authored-by: Shanlan(Lily) <[email protected]>
| snowflake.account_identifier | The unique `account_identifier` provided by Snowflake. Please use the form `<orgname>-<account_name>`. See [Account identifiers](https://docs.snowflake.com/en/user-guide/admin-account-identifier) for more details.| | ||
| snowflake.user | The user that owns the table to be sinked. The user should have been granted corresponding *role*. See [Grant role](https://docs.snowflake.com/en/sql-reference/sql/grant-role) for more details. | | ||
| snowflake.rsa_public_key_fp | The public key fingerprint used when generating custom `jwt_token`. See [Authenticating to the server](https://docs.snowflake.com/en/developer-guide/sql-api/authenticating) for more details. | | ||
| snowflake.private_key | The rsa pem key **without** encryption. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think giving the pem content directly is somehow counterintuitive. We may give a more concrete example of this. cc @xxhZs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is paired with the rsa_public_key_fp
above, is it possible to post the same connection from above again
https://docs.snowflake.com/en/developer-guide/sql-api/authenticating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is paired with the rsa_public_key_fp above, is it possible to post the same connection from above again
https://docs.snowflake.com/en/developer-guide/sql-api/authenticating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to merge this PR now, so create this issue to track, as it takes a while and it's nice-to-have.
docs/guides/sink-to-snowflake.md
Outdated
|
||
## Required permission | ||
|
||
To successfully sink data into Snowflake, the user account must have the appropriate permissions. These permissions include: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be helpful to clarify if it's the RisingWave user account or the Snowflake user account that needs the permissions.
docs/guides/sink-to-snowflake.md
Outdated
snowflake.s3_path = 'EXAMPLE_S3_PATH', | ||
-- depends on your mv setup, note that snowflake sink *only* supports | ||
-- `append-only` mode at present. | ||
force_append_only = 'true' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
force_append_only
should be included in the Parameters section.
docs/guides/sink-to-snowflake.md
Outdated
snowflake.aws_region = 'EXAMPLE_REGION', | ||
snowflake.s3_path = 'EXAMPLE_S3_PATH', | ||
-- depends on your mv setup, note that snowflake sink *only* supports | ||
-- `append-only` mode at present. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should note these limitations elsewhere if necessary.
Co-authored-by: emile-00 <[email protected]>
Co-authored-by: emile-00 <[email protected]>
Co-authored-by: emile-00 <[email protected]>
Co-authored-by: emile-00 <[email protected]>
Co-authored-by: emile-00 <[email protected]>
Co-authored-by: emile-00 <[email protected]>
docs/guides/sink-to-snowflake.md
Outdated
| Parameter | Description | | ||
|-------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| snowflake.database | The Snowflake database used for sinking. | | ||
| snowflake.schema | The corresponding schema where sink table exists. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be important to emphasize here that only the name of the schema is needed
Info
Description
Notes
Let me know if any comments, thanks!
Related code PR
file_suffix
risingwave#16241Related doc issue
Resolves Document: feat(sink): implement snowflake sink #2057
Resolves Document: feat(snowflake-sink): add example use case & detailed spec; fix a subtle problem regarding
file_suffix
#2059For reviewers
Preview
Key points
Before merging
I have checked the doc site preview, and the updated parts look good.
I have acquired the approval from the owner (and optionally the reviewers) of the code PR and at least one tech writer (
CharlieSYH
,emile-00
, &hengm3467
).