Skip to content

Commit

Permalink
Merge pull request #802 from bcongdon/function_checker
Browse files Browse the repository at this point in the history
Sanity checking for function imports
  • Loading branch information
Rich Jones authored Apr 24, 2017
2 parents c2525a3 + ca39b8c commit d0fe596
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 10 deletions.
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

0 comments on commit d0fe596

Please sign in to comment.