feat(cli): add lightweight transformer cli #459

brettstack · 2018-06-06T23:45:28Z

Issue #, if available:

Description of changes:
bin/sam-translate.py --input-file=sam-template.yaml outputs a cfn-template.json file (can specify output with --output-file=

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…tchSize from 200 to 10 (max)

jlhood · 2018-06-07T15:32:38Z

bin/sam-translate.py

+  --output-file=<o>    Location to store resulting CloudFormation template [default: cfn-template.json].
+
+"""
+import os


Not sure if we're trying to follow PEP8 imports style guide, but if we are, the imports should be separated into 3 groups.

Imports should be grouped in the following order:

Standard library imports.

Related third party imports.

Local application/library specific imports.

You should put a blank line between each group of imports.

jlhood · 2018-06-07T15:34:07Z

bin/sam-translate.py

+#!/usr/bin/env python2
+
+"""Convert SAM templates to CloudFormation templates.
+Known limitations: cannot transform CodeUri pointing at local directory.


Put a blank line between the summary and this line: https://www.python.org/dev/peps/pep-0257/#multi-line-docstrings

jlhood · 2018-06-07T15:35:14Z

bin/sam-translate.py

+cli_options = docopt(__doc__, sys.argv[1:])
+iam_client = boto3.client('iam')
+
+if __name__ == '__main__':


Put the logic in a main() method and then call it from the conditional:

if __name__ == '__main__': main()

jlhood · 2018-06-07T15:36:51Z

bin/sam-translate.py

+    output_file_option = cli_options.get('--output-file')
+    input_file_path = os.path.join(cwd, input_file_option)
+    output_file_path = os.path.join(cwd, output_file_option)
+    samTemplate = yaml_parse(open(input_file_path, 'r'))


samTemplate -> sam_template

Also use blank lines to separate logical sections (arg parsing, transform template, write output)

jlhood · 2018-06-07T15:38:46Z

bin/sam-translate.py

+    samTemplate = yaml_parse(open(input_file_path, 'r'))
+    cloud_formation_template = transform(samTemplate, {}, ManagedPolicyLoader(iam_client))
+    cloud_formation_template_prettified = json.dumps(cloud_formation_template, indent=2)
+    print(cloud_formation_template_prettified)


Remove? Seems weird to print and write to an output file. If you really want to print it also, you should add a --quiet,-q option to suppress this behavior or conversely a --print option to enable it.

jlhood · 2018-06-07T15:39:55Z

bin/sam-translate.py

+    cloud_formation_template = transform(samTemplate, {}, ManagedPolicyLoader(iam_client))
+    cloud_formation_template_prettified = json.dumps(cloud_formation_template, indent=2)
+    print(cloud_formation_template_prettified)
+    cloud_formation_template_file = open(output_file_path, 'w')


Unless there's a good reason not to, always use the with statement to ensure files are closed after use:

with open(output_file_path, 'w') as f: f.write(cloud_formation_template_prettified)

jlhood · 2018-06-07T15:40:31Z

DEVELOPMENT_GUIDE.rst

+.. _installation instructions: https://github.com/pyenv/pyenv#installation
+
+Profiling


Really appreciate you documenting this!

jlhood

Nice. This is a super useful tool! Mostly style/readability comments.

jlhood · 2018-06-07T18:20:43Z

bin/sam-translate.py

+
+import boto3
+
+from docopt import docopt


docopt should go in the same section as boto3, right?

jlhood · 2018-06-07T18:21:48Z

bin/sam-translate.py

+cwd = os.getcwd()
+
+
+def getInputOutputFilePaths():


getInputOutputFilePaths -> get_input_output_file_paths

jlhood · 2018-06-07T18:22:23Z

bin/sam-translate.py

+from samtranslator.translator.transform import transform
+from samtranslator.yaml_helper import yaml_parse
+
+cli_options = docopt(__doc__, sys.argv[1:])


Why do you need to pass the additional param?

jlhood · 2018-06-07T18:31:37Z

bin/sam-translate.py

+
+def main():
+    input_file_path, output_file_path = getInputOutputFilePaths()
+    sam_template = yaml_parse(open(input_file_path, 'r'))


Does yaml_parse close this file handle? Hopefully it doesn't and you should:

with open(input_file_path, 'r') as f: sam_template = yaml_parse(f)

jlhood

Just a few more minor things, then it should be good to go.

jlhood

Looks great! Thanks for iterating!

* doc: update DESIGN.md to include an Overview and entry point for SAM (#370) * Update DESIGN.md to include an Overview and entry point for SAM * Updated wording in DESIGN.md based on PR feedback * docs(example): improve microservice-http-endpoint-python3 example (#415) * Remove unnecessary index.js" * Moved common config to template Globals and uncomment debug log. * Add sample payload and commands for testing. * Rename test script as a script. * Exit from script after command. * Simplify use case for easier startup. - Move table creation into template. - Eliminates need to provide table name in every REST call. - Fix GET with no params. - Split test.sh script from output log. * Address requested changes. * doc(example): update api_backend example to use ES6 arrow function immediate return (#446) Since this code is using ES6 we can use the short version of returning a param * feat(event-source): add SQS as Lambda event source (#451) * feat(cli): add lightweight transformer cli (#459) * bug: fix ANY method ARN generation using wildcard (#449) * fix(event-source): fix SQS Queue schema to allow intrinsic functions (#465) * fix: log a warning on invalid schema validation (#466)

brettstack added 13 commits April 18, 2018 10:54

release: 1.5.1

bcb74c0

Merge remote-tracking branch 'upstream/develop' into develop

95aafd7

Merge remote-tracking branch 'upstream/develop' into develop

a5fc1b1

feat(event-source): add SQS event source

01451e1

Merge remote-tracking branch 'upstream/develop' into feature/sqs-event

062194b

uncomment code coverage

9bbee00

test: fix aws-cn output for streams with SQS

7f186ce

fix comment 'SQS Streams' -> 'SQS Queues'

bb325a1

move sqs tests into separate file; fix doc (Stream=>Queue); reduce Ba…

c100723

…tchSize from 200 to 10 (max)

add validation for missing Queue or Stream

a26e50c

add validation for StartingPosition when Stream is specified

e6d4910

feat(cli): add lightweight transformer cli

73d54b5

Merge remote-tracking branch 'upstream/develop' into feature/sqs-event

b1e4e5a

ylynn previously approved these changes Jun 7, 2018

View reviewed changes

jlhood reviewed Jun 7, 2018

View reviewed changes

refactor(cli): address PR feedback

a9c5974

brettstack dismissed ylynn’s stale review via a9c5974 June 7, 2018 18:14

jlhood reviewed Jun 7, 2018

View reviewed changes

jlhood suggested changes Jun 7, 2018

View reviewed changes

refactor(cli): address PR feedback

04bc87c

jlhood approved these changes Jun 7, 2018

View reviewed changes

ylynn approved these changes Jun 7, 2018

View reviewed changes

brettstack merged commit 0e89b42 into aws:develop Jun 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): add lightweight transformer cli #459

feat(cli): add lightweight transformer cli #459

brettstack commented Jun 6, 2018

jlhood Jun 7, 2018

jlhood Jun 7, 2018

jlhood Jun 7, 2018

jlhood Jun 7, 2018

jlhood Jun 7, 2018 •

edited

Loading

jlhood Jun 7, 2018 •

edited

Loading

jlhood Jun 7, 2018

jlhood left a comment

jlhood Jun 7, 2018 •

edited

Loading

jlhood Jun 7, 2018

jlhood Jun 7, 2018

jlhood Jun 7, 2018

jlhood left a comment

jlhood left a comment

		.. _installation instructions: https://github.com/pyenv/pyenv#installation

		Profiling

		cwd = os.getcwd()


		def getInputOutputFilePaths():

feat(cli): add lightweight transformer cli #459

feat(cli): add lightweight transformer cli #459

Conversation

brettstack commented Jun 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlhood Jun 7, 2018 • edited Loading

Choose a reason for hiding this comment

jlhood Jun 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlhood left a comment

Choose a reason for hiding this comment

jlhood Jun 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlhood left a comment

Choose a reason for hiding this comment

jlhood left a comment

Choose a reason for hiding this comment

jlhood Jun 7, 2018 •

edited

Loading

jlhood Jun 7, 2018 •

edited

Loading

jlhood Jun 7, 2018 •

edited

Loading