From 7106fb63578deef49741096d3d5aa04db89e15ae Mon Sep 17 00:00:00 2001 From: Hendrik Makait Date: Tue, 23 May 2017 16:56:34 -0700 Subject: [PATCH 1/3] Handle app factory with arguments in FLASK_APP --- flask/cli.py | 52 ++++++++++++++++++++++++++----- tests/test_apps/cliapp/factory.py | 15 +++++++++ tests/test_cli.py | 32 ++++++++++++++----- 3 files changed, 84 insertions(+), 15 deletions(-) create mode 100644 tests/test_apps/cliapp/factory.py diff --git a/flask/cli.py b/flask/cli.py index e6e8f65f57..befa29d35c 100644 --- a/flask/cli.py +++ b/flask/cli.py @@ -9,7 +9,10 @@ :license: BSD, see LICENSE for more details. """ +import ast +import inspect import os +import re import sys import traceback from functools import update_wrapper @@ -55,20 +58,20 @@ def find_best_app(script_info, module): ' one.'.format(module=module.__name__) ) - # Search for app factory callables. + # Search for app factory functions. for attr_name in ('create_app', 'make_app'): app_factory = getattr(module, attr_name, None) - if callable(app_factory): + if inspect.isfunction(app_factory): try: app = call_factory(app_factory, script_info) if isinstance(app, Flask): return app except TypeError: raise NoAppException( - 'Auto-detected "{callable}()" in module "{module}", but ' + 'Auto-detected "{function}()" in module "{module}", but ' 'could not call it without specifying arguments.'.format( - callable=attr_name, module=module.__name__ + function=attr_name, module=module.__name__ ) ) @@ -150,10 +153,45 @@ def locate_app(script_info, app_id): if app_obj is None: app = find_best_app(script_info, mod) else: - app = getattr(mod, app_obj, None) + function_regex = r'^([\w_][\w_\d]*)\((.*)\)$' + match = re.match(function_regex, app_obj) + try: + if match: + function_name = match.group(1) + arguments = match.group(2) + if arguments: + arguments = ast.literal_eval( + "({arguments}, )".format(arguments=arguments)) + else: + arguments = () + app_factory = getattr(mod, function_name, None) + app_factory_arg_names = getargspec(app_factory).args + if 'script_info' in app_factory_arg_names: + app = app_factory(*arguments, script_info=script_info) + elif arguments: + app = app_factory(*arguments) + elif not arguments and len(app_factory_arg_names) == 1: + app = app_factory(script_info) + else: + app = app_factory() + else: + attr = getattr(mod, app_obj, None) + if inspect.isfunction(attr): + app = call_factory(attr, script_info) + else: + app = attr + except TypeError as e: + new_error = NoAppException( + '{e}\nThe app factory "{factory}" in module "{module}" could' + ' not be called with the specified arguments (and a' + ' script_info argument automatically added if applicable).' + ' Did you make sure to use the right number of arguments as' + ' well as not using keyword arguments or' + ' non-literals?'.format(e=e, factory=app_obj, module=module)) + reraise(NoAppException, new_error, sys.exc_info()[2]) if app is None: - raise RuntimeError('Failed to find application in module "%s"' - % module) + raise RuntimeError('Failed to find application in module ' + '"{name}"'.format(name=module)) return app diff --git a/tests/test_apps/cliapp/factory.py b/tests/test_apps/cliapp/factory.py new file mode 100644 index 0000000000..b0d4771e8c --- /dev/null +++ b/tests/test_apps/cliapp/factory.py @@ -0,0 +1,15 @@ +from __future__ import absolute_import, print_function + +from flask import Flask + + +def create_app(): + return Flask('create_app') + + +def create_app2(foo, bar): + return Flask("_".join(['create_app2', foo, bar])) + + +def create_app3(foo, bar, script_info): + return Flask("_".join(['create_app3', foo, bar])) diff --git a/tests/test_cli.py b/tests/test_cli.py index 459d6ef95f..4f92f50da4 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -150,15 +150,31 @@ def test_locate_app(test_apps): script_info = ScriptInfo() assert locate_app(script_info, "cliapp.app").name == "testapp" assert locate_app(script_info, "cliapp.app:testapp").name == "testapp" + assert locate_app(script_info, "cliapp.factory").name == "create_app" + assert locate_app( + script_info, "cliapp.factory").name == "create_app" + assert locate_app( + script_info, "cliapp.factory:create_app").name == "create_app" + assert locate_app( + script_info, "cliapp.factory:create_app()").name == "create_app" + assert locate_app( + script_info, "cliapp.factory:create_app2('foo', 'bar')" + ).name == "create_app2_foo_bar" + assert locate_app( + script_info, "cliapp.factory:create_app3('baz', 'qux')" + ).name == "create_app3_baz_qux" assert locate_app(script_info, "cliapp.multiapp:app1").name == "app1" - pytest.raises(NoAppException, locate_app, - script_info, "notanpp.py") - pytest.raises(NoAppException, locate_app, - script_info, "cliapp/app") - pytest.raises(RuntimeError, locate_app, - script_info, "cliapp.app:notanapp") - pytest.raises(NoAppException, locate_app, - script_info, "cliapp.importerrorapp") + pytest.raises( + NoAppException, locate_app, script_info, "notanpp.py") + pytest.raises( + NoAppException, locate_app, script_info, "cliapp/app") + pytest.raises( + RuntimeError, locate_app, script_info, "cliapp.app:notanapp") + pytest.raises( + NoAppException, locate_app, + script_info, "cliapp.factory:create_app2('foo')") + pytest.raises( + NoAppException, locate_app, script_info, "cliapp.importerrorapp") def test_find_default_import_path(test_apps, monkeypatch, tmpdir): From 7a1a594b2649890fca5d0e2a24db111046d68f39 Mon Sep 17 00:00:00 2001 From: Hendrik Makait Date: Tue, 23 May 2017 22:33:48 -0700 Subject: [PATCH 2/3] Factor out call_factory_from_regex function --- flask/cli.py | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/flask/cli.py b/flask/cli.py index befa29d35c..c4f2fbc189 100644 --- a/flask/cli.py +++ b/flask/cli.py @@ -96,6 +96,29 @@ def call_factory(func, script_info): return func() +def call_factory_from_regex(match, mod, script_info): + """Checks if the given regex match has specified arguments and if the + function takes a ``script_info`` argument and calls the function with + the appropriate arguments.""" + function_name = match.group(1) + arguments = match.group(2) + if arguments: + arguments = ast.literal_eval( + "({arguments}, )".format(arguments=arguments)) + else: + arguments = () + app_factory = getattr(mod, function_name, None) + app_factory_arg_names = getargspec(app_factory).args + if 'script_info' in app_factory_arg_names: + app = app_factory(*arguments, script_info=script_info) + elif arguments: + app = app_factory(*arguments) + elif not arguments and len(app_factory_arg_names) == 1: + app = app_factory(script_info) + else: + app = app_factory() + return app + def prepare_exec_for_file(filename): """Given a filename this will try to calculate the python path, add it to the search path and return the actual module name that is expected. @@ -157,23 +180,7 @@ def locate_app(script_info, app_id): match = re.match(function_regex, app_obj) try: if match: - function_name = match.group(1) - arguments = match.group(2) - if arguments: - arguments = ast.literal_eval( - "({arguments}, )".format(arguments=arguments)) - else: - arguments = () - app_factory = getattr(mod, function_name, None) - app_factory_arg_names = getargspec(app_factory).args - if 'script_info' in app_factory_arg_names: - app = app_factory(*arguments, script_info=script_info) - elif arguments: - app = app_factory(*arguments) - elif not arguments and len(app_factory_arg_names) == 1: - app = app_factory(script_info) - else: - app = app_factory() + app = call_factory_from_regex(match, mod, script_info) else: attr = getattr(mod, app_obj, None) if inspect.isfunction(attr): From 48c2925664e154711e97385cc197be8bbb59572d Mon Sep 17 00:00:00 2001 From: Hendrik Makait Date: Thu, 25 May 2017 11:28:20 -0700 Subject: [PATCH 3/3] Factor in code review comments and refactor functions to be more naturally split. --- flask/cli.py | 119 +++++++++++++++++++++++----------------------- tests/test_cli.py | 6 +++ 2 files changed, 66 insertions(+), 59 deletions(-) diff --git a/flask/cli.py b/flask/cli.py index c4f2fbc189..fd62300d2a 100644 --- a/flask/cli.py +++ b/flask/cli.py @@ -82,42 +82,67 @@ def find_best_app(script_info, module): ) -def call_factory(func, script_info): - """Checks if the given app factory function has an argument named - ``script_info`` or just a single argument and calls the function passing - ``script_info`` if so. Otherwise, calls the function without any arguments - and returns the result. +def call_factory(app_factory, script_info, arguments=()): + """Takes an app factory, a ``script_info` object and optionally a tuple + of arguments. Checks for the existence of a script_info argument and calls + the app_factory depending on that and the arguments provided. """ - arguments = getargspec(func).args - if 'script_info' in arguments: - return func(script_info=script_info) - elif len(arguments) == 1: - return func(script_info) - return func() - - -def call_factory_from_regex(match, mod, script_info): - """Checks if the given regex match has specified arguments and if the - function takes a ``script_info`` argument and calls the function with - the appropriate arguments.""" - function_name = match.group(1) - arguments = match.group(2) - if arguments: - arguments = ast.literal_eval( - "({arguments}, )".format(arguments=arguments)) - else: - arguments = () - app_factory = getattr(mod, function_name, None) - app_factory_arg_names = getargspec(app_factory).args - if 'script_info' in app_factory_arg_names: - app = app_factory(*arguments, script_info=script_info) + arg_names = getargspec(app_factory).args + if 'script_info' in arg_names: + return app_factory(*arguments, script_info=script_info) elif arguments: - app = app_factory(*arguments) - elif not arguments and len(app_factory_arg_names) == 1: - app = app_factory(script_info) + return app_factory(*arguments) + elif not arguments and len(arg_names) == 1: + return app_factory(script_info) + return app_factory() + + +def find_app_by_string(string, script_info, module): + """Checks if the given string is a variable name or a function. If it is + a function, it checks for specified arguments and whether it takes + a ``script_info`` argument and calls the function with the appropriate + arguments. If it is a """ + from . import Flask + function_regex = r'^(?P\w+)(?:\((?P.*)\))?$' + match = re.match(function_regex, string) + if match: + name, args = match.groups() + try: + if args is not None: + args = args.rstrip(' ,') + if args: + args = ast.literal_eval( + "({args}, )".format(args=args)) + else: + args = () + app_factory = getattr(module, name, None) + app = call_factory(app_factory, script_info, args) + else: + attr = getattr(module, name, None) + if inspect.isfunction(attr): + app = call_factory(attr, script_info) + else: + app = attr + + if isinstance(app, Flask): + return app + else: + raise RuntimeError('Failed to find application in module ' + '"{name}"'.format(name=module)) + except TypeError as e: + new_error = NoAppException( + '{e}\nThe app factory "{factory}" in module "{module}" could' + ' not be called with the specified arguments (and a' + ' script_info argument automatically added if applicable).' + ' Did you make sure to use the right number of arguments as' + ' well as not using keyword arguments or' + ' non-literals?'.format(e=e, factory=string, module=module)) + reraise(NoAppException, new_error, sys.exc_info()[2]) else: - app = app_factory() - return app + raise NoAppException( + 'The provided string "{string}" is not a valid variable name' + 'or function expression.'.format(string=string)) + def prepare_exec_for_file(filename): """Given a filename this will try to calculate the python path, add it @@ -174,33 +199,9 @@ def locate_app(script_info, app_id): mod = sys.modules[module] if app_obj is None: - app = find_best_app(script_info, mod) + return find_best_app(script_info, mod) else: - function_regex = r'^([\w_][\w_\d]*)\((.*)\)$' - match = re.match(function_regex, app_obj) - try: - if match: - app = call_factory_from_regex(match, mod, script_info) - else: - attr = getattr(mod, app_obj, None) - if inspect.isfunction(attr): - app = call_factory(attr, script_info) - else: - app = attr - except TypeError as e: - new_error = NoAppException( - '{e}\nThe app factory "{factory}" in module "{module}" could' - ' not be called with the specified arguments (and a' - ' script_info argument automatically added if applicable).' - ' Did you make sure to use the right number of arguments as' - ' well as not using keyword arguments or' - ' non-literals?'.format(e=e, factory=app_obj, module=module)) - reraise(NoAppException, new_error, sys.exc_info()[2]) - if app is None: - raise RuntimeError('Failed to find application in module ' - '"{name}"'.format(name=module)) - - return app + return find_app_by_string(app_obj, script_info, mod) def find_default_import_path(): diff --git a/tests/test_cli.py b/tests/test_cli.py index 4f92f50da4..899fb1f070 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -160,6 +160,9 @@ def test_locate_app(test_apps): assert locate_app( script_info, "cliapp.factory:create_app2('foo', 'bar')" ).name == "create_app2_foo_bar" + assert locate_app( + script_info, "cliapp.factory:create_app2('foo', 'bar', )" + ).name == "create_app2_foo_bar" assert locate_app( script_info, "cliapp.factory:create_app3('baz', 'qux')" ).name == "create_app3_baz_qux" @@ -173,6 +176,9 @@ def test_locate_app(test_apps): pytest.raises( NoAppException, locate_app, script_info, "cliapp.factory:create_app2('foo')") + pytest.raises( + NoAppException, locate_app, + script_info, "cliapp.factory:create_app ()") pytest.raises( NoAppException, locate_app, script_info, "cliapp.importerrorapp")