Allow sessionvariables jdbc url param in source-mysql #25859

rodireich · 2023-05-05T23:31:00Z

What

source mysql can accept a jdbc url param looking like sessionVariables=MAX_EXECUTION_TIME=123456666

How

Allow adding a sessionVariables param in jdbc url parsing.
In order to reduce blast radius this only applies to mysql and mysql strict encrypt.
This is an adapted code change of #25386

github-actions · 2023-05-05T23:31:19Z

rodireich · 2023-05-05T23:33:04Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/4898396950
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/4898396950
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
================= 22 passed, 6 skipped, 30 warnings in 17.78s ==================

rodireich · 2023-05-05T23:33:40Z

/test connector=connectors/source-mysql-strict-encrypt

🕑 connectors/source-mysql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/4898400169
✅ connectors/source-mysql-strict-encrypt https://github.com/airbytehq/airbyte/actions/runs/4898400169
No Python unittests run

Build Passed

Test summary info:

All Passed

github-actions · 2023-05-05T23:35:20Z

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

Run integration tests
Bump connector or module version
Add changelog
Publish the new version

❌ Sources (22)

Connector	Version	Changelog	Publish
`source-alloydb`	`2.0.28`	✅	✅
`source-alloydb-strict-encrypt`	`2.0.28`	🔵 (ignored)	🔵 (ignored)
`source-bigquery`	`0.2.3`	✅	✅
`source-clickhouse`	`0.1.17`	✅	✅
`source-clickhouse-strict-encrypt`	`0.1.17`	🔵 (ignored)	🔵 (ignored)
`source-cockroachdb`	`0.1.22`	✅	✅
`source-cockroachdb-strict-encrypt`	`0.1.22`	🔵 (ignored)	🔵 (ignored)
`source-db2`	`0.1.19`	✅	✅
`source-db2-strict-encrypt`	`0.1.19`	🔵 (ignored)	🔵 (ignored)
`source-mssql`	`1.0.16`	✅	✅
`source-mssql-strict-encrypt`	`1.0.16`	🔵 (ignored)	🔵 (ignored)
`source-mysql`	`2.0.22`	✅	❌ (diff seed version)
`source-mysql-strict-encrypt`	`2.0.22`	🔵 (ignored)	🔵 (ignored)
`source-oracle`	`0.3.24`	✅	✅
`source-oracle-strict-encrypt`	`0.3.24`	🔵 (ignored)	🔵 (ignored)
`source-postgres`	`2.0.29`	✅	✅
`source-postgres-strict-encrypt`	`2.0.29`	🔵 (ignored)	🔵 (ignored)
`source-redshift`	`0.3.16`	✅	✅
`source-scaffold-java-jdbc`	`0.1.0`	🔵 (ignored)	🔵 (ignored)
`source-snowflake`	`0.1.34`	✅	✅
`source-teradata`	`0.1.0`	✅	✅
`source-tidb`	`0.2.4`	✅	✅

See "Actionable Items" below for how to resolve warnings and errors.

✅ Destinations (0)

Connector	Version	Changelog	Publish

See "Actionable Items" below for how to resolve warnings and errors.

✅ Other Modules (0)

Actionable Items

(click to expand)

Category	Status	Actionable Item
Version	❌ mismatch	The version of the connector is different from its normal variant. Please bump the version of the connector.
	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
Changelog	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
	❌ changelog missing	There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish	⚠ not in seed	The connector is not in the cloud or oss registry, so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that you have added a metadata.yaml file and the expected registries are enabled.

marcosmarxm

Thanks @rodireich

.../connectors/source-mysql/src/main/java/io/airbyte/integrations/source/mysql/MySqlSource.java

...ectors/source-mysql/src/test/java/io/airbyte/integrations/source/mysql/MySqlSourceTests.java

akashkulk

Quick question : I understand this is mainly to reduce the blast radius of the change. But, is this something that is ONLY specific to mysql and not desired in other connectors?

I guess my thinking is that this seems like generally useful code that will enable all JDBC users to have params in the form of sessionVariables=MAX_EXECUTION_TIME=123456666

What we actually publish can be something else altogether.

rodireich · 2023-05-08T21:11:47Z

I didn't see anything equivalent in postgres jdbc url parameters.
This isn't even a legal url as far as I can tell as this = was supposed to be url encoded %3D.
mysql however doesn't accept an encoded equals to sign.
That's why I was struggling to only put this in mysql. Not a really good change but necessary for somebody who needs to set a session variable.

