You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/04/07 14:48:55 UTC
[GitHub] [airflow] ashb opened a new pull request #8176: Improve
add_dag_code_table migration
ashb opened a new pull request #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176
This migration was added in #7217 had a few problems that could cause us trouble in the
future:
1. It was importing the live model definition, which could break in the
future (if for instance it ever got a new column added, when this
migration runs it would try to use that new definition, but the
column won't exist yet).
2. It was selecting the (large) `data` column needlessly
3. It was dropping and re-creating the index but that is only needed on
MSSQL, not for MySQL or Postgresql.
---
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil merged pull request #8176: Improve
add_dag_code_table migration
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on a change in pull request #8176: Improve
add_dag_code_table migration
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176#discussion_r404933503
##########
File path: airflow/migrations/versions/952da73b5eff_add_dag_code_table.py
##########
@@ -38,6 +37,18 @@
def upgrade():
+ from sqlalchemy.ext.declarative import declarative_base
+
+ Base = declarative_base()
+
+ class SerializedDagModel(Base):
+ __tablename__ = 'serialized_dag'
+
+ # There are other columns here, but these are the only ones we need for the SELECT/UPDATE we are doing
+ dag_id = sa.Column(sa.String(250), primary_key=True)
+ fileloc = sa.Column(sa.String(2000), nullable=False)
+ fileloc_hash = sa.Column(sa.BigInteger, nullable=False)
Review comment:
Yup.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #8176:
Improve add_dag_code_table migration
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176#discussion_r404885089
##########
File path: airflow/migrations/versions/952da73b5eff_add_dag_code_table.py
##########
@@ -38,6 +37,18 @@
def upgrade():
+ from sqlalchemy.ext.declarative import declarative_base
+
+ Base = declarative_base()
+
+ class SerializedDagModel(Base):
+ __tablename__ = 'serialized_dag'
+
+ # There are other columns here, but these are the only ones we need for the SELECT/UPDATE we are doing
+ dag_id = sa.Column(sa.String(250), primary_key=True)
+ fileloc = sa.Column(sa.String(2000), nullable=False)
+ fileloc_hash = sa.Column(sa.BigInteger, nullable=False)
Review comment:
Should the column type be old one here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #8176:
Improve add_dag_code_table migration
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176#discussion_r404885717
##########
File path: airflow/migrations/versions/952da73b5eff_add_dag_code_table.py
##########
@@ -38,6 +37,18 @@
def upgrade():
+ from sqlalchemy.ext.declarative import declarative_base
+
+ Base = declarative_base()
+
+ class SerializedDagModel(Base):
+ __tablename__ = 'serialized_dag'
+
+ # There are other columns here, but these are the only ones we need for the SELECT/UPDATE we are doing
+ dag_id = sa.Column(sa.String(250), primary_key=True)
+ fileloc = sa.Column(sa.String(2000), nullable=False)
+ fileloc_hash = sa.Column(sa.BigInteger, nullable=False)
Review comment:
i.e. `Integer` ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #8176: Improve add_dag_code_table
migration
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176#issuecomment-610438757
/cc @anitakar
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #8176:
Improve add_dag_code_table migration
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176#discussion_r404885089
##########
File path: airflow/migrations/versions/952da73b5eff_add_dag_code_table.py
##########
@@ -38,6 +37,18 @@
def upgrade():
+ from sqlalchemy.ext.declarative import declarative_base
+
+ Base = declarative_base()
+
+ class SerializedDagModel(Base):
+ __tablename__ = 'serialized_dag'
+
+ # There are other columns here, but these are the only ones we need for the SELECT/UPDATE we are doing
+ dag_id = sa.Column(sa.String(250), primary_key=True)
+ fileloc = sa.Column(sa.String(2000), nullable=False)
+ fileloc_hash = sa.Column(sa.BigInteger, nullable=False)
Review comment:
Should the column type be the old one here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb edited a comment on issue #8176: Improve
add_dag_code_table migration
Posted by GitBox <gi...@apache.org>.
ashb edited a comment on issue #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176#issuecomment-610438757
/cc @anitakar - just as an FYI mostly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #8176:
Improve add_dag_code_table migration
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176#discussion_r404886314
##########
File path: airflow/migrations/versions/952da73b5eff_add_dag_code_table.py
##########
@@ -38,6 +37,18 @@
def upgrade():
+ from sqlalchemy.ext.declarative import declarative_base
+
+ Base = declarative_base()
+
+ class SerializedDagModel(Base):
+ __tablename__ = 'serialized_dag'
+
+ # There are other columns here, but these are the only ones we need for the SELECT/UPDATE we are doing
+ dag_id = sa.Column(sa.String(250), primary_key=True)
+ fileloc = sa.Column(sa.String(2000), nullable=False)
+ fileloc_hash = sa.Column(sa.BigInteger, nullable=False)
Review comment:
Ignore me. I see we use the model after changing Column type
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil commented on a change in pull request #8176:
Improve add_dag_code_table migration
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #8176: Improve add_dag_code_table migration
URL: https://github.com/apache/airflow/pull/8176#discussion_r404886314
##########
File path: airflow/migrations/versions/952da73b5eff_add_dag_code_table.py
##########
@@ -38,6 +37,18 @@
def upgrade():
+ from sqlalchemy.ext.declarative import declarative_base
+
+ Base = declarative_base()
+
+ class SerializedDagModel(Base):
+ __tablename__ = 'serialized_dag'
+
+ # There are other columns here, but these are the only ones we need for the SELECT/UPDATE we are doing
+ dag_id = sa.Column(sa.String(250), primary_key=True)
+ fileloc = sa.Column(sa.String(2000), nullable=False)
+ fileloc_hash = sa.Column(sa.BigInteger, nullable=False)
Review comment:
Ignore me. I see we use the model after changing COlumn type
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services