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

Feature/cli rework #432

Closed
wants to merge 5 commits into from
Closed
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
6 changes: 5 additions & 1 deletion tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,11 @@ def test_cli_args(self):
zappa_cli = ZappaCLI()
# Sanity
argv = '-s test_settings.json derp ttt888'.split()
zappa_cli.handle(argv)
with self.assertRaises(SystemExit) as system_exit:
# https://github.com/Miserlou/Zappa/issues/404
# Previously returned with printed warning. Now exists as derp is not a valid command.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exists -> exits

zappa_cli.handle(argv)
self.assertNotEqual(system_exit.exception.code, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not check this against the actual return code? argparse will always exit with a code 2 for an incorrect argument.


def test_cli_error_exit_code(self):
# Discussion: https://github.com/Miserlou/Zappa/issues/407
Expand Down
124 changes: 72 additions & 52 deletions zappa/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,21 +54,6 @@
'touch',
]

CLI_COMMANDS = [
'certify',
'deploy',
'init',
'invoke',
'manage',
'rollback',
'schedule',
'status',
'tail',
'undeploy',
'unschedule',
'update'
]

##
# Main Input Processing
##
Expand Down Expand Up @@ -132,49 +117,84 @@ def handle(self, argv=None):
Parses command, load settings and dispatches accordingly.

