[GitHub] [airflow] kaxil commented on a change in pull request #20962: Support generating SQL script for upgrades

[GitBox <gi...@apache.org>]

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



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

Review comment:
       ```suggestion
       help='Migration revision range(start:end) to use for offline sql generation. Example: 'a13f7613ad25:7b2661a43ba3'',
   ```

##########
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 even if there are no migrations.

Review comment:
       We should have a pre-commit to verify that a tuple containing the current Airflow version is added to `REVISION_HEADS_MAP` to avoid forgetting it.

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

Review comment:
       ```suggestion
       ("-r", "--range"), help="Version range(start:end) for offline sql generation. Example: '2.0.2:2.2.3'", default=None
   ```

##########
File path: airflow/models/taskinstance.py
##########
@@ -354,7 +354,7 @@ class TaskInstance(Base, LoggingMixin):
     unixname = Column(String(1000))
     job_id = Column(Integer)
     pool = Column(String(256), nullable=False)
-    pool_slots = Column(Integer, default=1, nullable=False)
+    pool_slots = Column(Integer, default=1, nullable=False, server_default=text("1"))

Review comment:
       Why does it need `text("1")` instead of just `1`




-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org