PR#102: DB Init/Upgrade on demand during runapp - fedora-qa/blockerbugs

fedora-qa / blockerbugs

#102 DB Init/Upgrade on demand during runapp

Merged 4 years ago by frantisekz. Opened 4 years ago by frantisekz.

auto_init into develop

DB Init/Upgrade on demand during runapp

František Zatloukal • 4 years ago

5e67d92

blockerbugs/cli.py

file modified

+40 -10

		`@@ -12,7 +12,9 @@`
		`from blockerbugs.models.bug import Bug`
		`from alembic.config import Config`
		`from alembic import command as al_command`
		`+ from alembic import script`
		`from alembic.migration import MigrationContext`
		`+ from sqlalchemy.engine import reflection`


		`def get_alembic_config():`
		`@@ -25,25 +27,50 @@`
		`ini_section='alembic-packaged')`
		`return alembic_cfg`

		`+ def init_db(args):`
		`+ initialize_db(args.destructive)`

		`- def initialize_db(args):`
		`+ def initialize_db(destructive=False):`
		`alembic_cfg = get_alembic_config()`

		`+ if destructive:`
		`+ print("Initializing Database")`
		`+ db.drop_all()`
		`+ db.create_all()`
		`+ print("Initializing alembic version")`
		`+ al_command.stamp(alembic_cfg, "head")`
		`+ return True`
		`+`
		`+ # check whether the table 'milestone' exists`
		`+ # if it does not, we assume that the database is empty`
		`+ insp = reflection.Inspector.from_engine(db.engine)`
		`+ table_names = insp.get_table_names()`
		`+ if 'milestone' not in table_names:`
		`+ print(" - Creating tables")`
		`+ db.create_all()`
		`+ print(" - Stamping alembic's current version to 'head'")`
		`+ al_command.stamp(alembic_cfg, "head")`
		`+ return True`
		`+`
		`# check to see if the db has already been initialized by checking for an`
		`# alembic revision`
		`+ directory = script.ScriptDirectory.from_config(alembic_cfg)`
		`context = MigrationContext.configure(db.engine.connect())`
		`current_rev = context.get_current_revision()`
		`-`
		`- if current_rev:`
		`- print("Database already initialized and at rev %s - not re-initializing" % current_rev)`
		`+ if not current_rev:`
		`+ print("Couldn't determine alembic version in db...")`
		`+ return False`
		`+ if set(context.get_current_heads()) == set(directory.get_heads()):`
		`+ print("Database already initialized and at the latest rev %s - not re-initializing" % current_rev)`
		`+ return True`
		`else:`
		`- print("Initializing Database")`
		`- db.drop_all()`
		`- db.create_all()`
		`+ print("Database is behind alembic scripts, upgrading...")`
		`+ upgrade_db(None)`
		`+ return True`

		`- print("Initializing alembic version")`
		`+ # If we hit this point, database is probably screwed`
		`+ return False`

		`- al_command.stamp(alembic_cfg, "head")`


		`def upgrade_db(args):`
		`@@ -188,7 +215,10 @@`
		`subparsers = parser.add_subparsers(dest='command', metavar="<COMMAND>")`

		`init_db_parser = subparsers.add_parser('init_db', help='Initialize DB')`
		`- init_db_parser.set_defaults(func=initialize_db)`
		`+ init_db_parser.add_argument('-rm', '--destructive', action='store_true',`
		`+ dest='destructive', default=False,`
		`+ help='Force database recreation (will ERASE your DB!!!)')`
		`+ init_db_parser.set_defaults(func=init_db)`

		`add_milestone_parser = subparsers.add_parser('add_milestone',`
		`help='Add milestone')`

runapp.py

file modified

		`@@ -21,10 +21,19 @@`
		`# Tim Flink <tflink@redhat.com>`

		`import os`
		`+ import sys`
		`import blockerbugs`
		`+ from blockerbugs import cli`

		`if __name__ == '__main__':`

		`+ # verify we have a database ready, bail out if not or if it's not in`
		`+ # consistent state`
		`+`
		`+ if not cli.initialize_db(destructive=False):`
		`+ print("Database is in unknown state, manual intervention needed")`
		`+ sys.exit(1)`
		`+`
		`# now that we have FAS integration, we don't want to default to production`
		`# when we're not running though mod_wsgi`

frantisekz commented 4 years ago

Tries to perform init_db and upgrade_db steps automatically as needed during app startup (both through python development server and mod_wsgi).

Adds --rm/--destructive option to force db to recreate.

rebased onto b8867b1

4 years ago

rebased onto fd756a7

4 years ago

frantisekz commented 4 years ago

@jskladan I think this one should be ready for review :)

jskladan commented 4 years ago

Beware of the S-word! I'll give it a look. @tflink might also be interested

jskladan commented on line 36 of blockerbugs/cli.py 4 years ago

https://alembic.sqlalchemy.org/en/latest/cookbook.html#test-current-database-revision-is-at-head-s might be interesting

jskladan commented on line 14 of runapp.py 4 years ago

I'd rather see ...(destructive=False) to keep it explicit. Not that it changes anything functionally.

jskladan commented 4 years ago

Looks good on paper, I'll run&test it tomorrow, but I don't see any glaring issues.

1 new commit added

Few improvements for PR review

4 years ago

frantisekz commented 4 years ago

Updated to reflect proposed changes.

If you want to test downgrades, comment out upgrade lines in 'ea9712463f84_drop_spins_support.py' . I wonder who added that file ... :O :D

I've tested a few random 'alembic downgrade' steps and upgrades from that point, seemed to work fine.

jskladan commented 4 years ago

I honestly don't feel like working downgrades are strictly necessary (since that usually means unrecoverable data-loss), but it's good to know that you keep them in mind.

rebased onto 9310af5

4 years ago

jskladan commented 4 years ago

LGTM, be aware of #105 though

rebased onto 5e67d92

4 years ago

Pull-Request has been merged by frantisekz

4 years ago