You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/02/17 13:18:28 UTC
[GitHub] [airflow] XD-DENG opened a new pull request #14274: Handle possible suffix in MySQL version + avoid hard-coding
XD-DENG opened a new pull request #14274:
URL: https://github.com/apache/airflow/pull/14274
closes #14262, and do additional minor improvement.
- Fix the issue in #14262 . The issue arose because MySQL version may include special suffix like "-log", which is not accepted by `packaging.version.Version()`. A special treatment is added. It's unnecessary for Postgres or SQLite.
- Minor changes to avoid hard-coding minimum version numbers in the string returned (using `.format()` rather than f-string to consider potential lower Python versions used by users).
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #14262
related: #14262
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #14274: Handle possible suffix in MySQL version + avoid hard-coding
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14274:
URL: https://github.com/apache/airflow/pull/14274#discussion_r577609833
##########
File path: airflow/upgrade/rules/postgres_mysql_sqlite_version_upgrade_check.py
##########
@@ -43,16 +43,23 @@ def check(self, session=None):
min_req_sqlite_version = Version('3.15')
installed_sqlite_version = Version(session.execute('select sqlite_version();').scalar())
if installed_sqlite_version < min_req_sqlite_version:
- return "From Airflow 2.0, SQLite version below 3.15 is no longer supported. \n" + more_info
+ return "From Airflow 2.0, SQLite version below {} is no longer supported. \n{}".format(
+ min_req_sqlite_version, more_info
+ )
elif "postgres" in conn_str:
min_req_postgres_version = Version('9.6')
installed_postgres_version = Version(session.execute('SHOW server_version;').scalar())
if installed_postgres_version < min_req_postgres_version:
- return "From Airflow 2.0, PostgreSQL version below 9.6 is no longer supported. \n" + more_info
+ return "From Airflow 2.0, PostgreSQL version below {} is no longer supported. \n{}".format(
+ min_req_postgres_version, more_info
+ )
elif "mysql" in conn_str:
min_req_mysql_version = Version('5.7')
- installed_mysql_version = Version(session.execute('SELECT VERSION();').scalar())
+ # special treatment is needed here, because MySQL version may include a suffix like '-log'
+ installed_mysql_version = Version(session.execute('SELECT VERSION();').scalar().split('-')[0])
if installed_mysql_version < min_req_mysql_version:
- return "From Airflow 2.0, MySQL version below 5.7 is no longer supported. \n" + more_info
+ return "From Airflow 2.0, MySQL version below {} is no longer supported. \n{}".format(
Review comment:
Whoops my bad!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] XD-DENG merged pull request #14274: Handle possible suffix in MySQL version + avoid hard-coding
Posted by GitBox <gi...@apache.org>.
XD-DENG merged pull request #14274:
URL: https://github.com/apache/airflow/pull/14274
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] zappallot commented on pull request #14274: Handle possible suffix in MySQL version + avoid hard-coding
Posted by GitBox <gi...@apache.org>.
zappallot commented on pull request #14274:
URL: https://github.com/apache/airflow/pull/14274#issuecomment-782239335
awesome, thanks very much
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] XD-DENG commented on a change in pull request #14274: Handle possible suffix in MySQL version + avoid hard-coding
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #14274:
URL: https://github.com/apache/airflow/pull/14274#discussion_r577608345
##########
File path: airflow/upgrade/rules/postgres_mysql_sqlite_version_upgrade_check.py
##########
@@ -43,16 +43,23 @@ def check(self, session=None):
min_req_sqlite_version = Version('3.15')
installed_sqlite_version = Version(session.execute('select sqlite_version();').scalar())
if installed_sqlite_version < min_req_sqlite_version:
- return "From Airflow 2.0, SQLite version below 3.15 is no longer supported. \n" + more_info
+ return "From Airflow 2.0, SQLite version below {} is no longer supported. \n{}".format(
+ min_req_sqlite_version, more_info
+ )
elif "postgres" in conn_str:
min_req_postgres_version = Version('9.6')
installed_postgres_version = Version(session.execute('SHOW server_version;').scalar())
if installed_postgres_version < min_req_postgres_version:
- return "From Airflow 2.0, PostgreSQL version below 9.6 is no longer supported. \n" + more_info
+ return "From Airflow 2.0, PostgreSQL version below {} is no longer supported. \n{}".format(
+ min_req_postgres_version, more_info
+ )
elif "mysql" in conn_str:
min_req_mysql_version = Version('5.7')
- installed_mysql_version = Version(session.execute('SELECT VERSION();').scalar())
+ # special treatment is needed here, because MySQL version may include a suffix like '-log'
+ installed_mysql_version = Version(session.execute('SELECT VERSION();').scalar().split('-')[0])
if installed_mysql_version < min_req_mysql_version:
- return "From Airflow 2.0, MySQL version below 5.7 is no longer supported. \n" + more_info
+ return "From Airflow 2.0, MySQL version below {} is no longer supported. \n{}".format(
Review comment:
Hi Kaxil, I'm not sure if I get you correctly, but this PR is raised against v1-10-stable, so it should be doing what you are suggesting.
This `airflow/upgrade` directory does not exist in `master` branch anyway :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #14274: Handle possible suffix in MySQL version + avoid hard-coding
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14274:
URL: https://github.com/apache/airflow/pull/14274#discussion_r577605240
##########
File path: airflow/upgrade/rules/postgres_mysql_sqlite_version_upgrade_check.py
##########
@@ -43,16 +43,23 @@ def check(self, session=None):
min_req_sqlite_version = Version('3.15')
installed_sqlite_version = Version(session.execute('select sqlite_version();').scalar())
if installed_sqlite_version < min_req_sqlite_version:
- return "From Airflow 2.0, SQLite version below 3.15 is no longer supported. \n" + more_info
+ return "From Airflow 2.0, SQLite version below {} is no longer supported. \n{}".format(
+ min_req_sqlite_version, more_info
+ )
elif "postgres" in conn_str:
min_req_postgres_version = Version('9.6')
installed_postgres_version = Version(session.execute('SHOW server_version;').scalar())
if installed_postgres_version < min_req_postgres_version:
- return "From Airflow 2.0, PostgreSQL version below 9.6 is no longer supported. \n" + more_info
+ return "From Airflow 2.0, PostgreSQL version below {} is no longer supported. \n{}".format(
+ min_req_postgres_version, more_info
+ )
elif "mysql" in conn_str:
min_req_mysql_version = Version('5.7')
- installed_mysql_version = Version(session.execute('SELECT VERSION();').scalar())
+ # special treatment is needed here, because MySQL version may include a suffix like '-log'
+ installed_mysql_version = Version(session.execute('SELECT VERSION();').scalar().split('-')[0])
if installed_mysql_version < min_req_mysql_version:
- return "From Airflow 2.0, MySQL version below 5.7 is no longer supported. \n" + more_info
+ return "From Airflow 2.0, MySQL version below {} is no longer supported. \n{}".format(
Review comment:
Upgrade check needs to be fixed in v1-10-stable too
https://github.com/apache/airflow/blob/v1-10-stable/airflow/upgrade/rules/postgres_mysql_sqlite_version_upgrade_check.py#L54-L58
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org