Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AIRFLOW-6185] SQLAlchemy Connection model schema not aligned with Alembic schema #6754

Merged
merged 2 commits into from
Dec 9, 2019

Conversation

xinbinhuang
Copy link
Contributor

Make sure you have checked all steps below.

Jira

Description

  • Here are some details about my PR, including screenshots of any UI changes:
    • Create a new alembic revision to increase Connection.password to 5000, syncing with the SQLAlchemy model, and set render_as_batch=True for both online & offline migrations to work with SQLite backend.
    • Import SerializedDagModel to migrations/env.py. As SerializedDagModel was removed from models ([AIRFLOW-6010] Remove cyclic imports and pylint disables #6601 ), alembic thinks that the object was removed, and will try to remove the table in autogenerated schema migration. This can avoid the behavior

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

  • My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
    • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

@codecov-io
Copy link

codecov-io commented Dec 8, 2019

Codecov Report

Merging #6754 into master will decrease coverage by 0.29%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #6754     +/-   ##
=========================================
- Coverage   84.84%   84.55%   -0.3%     
=========================================
  Files         669      669             
  Lines       37851    37851             
=========================================
- Hits        32114    32004    -110     
- Misses       5737     5847    +110
Impacted Files Coverage Δ
airflow/kubernetes/volume_mount.py 44.44% <0%> (-55.56%) ⬇️
airflow/kubernetes/volume.py 52.94% <0%> (-47.06%) ⬇️
airflow/kubernetes/pod_launcher.py 45.25% <0%> (-46.72%) ⬇️
airflow/kubernetes/refresh_config.py 50.98% <0%> (-23.53%) ⬇️
...rflow/contrib/operators/kubernetes_pod_operator.py 78.2% <0%> (-20.52%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4561978...71314c6. Read the comment docs.

@xinbinhuang xinbinhuang changed the title [Airflow 6185] SQLAlchemy Connection model schema not aligned with Alembic schema [AIRFLOW-6185] SQLAlchemy Connection model schema not aligned with Alembic schema Dec 9, 2019
target_metadata=target_metadata,
literal_binds=True,
compare_type=COMPARE_TYPE,
render_as_batch=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@@ -22,6 +22,7 @@
from alembic import context

from airflow import models, settings
from airflow.models.serialized_dag import SerializedDagModel # noqa
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Did not realise that was the side effect. Will remember about that!

@potiuk potiuk merged commit e04059d into apache:master Dec 9, 2019
ashb added a commit that referenced this pull request Dec 10, 2019
…umn to 5000 (#6241)" (#6783)

This reverts commit da7a353.

This was already fixed by by #6754 which was already merged, and we ended up with
two migration files (trying) to do the same thing.
ashb pushed a commit to ashb/airflow that referenced this pull request Dec 18, 2019
…embic schema (apache#6754)

* [AIRFLOW-6185] SQLAlchemy Connection model schema aligned with Alembic schema

(cherry picked from commit e04059d)
ashb pushed a commit that referenced this pull request Dec 18, 2019
…embic schema (#6754)

* [AIRFLOW-6185] SQLAlchemy Connection model schema aligned with Alembic schema

(cherry picked from commit e04059d)
ashb pushed a commit that referenced this pull request Dec 19, 2019
…embic schema (#6754)

* [AIRFLOW-6185] SQLAlchemy Connection model schema aligned with Alembic schema

(cherry picked from commit e04059d)
kaxil pushed a commit that referenced this pull request Dec 19, 2019
…embic schema (#6754)

* [AIRFLOW-6185] SQLAlchemy Connection model schema aligned with Alembic schema

(cherry picked from commit e04059d)
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
…embic schema (apache#6754)

* [AIRFLOW-6185] SQLAlchemy Connection model schema aligned with Alembic schema
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
…umn to 5000 (apache#6241)" (apache#6783)

This reverts commit da7a353.

This was already fixed by by apache#6754 which was already merged, and we ended up with
two migration files (trying) to do the same thing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants