-
Notifications
You must be signed in to change notification settings - Fork 7
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
Move dev DB schema creation and stamping to a separate process #1123
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,35 @@ | ||
"""Initialize the dev environment's DB.""" | ||
#!/usr/bin/env python3 | ||
import sys | ||
"""Initialize the dev environment's DB.""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
|
||
import alembic.command | ||
import alembic.config | ||
from pyramid.paster import bootstrap | ||
bootstrap("conf/development.ini") | ||
from sqlalchemy import text | ||
from sqlalchemy.exc import ProgrammingError | ||
|
||
from via.db import Base, create_engine | ||
|
||
|
||
def is_stamped(engine) -> bool: | ||
"""Return True if the DB is stamped with an Alembic revision ID.""" | ||
with engine.connect() as connection: | ||
try: | ||
if connection.execute(text("select * from alembic_version")).first(): | ||
return True | ||
except ProgrammingError: | ||
pass | ||
|
||
return False | ||
|
||
|
||
def main(): | ||
with bootstrap("conf/development.ini") as env: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
settings = env["registry"].settings | ||
engine = create_engine(settings) | ||
|
||
if not is_stamped(engine): | ||
Base.metadata.create_all(engine) | ||
alembic.command.stamp(alembic.config.Config("conf/alembic.ini"), "head") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I mentioned on the other PR I reckon either here or in the Makefile https://github.com/hypothesis/lms/blob/main/Makefile#L18 I reckon with that we'll get a "it just works" experience for dev that are not making changes here often and just want to have the service running. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Via does run the migrations in It may be a bit confusing that there's a |
||
|
||
|
||
if __name__ == "__main__": | ||
main() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
import alembic.command | ||
import alembic.config | ||
import sqlalchemy | ||
import zope.sqlalchemy | ||
from sqlalchemy import MetaData, create_engine, text | ||
from sqlalchemy.exc import ProgrammingError | ||
from sqlalchemy import MetaData | ||
from sqlalchemy.orm import DeclarativeBase, MappedAsDataclass, Session | ||
|
||
|
||
|
@@ -18,23 +16,13 @@ class Base(MappedAsDataclass, DeclarativeBase): | |
) | ||
|
||
|
||
def stamp(engine): # pragma: no cover | ||
"""If the DB isn't already stamped then stamp it with the latest revision.""" | ||
stamped = False | ||
|
||
with engine.connect() as connection: | ||
try: | ||
if connection.execute(text("select * from alembic_version")).first(): | ||
stamped = True | ||
except ProgrammingError: | ||
pass | ||
|
||
if not stamped: | ||
alembic.command.stamp(alembic.config.Config("conf/alembic.ini"), "head") | ||
def create_engine(settings): # pragma: no cover | ||
"""Return a SQLAlchemy engine.""" | ||
return sqlalchemy.create_engine(settings["database_url"]) | ||
|
||
|
||
def includeme(config): # pragma: no cover | ||
engine = create_engine(config.registry.settings["database_url"]) | ||
engine = create_engine(config.registry.settings) | ||
|
||
def db_session(request): | ||
"""Return the SQLAlchemy session for the given request.""" | ||
|
@@ -47,11 +35,6 @@ def db_session(request): | |
|
||
return session | ||
|
||
if config.registry.settings.get("dev"): | ||
# Create the database tables if they don't already exist. | ||
Base.metadata.create_all(engine) | ||
stamp(engine) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly not having dev specific stuff in here seems like a win to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
# Make the SQLAlchemy session available as `request.db`. | ||
# `reify=True` means it'll create only one session per request. | ||
config.add_request_method(db_session, name="db", reify=True) |
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.
By default if
bin/make_db
exits in under a second (which it easily can do) Supervisor will consider this a crash regardless of the exit code and will restart the process. Settingstartsecs
to 0 tells Supervisor not to consider this a crash and not to restart the process (unless the exit code is non-zero)