Skip to content

Added MySQL Database Support (updated pyproject dependencies)#7

Merged
s-bose merged 10 commits intos-bose:masterfrom
CodeStrate:master
Nov 1, 2025
Merged

Added MySQL Database Support (updated pyproject dependencies)#7
s-bose merged 10 commits intos-bose:masterfrom
CodeStrate:master

Conversation

@CodeStrate
Copy link
Contributor

  • Added mysql connector dependency and updated README and pyproject to reflect it (similar to postgresql)
  • Added utility to parse individual connection params from DSN for MySQL
  • Migration Logic borrowed from SQLite with MySQL syntax specific changes

Copy link
Owner

@s-bose s-bose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Documentations missing.
  • Several issues with code structure and refactor
  • Unit tests missing.

Overall nice work! Please fix the highlighted bits and add comprehensive unit tests.

@CodeStrate
Copy link
Contributor Author

Could you elaborate on the documentation part?

@s-bose
Copy link
Owner

s-bose commented Oct 22, 2025

Could you elaborate on the documentation part?

I mean in the readme there should be a mention of mysql and the dsn format.

- Removed unnecessary comments.
- Added a MySQLConnectionParams typedDict in place of typing.Dict
- Reworked generic exceptions to be more specific. and removed unnecessary exceptions.
- Removed defaults in params and structured query string params better.
- Changed TEXT to VARCHAR with fixed length as per MySQL syntax.
- Fixed a typo in providers to implement MySQLProvider correctly.
- Fixed Unread result errors while testing by correctly consuming cursor results.
@CodeStrate
Copy link
Contributor Author

@s-bose Added unit tests using the existing implementations as base.
30 tests passed and 1 failed due assertion on autocommit
Changed the documentation and added the structural changes as requested. Could you review?

@CodeStrate
Copy link
Contributor Author

CodeStrate commented Oct 24, 2025

I need your assistance since unit testing is not my strong suit, any suggestions on the autocommit test might help.

EDIT: I saw postgres's autocommit true in connect, so followed that.

…syntax and added autocommit True to match Postgres.
Copy link
Contributor Author

@CodeStrate CodeStrate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All requested changes are made @s-bose . Please review

@s-bose
Copy link
Owner

s-bose commented Oct 27, 2025

@CodeStrate Just a few more nitpicky reviews. Please review and lmk. Otherwise LGTM for merge.

- Also removed self.connection_params making it a local variable.
@CodeStrate
Copy link
Contributor Author

@s-bose Commit 12acbc6 fixes the two nitpicks you had. Please review and LMK if looks good to merge.

@s-bose
Copy link
Owner

s-bose commented Oct 30, 2025

Test CI is failing. Could you check please?

@CodeStrate
Copy link
Contributor Author

I didn't add uv lock file to the repository so probably that's the reason

@CodeStrate
Copy link
Contributor Author

@s-bose can you test the CI now?

@s-bose
Copy link
Owner

s-bose commented Oct 30, 2025

@CodeStrate
Copy link
Contributor Author

CodeStrate commented Oct 30, 2025

@s-bose try 73a6da7. it was failing due to the unsupported testcase using mysql as a reference.

@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@s-bose s-bose self-assigned this Oct 31, 2025
@s-bose s-bose self-requested a review October 31, 2025 15:08
@s-bose
Copy link
Owner

s-bose commented Oct 31, 2025

@CodeStrate Two more things.

wandern/databases/mysql.py#L29-L90

Complex Method

Could you please try to shorten the code a bit?
And, could you please bump the package version to the next minor release?

Sorry for being a pain! :)

- Bumped Package version to 0.0.4
- Simplified the parser function in mysql.py and modified the tests accordingly.
@CodeStrate
Copy link
Contributor Author

@s-bose with 095431a CodeFactor doesn't report an error now. I simplified the parser code, changed the 2 tests for missing host and port (combined them into one since we checked for either in the parser as both are required, and we don't assume any default fallbacks)

also bumped 0.0.3 to 0.0.4 :)

@s-bose s-bose merged commit cd1c84c into s-bose:master Nov 1, 2025
3 checks passed
@s-bose
Copy link
Owner

s-bose commented Nov 1, 2025

@CodeStrate Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants