Merge #1767 Use click instead of flask script

module_build_service/manage.py

@@ -2,15 +2,14 @@
 # SPDX-License-Identifier: MIT
 from __future__ import absolute_import, print_function
 from collections import defaultdict
-from functools import wraps
+import click
 import getpass
 import logging
 import os
 import sys
-import textwrap
 
 import flask_migrate
-from flask_script import Manager, prompt_bool
+from flask.cli import FlaskGroup
 from werkzeug.datastructures import FileStorage
 
 from module_build_service import app, db
@@ -19,76 +18,37 @@ from module_build_service.builder.MockModuleBuilder import (
 )
 from module_build_service.common import conf, models
 from module_build_service.common.errors import StreamAmbigous, StreamNotXyz
-from module_build_service.common.logger import level_flags
 from module_build_service.common.utils import load_mmd_file, import_mmd
 import module_build_service.scheduler.consumer
 from module_build_service.scheduler.db_session import db_session
 import module_build_service.scheduler.local
 from module_build_service.web.submit import submit_module_build_from_yaml
 
-
-def create_app(debug=False, verbose=False, quiet=False):
-    # logging (intended for flask-script, see manage.py)
-    log = logging.getLogger(__name__)
-    if debug:
-        log.setLevel(level_flags["debug"])
-    elif verbose:
-        log.setLevel(level_flags["verbose"])
-    elif quiet:
-        log.setLevel(level_flags["quiet"])
-
-    return app
-
-
-manager = Manager(create_app)
-help_args = ("-?", "--help")
-manager.help_args = help_args
-migrations_dir = os.path.join(os.path.abspath(os.path.dirname(__file__)),
-                              'migrations')
+migrations_dir = os.path.join(os.path.abspath(os.path.dirname(__file__)), "migrations")
 migrate = flask_migrate.Migrate(app, db, directory=migrations_dir)
-manager.add_command("db", flask_migrate.MigrateCommand)
-manager.add_option("-d", "--debug", dest="debug", action="store_true")
-manager.add_option("-v", "--verbose", dest="verbose", action="store_true")
-manager.add_option("-q", "--quiet", dest="quiet", action="store_true")
 
 
-def console_script_help(f):
-    @wraps(f)
-    def wrapped(*args, **kwargs):
-        import sys
-
-        if any([arg in help_args for arg in sys.argv[1:]]):
-            command = os.path.basename(sys.argv[0])
-            print(textwrap.dedent(
-                """\
-                    {0}
-
-                    Usage: {0} [{1}]
-
-                    See also:
-                    mbs-manager(1)
-                """).strip().format(command, "|".join(help_args))
-            )
-            sys.exit(2)
-        r = f(*args, **kwargs)
-        return r
-
-    return wrapped
+@click.group(cls=FlaskGroup, create_app=lambda *args, **kwargs: app)
+def cli():
+    """MBS manager"""
 
 
-@console_script_help
-@manager.command
+@cli.command("upgradedb")
 def upgradedb():
     """ Upgrades the database schema to the latest revision
     """
     app.config["SERVER_NAME"] = "localhost"
-    # TODO: configurable?
-    migrations_dir = os.path.join(os.path.abspath(os.path.dirname(__file__)), "migrations")
     with app.app_context():
         flask_migrate.upgrade(directory=migrations_dir)
 
 
-@manager.command
+def upgradedb_entrypoint():
+    """Entrypoint for command mbs-upgradedb"""
+    # Work around issue with FlaskGroup not being initiated
+    cli(["upgradedb"])
+
+
+@cli.command("cleardb")
 def cleardb():
     """ Clears the database
     """
@@ -96,8 +56,8 @@ def cleardb():
     models.ComponentBuild.query.delete()
 
 
-@console_script_help
-@manager.command
+@cli.command("import_module")
+@click.argument("mmd_file", type=click.Path(exists=True))
 def import_module(mmd_file):
     """ Imports the module from mmd_file
     """
@@ -117,41 +77,56 @@ def collect_dep_overrides(overrides):
     return collected
 
 
