🎉 Source and Destination Snowflake: Add jdbc_url_params support for optional JDBC parameters

[noahkawasakigoogle]

What

Add support for Snowflakes Source and Destination's to take in a JDBC URL Params input to specify any additional JDBC parameters for the connection.

Solves Snowflake for #9501

Eventually I would like to have this for all connectors that support this: #9598

How

Add in jdbc_url_params for Snowflake in the config Jsonode. Optionally append to the JDBC string if exists.

Also, I'm unsure on how I am supposed to run the integration tests myself without the secrets, is it expected that I should create my own personal Snowflake account for this? Or is there an assumption that I am the original Snowflake connector developer and already have these credentials?

Recommended reading order

Very simple, any file

🚨 User Impact 🚨

Pre-merge Checklist

Updating a connector

Community member or Airbyter

• Grant edit access to maintainers (instructions)
• Secrets in the connector's spec are annotated with airbyte_secret
• Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
• Code reviews completed
• Documentation updated
  • Connector's README.md
  • Connector's bootstrap.md. See description and examples
  • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
• PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

• Create a non-forked branch based on this PR and test the below items on it
• Build is successful
• Credentials added to Github CI. Instructions.
• /test connector=connectors/<name> command is passing.
• New Connector version released on Dockerhub by running the /publish command described here
• After the new connector version is published, connector version bumped in the seed directory as described here
• Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

[noahkawasakigoogle]

/test connector=source-snowflake
/test connector=destination-snowflake

[alafanechere]

Hi @noahkawasakigoogle, thank you very much for this contrib!
I made minor suggestions, let me know what you think.
Could you also please bump the version of the connectors you edited:

• In the Dockerfiles
• In airbyte-config/init/src/main/resources/seed/source_definitions.yaml
• In airbyte-config/init/src/main/resources/seed/destination_definitions.yaml
• In airbyte-config/init/src/main/resources/config/STANDARD_SOURCE_DEFINITION/e2d65910-8c8b-40a1-ae7d-ee2416b2bfa2.json
• In airbyte-config/init/src/main/resources/config/STANDARD_DESTINATION_DEFINITION/424892c4-daac-4491-b35d-c6688ba547ba.json

I'll run the integration test in our CI and send you the results.

[noahkawasakigoogle]

Thanks for reviewing! For some reason I thought the rest of the docker tag bumps happened in some automatic workflow. Just made them. Also removed the LICENSE file in the docs directory. Will open a separate PR for that. I had some responses to the JDBC nits, lemme know what you think.

#9701

[alafanechere]

I confirm acceptance tests pass for the source connector. I'll keep you updated about the destination as the run is longer.

[alafanechere]

Acceptance tests passed for both connectors. @misteryeo / @sherifnada could you please give a final review as the spec.json was changed?

[noahkawasakigoogle]

Can we make sure to approve and merge this before this PR if approved? As I am only supposed to contribute to non Elastic licensed files

[pjleiwa6g]

To make it possible to enter a value like query_tag={'TOOL':'AIRBYTE','CUSTID':'1001'} can url encoding be added too? Example is in the attached file.

SnowflakeDatabase.java.txt

import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.io.UnsupportedEncodingException;

....
  if (config.has("jdbc_url_params")) {
        jdbcUrl.append(encodeValue(config.get("jdbc_url_params").asText()));
.... 

  private static String encodeValue(String value) {
        try {
            return URLEncoder.encode(value, StandardCharsets.UTF_8.toString()).replace("%3D","=").replace("%26","&");
        } catch (UnsupportedEncodingException ex) {
            throw new RuntimeException(ex.getCause());
        }
  }

[sherifnada]

Hey @noahkawasakigoogle ! Thanks for creating the PR. It's actually my first time seeing the epic linked to this PR, so I asked a bunch of questions on the main issue here: #9501 apologies for being a little behind you on this. Feel free to chime in on those questions as well

@misteryeo do you have thoughts on the UX questions in that issue?

[noahkawasakigoogle]

@sherifnada No prob! I replied with some thoughts.

@pjleiwa6g Hm, yeah this could happen. Do you think it would be possible to just put the URL encoded characters directly in the connection though? Snowflake documents this example:

String connectionURL = "jdbc:snowflake://myorganization-myaccount.snowflakecomputing.com/?query_tag='folder%3Dfolder1%20folder2%26'

My concern is just that Airbyte doing string manipulation behind the scenes makes it more vague whats going on to end users, and that there could be more and more characters to add search and replace logic for. This approach is more transparent: "What you put in will get sent to Snowflake". I'm open to either choice though. Also to answer your question on the other ticket, QUERY_TAG works here, tested that one specifically. Any of the Snowflake parameters can also be set this way.

[pjleiwa6g]

@noahkawasakigoogle I'm think you are right that it makes it more vague for end users. I tested it by entering an encoded string and it works fine. You can forget my comment.

[misteryeo]

Just added one comment but otherwise LGTM!

[misteryeo]

Yes please!

…

--- Andy Yeo Senior Product Manager Github <https://github.com/airbytehq/airbyte> | Twitter <https://twitter.com/AirbyteHQ> | LinkedIn <https://www.linkedin.com/company/airbytehq/> We're hiring, come work with me! <https://airbyte.io/careers> [image: 🚀]

On Wed, Jan 26, 2022 at 3:49 PM, Noah Kawasaki ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In airbyte-integrations/connectors/destination-snowflake/src/main/ resources/spec.json <#9623 (comment)>: > @@ -68,11 +68,17 @@ "title": "Password", "order": 6 }, + "jdbc_url_params": { + "description": "Additional properties to pass to the jdbc url string when connecting to the database formatted as 'key=value' pairs separated by the symbol '&'. (example: key1=value1&key2=value2&key3=value3).", Hey @misteryeo <https://github.com/misteryeo> @alafanechere <https://github.com/alafanechere> said the same thing here and I replied, would you prefer me to just update the capitalization everywhere? — Reply to this email directly, view it on GitHub <#9623 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEASBTPWXEDRTBW2X2JCW2LUYCB65ANCNFSM5MLMFB6Q> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[noahkawasakigoogle]

I see this in CI:

airbyte/destination-snowflake: 0.4.5
	URL: https://hub.docker.com/v2/repositories/airbyte/destination-snowflake/tags/0.4.5
curl: (22) The requested URL returned error: 404 Not Found
	ERROR: not found!

I'm assuming this is expected from my fork since this hasnt merged?

[alafanechere]

I see this in CI:

airbyte/destination-snowflake: 0.4.5
	URL: https://hub.docker.com/v2/repositories/airbyte/destination-snowflake/tags/0.4.5
curl: (22) The requested URL returned error: 404 Not Found
	ERROR: not found!

I'm assuming this is expected from my fork since this hasnt merged?

Yes exactly, I was running final tests after your latest commit on a side branch. They are passing, so I'm going to publish the connectors now.

[alafanechere]

Thank you @noahkawasakigoogle for this contribution! I re-bumped the version myself because of some conflicts with the destination connectors.