🎉 Destination MySQL: Add jdbc_url_params support for optional JDBC parameters #10362

girarda · 2022-02-15T23:12:38Z

What

Add support for MySQL Destinations to take in a JDBC URL Params input to specify any additional JDBC parameters for the connection

How

Add in jdbc_url_params for MySQLDestination in the config JsonNode. Optionally append to the JDBC string if it exists and is not empty.

🚨 User Impact 🚨

Pre-merge Checklist

Expand the relevant checklist and delete the others.

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]~(https://docs.google.com/document/d/1ypdgmwmEHWv-TrO4_YOQ7pAJGVrMp5BOkEVh831N260/edit?usp=sharing)
- 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

CLAassistant · 2022-02-15T23:12:44Z

All committers have signed the CLA.

...tination-mysql/src/main/java/io/airbyte/integrations/destination/mysql/MySQLDestination.java

girarda · 2022-02-16T01:30:51Z

...tination-mysql/src/main/java/io/airbyte/integrations/destination/mysql/MySQLDestination.java

    if (!additionalParameters.isEmpty()) {
-      jdbcUrl.append("&");
+      jdbcUrl.append("?");


append "?" instead of "&" since this zeroDateTimeBehavior=convertToNull is already included in additionalParameters

...tination-mysql/src/main/java/io/airbyte/integrations/destination/mysql/MySQLDestination.java

tuliren · 2022-02-16T23:40:58Z

@tuliren should this be throwing a more concrete exception type?

Oh, I missed this question during the review. Yes, I think the IllegalArgumentException is better for this case.

tuliren

Thanks for the updates. I think the only thing left is to replace RuntimeException with a more specific one, like IllegalArgumentException. Next you can publish a new version of this connector, update the version in the seed, and update the spec. You can follow the pre-merge checklist for details.

girarda · 2022-02-17T00:33:11Z

/test connector=connectors/destination-mysql

🕑 connectors/destination-mysql https://github.com/airbytehq/airbyte/actions/runs/1856011375
✅ connectors/destination-mysql https://github.com/airbytehq/airbyte/actions/runs/1856011375
Python tests coverage:

Name                                                                                                                            Stmts   Miss  Cover
---------------------------------------------------------------------------------------------------------------------------------------------------
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/__init__.py                      2      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/__init__.py               1      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/airbyte_protocol.py     124      0   100%
normalization/__init__.py                                                                                                           4      0   100%
normalization/destination_type.py                                                                                                  13      0   100%
normalization/transform_catalog/__init__.py                                                                                         2      0   100%
normalization/transform_catalog/catalog_processor.py                                                                              143     77    46%
normalization/transform_catalog/destination_name_transformer.py                                                                   155      8    95%
normalization/transform_catalog/reserved_keywords.py                                                                               13      0   100%
normalization/transform_catalog/stream_processor.py                                                                               520    333    36%
normalization/transform_catalog/table_name_registry.py                                                                            174     34    80%
normalization/transform_catalog/transform.py                                                                                       45     26    42%
normalization/transform_catalog/utils.py                                                                                           33      7    79%
normalization/transform_config/__init__.py                                                                                          2      0   100%
normalization/transform_config/transform.py                                                                                       148     34    77%
---------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                                                            1379    519    62%

girarda · 2022-02-17T01:20:54Z

/publish connector=connectors/destination-mysql

🕑 connectors/destination-mysql https://github.com/airbytehq/airbyte/actions/runs/1856175865
✅ connectors/destination-mysql https://github.com/airbytehq/airbyte/actions/runs/1856175865

girarda added 8 commits February 15, 2022 12:49

pass through jdbc params

4d751f4

fail if contains verifyServerCertificate

9b6c5bd

do the same for all ssl params

c924611

delete dead file

c959dd8

slight refactor

4b7d8fe

new method

e401db1

remove default value

76d8595

error message

4e592ed

github-actions bot added the area/connectors Connector related issues label Feb 15, 2022

girarda changed the title ~~Alex/mysql jdbc params~~ Destination MySQL: Add jdbc_url_params support for optional JDBC parameters Feb 15, 2022

girarda commented Feb 15, 2022

View reviewed changes

...tination-mysql/src/main/java/io/airbyte/integrations/destination/mysql/MySQLDestination.java Outdated Show resolved Hide resolved

girarda temporarily deployed to more-secrets February 15, 2022 23:14 Inactive

rename

c90021b

girarda temporarily deployed to more-secrets February 15, 2022 23:17 Inactive

Merge branch 'master' into alex/mysql-jdbc-params

59415f8

girarda temporarily deployed to more-secrets February 15, 2022 23:25 Inactive

tuliren reviewed Feb 15, 2022

View reviewed changes

girarda marked this pull request as draft February 16, 2022 00:13

update as per comments

51b13aa

girarda temporarily deployed to more-secrets February 16, 2022 01:27 Inactive

Update exception message

95e1a83

girarda temporarily deployed to more-secrets February 16, 2022 01:29 Inactive

girarda commented Feb 16, 2022

View reviewed changes

...tination-mysql/src/main/java/io/airbyte/integrations/destination/mysql/MySQLDestination.java Show resolved Hide resolved

Remove trailing &

e9e2d7a

girarda temporarily deployed to more-secrets February 16, 2022 23:40 Inactive

tuliren approved these changes Feb 16, 2022

View reviewed changes

Throw IllegalArgumentException

80352b6

girarda temporarily deployed to more-secrets February 17, 2022 00:19 Inactive

extract to constants

d3a1263

girarda temporarily deployed to more-secrets February 17, 2022 00:29 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 17, 2022 00:35 Inactive

girarda changed the title ~~Destination MySQL: Add jdbc_url_params support for optional JDBC parameters~~ 🎉 Destination MySQL: Add jdbc_url_params support for optional JDBC parameters Feb 17, 2022

girarda linked an issue Feb 17, 2022 that may be closed by this pull request

Destination MySQL: custom JDBC parameters #9434

Closed

octavia-squidington-iii temporarily deployed to more-secrets February 17, 2022 01:23 Inactive

girarda added 2 commits February 16, 2022 18:00

Bump version in seed

449e2cf

Update destination specs

5800b6d

girarda temporarily deployed to more-secrets February 17, 2022 02:03 Inactive

girarda merged commit 3db3e88 into master Feb 17, 2022

girarda deleted the alex/mysql-jdbc-params branch February 17, 2022 18:58

octavia-squidington-iii mentioned this pull request Feb 17, 2022

Bump Airbyte version from 0.35.30-alpha to 0.35.31-alpha #10446

Merged

girarda restored the alex/mysql-jdbc-params branch February 18, 2022 23:29

girarda temporarily deployed to more-secrets February 18, 2022 23:31 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 18, 2022 23:32 Inactive

octavia-squidington-iii temporarily deployed to more-secrets February 19, 2022 00:17 Inactive

girarda deleted the alex/mysql-jdbc-params branch February 19, 2022 00:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Destination MySQL: Add jdbc_url_params support for optional JDBC parameters #10362

🎉 Destination MySQL: Add jdbc_url_params support for optional JDBC parameters #10362

girarda commented Feb 15, 2022 •

edited

Loading

CLAassistant commented Feb 15, 2022 •

edited

Loading

girarda Feb 16, 2022

tuliren commented Feb 16, 2022

tuliren left a comment

girarda commented Feb 17, 2022 •

edited by github-actions bot

Loading

girarda commented Feb 17, 2022 •

edited by github-actions bot

Loading

🎉 Destination MySQL: Add jdbc_url_params support for optional JDBC parameters #10362

🎉 Destination MySQL: Add jdbc_url_params support for optional JDBC parameters #10362

Conversation

girarda commented Feb 15, 2022 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

CLAassistant commented Feb 15, 2022 • edited Loading

girarda Feb 16, 2022

Choose a reason for hiding this comment

tuliren commented Feb 16, 2022

tuliren left a comment

Choose a reason for hiding this comment

girarda commented Feb 17, 2022 • edited by github-actions bot Loading

girarda commented Feb 17, 2022 • edited by github-actions bot Loading

girarda commented Feb 15, 2022 •

edited

Loading

CLAassistant commented Feb 15, 2022 •

edited

Loading

girarda commented Feb 17, 2022 •

edited by github-actions bot

Loading

girarda commented Feb 17, 2022 •

edited by github-actions bot

Loading