-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Add user and password config to MySQL event listener #26260
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
base: master
Are you sure you want to change the base?
Add user and password config to MySQL event listener #26260
Conversation
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
ad6a7cb
to
7eb43bb
Compare
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
...ent-listener/src/main/java/io/trino/plugin/eventlistener/mysql/MysqlEventListenerConfig.java
Outdated
Show resolved
Hide resolved
...ent-listener/src/main/java/io/trino/plugin/eventlistener/mysql/MysqlEventListenerConfig.java
Outdated
Show resolved
Hide resolved
...ent-listener/src/main/java/io/trino/plugin/eventlistener/mysql/MysqlEventListenerConfig.java
Show resolved
Hide resolved
...ent-listener/src/main/java/io/trino/plugin/eventlistener/mysql/MysqlEventListenerConfig.java
Show resolved
Hide resolved
7eb43bb
to
1d09790
Compare
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
1d09790
to
e98d7cf
Compare
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
e98d7cf
to
8968d7a
Compare
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
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.
Remove Feat(mysql-event-listener):
from the commit title, and wrap the commit body at 72 characters. See https://trino.io/development/process.html#pull-request-and-commit-guidelines- for more details.
8968d7a
to
41124be
Compare
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
41124be
to
a3a6c3f
Compare
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
Allow configuring MySQL credentials for the event listener using separate mysql-event-listener.db.user and mysql-event-listener.db.password properties. This makes it easier and safer to provide credentials, especially when passwords contain special characters, and aligns with the configuration style of other Trino connectors. The connection logic now uses these properties if provided, falling back to credentials in the JDBC URL if not set.
a3a6c3f
to
8dff8ac
Compare
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
if (getUser().isPresent() && url != null && url.matches(".*[?&]user=.+")) { | ||
throw new RuntimeException("'user' property is set, but JDBC URL also contains 'user' in the query string. Please specify the user only once."); | ||
} | ||
// Check for password property | ||
if (getPassword().isPresent() && url != null && url.matches(".*[?&]password=.+")) { | ||
throw new RuntimeException("'password' property is set, but JDBC URL also contains 'password' in the query string. Please specify the password only once."); | ||
} |
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.
I don't think we want to parse the URL by ourselves. Please consider using helper methods in MySQL JDBC driver, e.g. ConnectionUrlParser.parseConnectionString
method
@cla-bot check |
The cla-bot has been summoned, and re-checked this pull request! |
Description
This PR introduces support for configuring MySQL credentials in the event listener using the new properties
mysql-event-listener.db.user
andmysql-event-listener.db.password
.These properties allow users to specify the database username and password separately from the JDBC URL, making configuration more secure and user-friendly, especially when passwords contain special characters. The connection logic now uses these properties if provided, falling back to credentials in the JDBC URL if not set.
Documentation has been updated to reflect the new configuration options.
Additional context and related issues
This change aligns the MySQL event listener configuration with the approach used by other Trino connectors, improving consistency and usability. It addresses issues encountered when embedding credentials with special characters in the JDBC URL and enhances security by allowing sensitive information to be managed separately. No related issues were referenced, but this update is based on common best practices and user feedback.
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(X) Release notes are required, with the following suggested text:
Closes: #25941