Skip to content

Add support for MySQL/MariaDB #89

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

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Conversation

phihos
Copy link
Contributor

@phihos phihos commented Sep 9, 2024

This PR adds support for the Exto adapter Ecto.Adapters.MyXQL.

Closes #85.

@phihos
Copy link
Contributor Author

phihos commented Sep 9, 2024

I ran the tests locally and they passed. Someone needs tot trigger the CI so I can fix potential CI issues.

@phihos phihos force-pushed the myxql branch 4 times, most recently from 84b531b to 3a328d3 Compare September 9, 2024 22:37
@crbelaus
Copy link
Contributor

Thanks for this PR @phihos! It looks great, I will take a deeper look later this week. Apparently GitHub Actions doesn't like the --connect parameter and the CI is failing.

@phihos
Copy link
Contributor Author

phihos commented Sep 10, 2024

Hi, thanks for the feedback. I checked the health command and according to an official blogpost the command is correct. I also tried it myself and the script and its argument are accepted. I guess GH actions/docker does not like the single quotes very much. I replaced them with double quotes and added --innodb_initialized which is also recommended.

@phihos
Copy link
Contributor Author

phihos commented Sep 10, 2024

Fixed the formatting issue. Next run should at least go to the database tests.

@phihos
Copy link
Contributor Author

phihos commented Sep 11, 2024

I forgot a line in the example test config which I put in my local test config. The tables should exist now. @crbelaus Can you trigger CI again?

@phihos
Copy link
Contributor Author

phihos commented Sep 11, 2024

Sorry, I accidentally reverted the health-cmd fix. Should work again now.

@phihos
Copy link
Contributor Author

phihos commented Sep 11, 2024

I also accidentally reverted the formatter fix from yesterday 🙈
Fixed it now.

@phihos
Copy link
Contributor Author

phihos commented Sep 11, 2024

Finally 🥳

Copy link
Contributor

@crbelaus crbelaus left a comment

Choose a reason for hiding this comment

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

Honestly this is looking great. I've been playing with it and it works flawlessly. Thanks for the work @phihos

Any thoughts @odarriba ?

@crbelaus crbelaus changed the title Support myxql (#85) Add support for MySQL/MariaDB Sep 11, 2024
Copy link
Contributor

@odarriba odarriba left a comment

Choose a reason for hiding this comment

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

Looks good!! Thanks for this amazing contribution ❤️

@crbelaus crbelaus merged commit cc0c7d3 into elixir-error-tracker:main Sep 11, 2024
3 checks passed
@phihos phihos deleted the myxql branch September 11, 2024 15:55
@phihos
Copy link
Contributor Author

phihos commented Sep 11, 2024

Thank you for merging my PR. Now I do not have to maintain a fork for my use-case. Your code was already very well structured and implementation went smoothly. Looking forward to future releases :-)

@crbelaus
Copy link
Contributor

Thank you for merging my PR. Now I do not have to maintain a fork for my use-case. Your code was already very well structured and implementation went smoothly. Looking forward to future releases :-)

Thank you for contributing @phihos! BTW, we have just released v0.3.0 with MySQL/MariaDB support.

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.

Support myxql
3 participants