-@manager.option("--stream", action="store", dest="stream")
-@manager.option("--file", action="store", dest="yaml_file")
-@manager.option("--srpm", action="append", default=[], dest="srpms", metavar="SRPM")
-@manager.option("--skiptests", action="store_true", dest="skiptests")
-@manager.option("--offline", action="store_true", dest="offline")
-@manager.option(
-    '--buildrequires', action='append', metavar='name:stream',
-    dest='buildrequires', default=[],
+@cli.command("build_module_locally")
+@click.option("--stream", metavar="STREAM")
+@click.option(
+    "--file", "yaml_file",
+    metavar="FILE",
+    required=True,
+    type=click.Path(exists=True),
+)
+@click.option("--srpm", "srpms", metavar="SRPM", multiple=True)
+@click.option("--skiptests", is_flag=True)
+@click.option("--offline", is_flag=True)
+@click.option(
+    '--buildrequires', "buildrequires", multiple=True,
+    metavar='name:stream', default=[],
     help='Buildrequires to override in the form of "name:stream"'
 )
-@manager.option(
-    '--requires', action='append', metavar='name:stream',
-    dest='requires', default=[],
+@click.option(
+    '--requires', "requires", multiple=True,
+    metavar='name:stream', default=[],
     help='Requires to override in the form of "name:stream"'
 )
-@manager.option("-d", "--debug", action="store_true", dest="log_debug")
-@manager.option("-l", "--add-local-build", action="append", default=None, dest="local_build_nsvs")
-@manager.option("-s", "--set-stream", action="append", default=[], dest="default_streams")
-@manager.option(
-    "-r", "--platform-repo-file", action="append", default=[], dest="platform_repofiles"
+@click.option("-d", "--debug", "log_debug", is_flag=True)
+@click.option(
+    "-l", "--add-local-build", "local_build_nsvs",
+    metavar="NSV", multiple=True
 )
-@manager.option("-p", "--platform-id", action="store", default=None, dest="platform_id")
+@click.option(
+    "-s", "--set-stream", "default_streams",
+    metavar="STREAM", multiple=True
+)
+@click.option(
+    "-r", "--platform-repo-file", "platform_repofiles",
+    metavar="FILE",
+    type=click.Path(exists=True),
+    multiple=True
+)
+@click.option("-p", "--platform-id", metavar="PLATFORM_ID")
 def build_module_locally(
-    local_build_nsvs=None,
+    stream=None,
     yaml_file=None,
     srpms=None,
-    stream=None,
     skiptests=False,
-    default_streams=None,
     offline=False,
+    log_debug=False,
+    local_build_nsvs=None,
+    default_streams=None,
     platform_repofiles=None,
     platform_id=None,
     requires=None,
     buildrequires=None,
-    log_debug=False,
 ):
     """ Performs local module build using Mock
     """
@@ -233,14 +208,11 @@ def build_module_locally(
         sys.exit(1)
 
 
-@manager.option(
-    "identifier",
-    metavar="NAME:STREAM[:VERSION[:CONTEXT]]",
-    help="Identifier for selecting module builds to retire",
-)
-@manager.option(
+@cli.command("retire")
+@click.argument("identifier", metavar="NAME:STREAM[:VERSION[:CONTEXT]]")
+@click.option(
     "--confirm",
-    action="store_true",
+    is_flag=True,
     default=False,
     help="Perform retire operation without prompting",
 )
@@ -273,7 +245,8 @@ def retire(identifier, confirm=False):
         logging.info("\t%s", ":".join((build.name, build.stream, build.version, build.context)))
 
     # Prompt for confirmation
-    is_confirmed = confirm or prompt_bool("Retire {} module builds?".format(len(module_builds)))
+    confirm_msg = "Retire {} module builds?".format(len(module_builds))
+    is_confirmed = confirm or click.confirm(confirm_msg, abort=False)
     if not is_confirmed:
         logging.info("Module builds were NOT retired.")
         return
@@ -288,8 +261,10 @@ def retire(identifier, confirm=False):
     logging.info("Module builds retired.")
 
 
-@console_script_help
-@manager.command
+@cli.command("run")
+@click.option("-h", "--host", metavar="HOST", help="Bind to this host.")
+@click.option("-p", "--port", metavar="PORT", help="Bind to this port along with --host.")
+@click.option("-d", "--debug", is_flag=True, default=False, help="Run frontend in debug mode.")
 def run(host=None, port=None, debug=None):
     """ Runs the Flask app, locally. Intended for dev instances, should not be used for production.
     """
@@ -302,9 +277,5 @@ def run(host=None, port=None, debug=None):
     app.run(host=host, port=port, debug=debug)
 
 
-def manager_wrapper():
-    manager.run()
-
-
 if __name__ == "__main__":
-    manager_wrapper()
+    cli()

module_build_service/scheduler/route.py

@@ -31,7 +31,7 @@ def route_task(name, args, kwargs, options, task=None, **kw):
     # handlers can be decorated, inspect the original function
     while getattr(handler, "__wrapped__", None):
         handler = handler.__wrapped__
-    handler_args = inspect.getargspec(handler).args
+    handler_args = inspect.getfullargspec(handler).args
 
     def _get_handler_arg(name):
         index = handler_args.index(name)

openshift/backend/Dockerfile

@@ -29,6 +29,7 @@ COPY --from=builder /src/dist /src/dist
 
 RUN $DNF_CMD install \
       python3-celery \
+      python3-click \
       python3-distro \
       python3-dnf \
       python3-docopt \
@@ -59,12 +60,14 @@ RUN $DNF_CMD install \
       python3-solv \
       python3-sqlalchemy \
       git-core \
+      rpm-build \
       $EXTRA_RPMS && \
     rm -f /etc/fedmsg.d/* && \
     $DNF_CMD clean all
 # Install locally available python wheels only.
 # Any new/updated dependencies should be installed via rpm.
 RUN pip-3 install --no-deps /src/dist/*.whl
+RUN mkdir /etc/mbs-messaging-umb && ln -s $(pip-3 show mbs_messaging_umb | awk '/^Location:/ {print $2}')/etc/mbs-messaging-umb/config.py /etc/mbs-messaging-umb/config.py
 
 USER 1001
 

requirements.txt

@@ -1,10 +1,10 @@
+click
 distro
 dogpile.cache
 enum34
 fedmsg
 Flask
 Flask-Migrate
-Flask-Script
 Flask-SQLAlchemy
 funcsigs # Python2 only
 futures # Python 2 only

setup.py

@@ -39,9 +39,9 @@ setup(
     dependency_links=deps_links,
     entry_points={
         "console_scripts": [
-            "mbs-upgradedb = module_build_service.manage:upgradedb",
+            "mbs-upgradedb = module_build_service.manage:upgradedb_entrypoint",
             "mbs-frontend = module_build_service.manage:run",
-            "mbs-manager = module_build_service.manage:manager_wrapper",
+            "mbs-manager = module_build_service.manage:cli",
         ],
         "moksha.consumer": "mbsconsumer = module_build_service.scheduler.consumer:MBSConsumer",
         "mbs.messaging_backends": [

tests/test_manage.py

@@ -6,10 +6,9 @@ import logging
 from mock import patch
 import pytest
 
-from module_build_service import app
+from module_build_service import app, manage as mbs_manager
 from module_build_service.common import models
 from module_build_service.common.models import BUILD_STATES, ModuleBuild
-from module_build_service.manage import manager_wrapper, retire
 from module_build_service.scheduler.db_session import db_session
 from module_build_service.web.utils import deps_to_dict
 from tests import clean_database, staged_data_filename
@@ -31,10 +30,12 @@ class TestMBSManage:
     )
     def test_retire_identifier_validation(self, identifier, is_valid):
         if is_valid:
-            retire(identifier)
+            with pytest.raises(SystemExit) as exc_info:
+                mbs_manager.cli(["retire", identifier])
+                assert 0 == exc_info
         else:
             with pytest.raises(ValueError):
-                retire(identifier)
+                mbs_manager.cli(["retire", identifier])
 
     @pytest.mark.parametrize(
         ("overrides", "identifier", "changed_count"),
@@ -48,9 +49,9 @@ class TestMBSManage:
             ({"context": "pickme"}, "spam:eggs:ham", 2),
         ),
     )
-    @patch("module_build_service.manage.prompt_bool")
-    def test_retire_build(self, prompt_bool, overrides, identifier, changed_count):
-        prompt_bool.return_value = True
+    @patch("click.confirm")
+    def test_retire_build(self, confirm, overrides, identifier, changed_count):
+        confirm.return_value = True
 
         module_builds = (
             db_session.query(ModuleBuild)
@@ -72,7 +73,10 @@ class TestMBSManage:
 
         db_session.commit()
 
-        retire(identifier)
+        with pytest.raises(SystemExit) as exc_info:
+            mbs_manager.cli(["retire", identifier])
+            assert 0 == exc_info.value
+
         retired_module_builds = (
             db_session.query(ModuleBuild)
             .filter_by(state=BUILD_STATES["garbage"])
@@ -94,11 +98,11 @@ class TestMBSManage:
             (False, True, True)
         ),
     )
-    @patch("module_build_service.manage.prompt_bool")
+    @patch("click.confirm")
     def test_retire_build_confirm_prompt(
-        self, prompt_bool, confirm_prompt, confirm_arg, confirm_expected
+        self, confirm, confirm_prompt, confirm_arg, confirm_expected
     ):
-        prompt_bool.return_value = confirm_prompt
+        confirm.return_value = confirm_prompt
 
         module_builds = db_session.query(ModuleBuild).filter_by(state=BUILD_STATES["ready"]).all()
         # Verify our assumption of the amount of ModuleBuilds in database
@@ -107,15 +111,17 @@ class TestMBSManage:
         for x, build in enumerate(module_builds):
             build.name = "spam" + str(x) if x > 0 else "spam"
             build.stream = "eggs"
-
         db_session.commit()
 
-        retire("spam:eggs", confirm_arg)
+        cmd = ["retire", "spam:eggs"] + (["--confirm"] if confirm_arg else [])
+        with pytest.raises(SystemExit) as exc_info:
+            mbs_manager.cli(cmd)
+            assert 0 == exc_info.value
+
+        expected_changed_count = 1 if confirm_expected else 0
         retired_module_builds = (
             db_session.query(ModuleBuild).filter_by(state=BUILD_STATES["garbage"]).all()
         )
-
-        expected_changed_count = 1 if confirm_expected else 0
         assert len(retired_module_builds) == expected_changed_count
 
 
@@ -157,7 +163,7 @@ class TestCommandBuildModuleLocally:
         original_db_uri = app.config["SQLALCHEMY_DATABASE_URI"]
         try:
             with patch("sys.argv", new=cli_cmd):
-                manager_wrapper()
+                mbs_manager.cli()
         finally:
             app.config["SQLALCHEMY_DATABASE_URI"] = original_db_uri