akashkulk · 2023-05-08T21:57:06Z

I didn't see anything equivalent in postgres jdbc url parameters. This isn't even a legal url as far as I can tell as this = was supposed to be url encoded %3D. mysql however doesn't accept an encoded equals to sign. That's why I was struggling to only put this in mysql. Not a really good change but necessary for somebody who needs to set a session variable.

Out of curiosity, how many people are facing this issue? Longer term, I think we'd not want to expose JDBC param setting. Another option here is to just expose the MAX_EXECUTION_TIME as a setting in the UX

I think given that this is currently scoped to Mysql, this should be fine - but I really think we should file a ticket to revisit how we're setting JDBC args in the future. For example :

How many users are setting them?
Are there only a couple of distinct parameters that only set? If so, perhaps only those can be exposed in the UX?

I think this will make some of the existing code confusing. it also seems like the user found a workaround by setting a top-level JDBC param instead of a sessionVariable

I'd prefer to shelf this issue for now before we have a good understanding of how often this is used. WDYT?

rodireich · 2023-05-08T22:07:34Z

This came form one user in #25386.
This user did find a workaround and I'm not very happy with this code change either but if somebody is stuck and need to set a session variable they may have no other recourse.

Any of the following variables are valid so there may be other cases - current or future - that can benefit from us supporting it:
https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html

...ectors/source-jdbc/src/main/java/io/airbyte/integrations/source/jdbc/AbstractJdbcSource.java

akashkulk

Small nit, looks good otherwise

rodireich · 2023-05-22T21:07:12Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5050421334
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5050421334
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
================= 22 passed, 6 skipped, 30 warnings in 18.39s ==================

rodireich · 2023-05-22T21:21:28Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5050538066
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/5050538066
No Python unittests run

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: not found in the config.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
================= 22 passed, 6 skipped, 30 warnings in 17.68s ==================

…blish-25386

rodireich · 2023-05-23T01:51:14Z

/publish connector=connectors/source-mysql

⚠️ The publish slash command is now deprecated.

The connector publication happens on merge to the master branch.

Please use /legacy-publish if you need to publish normalization images.

Please join the #publish-on-merge-updates slack channel to track ongoing publish pipelines.

Please reach out to the @dev-connector-ops team if you need support in publishing a connector.

* initial commit * cleanup * cleanup * Throw a configuration exception in case of bad jdbc url param * End-to-End test session variable jdbc param * reafctoring sanity * sanity * bump version and update note * cherry pick fix to unrelated build error * fix failing test --------- Co-authored-by: Ryan Fu <[email protected]>

rodireich added 3 commits May 4, 2023 11:28

initial commit

1efc6bb

cleanup

6281b5f

cleanup

a1c26ca

octavia-squidington-iii added the area/connectors Connector related issues label May 5, 2023

octavia-squidington-iii added connectors/source/mysql connectors/source/jdbc labels May 5, 2023

rodireich requested a review from akashkulk May 5, 2023 23:33

rodireich changed the title ~~allow sessionvariables jdbc url param in source-mysql~~ Allow sessionvariables jdbc url param in source-mysql May 5, 2023

rodireich requested a review from marcosmarxm May 5, 2023 23:37

rodireich marked this pull request as ready for review May 6, 2023 00:43

rodireich requested a review from a team as a code owner May 6, 2023 00:43

marcosmarxm approved these changes May 8, 2023

View reviewed changes

akashkulk reviewed May 8, 2023

View reviewed changes

.../connectors/source-mysql/src/main/java/io/airbyte/integrations/source/mysql/MySqlSource.java Outdated Show resolved Hide resolved

akashkulk reviewed May 8, 2023

View reviewed changes

...ectors/source-mysql/src/test/java/io/airbyte/integrations/source/mysql/MySqlSourceTests.java Show resolved Hide resolved

akashkulk reviewed May 8, 2023

View reviewed changes

akashkulk reviewed May 10, 2023

View reviewed changes

...ectors/source-jdbc/src/main/java/io/airbyte/integrations/source/jdbc/AbstractJdbcSource.java Outdated Show resolved Hide resolved

akashkulk reviewed May 10, 2023

View reviewed changes

rodireich added 2 commits May 10, 2023 13:56

Throw a configuration exception in case of bad jdbc url param

02ae5af

End-to-End test session variable jdbc param

