-
Notifications
You must be signed in to change notification settings - Fork 97
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
Conversation
@@ -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{ |
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.
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
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.
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
Adds a section under Create Peer for Postgres for inputting fields for ssh config To land after PeerDB-io/peerdb#1704 lands
Also you can now specify
sync_interval
in the create mirror commandRemove 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