From b2198e3c78eb5bf2074ac714927d9f28b6bdc153 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Mar 11 2020 15:07:04 +0000 Subject: mbs-manager uses click to replace Flask-Script Resolves: FACTORY-4876 Signed-off-by: Chenxiong Qi --- diff --git a/module_build_service/manage.py b/module_build_service/manage.py index 925b503..d07b7c5 100755 --- a/module_build_service/manage.py +++ b/module_build_service/manage.py @@ -1,14 +1,13 @@ # -*- coding: utf-8 -*- # SPDX-License-Identifier: MIT from __future__ import absolute_import, print_function -from functools import wraps +import click import getpass import logging import os -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 @@ -17,76 +16,39 @@ from module_build_service.builder.MockModuleBuilder import ( ) from module_build_service.common import conf, models from module_build_service.common.errors import StreamAmbigous -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}] +@click.group(cls=FlaskGroup, create_app=lambda *args, **kwargs: app) +def cli(): + """MBS manager""" - See also: - mbs-manager(1) - """).strip().format(command, "|".join(help_args)) - ) - sys.exit(2) - r = f(*args, **kwargs) - return r - return wrapped +cli.command("db", flask_migrate.MigrateCommand) -@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""" + cli(["db", "upgrade"]) + + +@cli.command("cleardb") def cleardb(): """ Clears the database """ @@ -94,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 """ @@ -103,29 +65,44 @@ def import_module(mmd_file): import_mmd(db.session, mmd) -@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("-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" +@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("-d", "--debug", "log_debug", is_flag=True) +@click.option( + "-l", "--add-local-build", "local_build_nsvs", + metavar="NSV", multiple=True +) +@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 ) -@manager.option("-p", "--platform-id", action="store", default=None, dest="platform_id") +@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, - log_debug=False, ): """ Performs local module build using Mock """ @@ -205,14 +182,11 @@ def build_module_locally( raise RuntimeError("Module build failed") -@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", ) @@ -245,7 +219,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 @@ -260,8 +235,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. """ @@ -274,9 +251,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() diff --git a/requirements.txt b/requirements.txt index 9ea96f5..17ea4df 100644 --- a/requirements.txt +++ b/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 diff --git a/setup.py b/setup.py index 5ead53c..fcd2b2c 100644 --- a/setup.py +++ b/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": [ diff --git a/tests/test_manage.py b/tests/test_manage.py index f70209b..4bf134b 100644 --- a/tests/test_manage.py +++ b/tests/test_manage.py @@ -5,10 +5,9 @@ from __future__ import absolute_import 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 @@ -30,10 +29,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"), @@ -47,9 +48,9 @@ class TestMBSManage: ({"context": "pickme"}, "spam:eggs:ham", 3), ), ) - @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) @@ -71,7 +72,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"]) @@ -93,11 +97,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 @@ -106,15 +110,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 @@ -156,7 +162,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