Feature/cli rework #432
Feature/cli rework #432
Conversation
|
Looks like it has issues with how we're getting the versions. This might be easier to merge after #426 or implement a different version method. |
|
It looks like there is interest in progressing the CLI, If this PR seems more appealing than #519 I can bring it up to date with master. |
| 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. |
| # https://github.com/Miserlou/Zappa/issues/404 | ||
| # Previously returned with printed warning. Now exists as derp is not a valid command. | ||
| zappa_cli.handle(argv) | ||
| self.assertNotEqual(system_exit.exception.code, 0) |
There was a problem hiding this comment.
Why not check this against the actual return code? argparse will always exit with a code 2 for an incorrect argument.
| # Functionality shared by all commands | ||
| parent_parser = argparse.ArgumentParser(add_help=False) | ||
| parent_parser.add_argument('environment', type=str, help='Deployment environment', default=None, nargs='?') | ||
| parent_parser.add_argument('-y', '--yes', action='store_true', help='Auto confirm yes', default=False) |
There was a problem hiding this comment.
AFAICT the yes argument is only used by the undeploy command.
| subparsers = parser.add_subparsers(title="Zappa Commands", help="Choose a valid zappa command", dest='command') | ||
|
|
||
| # Parent Parser | ||
| # Functionality shared by all commands |
There was a problem hiding this comment.
I'm not sure this functionality is shared by all commands. zappa --all init doesn't make much sense for example.
|
|
||
| # Deploy command | ||
| deploy_help = "Deploy the Zappa application." | ||
| deploy_parser = subparsers.add_parser('deploy', description=deploy_help, help=deploy_help, parents=[parent_parser]) |
There was a problem hiding this comment.
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.
| # 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) |
There was a problem hiding this comment.
When the action is 'store_true' argparse assigns a default of False so it doesn't need to be specified here.
| @@ -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 | |||
| 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() |
There was a problem hiding this comment.
This no longer lets you bypass check_for_update when specifying --json. Was that intended?
| # 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) |
There was a problem hiding this comment.
I know this isn't yours but this line will never be reached. parser.error() exits with a status code of 2.
| 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") |
There was a problem hiding this comment.
Neat, I've never seen version= used here to provide a version argument. TIL.
There was a problem hiding this comment.
yea, argparse has version built in. https://docs.python.org/2.7/library/argparse.html#action
Description
Redid the CLI so we could have help on the commands. A few questions:
GitHub Issues
#404