-
Notifications
You must be signed in to change notification settings - Fork 12
psdk(doc): development guide update for single response for AlterTableResponse #153
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR updates the development guide to clarify the proper way to report warnings in destination connector responses, specifically addressing the single-response constraint for AlterTableResponse and similar non-streaming RPCs. The documentation now explicitly distinguishes between source connectors (which support streaming responses) and destination connectors (which return single responses).
Key Changes
- Added a new "Response Types and Multiple Messages" section explaining the difference between streaming (source) and single (destination) response patterns
- Reorganized usage examples into separate sections for source and destination connectors with concrete code samples
- Updated the NOTE formatting to use bold text for consistency
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **Source Connectors (Streaming Responses):** | ||
| - The `Update` RPC is defined as: `rpc Update (UpdateRequest) returns (stream UpdateResponse) {}` | ||
| - The `stream` keyword means you can call `responseObserver.onNext()` **multiple times** to send multiple responses. | ||
| - You can send multiple warnings, records, checkpoints, etc. in separate `UpdateResponse` messages. | ||
| - You can send **only one task message** - once a task is issued, the sync stops immediately. | ||
|
|
||
| **Destination Connectors (Single Responses):** | ||
| - RPCs like `AlterTable`, `CreateTable`, `WriteBatch`, etc. return single (non-streaming) responses. | ||
| - Example: `rpc AlterTable(AlterTableRequest) returns (AlterTableResponse) {}` | ||
| - You can call `responseObserver.onNext()` **only once**, followed by `responseObserver.onCompleted()`. | ||
| - Each response uses a `oneof` field, meaning you can return **only one of**: success, warning, or task. |
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.
Should we make this nested bullets instead of Bold?
|
|
||
| #### Usage Examples | ||
|
|
||
| **Source Connector - Multiple responses with Update (streaming):** |
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.
can we convert this to bullets instead of bold?
| ); | ||
| ``` | ||
|
|
||
| **Destination Connector - Single response with AlterTable:** |
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.
can we convert this to bullets instead of bold?
| ``` | ||
| > NOTE: We continue with the sync in case of Warnings, and break execution when Tasks are thrown. | ||
|
|
||
| > **NOTE:** We continue with the sync in case of Warnings, and break execution when Tasks are thrown. |
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.
| > **NOTE:** We continue with the sync in case of Warnings, and break execution when Tasks are thrown. | |
| > NOTE: We continue with the sync in case of Warnings, and break execution when Tasks are thrown. |
| - Partner code should capture and relay a clear message when the account permissions are not sufficient. | ||
|
|
||
| ### User alerts | ||
| > NOTE: Available in V2 only. |
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.
Please remove this line, as all partners are using V2 now.
| - Descriptive dropdown: A dropdown field with contextual descriptions for each option, helping users choose the right value. Use a label–description pair for each option in `DescriptiveDropDownFields`. | ||
| - Toggle field: A toggle switch for binary options (e.g., on/off or yes/no). | ||
| - Upload field: Lets users upload files (e.g., certificates, keys) through the setup form. Use `allowed_file_type` to specify permitted file types and `max_file_size_bytes` to set a file size limit. Uploaded files are automatically converted to Base64-encoded strings in the SDK configuration object. You must implement decoding of the Base64 string to reconstruct the uploaded file locally. | ||
| - Conditional fields (Available in V2): This feature allows you to define fields that are dependent on the value of a specific parent field. The message consists of two nested-messages: `VisibilityCondition` and a list of dependent form fields. The `VisibilityCondition` message specifies the parent field and its condition value. The list of dependent fields defines the fields that are shown when the value of the parent field provided in the setup form matches the specified condition field. |
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.
Same here, remove Available in V2
|
|
||
| The ability to send multiple responses depends on whether the RPC returns a streaming response: | ||
|
|
||
| **Source Connectors (Streaming Responses):** |
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 all source connectors related rpc calls have streaming response, it is just Update method. Adding it in the braces feels incorrect.
|
|
||
| #### Response Types and Multiple Messages | ||
|
|
||
| The ability to send multiple responses depends on whether the RPC returns a streaming response: |
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 we should have separate split for source and destination connector, we can split section based on where stream response is sent and where it is not, and how would it behave in both case.
| - Partner code should use [Warning](https://github.com/fivetran/fivetran_sdk/blob/main/common.proto#L160) and [Task](https://github.com/fivetran/fivetran_sdk/blob/main/common.proto#L164) messages defined in the proto files to relay information or errors to Fivetran. | ||
| - Usage example: | ||
|
|
||
| #### Response Types and Multiple Messages |
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.
From the title, it seems confusing, if we want to point only about warning and tasks, we should change this title accordingly, something like Guidelines for using warnings and tasks.
Closes https://fivetran.atlassian.net/browse/RD-1055552
Description of Change
Update the connector documentation and examples with a proper way to report warnings