-
Notifications
You must be signed in to change notification settings - Fork 24
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
Remove required table field for share point connector #354
Remove required table field for share point connector #354
Conversation
da96a3b
to
d4ae7f7
Compare
fivetran/common/fields.json
Outdated
@@ -659,7 +659,8 @@ | |||
"key_field": "", | |||
"item_type": {}, | |||
"description": { | |||
"adobe_analytics": "The table name unique within the schema to which connector will sync the data. Required for connector creation." | |||
"adobe_analytics": "The table name unique within the schema to which connector will sync the data. Required for connector creation.", | |||
"share_point": "The table name within the schema to which the connector will sync the data." |
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.
how come we have it here? it's totally unrelated field for share_point.
fivetran/common/fields.json
Outdated
@@ -10508,7 +10509,6 @@ | |||
"kinesis": "Destination table. Table is permanent and cannot be changed after connection creation", | |||
"s3": "Destination table. Table is permanent and cannot be changed after connection creation", | |||
"sftp": "Destination table. Table is permanent and cannot be changed after connection creation", | |||
"share_point": "Destination table. Table is permanent and cannot be changed after connection creation", |
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.
It should be still here, but we should handle it as optional field for share_point.
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.
LGTM, this is the correct solution so should work. Please check my minor suggestion below.
@@ -328,10 +328,9 @@ func getDestinatonSchemaForConfig(serviceId, nameAttr, tableAttr, prefixAttr att | |||
"schema": nameAttr.(types.String).ValueString(), | |||
} | |||
if common.GetDestinationSchemaFields()[service]["table"] { | |||
if tableAttr.IsNull() { | |||
return nil, fmt.Errorf("`destination_schema.table` field is required to create `%v` connector", service) | |||
if !tableAttr.IsNull() { |
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.
We also should check if it is not empty string, just in case.
No description provided.