Skip to content
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

Sanity checking for function imports #802

Merged
merged 5 commits into from
Apr 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"expression": "rate(1 minute)"
},
{
"function": "test.test_app.method",
"function": "tests.test_app.method",
"event_source": {
"arn": "arn:aws:sns:::1",
"events": [
Expand Down
2 changes: 2 additions & 0 deletions tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@ def callback(self):
def prebuild_me():
print("this is a prebuild script")

def method():
print("this is a method")
2 changes: 1 addition & 1 deletion tests/test_bad_circular_extends_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"expression": "rate(1 minute)"
},
{
"function": "test.test_app.method",
"function": "tests.test_app.method",
"event_source": {
"arn": "arn:aws:sns:::1",
"events": [
Expand Down
16 changes: 16 additions & 0 deletions tests/test_bad_module_paths.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"ttt888": {
"s3_bucket": "lmbda",
"app_function": "tests.test_app.hello_world",
"delete_local_zip": true,
"debug": true,
"parameter_depth": 2,
"prebuild_script": "tests.test_app.prebuild_me",
"events": [
{
"function": "tests.test_app.not_a_function",
"expression": "rate(1 minute)"
}
]
}
}
2 changes: 1 addition & 1 deletion tests/test_bad_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"expression": "rate(1 minute)"
},
{
"function": "test.test_app.method",
"function": "tests.test_app.method",
"event_source": {
"arn": "arn:aws:sns:::1",
"events": [
Expand Down
2 changes: 1 addition & 1 deletion tests/test_bad_stage_name_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"expression": "rate(1 minute)"
},
{
"function": "test.test_app.method",
"function": "tests.test_app.method",
"event_source": {
"arn": "arn:aws:sns:::1",
"events": [
Expand Down
2 changes: 1 addition & 1 deletion tests/test_settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ ttt888:
events:
- function: tests.test_app.schedule_me
expression: rate(1 minute)
- function: test.test_app.method
- function: tests.test_app.method
event_source:
arn: arn:aws:sns:::1
events:
Expand Down
6 changes: 6 additions & 0 deletions tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,12 @@ def test_bad_environment_vars_catch(self):
zappa_cli.api_stage = 'ttt888'
self.assertRaises(ValueError, zappa_cli.load_settings, 'tests/test_bad_environment_vars.json')

def test_function_sanity_check(self):
zappa_cli = ZappaCLI()
self.assertRaises(ClickException, zappa_cli.function_sanity_check, 'not_a_module.foo')
self.assertRaises(ClickException, zappa_cli.function_sanity_check, 'tests.test_app.not_a_function')
self.assertRaises(ClickException, zappa_cli.load_settings, 'test/test_bad_module_paths.json')

# @mock.patch('botocore.session.Session.full_config', new_callable=mock.PropertyMock)
# def test_cli_init(self, mock_config):

Expand Down
33 changes: 28 additions & 5 deletions zappa/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,9 @@ def schedule(self):
return

for event in events:
self.collision_warning(event.get('function'))
if event.get('name') == 'zappa-keep-warm':
continue
self.function_sanity_check(event.get('function'))

if self.stage_config.get('keep_warm', True):
if not events:
Expand Down Expand Up @@ -1724,12 +1726,15 @@ def load_settings(self, settings_file=None, session=None):
setattr(self.zappa, setting, setting_val)

if self.app_function:
self.collision_warning(self.app_function)
self.function_sanity_check(self.app_function)
if self.app_function[-3:] == '.py':
click.echo(click.style("Warning!", fg="red", bold=True) +
" Your app_function is pointing to a " + click.style("file and not a function", bold=True) +
"! It should probably be something like 'my_file.app', not 'my_file.py'!")

if self.exception_handler:
self.function_sanity_check(self.exception_handler)

return self.zappa

def get_json_or_yaml_settings(self, settings_name="zappa_settings"):
Expand Down Expand Up @@ -2184,12 +2189,13 @@ def execute_prebuild_script(self):
prebuild_function = getattr(module_, pb_func)
prebuild_function() # Call the function

def collision_warning(self, item):
def function_sanity_check(self, item):
"""
Given a string, print a warning if this could
collide with a Zappa core package module.
collide with a Zappa core package module, or if the
given function could not be correctly imported.

Use for app functions and events.
Use for app functions, event functions, and exception handlers.
"""

namespace_collisions = [
Expand All @@ -2201,6 +2207,23 @@ def collision_warning(self, item):
" You may have a namespace collision with " + click.style(item, bold=True) +
"! You may want to rename that file.")

module_path, function_path = item.rsplit('.', 1)
try:
module = importlib.import_module(module_path)
except ImportError: # pragma: no cover
raise ClickException(
"You have tried to reference the module " + click.style(module_path, bold=True) +
" when importing " + click.style(item, bold=True) + ", but this module cannot be " +
"imported. Please check for typos and ensure that this module is reachable.")

try:
getattr(module, function_path)
except AttributeError: # pragma: no cover
raise ClickException(
"You have tried to reference the function " + click.style(function_path, bold=True) +
" when importing " + click.style(item, bold=True) + ", but this function cannot be " +
"imported. Please check for typos and ensure that this function is reachable.")

def deploy_api_gateway(self, api_id):
cache_cluster_enabled = self.stage_config.get('cache_cluster_enabled', False)
cache_cluster_size = str(self.stage_config.get('cache_cluster_size', .5))
Expand Down