d919a8d

rodireich added 2 commits May 22, 2023 14:19

reafctoring sanity

fcd5439

sanity

989d3e6

rodireich requested a review from akashkulk May 22, 2023 21:23

akashkulk approved these changes May 22, 2023

View reviewed changes

rodireich enabled auto-merge (squash) May 22, 2023 22:19

Merge branch 'master' into rodi/publish-25386

13eebe2

rodireich disabled auto-merge May 22, 2023 22:20

rodireich and others added 2 commits May 22, 2023 15:25

bump version and update note

7b50c42

cherry pick fix to unrelated build error

23eec8b

rodireich requested a review from a team as a code owner May 23, 2023 00:16

octavia-squidington-iii added area/documentation Improvements or additions to documentation connectors/source/mysql-strict-encrypt labels May 23, 2023

Merge branch 'master' into rodi/publish-25386

fbb22af

marcosmarxm mentioned this pull request May 23, 2023

Fix db-lib JdbcUtils.java to accept JDBC parameters with = sign. #25386

Closed

rodireich added 3 commits May 22, 2023 18:21

Merge branch 'master' into rodi/publish-25386

435d27e

fix failing test

4806d1c

Merge remote-tracking branch 'origin/rodi/publish-25386' into rodi/pu…

7e662df

…blish-25386

rodireich merged commit 2aa8a0d into master May 23, 2023

rodireich deleted the rodi/publish-25386 branch May 23, 2023 01:53

rodireich mentioned this pull request Aug 8, 2023

Allow configuring "options=" in source-postgres jdbc url #29238

Closed

1 task

This was referenced Aug 16, 2023

✨ Allow configuring "options=" in source-postgres jdbc url #29497

Closed

✨ Allow configuring "options=" in source-postgres jdbc url #29516

Closed

✨ Allow configuring "options=" in source-postgres jdbc url #29534

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow sessionvariables jdbc url param in source-mysql #25859

Allow sessionvariables jdbc url param in source-mysql #25859

rodireich commented May 5, 2023

github-actions bot commented May 5, 2023

rodireich commented May 5, 2023 •

edited by github-actions bot

Loading

rodireich commented May 5, 2023 •

edited by github-actions bot

Loading

github-actions bot commented May 5, 2023 •

edited

Loading

❌ Sources (22)

✅ Destinations (0)

✅ Other Modules (0)

Actionable Items

marcosmarxm left a comment

akashkulk left a comment

rodireich commented May 8, 2023

akashkulk commented May 8, 2023

rodireich commented May 8, 2023

akashkulk left a comment

rodireich commented May 22, 2023 •

edited by github-actions bot

Loading

rodireich commented May 22, 2023 •

edited by github-actions bot

Loading

rodireich commented May 23, 2023 •

edited by github-actions bot

Loading

Allow sessionvariables jdbc url param in source-mysql #25859

Allow sessionvariables jdbc url param in source-mysql #25859

Conversation

rodireich commented May 5, 2023

What

How

github-actions bot commented May 5, 2023

Before Merging a Connector Pull Request

rodireich commented May 5, 2023 • edited by github-actions bot Loading

Build Passed

rodireich commented May 5, 2023 • edited by github-actions bot Loading

Build Passed

github-actions bot commented May 5, 2023 • edited Loading

Affected Connector Report

❌ Sources (22)

✅ Destinations (0)

✅ Other Modules (0)

Actionable Items

marcosmarxm left a comment

Choose a reason for hiding this comment

akashkulk left a comment

Choose a reason for hiding this comment

rodireich commented May 8, 2023

akashkulk commented May 8, 2023

rodireich commented May 8, 2023

akashkulk left a comment

Choose a reason for hiding this comment

rodireich commented May 22, 2023 • edited by github-actions bot Loading

Build Passed

rodireich commented May 22, 2023 • edited by github-actions bot Loading

Build Passed

rodireich commented May 23, 2023 • edited by github-actions bot Loading

rodireich commented May 5, 2023 •

edited by github-actions bot

Loading

rodireich commented May 5, 2023 •

edited by github-actions bot

Loading

github-actions bot commented May 5, 2023 •

edited

Loading

rodireich commented May 22, 2023 •

edited by github-actions bot

Loading

rodireich commented May 22, 2023 •

edited by github-actions bot

Loading

rodireich commented May 23, 2023 •

edited by github-actions bot

Loading