-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Add MariaDB support to Breeze development environment #60133
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
base: main
Are you sure you want to change the base?
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
This doesn't add a specific mariadb Dialect, just permit easy testing. to test, in place of setting mysql version, like '8.4', just set 'mariadb:11.8' for example. example : breeze --backend mysql --mysql-version mariadb:11.8
potiuk
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.
Even if you propose it for testing only, we should have separate mariadb backend type (and devlist discussion about what to do with mariadb should be started). It makes very little sense to add mariadb as variant of mysql - those two databases have different dialects and they separated a long time ago and have different set of features and versions. There are already known issues and differences betwen mariadb and mysql.
Please start discussion about it and dev community should decide what level of suppor we have for mariadb - even if it is only for testing, we will likely still need to support it and it means some maintenance overhead.
Note, that I am not saying it's bad to add it - I am just saying that this is the kind of decision that MUST be done on devlist - not in PR.
And I am not against it by default to be perfecly honest - we are already using mariadbclient (because mysql and mariadb share the same protocol - by design) and we are using mariadb client in our image. But those databases are different now and if we are to support it in the community, we need ot umderstand what maintenance overhead it is going to create.
Requesting changes just to make sure it's not accidentally merged before the discussion in devlist happens.
When `latest_versions_only` is set, return both the latest MySQL and latest MariaDB versions instead of only the last version in the list. This ensures both database types are tested when running with latest versions.
|
While MariaDB and MySQL have differences, they remain similar enough for practical testing purposes. PR #60047 represents the minimal change needed to enable the MariaDB test suite, though it still requires a correction to maintain PostgreSQL and SQLite compatibility. The goal of this PR is to enable immediate MariaDB testing capabilities to catch any breaking changes early. Long-term vision: please tell if your ok with that |
Again - not my decision, by community decision that should be discussed on devlist. What I think will matter only in the context of what others - who will be maintaining it - think. |
|
BTW. You might freely continue rebasing your PR and base your testing on top of it - while the discussion is happening. There is no need to merge it to do it, what is more important though is that we decide - as a community whether we are going to support it - even for testing purposes. Context: We had very bad experience with MsSQL in the past and we eventually had to remove it (we were lucky it was merged as "experimental" - but a lot of our users who used it had problems when we removed it and wanted it to continue - even if it was clearly marked as "experimental". Now I understand what you are saying that it's "close enough to MySQL", but this is a completely vague statement that does not even attempt to describe "how close" it is. In the past for example there was a LONG period of time SKIP LOCKED was not supported by MariaDB. And we are currently using some features from MySQL like json fields etc. that might prove to behave differently, even the PR #60047 shows that there are some significant changes that will make it more difficult to maintain and keep mariadb support in parallel to mysql. This PR is a proof that there are differences - and ones that add maintenance overhead. So my best guess (but again - you MUST start devlist discussion if you want to support it) you will have to show exactly what kind of changes are necessary to support MariaDB in order to even begin a discussion "should we merge it". Which basically means that you will have to heave the PR green, when running it with your changes and have all the tests passing. And you - rebasing it continuously - will be able to see also - while other changes is implemented, how much extra maintenance it is to keep your PR is green - this is a great opportunity to either proof that it is fine - if it will be relatively painless to keep it green over the course of weeks or months, or whether it is actually difficult and will require maintainers to spend extra time to test and fix issues. So there are only good things with keeping your PR open and continuously rebased - while the discussion is running. |
This doesn't add a specific mariadb Dialect, just permit easy testing.
to test, in place of setting mysql version, like '8.4', just set 'mariadb:11.8' for example. example :
breeze --backend mysql --mysql-version mariadb:11.8