-
Notifications
You must be signed in to change notification settings - Fork 17.5k
Support generating SQL script for upgrades #20962
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,9 @@ | |
| import sys | ||
| import time | ||
| from tempfile import gettempdir | ||
| from typing import TYPE_CHECKING, Any, Callable, Iterable, List, Tuple | ||
| from typing import TYPE_CHECKING, Any, Callable, Iterable, List, Optional, Tuple | ||
|
|
||
| from bcrypt import warnings | ||
| from sqlalchemy import Table, exc, func, inspect, or_, text | ||
| from sqlalchemy.orm.session import Session | ||
|
|
||
|
|
@@ -68,6 +69,21 @@ | |
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
| REVISION_HEADS_MAP = { | ||
| "2.0.0": "e959f08ac86c", | ||
| "2.0.1": "82b7c48c147f", | ||
| "2.0.2": "2e42bb497a22", | ||
| "2.1.0": "a13f7613ad25", | ||
| "2.1.1": "a13f7613ad25", | ||
| "2.1.2": "a13f7613ad25", | ||
| "2.1.3": "97cdd93827b8", | ||
| "2.1.4": "ccde3e26fe78", | ||
| "2.2.0": "7b2661a43ba3", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we include all releases here? If we pass
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no migration head for that version unless we should use the head for a previous release that has migration but I think that a user using this feature is more likely to understand the message. WDYT?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should add all versions. It's very easy to add another command which will filter out the versions that have the same migration head as the previous one if you want "miigration versions only". And It also gives a great opportunity to print a "happy" message to the users when they run "get me the script to migrate from "2.2.1 to 2.2.2": |
||
| "2.2.1": "7b2661a43ba3", | ||
| "2.2.2": "7b2661a43ba3", | ||
| "2.2.3": "be2bfac3da23", | ||
| } | ||
|
|
||
|
|
||
| def _format_airflow_moved_table_name(source_table, version): | ||
| return "__".join([settings.AIRFLOW_MOVED_TABLE_PREFIX, version.replace(".", "_"), source_table]) | ||
|
|
@@ -1001,8 +1017,93 @@ def _check_migration_errors(session: Session = NEW_SESSION) -> Iterable[str]: | |
| session.commit() | ||
|
|
||
|
|
||
| def _offline_migration(command, config, revision): | ||
| log.info("Running offline migrations for revision range %s", revision) | ||
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore") | ||
| logging.disable(logging.CRITICAL) | ||
| command.upgrade(config, revision, sql=True) | ||
| logging.disable(logging.NOTSET) | ||
|
|
||
|
|
||
| def _validate_version_range(command, config, version_range): | ||
| if ':' not in version_range: | ||
| raise AirflowException( | ||
| 'Please provide Airflow version range with the format "old_version:new_version"' | ||
| ) | ||
| lower, upper = version_range.split(':') | ||
|
|
||
| if not REVISION_HEADS_MAP.get(lower) or not REVISION_HEADS_MAP.get(upper): | ||
| raise AirflowException('Please provide valid Airflow versions above 2.0.0.') | ||
| if REVISION_HEADS_MAP.get(lower) == REVISION_HEADS_MAP.get(upper): | ||
| if sys.stdout.isatty(): | ||
| size = os.get_terminal_size().columns | ||
| else: | ||
| size = 0 | ||
| print(f"Hey this is your migration script from {lower}, to {upper}, but guess what?".center(size)) | ||
|
ephraimbuddy marked this conversation as resolved.
Outdated
|
||
| print( | ||
| "There is no migration needed as the database has not changed between those versions. " | ||
| "You are done.".center(size) | ||
| ) | ||
| print("""/\\_/\\""".center(size)) | ||
| print("""(='_' )""".center(size)) | ||
| print("""(,(") (")""".center(size)) | ||
| print("""^^^""".center(size)) | ||
| return | ||
| dbname = settings.engine.dialect.name | ||
| if dbname == 'sqlite': | ||
| raise AirflowException('SQLite is not supported for offline migration.') | ||
| elif dbname == 'mssql' and (lower != '2.2.0' or int(lower.split('.')[1]) < 2): | ||
| raise AirflowException( | ||
| 'MSSQL is not supported for offline migration in Airflow versions less than 2.2.0.' | ||
| ) | ||
| revision = f"{REVISION_HEADS_MAP[lower]}:{REVISION_HEADS_MAP[upper]}" | ||
| try: | ||
| command.history(config, rev_range=revision) | ||
| except Exception: | ||
| raise AirflowException( | ||
| f"Error while checking history for revision range {revision}. " | ||
| f"Check that the supplied airflow version is in the format 'old_version:new_version'." | ||
| ) | ||
| return revision | ||
|
|
||
|
|
||
| def _validate_revision(command, config, revision_range): | ||
| if ':' not in revision_range: | ||
| raise AirflowException( | ||
| 'Please provide Airflow revision range with the format "old_revision:new_revision"' | ||
| ) | ||
| dbname = settings.engine.dialect.name | ||
| if dbname == 'sqlite': | ||
| raise AirflowException('SQLite is not supported for offline migration.') | ||
| start_version = '2.0.0' | ||
| rev_2_0_0_head = 'e959f08ac86c' | ||
| _lowerband, _upperband = revision_range.split(':') | ||
| if dbname == 'mssql': | ||
| rev_2_2_0_head = '7b2661a43ba3' | ||
| head_to_lowerband_range = f"{rev_2_2_0_head}:{_lowerband}" | ||
| head_to_upperband_range = f"{rev_2_2_0_head}:{_upperband}" | ||
| rev_2_0_0_head = rev_2_2_0_head # for logging purposes | ||
| start_version = '2.2.0' | ||
| else: | ||
| head_to_lowerband_range = f"{rev_2_0_0_head}:{_lowerband}" | ||
| head_to_upperband_range = f"{rev_2_0_0_head}:{_upperband}" | ||
| for i in [head_to_lowerband_range, head_to_upperband_range]: | ||
| try: | ||
| command.history(config, rev_range=i) | ||
| except Exception: | ||
| raise AirflowException( | ||
| f"Error while checking history for revision range {i}. " | ||
| f"Check that {i.split(':')[1]} is a valid revision. " | ||
| f"Supported revision for offline migration is from {rev_2_0_0_head} " | ||
| f"which is airflow {start_version} head" | ||
| ) | ||
|
|
||
|
|
||
| @provide_session | ||
| def upgradedb(session: Session = NEW_SESSION): | ||
| def upgradedb( | ||
| version_range: Optional[str] = None, revision_range: Optional[str] = None, session: Session = NEW_SESSION | ||
| ): | ||
| """Upgrade the database.""" | ||
| # alembic adds significant import time, so we import it lazily | ||
| if not settings.SQL_ALCHEMY_CONN: | ||
|
|
@@ -1012,6 +1113,14 @@ def upgradedb(session: Session = NEW_SESSION): | |
| config = _get_alembic_config() | ||
|
|
||
| config.set_main_option('sqlalchemy.url', settings.SQL_ALCHEMY_CONN.replace('%', '%%')) | ||
| if version_range: | ||
| revision = _validate_version_range(command, config, version_range) | ||
| if not revision: | ||
| return | ||
| return _offline_migration(command, config, revision) | ||
| elif revision_range: | ||
| _validate_revision(command, config, revision_range) | ||
| return _offline_migration(command, config, revision_range) | ||
|
|
||
| errors_seen = False | ||
| for err in _check_migration_errors(session=session): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -205,6 +205,7 @@ The Release Candidate artifacts we vote upon should be the exact ones we vote ag | |
| - Add a commit that updates `CHANGELOG.md` to add changes from previous version if it has not already added. | ||
| For now this is done manually, example run `git log --oneline v2-2-test..HEAD --pretty='format:- %s'` and categorize them. | ||
| - Add section for the release in `UPDATING.md`. If no new entries exist, put "No breaking changes" (e.g. `2.1.4`). | ||
| - Update the `REVISION_HEADS_MAP` at airflow/utils/db.py to include the revision head of the release even if there are no migrations. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should have a pre-commit to verify that a tuple containing the current Airflow version is added to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like a pre-commit won't be possible in this case as we only update the migration head when there's a new airflow release.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about something as simple as: from airflow.utils.db import REVISION_HEADS_MAP
from airflow.version import version as airflow_version
from packaging.version import Version
if not Version(airflow_version).is_prerelease and airflow_version not in REVISION_HEADS_MAP:
raise Exception(f"Airflow version {airflow_version} is not in the revision map {REVISION_HEADS_MAP}")
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i was also thinking about this issue. i saw that we had a table in the docs which contains this mapping. so what i was thinking to do was store that table in yaml form, and it could be used to generate the docs table and to support CLI commands which need to map version to revision, and that's what i do in #21601
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good, I think the challenge would be to get the migration heads of the versions and discard every other migration in-between when going from version to version. It's still necessary to have a pre-commit or a ci job that makes sure we have the migration head updated before a release
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes that's right. |
||
| - Commit the version change. | ||
| - PR from the 'test' branch to the 'stable' branch, and manually merge it once approved. | ||
| - Check out the 'stable' branch | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.