-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fix missing migrations #6483
Fix missing migrations #6483
Conversation
And shouldn't need the other rubrik when upgrading because we're now passing the migrations directory in earlier
7e2e4c2
to
7d5a7de
Compare
Passing run #21961 ↗︎
Details:
Review all test suite changes for PR #6483 ↗︎ |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6483 +/- ##
===========================================
- Coverage 92.94% 77.20% -15.75%
===========================================
Files 264 267 +3
Lines 10307 10417 +110
Branches 835 835
===========================================
- Hits 9580 8042 -1538
- Misses 599 2131 +1532
- Partials 128 244 +116 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to exclude migrations from codecov tests?
db.create_all() | ||
flask_migrate.stamp() | ||
|
||
flask_migrate.upgrade() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a noop if force == True
, but do we want in in an else
clause just in case something goes wrong with a migration script and we need to rebuild?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reckon we want to boot this stuff out of the start up sequence and separate it all out from one another really, but not right now.
Closes #6480
I have:
Description
Fix for a second time the missing flowauth migrations. These are now included in the FlowAuth package, and I've added some logging so one can see them actually being run as well.