jedcunningham commented on a change in pull request #21273:
URL: https://github.com/apache/airflow/pull/21273#discussion_r803221334
##########
File path:
airflow/migrations/versions/5e3ec427fdd3_increase_length_of_email_and_username.py
##########
@@ -46,9 +46,11 @@ def upgrade():
def downgrade():
"""Revert length of email from 256 to 64 characters"""
- with op.batch_alter_table('ab_user') as batch_op:
- batch_op.alter_column('username', type_=sa.String(64))
- batch_op.alter_column('email', type_=sa.String(64))
- with op.batch_alter_table('ab_register_user') as batch_op:
- batch_op.alter_column('username', type_=sa.String(64))
- batch_op.alter_column('email', type_=sa.String(64))
+ conn = op.get_bind()
+ if conn.dialect.name != 'mssql':
Review comment:
Why not for mssql?
##########
File path:
airflow/migrations/versions/7b2661a43ba3_taskinstance_keyed_to_dagrun.py
##########
@@ -337,7 +337,7 @@ def downgrade():
batch_op.drop_constraint('task_instance_pkey', type_='primary')
batch_op.alter_column('execution_date', existing_type=dt_type,
existing_nullable=True, nullable=False)
batch_op.alter_column(
- 'dag_id', existing_type=string_id_col_type,
existing_nullable=True, nullable=True
+ 'dag_id', existing_type=string_id_col_type,
existing_nullable=True, nullable=False
Review comment:
```suggestion
'dag_id', existing_type=string_id_col_type,
existing_nullable=False, nullable=True
```
Shouldn't it be like this? Upgrade went from nullable -> nonnullable, we
switch it back in downgrade?
##########
File path:
airflow/migrations/versions/b25a55525161_increase_length_of_pool_name.py
##########
@@ -45,6 +45,8 @@ def upgrade():
def downgrade():
"""Revert Increased length of pool name from 256 to 50 characters"""
- # use batch_alter_table to support SQLite workaround
- with op.batch_alter_table('slot_pool',
table_args=sa.UniqueConstraint('pool')) as batch_op:
- batch_op.alter_column('pool', type_=sa.String(50))
+ conn = op.get_bind()
+ if conn.dialect.name != 'mssql':
Review comment:
Why not mssql?
##########
File path:
airflow/migrations/versions/6e96a59344a4_make_taskinstance_pool_not_nullable.py
##########
@@ -42,25 +38,8 @@
ID_LEN = 250
-class TaskInstance(Base): # type: ignore
- """Minimal model definition for migrations"""
-
- __tablename__ = "task_instance"
-
- task_id = Column(String(), primary_key=True)
- dag_id = Column(String(), primary_key=True)
- execution_date = Column(UtcDateTime, primary_key=True)
- pool = Column(String(50), nullable=False)
-
-
def upgrade():
"""Make TaskInstance.pool field not nullable."""
- with create_session() as session:
- session.query(TaskInstance).filter(TaskInstance.pool.is_(None)).update(
- {TaskInstance.pool: 'default_pool'}, synchronize_session=False
- ) # Avoid select updated rows
- session.commit()
Review comment:
Why don't we need to do this anymore?
##########
File path:
airflow/migrations/versions/e9304a3141f0_make_xcom_pkey_columns_non_nullable.py
##########
@@ -48,14 +48,11 @@ def downgrade():
"""Unapply make xcom pkey columns non-nullable"""
conn = op.get_bind()
with op.batch_alter_table('xcom') as bop:
- if conn.dialect.name == 'mssql':
- bop.drop_constraint('pk_xcom', 'primary')
-
# regardless of what the model defined, the `key` and `execution_date`
- # columns were always non-nullable for sqlite and postgres, so leave
them alone
+ # columns were always non-nullable for mysql, sqlite and postgres, so
leave them alone
Review comment:
Are you sure about this? I remember being pretty careful when I tried
fixing this downgrade the last time.
##########
File path: airflow/migrations/versions/849da589634d_prefix_dag_permissions.py
##########
@@ -233,6 +233,7 @@ def upgrade():
def downgrade():
- db = SQLA()
- db.session = settings.Session
- undo_migrate_to_new_dag_permissions(db.session)
+ # db = SQLA()
+ # db.session = settings.Session
+ # undo_migrate_to_new_dag_permissions(db.session)
Review comment:
Since this is a 2.0.0 migration, probably not worth spending time on it.
--
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]
