Merge #606 Create app_context before database session.

module_build_service/manage.py

@@ -125,37 +125,41 @@ def cleardb():
 def build_module_locally(url, branch, skiptests=False):
     """ Performs local module build using Mock
     """
-    conf.set_item("system", "mock")
+    if 'SERVER_NAME' not in app.config:
+        app.config["SERVER_NAME"] = 'localhost'
 
-    # Use our own local SQLite3 database.
+    with app.app_context():
-    confdir = os.path.abspath(os.getcwd())
+        conf.set_item("system", "mock")
-    dbdir = os.path.abspath(os.path.join(confdir, '..')) if confdir.endswith('conf') \
-        else confdir
-    dbpath = '/{0}'.format(os.path.join(dbdir, '.mbs_local_build.db'))
-    dburi = 'sqlite://' + dbpath
-    app.config["SQLALCHEMY_DATABASE_URI"] = dburi
-    conf.set_item("sqlalchemy_database_uri", dburi)
-    if os.path.exists(dbpath):
-        os.remove(dbpath)
 
-    # Create the database and insert fake base-runtime module there. This is
+        # Use our own local SQLite3 database.
-    # normally done by the flask_migrate.upgrade(), but I (jkaluza) do not
+        confdir = os.path.abspath(os.getcwd())
-    # call it here, because after that call, all the logged messages are not
+        dbdir = os.path.abspath(os.path.join(confdir, '..')) if confdir.endswith('conf') \
-    # printed to stdout/stderr and are ignored... I did not find a way how to
+            else confdir
-    # fix that.
+        dbpath = '/{0}'.format(os.path.join(dbdir, '.mbs_local_build.db'))
-    #
+        dburi = 'sqlite://' + dbpath
-    # In the future, we should use PDC to get what we need from the fake module,
+        app.config["SQLALCHEMY_DATABASE_URI"] = dburi
-    # so it's probably not big problem.
+        conf.set_item("sqlalchemy_database_uri", dburi)
-    db.create_all()
+        if os.path.exists(dbpath):
+            os.remove(dbpath)
 
-    username = getpass.getuser()
+        # Create the database and insert fake base-runtime module there. This is
-    submit_module_build_from_scm(username, url, branch, allow_local_url=True,
+        # normally done by the flask_migrate.upgrade(), but I (jkaluza) do not
-                                 skiptests=skiptests)
+        # call it here, because after that call, all the logged messages are not
+        # printed to stdout/stderr and are ignored... I did not find a way how to
+        # fix that.
+        #
+        # In the future, we should use PDC to get what we need from the fake module,
+        # so it's probably not big problem.
+        db.create_all()
 
-    stop = module_build_service.scheduler.make_simple_stop_condition(db.session)
+        username = getpass.getuser()
+        submit_module_build_from_scm(username, url, branch, allow_local_url=True,
+                                    skiptests=skiptests)
 
-    # Run the consumer until stop_condition returns True
+        stop = module_build_service.scheduler.make_simple_stop_condition(db.session)
-    module_build_service.scheduler.main([], stop)
+
+        # Run the consumer until stop_condition returns True
+        module_build_service.scheduler.main([], stop)
 
 
 @manager.command

module_build_service/models.py

@@ -31,9 +31,10 @@ import contextlib
 from datetime import datetime
 from sqlalchemy import engine_from_config, event
 from sqlalchemy.orm import validates, scoped_session, sessionmaker
+from flask import has_app_context
 import modulemd as _modulemd
 
-from module_build_service import db, log, get_url_for
+from module_build_service import db, log, get_url_for, app
 import module_build_service.messaging
 
 from sqlalchemy.orm import lazyload
@@ -66,25 +67,44 @@ BUILD_STATES = {
 
 INVERSE_BUILD_STATES = {v: k for k, v in BUILD_STATES.items()}
 
+@contextlib.contextmanager
+def _dummy_context_mgr():
+    """
+    Yields None. Used in the make_session to not duplicate the code when
+    app_context exists.
+    """
+    yield None
 
 @contextlib.contextmanager
 def make_session(conf):
-    # TODO - we could use ZopeTransactionExtension() here some day for
+    """
-    # improved safety on the backend.
+    Yields new SQLAlchemy database sesssion.
-    engine = engine_from_config({
+    """
-        'sqlalchemy.url': conf.sqlalchemy_database_uri,
+    # Needs to be set to create app_context.
-    })
+    if 'SERVER_NAME' not in app.config:
-    session = scoped_session(sessionmaker(bind=engine))()
+        app.config['SERVER_NAME'] = 'localhost'
-    event.listen(session, "before_commit", session_before_commit_handlers)
+
-    try:
+    # If there is no app_context, we have to create one before creating
-        yield session
+    # the session. If we would create app_context after the session (this
-        session.commit()
+    # happens in get_url_for() method), new concurrent session would be
-    except:
+    # created and this would lead to "database is locked" error for SQLite.
-        # This is a no-op if no transaction is in progress.
+    with app.app_context() if not has_app_context() else _dummy_context_mgr():
-        session.rollback()
+        # TODO - we could use ZopeTransactionExtension() here some day for
-        raise
+        # improved safety on the backend.
-    finally:
+        engine = engine_from_config({
-        session.close()
+            'sqlalchemy.url': conf.sqlalchemy_database_uri,
+        })
+        session = scoped_session(sessionmaker(bind=engine))()
+        event.listen(session, "before_commit", session_before_commit_handlers)
+        try:
+            yield session
+            session.commit()
+        except:
+            # This is a no-op if no transaction is in progress.
+            session.rollback()
+            raise
+        finally:
+            session.close()
 
 
 class MBSBase(db.Model):

tests/test_scheduler/test_poller.py

@@ -82,6 +82,7 @@ class TestPoller(unittest.TestCase):
         poller.poll()
 
         # Refresh our module_build object.
+        module_build = models.ModuleBuild.query.filter_by(id=2).one()
         db.session.refresh(module_build)
 
         # Components should be in BUILDING state now.
@@ -156,6 +157,7 @@ class TestPoller(unittest.TestCase):
         poller.poll()
 
         # Refresh our module_build object.
+        module_build = models.ModuleBuild.query.filter_by(id=2).one()
         db.session.refresh(module_build)
 
         self.assertTrue(not koji_session.newRepo.called)
@@ -190,6 +192,7 @@ class TestPoller(unittest.TestCase):
         poller.poll()
 
         # Refresh our module_build object.
+        module_build = models.ModuleBuild.query.filter_by(id=2).one()
         db.session.refresh(module_build)
 
         # Components should not be in building state
@@ -232,6 +235,7 @@ class TestPoller(unittest.TestCase):
             poller = MBSProducer(hub)
             poller.delete_old_koji_targets(conf, db.session)
 
+            module_build = models.ModuleBuild.query.filter_by(id=2).one()
             db.session.refresh(module_build)
             module_build.time_completed = datetime.utcnow() - timedelta(hours=23)
             db.session.commit()