-
Notifications
You must be signed in to change notification settings - Fork 597
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
feat(sink): support delta sink with gcs #16182
Conversation
src/connector/src/sink/deltalake.rs
Outdated
let mut storage_options = HashMap::new(); | ||
storage_options.insert( | ||
GCS_SERVICE_ACCOUNT.to_string(), | ||
self.gcs_service_account.clone().unwrap(), |
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.
If the gcs_service_account
is not specified, the CN will panic?
If so, sounds unacceptable to me. Please do validation when create sink
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.
fixed
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.
The fix is inside create_deltalake_client()
, you are sure that it will be called during create sink
, right?
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.
Yes, It will call it on create sink
and sink write
, create sink
is called first, so when the user just makes a write error, it will report the error to the psql client, not recovery
src/connector/src/sink/deltalake.rs
Outdated
storage_options.insert( | ||
GCS_SERVICE_ACCOUNT.to_string(), | ||
self.gcs_service_account.clone().ok_or_else(|| { | ||
SinkError::Config(anyhow!("gcs.service.account is required with aws gcs")) |
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.
not aws
SinkError::Config(anyhow!("gcs.service.account is required with aws gcs")) | |
SinkError::Config(anyhow!("gcs.service.account is required with Google Cloud Storage (GCS)")) |
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
#16140
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
we can create delta lake sink with gcs the option is :
location = 'gs://bucket/xxx’
gcs.service.account = ‘service account json string’