Drop execution_date unique constraint on DagRun #41818
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boring-cyborg
bot
added
area:db-migrations
uranusjr
force-pushed
the
remove-execution-date
branch
from
5282e03 to
c14aff3
Compare
uranusjr
commented
Aug 28, 2024
uranusjr
force-pushed
the
remove-execution-date
branch
from
c14aff3 to
518767f
Compare
sunank200
approved these changes
Aug 29, 2024
sunank200
reviewed
Aug 29, 2024
Collaborator
sunank200
left a comment
sunank200
left a comment
There was a problem hiding this comment.
I think the tests has to change as well
uranusjr
requested review from
ephraimbuddy and
pierrejeambrun
as code owners
Member
Author
|
Fixed. If this passes, I think it can be merged first. |
uranusjr
commented
Sep 4, 2024
uranusjr
force-pushed
the
remove-execution-date
branch
2 times, most recently
from
2181c46 to
b28fa5a
Compare
uranusjr
force-pushed
the
remove-execution-date
branch
from
d87ad4a to
5145387
Compare
uranusjr
commented
Sep 4, 2024
kaxil
approved these changes
Sep 4, 2024
Member
kaxil
left a comment
kaxil
left a comment
There was a problem hiding this comment.
It isn’t breaking change but might be worth mentioning in Towncrier notes as a note-worthy change if someone is using combination of dag_id & execution_date to retrieve DagRuns
The column has also been renamed to logical_date, although the Python model is not changed. This allows us to not need to fix all the Python code at once (we'll do that later), but still do the two changes in one migration instead of two.
This seems to affect SQLite.
This seems to work with SQLite. Alembic is stupid...
uranusjr
force-pushed
the
remove-execution-date
branch
from
aadab74 to
8d52673
Compare
Member
Author
|
I think it is more appropriate to attach the news fragment on the next PR that will rename the attribute on DagRun instead. That one is the actual breaking change most people will notice. This PR only affects the SQL layer, not Python. Having separate fragments for changes in each layer will likely be a bit confusing. |
This was referenced Sep 4, 2024
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The column has also been renamed to logical_date, although the Python model is not changed. This allows us to not need to fix all the Python code at once (we'll do that later), but still do the two changes in one migration instead of two.
Note that this one is not breaking—the metadatabase interface is not user-facing, we’re only removing a check that stopped impossible things.