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.

Sign up for GitHub

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

Jump to bottom

Interface changes revision-2 #58

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Interface changes revision-2 #58

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e394282
interface changes revision-2
manjutapali Jul 2, 2024
1bd70f9
updated the name
manjutapali Jul 2, 2024
92ef803
updated the code comments
manjutapali Jul 2, 2024
759ca84
renamed the operation
manjutapali Jul 5, 2024
5aeb6d3
reverted the field renumbering
manjutapali Jul 9, 2024
c7c4500
revert schema diff changes
manjutapali Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 33 additions & 23 deletions common.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
syntax = "proto3";
option optimize_for = SPEED;
option java_multiple_files = true;
option go_package = "fivetran.com/fivetran_sdk";
package fivetran_sdk;
option go_package = "fivetran.com/fivetran_sdk_r2";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we changing the package name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We are introducing breaking changes and will use the identifier revision (r2) to denote the version for these updates.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@georgewfraser as we discussed during the previous meeting, we need to be able to run one version in production while we need to be able to build and release different interface version in the testers to give to the partners. Since the testers are in the engineering repo too, there is no way to have two versions of grpc interface in the same package. So we introduced the "_r2" for "revision 2". (We chose to replace "version" with "revision" to indicate we are not going to use versions)

package fivetran_sdk.r2;

import "google/protobuf/timestamp.proto";

Expand All @@ -20,10 +20,12 @@ message FormField {
string label = 2;
bool required = 3;
optional string description = 4;
optional string default_value = 5;
optional string placeholder = 6;
oneof type {
TextField text_field = 5;
DropdownField dropdown_field = 6;
ToggleField toggle_field = 7;
TextField text_field = 7;
manjutapali marked this conversation as resolved.
Show resolved Hide resolved
DropdownField dropdown_field = 8;
ToggleField toggle_field = 9;
}
}

Expand Down Expand Up @@ -53,7 +55,6 @@ message TestResponse {
oneof response {
bool success = 1;
string failure = 2;
// potential future warning
}
}

Expand All @@ -79,21 +80,29 @@ enum DataType {
DECIMAL = 5;
FLOAT = 6;
DOUBLE = 7;
NAIVE_DATE = 8;
NAIVE_DATETIME = 9;
UTC_DATETIME = 10;
BINARY = 11;
XML = 12;
STRING = 13;
JSON = 14;
NAIVE_TIME = 8;
NAIVE_DATE = 9;
NAIVE_DATETIME = 10;
UTC_DATETIME = 11;
BINARY = 12;
XML = 13;
STRING = 14;
JSON = 15;
}

message DataTypeParams {
oneof params {
DecimalParams decimal = 1;
int32 string_byte_length = 2;
}
}

message DecimalParams {
uint32 precision = 1;
uint32 scale = 2;
}

