Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bug: hardcoded dialect/driver parameter in connection url in mysql container #727

Closed
nightblure opened this issue Oct 25, 2024 · 2 comments · Fixed by #739
Closed

Bug: hardcoded dialect/driver parameter in connection url in mysql container #727

nightblure opened this issue Oct 25, 2024 · 2 comments · Fixed by #739

Comments

@nightblure
Copy link
Contributor

nightblure commented Oct 25, 2024

Describe the bug

I find it strange that the dialect or driver parameter is hardcoded here (look screenshot below), because today there are many different methods and drivers for connecting to mysql. If I use something other than pymysql, I get the wrong connection address from the get_connection_url method.

Code:
Image

Possible solution

  1. Remove this hardcoded line and leave only mysql://...;
  2. Allow you to pass an optional dialect or driver parameter to the get_connection_url function or the MySqlContainer class initializer. If its value is non-empty, then in the get_connection_url method it will be inserted into the right place in the connection string

To Reproduce

  • just see to current code (testcontainers 4.8.2)

Runtime environment
Looks optional for this issue

@nightblure nightblure changed the title Bug or enhancement: hardcoded dialect/driver parameter in connection url in mysql container Bug and enhancement proposal: remove hardcoded dialect/driver parameter in connection url in mysql container Oct 25, 2024
@nightblure nightblure changed the title Bug and enhancement proposal: remove hardcoded dialect/driver parameter in connection url in mysql container Bug: hardcoded dialect/driver parameter in connection url in mysql container Oct 25, 2024
@nightblure
Copy link
Contributor Author

If this solution works, I could do it

@totallyzen
Copy link
Collaborator

Please feel free to contribute!

I would recommend putting the dialect in the constructor, not self.get_connection_url() - it's where the other settings go as well by the looks of it.

totallyzen pushed a commit that referenced this issue Dec 12, 2024
…739)

closes
#727

* add parameter `dialect`;
* tests fixing and add some assertions
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
2 participants