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

start connector node by default #484

Merged
merged 1 commit into from
Jul 27, 2023
Merged

start connector node by default #484

merged 1 commit into from
Jul 27, 2023

Conversation

lmatz
Copy link
Contributor

@lmatz lmatz commented Jul 27, 2023

What's changed and what's your intention?

PLEASE DO NOT LEAVE THIS EMPTY !!!

These two yaml files are the default ones that get used by users, e.g. reading README.md in this Repo or reading documentation at risingwave.dev.

I think we can start the connector node by default as it is a must for postgres-cdc and other advanced sources/sinks.

They typically encounter transport error when they try to establish source without starting a connector node.

Try to reduce the friction and confusion.

Checklist

  • I have written the necessary docs and comments
  • I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

@lmatz lmatz requested a review from arkbriar July 27, 2023 09:02
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Merging #484 (25649af) into main (272469d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #484   +/-   ##
=======================================
  Coverage   67.67%   67.67%           
=======================================
  Files          27       27           
  Lines        4210     4210           
=======================================
  Hits         2849     2849           
  Misses       1312     1312           
  Partials       49       49           
Flag Coverage Δ
unittests 67.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@arkbriar arkbriar left a comment

Choose a reason for hiding this comment

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

LGTM!

@arkbriar arkbriar added this pull request to the merge queue Jul 27, 2023
Merged via the queue into main with commit e434f97 Jul 27, 2023
7 of 8 checks passed
@arkbriar arkbriar deleted the lz/connector branch July 27, 2023 15:37
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.

2 participants