enum OpType {
enum RecordType {
UPSERT = 0;
UPDATE = 1;
DELETE = 2;
Expand All @@ -109,14 +118,15 @@ message ValueType {
int64 long = 5;
float float = 6;
double double = 7;
google.protobuf.Timestamp naive_date = 8;
google.protobuf.Timestamp naive_datetime = 9;
google.protobuf.Timestamp utc_datetime = 10;
string decimal = 11;
bytes binary = 12;
string string = 13;
string json = 14;
string xml = 15;
google.protobuf.Timestamp naive_time = 8;
google.protobuf.Timestamp naive_date = 9;
google.protobuf.Timestamp naive_datetime = 10;
google.protobuf.Timestamp utc_datetime = 11;
string decimal = 12;
bytes binary = 13;
string string = 14;
string json = 15;
string xml = 16;
}
}

Expand All @@ -129,5 +139,5 @@ message Column {
string name = 1;
DataType type = 2;
bool primary_key = 3;
optional DecimalParams decimal = 4;
optional DataTypeParams params = 4;
}
30 changes: 6 additions & 24 deletions connector_sdk.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
syntax = "proto3";
option optimize_for = SPEED;
option java_multiple_files = true;
option go_package = "fivetran.com/fivetran_sdk";
package fivetran_sdk;
option go_package = "fivetran.com/fivetran_sdk_r2";
package fivetran_sdk.r2;

import "common.proto";

// Fivetran (grpc client) <> Connector (grpc server)
service Connector {
// Fivetran (grpc client) <> SourceConnector (grpc server)
service SourceConnector {
rpc ConfigurationForm (ConfigurationFormRequest) returns (ConfigurationFormResponse) {}
rpc Test (TestRequest) returns (TestResponse) {}
rpc Schema (SchemaRequest) returns (SchemaResponse) {}
Expand Down Expand Up @@ -65,25 +65,7 @@ message TableSelection {
}

message UpdateResponse {
oneof response {
LogEntry log_entry = 1;
Operation operation = 2;
}
}

enum LogLevel {
INFO = 0;
WARNING = 1;
SEVERE = 2;
}

message LogEntry {
LogLevel level = 1;
string message = 2;
}

message Operation {
oneof op {
manjutapali marked this conversation as resolved.
Show resolved Hide resolved
oneof Operation {
Record record = 1;
SchemaChange schema_change = 2;
Checkpoint checkpoint = 3;
Expand All @@ -100,7 +82,7 @@ message SchemaChange {
message Record {
optional string schema_name = 1;
string table_name = 2;
OpType type = 3;
RecordType type = 3;
map<string, ValueType> data = 4;
}

Expand Down
31 changes: 25 additions & 6 deletions destination_sdk.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
syntax = "proto3";
option optimize_for = SPEED;
option java_multiple_files = true;
option go_package = "fivetran.com/fivetran_sdk";
package fivetran_sdk;
option go_package = "fivetran.com/fivetran_sdk_r2";
package fivetran_sdk.r2;

import "google/protobuf/timestamp.proto";
import "common.proto";

// Fivetran (grpc client) <> Destination (grpc server)
service Destination {
// Fivetran (grpc client) <> DestinationConnector (grpc server)
service DestinationConnector {
rpc ConfigurationForm (ConfigurationFormRequest) returns (ConfigurationFormResponse) {}
rpc Capabilities (CapabilitiesRequest) returns (CapabilitiesResponse) {}
rpc Test (TestRequest) returns (TestResponse) {}
Expand Down Expand Up @@ -55,7 +55,26 @@ message CreateTableResponse {
message AlterTableRequest {
map<string, string> configuration = 1;
string schema_name = 2;
Table table = 3;
string table_name = 3;
repeated SchemaDiff changes = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do our own DataWriter implementations get something like this? How is it generated?

Copy link
Collaborator Author

@manjutapali manjutapali Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No, our SDKDataWriter generates the schema difference. The same approach is followed across all the DataWriters.
For eg: In SQLLikeDataWriter, we compute the difference in primary key, update in column type, added new columns etc - SQLLikeDataWriter#updateSchema, SQLLikeDataWriter#updateTable

Copy link
Collaborator

@ediril ediril Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@georgewfraser no they don't "get" it from the core, they calculate it themselves. This is something new we introduced so the partner code doesn't have to calculate it. SdkDataWriter generates the diffs to pass to the partner code so they don't have to do it themselves.

It would be great if we can move this to the core so the other data writers didn't have to perform the diff either. (In fact, the core doesn't even do a simple compare between existing and new schema, calling DataWriter#updateSchema whether the two are different or not, expecting every data writer to figure it out)

}

message SchemaDiff {
oneof change {
Column add_column = 1;
ChangeType change_column_type = 2;
PrimaryKey update_primary_keys = 3;
}
}

message PrimaryKey {
repeated string column_name = 1;
}

message ChangeType {
string column_name = 1;
DataType new_type = 2;
optional DataTypeParams params = 3;
}

message AlterTableResponse {
Expand All @@ -75,7 +94,7 @@ message TruncateRequest {
}

message SoftTruncate {
string deleted_column = 3;
string deleted_column = 1;
}

message TruncateResponse {
Expand Down
Loading