-
Notifications
You must be signed in to change notification settings - Fork 612
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: tvf with cdc source #20439
feat: tvf with cdc source #20439
Conversation
@@ -117,6 +117,11 @@ pub fn generate_risedev_env(services: &Vec<ServiceConfig>) -> String { | |||
writeln!(env, r#"PGUSER="{user}""#,).unwrap(); | |||
writeln!(env, r#"PGPASSWORD="{password}""#,).unwrap(); | |||
writeln!(env, r#"PGDATABASE="{database}""#,).unwrap(); | |||
writeln!( | |||
env, | |||
r#"RISEDEV_POSTGRES_WITH_OPTIONS_COMMON="connector='postgres-cdc',hostname='{host}',port='{port}'""#, |
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 don't think test specific env vars should be set in risedev_env
. How about setting them inside the e2e-source-test script?
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 should be acceptable as we already have it for Mysql tests
risingwave/src/risedevtool/src/risedev_env.rs
Line 100 in cbf002f
writeln!(env, r#"RISEDEV_MYSQL_WITH_OPTIONS_COMMON="connector='mysql-cdc',hostname='{host}',port='{port}'""#,).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.
I like this approach, just need to improve the readability. Thanks for the work!
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.
LGTM~!
Signed-off-by: tabversion <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
allow
postgres_query
andmysql_query
by reusing the params of cdc-sourcepart of #18583
Checklist
Documentation
Release note
Note that we only take the (hostname, port, username, password, database_name) from the source, ignoring the
slot.name
andserver.id