"""
help_message = "Please supply a command to execute. Can be one of: {}".format(', '.join(x for x in sorted(CLI_COMMANDS)))

parser = argparse.ArgumentParser(description='Zappa - Deploy Python applications to AWS Lambda and API Gateway.\n')
parser.add_argument('command_env', metavar='U', type=str, nargs='*', help=help_message)
parser.add_argument('-n', '--num-rollback', type=int, default=0,
help='The number of versions to rollback.')
parser = argparse.ArgumentParser(description='Zappa - Deploy Python applications to AWS Lambda and API Gateway.\n', version=pkg_resources.require("zappa")[0].version, prog="zappa")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat, I've never seen version= used here to provide a version argument. TIL.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, argparse has version built in. https://docs.python.org/2.7/library/argparse.html#action

parser.add_argument('-s', '--settings_file', type=str, default='zappa_settings.json',
help='The path to a zappa settings file.')
help='The path to a zappa settings file.')
parser.add_argument('-a', '--app_function', type=str, default=None,
help='The WSGI application function.')
parser.add_argument('-v', '--version', action='store_true', help='Print the zappa version', default=False)
parser.add_argument('-y', '--yes', action='store_true', help='Auto confirm yes', default=False)
parser.add_argument('--remove-logs', action='store_true', help='Removes log groups of api gateway and lambda task during the undeployment.', default=False)
parser.add_argument('--raw', action='store_true', help='When invoking remotely, invoke this python as a string, not as a modular path.', default=False)
parser.add_argument('--no-cleanup', action='store_true', help="Don't remove certificate files from /tmp during certify. Dangerous.", default=False)
parser.add_argument('--all', action='store_true', help="Execute this command for all of our defined Zappa environments.", default=False)
parser.add_argument('--json', action='store_true', help='Returns status in JSON format', default=False) # https://github.com/Miserlou/Zappa/issues/407
help='The WSGI application function.')

subparsers = parser.add_subparsers(title="Zappa Commands", help="Choose a valid zappa command", dest='command')

# Parent Parser
# Functionality shared by all commands
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this functionality is shared by all commands. zappa --all init doesn't make much sense for example.

parent_parser = argparse.ArgumentParser(add_help=False)
parent_parser.add_argument('environment', type=str, help='Deployment environment', default=None, nargs='?')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str is the default type so you don't need to specify it. I also suspect this won't work. Taking only 0 or 1 arguments won't work for commands like manage that need to take an arbitrary number of arguments after the environment.

parent_parser.add_argument('-y', '--yes', action='store_true', help='Auto confirm yes', default=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT the yes argument is only used by the undeploy command.

parent_parser.add_argument('--all', action='store_true', help="Execute this command for all of the defined Zappa environments.", default=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of these lines are > 80 characters.


# Certify command
certify_help = "Use Let's Encrypt to setup SSL to the domain specified in the config"
certify_parser = subparsers.add_parser('certify', description=certify_help, help=certify_help, parents=[parent_parser])
certify_parser.add_argument('--no-cleanup', action='store_true', help="Don't remove certificate files from /tmp during certify. Dangerous.", default=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the action is 'store_true' argparse assigns a default of False so it doesn't need to be specified here.


# Deploy command
deploy_help = "Deploy the Zappa application."
deploy_parser = subparsers.add_parser('deploy', description=deploy_help, help=deploy_help, parents=[parent_parser])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parser doesn't need to be assigned to a variable since it's never used afterwards. Same applies to init_parser, manage_parser, schedule_parser, unschedule_parser, and update_parser.


# Init command
init_help = "Initialize a zappa_settings.json file."
init_parser = subparsers.add_parser('init', help=init_help, description=init_help, parents=[parent_parser])

# Invoke command
invoke_help = "Invoke functions directly on the Zappa deployment"
invoke_parser = subparsers.add_parser('invoke', help=invoke_help, description=invoke_help, parents=[parent_parser])
invoke_parser.add_argument('--raw', action='store_true', help='When invoking remotely, invoke this python as a string, not as a modular path.', default=False)

# Manage command
manage_help = "Remotely execute Django's manage command."
manage_parser = subparsers.add_parser('manage', help=manage_help, description=manage_help, parents=[parent_parser])

# Rollback command
rollback_help = "Rollback n versions of the application"
rollback_parser = subparsers.add_parser('rollback', help=rollback_help, description=rollback_help, parents=[parent_parser])
rollback_parser.add_argument('-n', '--num-rollback', type=int, default=0, help='The number of versions to rollback.')

# Schedule command
schedule_help = "Schedule CloudWatch Events to execute functions within the Zappa application."
schedule_parser = subparsers.add_parser('schedule', help=schedule_help, description=schedule_help, parents=[parent_parser])

# Status command
status_help = "Get the status of the Zappa deployment"
status_parser = subparsers.add_parser('status', help=status_help, description=status_help, parents=[parent_parser])
status_parser.add_argument('--json', action='store_true', help='Returns status in JSON format', default=False) # https://github.com/Miserlou/Zappa/issues/407

# Tail command
tail_help = 'Tail the CloudWatch logs of a deployed Zappa application'
tail_parser = subparsers.add_parser('tail', help=tail_help, description=tail_help, parents=[parent_parser])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some arguments missing here (--http and --no-color) but they were added after this PR so not your fault :)


# Undeploy command
undeploy_help = "Undeploy a Zappa application"
undeploy_parser = subparsers.add_parser('undeploy', help=undeploy_help, description=undeploy_help, parents=[parent_parser])
undeploy_parser.add_argument('--remove-logs', action='store_true', help='Removes log groups of api gateway and lambda task during the undeployment.', default=False)

# Unschedule command
unschedule_help = "Unschedule CloudWatch Events created during \'zappa schedule\'"
unschedule_parser = subparsers.add_parser('unschedule', help=unschedule_help, description=unschedule_help, parents=[parent_parser])

# Update command
update_help = "Update the deployed Zappa application."
update_parser = subparsers.add_parser('update', help=update_help, description=update_help, parents=[parent_parser])

args = parser.parse_args(argv)

self.vargs = vars(args)
vargs_nosettings = self.vargs.copy()
vargs_nosettings.pop('settings_file')
if not any(vargs_nosettings.values()): # pragma: no cover
parser.error(help_message)
return

# Version requires no arguments
if args.version: # pragma: no cover
self.print_version()
sys.exit(0)

# Parse the input
self.command_env = self.vargs['command_env']
self.command = self.command_env[0]
# Make sure there isn't a new version available
self.check_for_update()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This no longer lets you bypass check_for_update when specifying --json. Was that intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!


if self.command not in CLI_COMMANDS:
print("The command '{}' is not recognized. {}".format(self.command, help_message))
return
self.command = self.vargs['command']
self.command_env = self.vargs['environment']

# Make sure there isn't a new version available
if not self.vargs['json']:
self.check_for_update()

# We don't have any settings yet, so make those first!
# (Settings-based interactions will fail
Expand All @@ -193,16 +213,16 @@ def handle(self, argv=None):
if all_environments: # All envs!
environments = self.zappa_settings.keys()
else: # Just one env.
if len(self.command_env) < 2: # pragma: no cover
if self.command_env is None: # pragma: no cover
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not self.command_env:

# If there's only one environment defined in the settings,
# use that as the default.
if len(self.zappa_settings.keys()) is 1:
environments.append(self.zappa_settings.keys()[0])
else:
parser.error("Please supply an environment to interact with.")
parser.error("There is more than 1 environment in the settings file. Please supply an environment to interact with.")
sys.exit(1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't yours but this line will never be reached. parser.error() exits with a status code of 2.

else:
environments.append(self.command_env[1])
environments.append(self.command_env)

for environment in environments:
try:
Expand Down