mysql icon indicating copy to clipboard operation
mysql copied to clipboard

Add BeforeConnect callback to configuration object

Open ItalyPaleAle opened this issue 1 year ago • 7 comments

Description

This can be used to alter the connection options for each connection, right before it's established.

Our use case is that we want to connect to a MySQL server which supports authentication using a JWT (in the specific case, Azure Database for MySQL using tokens from Azure AD). The token is to be used in place of a password. Because the tokens have a limited lifetime, we cannot create a Config object with a hardcoded password, because connections in the pool can be created later in time and the token may be expired. By using the BeforeConnect callback, instead, we can ensure that the Config object used to open the connection has a fresh password every time.

This design is modeled after the "BeforeConnect" property offered by pgx.

Although our use case is for updating the password, there may be other uses for a BeforeConnect callback. The callback can also return an error to abort the connection.

We plan to use this for Dapr: dapr/components-contrib#2955

You can see how we implemented this for Postgres (using pgx) here: https://github.com/dapr/components-contrib/blob/566c7fd31adbd81740ba66995729a7eadeb3444a/internal/authentication/postgresql/metadata.go#L97-L109

Checklist

  • [X] Code compiles correctly
  • [X] Created tests which fail without the change (if possible)
  • [X] All tests passing
  • [X] Extended the README / documentation, if necessary
  • [X] Added myself / the copyright holder to the AUTHORS file

Summary by CodeRabbit

  • New Features

    • Introduced a pre-connection configuration step, allowing custom logic before database connections are established.
  • Bug Fixes

    • Ensured the correct database is targeted during connections by implementing additional checks.
  • Tests

    • Added new tests to verify the database connection process and pre-connection configurations.

ItalyPaleAle avatar Aug 02 '23 23:08 ItalyPaleAle

@methane Would it be possible to have this PR considered please? Or please, let me know if there's some other process I should follow.

ItalyPaleAle avatar Sep 12 '23 17:09 ItalyPaleAle

wait. I'm very busy and sick.

methane avatar Sep 12 '23 17:09 methane

Sorry to hear that, hope you get better soon 🙁 No worries, we're fine with waiting

ItalyPaleAle avatar Sep 12 '23 18:09 ItalyPaleAle

Walkthrough

The changes involve enhancing a database connection library to support pre-connection configuration adjustments. A new BeforeConnect callback function has been introduced, allowing custom logic to be executed before a connection is established. This functionality is tested with a new test case ensuring the connection is correctly modified before use. The changes affect the connection process, configuration structure, and testing suite to ensure the new feature works as intended.

Changes

File(s) Change Summary
connector.go, dsn.go Introduced BeforeConnect callback in Config and updated Connect method to handle pre-connection configuration changes. Added context package import.
driver_test.go Added TestBeforeConnect function to test the new BeforeConnect feature and ensure proper database connection handling.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

coderabbitai[bot] avatar Dec 12 '23 15:12 coderabbitai[bot]

Coverage Status

coverage: 82.055% (+0.1%) from 81.919% when pulling 6a4e24ee11ef6a81c552d1f4bbbe4a8e374bfb74 on ItalyPaleAle:add-beforeconnect into 6964272ffd13a41ad66383cd2ea738fded75ad06 on go-sql-driver:master.

coveralls avatar Dec 13 '23 04:12 coveralls

@shogo82148 How do you think this new API?

methane avatar Feb 05 '24 09:02 methane

Although I like this feature, I want to wait adding more fields to Config object. Please wait for the discussion in https://github.com/go-sql-driver/mysql/discussions/1548

methane avatar Feb 06 '24 04:02 methane

This seems to be an unnecessary solution to a true problem.

Instead the user should have created its own database/sql.Driver and database/sql.Connector that would build a *mysql.Config on the call to Connector's Connect and call mysql.NewConnector() and wrap its Connect.

See https://github.com/dolmen-go/mylogin-driver/blob/master/driver.go as an example of a database/sql.Driver that wraps github.com/go-sql-driver/mysql that implements a different DSN.

dolmen avatar May 14 '24 07:05 dolmen