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

Query layer: support ssh fields, sync interval, simplify code path #1704

Merged
merged 6 commits into from
May 9, 2024

Conversation

Amogh-Bharadwaj
Copy link
Contributor

@Amogh-Bharadwaj Amogh-Bharadwaj commented May 8, 2024

  • Adds parsing for SSHConfig fields in Create Peer command for Postgres
CREATE PEER postgres_peer FROM POSTGRES WITH
(
    host = '<hostname>',
    port = 5432,
    user = '<user>',
    password = '<password>',
    database = '<dbname>',
    ssh_config = '{
        "host": "<ssh_host>",
        "port": 22,
        "user": "<ssh_user>",
        "password": "<ssh_password>",
        "private_key": "<ssh_private>"
    }'
);
  • Also you can now specify sync_interval in the create mirror command

  • Remove catalog flows table entry and update operations in query layer side, as we are anyways hitting a grpc endpoint which takes care of that. This makes the create_catalog_entry flag in the request to create/validate mirror endpoints redundant, so that is removed from route.proto

  • Fixes a bug in validate mirror where select * from sourcetables limit 0 was not checked for empty publication

Functionally tested via query layer and UI

@Amogh-Bharadwaj Amogh-Bharadwaj requested a review from serprex May 8, 2024 20:27
@Amogh-Bharadwaj Amogh-Bharadwaj changed the title Query layer: support ssh fields in postgres create peer Query layer: support ssh fields, sync interval, simplify code path May 9, 2024
@@ -46,7 +46,9 @@ func (h *FlowRequestHandler) ValidateCDCMirror(
sourcePeerConfig := req.ConnectionConfigs.Source.GetPostgresConfig()
if sourcePeerConfig == nil {
slog.Error("/validatecdc source peer config is nil", slog.Any("peer", req.ConnectionConfigs.Source))
return nil, errors.New("source peer config is nil")
return &protos.ValidateCDCMirrorResponse{
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like something easy to screw up in future, ideally it'd be a model that returns by value so zero value is correct response. should try make code work with errors returning nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change the validate peer and mirror endpoints in a follow up PR to be something like

func (h *FlowRequestHandler) ValidateCDCMirror(
	ctx context.Context, req *protos.CreateCDCFlowRequest,
) protos.ValidateCDCMirrorResponse {

where the response has an error message field

@Amogh-Bharadwaj Amogh-Bharadwaj merged commit e2be383 into main May 9, 2024
12 checks passed
@Amogh-Bharadwaj Amogh-Bharadwaj deleted the ssh-sql branch May 9, 2024 09:57
Amogh-Bharadwaj added a commit to PeerDB-io/docs that referenced this pull request May 9, 2024
Adds a section under Create Peer for Postgres for inputting fields for
ssh config
To land after PeerDB-io/peerdb#1704 lands
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