-
-
Notifications
You must be signed in to change notification settings - Fork 629
feat: add MySQL storage support #1003
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
Conversation
| var query string | ||
| if s.driver == "mysql" { | ||
| query = `SELECT resolve_key, number_of_successes_in_a_row FROM endpoint_alerts_triggered WHERE endpoint_id = (SELECT endpoint_id FROM endpoints WHERE endpoint_key = ? LIMIT 1) AND configuration_checksum = ?` | ||
| } else { | ||
| query = `SELECT resolve_key, number_of_successes_in_a_row FROM endpoint_alerts_triggered WHERE endpoint_id = (SELECT endpoint_id FROM endpoints WHERE endpoint_key = $1 LIMIT 1) AND configuration_checksum = $2` | ||
| } |
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'm not really a fan of having those if/else blocks for each SQL query. With just postgres and sqlite it was easy because they both have the same syntax, but now with mysql, I think we might need a different solution.
Does ? work with sqlite and postgres as well? Or does postgres/sqlite require $1/$2/$...? Because in most cases, the query is exactly the same, outside of how prepared parameters are passed
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'm not really a fan of having those if/else blocks for each SQL query. With just postgres and sqlite it was easy because they both have the same syntax, but now with mysql, I think we might need a different solution.
Does
?work with sqlite and postgres as well? Or does postgres/sqlite require$1/$2/$...? Because in most cases, the query is exactly the same, outside of how prepared parameters are passed
Unfortunately, ? is MySQL-specific, and $... is Postgres-specific. If necessary, I recommend using an ORM (e.g. gorm, xorm, ent) to smooth over these differences.
|
Tested in my local environment and ready for review. |
TwiN
left a comment
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.
Just a heads up, I consider this a pretty large change, and it'll take a few months before I have the time to test this properly.
# Conflicts: # README.md # go.mod # storage/store/sql/sql.go # storage/store/store.go
|
I've given this a lot of thoughts, and I decided that supporting multiple storage providers is just too much work and it makes implementing new features even more difficult. Given that I'm doing this solo and not full time, I decided that I don't have enough time to support more storage types. I do appreciate your contribution, though! |
Summary
Add MySQL storage support, close #283
Checklist
README.md, if applicable.