jscheffl commented on code in PR #58852:
URL: https://github.com/apache/airflow/pull/58852#discussion_r2660460651
##########
airflow-core/src/airflow/models/dag.py:
##########
@@ -363,6 +363,8 @@ class DagModel(Base):
timetable_summary: Mapped[str | None] = mapped_column(Text, nullable=True)
# Timetable description
timetable_description: Mapped[str | None] = mapped_column(String(1000),
nullable=True)
+ # Timetable Type
+ timetable_type: Mapped[str] = mapped_column(String(255), nullable=False,
server_default="")
Review Comment:
The nullable definition here is now inconsistent between the table spec
which is being used when the schema is freshly created and the alembic
migration. Migration allows to have Nullable-values. This would be inconsistent.
I understand that during migration we can not (easily) de-serialize existing
Dags and fill the value for I understand that during migration time we can fill
it. It will be filled implicitly with then next Dag parsing during running
post-migration.
Therefore I propose to fill the value during migration with a placeholder so
that the field is not-nullable and filled and assume it is updated on next Dag
parsing.
I'd propose _not_ to define a server default, because the field should be
filled by Dag parser (forced).
```suggestion
timetable_type: Mapped[str] = mapped_column(String(255), nullable=False)
```
--
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]
