tweak snowflake destination to hide INSERT load method #9982
Conversation
subodh1810
changed the title
|
@subodh1810 is it true what's written in the SonarQube that we have zero test coverage on those classes? If so we should create a ticket and send to GL |
| "additionalProperties": false, | ||
| "description": "Uses <pre>INSERT</pre> statements to send batches of records to Snowflake. Easiest (no setup) but not recommended for large production workloads due to slow speed.", | ||
| "description": "Please select another option", |
There was a problem hiding this comment.
removing "another" to make it a bit more natural
| "description": "Please select another option", | |
| "description": "Select an option", |
There was a problem hiding this comment.
| "description": "Please select another option", | |
| "description": "This mode has been deprecated. It is now equivalent to the internal staging mode, which is more performant.", |
| @@ -94,9 +94,9 @@ | |||
| } | |||
| }, | |||
| { | |||
| "title": "Standard Inserts", | |||
There was a problem hiding this comment.
could you change the order of the options so this is the first in the list? this way it would look like this:
screenshot obtained using instructions here
| @@ -38,11 +37,6 @@ public static DestinationType getTypeFromConfig(final JsonNode config) { | |||
| } | |||
| } | |||
|
|
|||
| public static boolean isInternalStaging(final JsonNode config) { | |||
|
@tuliren if we delete the option then we will have to write a migration to fix the existing configs cause validation would start failing. I am not against that but this seems simpler. If you think we should write a migration am happy to do it |
|
Alright, deleting it will cause some UI issue. Alternatively, we can keep it, but make it equivalent to the staging method. Since the staging method requires no extra config, I think we actually don't need to migrate any existing configs. It works as follows:
In this way, all existing connections with standard loading method will just use internal staging. |
There was a problem hiding this comment.
Sorry, actually this PR already changes the backend to default standard insert to internal staging. Then my proposal is basically the same as what you changed in the PR. My only suggest is to point out in the description that the standard insert method is deprecated and default to internal staging, which is probably less confusing than "select another option".
| @@ -94,9 +94,9 @@ | |||
| } | |||
| }, | |||
| { | |||
| "title": "Standard Inserts", | |||
| "title": "Please select another option", | |||
There was a problem hiding this comment.
| "title": "Please select another option", | |
| "title": "Select another option", |
|
Tested the spec
|
|
This change was not versioned and published. I have published a new version with this change in #10297. |
Issue : #9602
In the PR https://github.com/airbytehq/airbyte/pull/8528/files we made INTERNAL LOAD method as default. This PR just hides INSERT load method from spec so that people dont select it any more