ashb commented on a change in pull request #20962:
URL: https://github.com/apache/airflow/pull/20962#discussion_r806707192



##########
File path: airflow/cli/cli_parser.py
##########
@@ -224,6 +224,16 @@ def _check(value):
     default=ColorMode.AUTO,
 )
 
+# DB args
+ARG_VERSION_RANGE = Arg(
+    ("-v", "--version-range"), help="Version range(start:end) for offline sql 
generation", default=None

Review comment:
       ```suggestion
       ("-r", "--version-range"), help="Version range(start:end) for offline 
sql generation", default=None
   ```
   
   `-v` is usually verbose or version, so lets avoid that confusion.

##########
File path: airflow/cli/cli_parser.py
##########
@@ -224,6 +224,16 @@ def _check(value):
     default=ColorMode.AUTO,
 )
 
+# DB args
+ARG_VERSION_RANGE = Arg(
+    ("-v", "--version-range"), help="Version range(start:end) for offline sql 
generation", default=None
+)
+ARG_REVISION_RANGE = Arg(
+    ('-r', '--revision-range'),

Review comment:
       This one is probably not going to be used very common, so we can remove 
the short opt here
   ```suggestion
       ('--revision-range',),
   ```

##########
File path: airflow/utils/db.py
##########
@@ -989,8 +1005,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))

Review comment:
       Should we prefix all these messages with `--` to make them SQL comments?

##########
File path: dev/README_RELEASE_AIRFLOW.md
##########
@@ -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 if any.

Review comment:
       ```suggestion
   - Update the `REVISION_HEADS_MAP` at airflow/utils/db.py to include the 
revision head of the release even if there are no migrations.
   ```

##########
File path: airflow/cli/cli_parser.py
##########
@@ -224,6 +224,16 @@ def _check(value):
     default=ColorMode.AUTO,
 )
 
+# DB args
+ARG_VERSION_RANGE = Arg(
+    ("-v", "--version-range"), help="Version range(start:end) for offline sql 
generation", default=None

Review comment:
       Short flags should be a single character.  We could have this be 
`--range` (and keep the other as `--revision-range`) -- individual revisions 
are almost never going to be used outside of developers working on the main 
branch.




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to