feat: SAM Init interactive flow redesign #3410

jonife · 2021-10-26T17:09:12Z

Which issue(s) does this change fix?

Why is this change necessary?

Improve the user experience on the sam init interactive experience. Today we "hide" the types of app we support through a layer of questions (custom or pre made examples, Package type for lambda, what runtime, then show apps). While this works, we are moving to the idea of surfacing what examples we have first.

The new flow is something like:

custom or pre-made ---> here are the types of examples we have ----> from that list here are the package types ---> from that here are the runtimes.

How does it address the issue?

Improve the user experience on the sam init interactive experience

What side effects does this change have?

Changed the entire cli interactive flow

Checklist

Write unit tests
Write/update integration tests
make pr passes

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

mndeveci

I left small comments for now but I want to test this locally and give more detailed feedback later.

samcli/commands/init/__init__.py

samcli/commands/init/init_templates.py

samcli/local/common/runtime_template.py

samcli/commands/init/init_templates.py

mndeveci · 2021-11-24T01:20:55Z

samcli/commands/init/interactive_init_flow.py

-        runtime = _get_runtime(runtime)
-    dependency_manager = _get_dependency_manager(dependency_manager, runtime)
+    templates = InitTemplates()
+    filter_value = runtime if runtime else base_image


2 comments;

Let's rename the filter_value to something more meaningful

What happens if runtime and base_image is None?

mndeveci · 2021-11-24T01:27:55Z

samcli/commands/init/interactive_init_flow.py

+    except KeyError as ex:
+        raise InvalidInitOptionException(f"Lambda Runtime {runtime} is not supported for {use_case} examples.") from ex


Please don't use exception driven development. This could easily change with another missing key on another dict. Please be explicit about which line can throw this.

mndeveci · 2021-11-24T01:28:20Z

samcli/commands/init/interactive_init_flow.py

+
+    try:
+        dependency_manager_options = package_types_options[package_type]
+    except KeyError as ex:


ditto. Please explicitly check the invalid case rather than raising an exception on KeyError

samcli/commands/init/interactive_init_flow.py

mndeveci · 2021-11-24T02:00:24Z

samcli/commands/init/interactive_init_flow.py

-        choice = click.prompt("Runtime", type=click.Choice(choices), show_choices=False)
-        runtime = INIT_RUNTIMES[int(choice) - 1]  # zero index
-    return runtime
+def _generate_default_hello_world_application(use_case, package_type, runtime, dependency_manager, pt_explicit):


Can we add documentation for these new methods?

hawflau · 2021-11-25T17:23:36Z

samcli/commands/init/init_templates.py

+def get_template_value(value, template):
+    if value not in template:
+        raise InvalidInitTemplateError(
+            f"Template is missing the value for {value} in manifest file. Please raise a a github issue."


nit: Is it possible to include the url to raise an issue?

…ase prompt when --app-template is provided

mndeveci · 2021-12-14T23:54:50Z

samcli/commands/init/interactive_init_flow.py

+    return (runtime, package_type, dependency_manager, pt_explicit)
+
+
+def _get_app_template_properties(preprocessed_options, use_case, base_image, template_properties):


We have some nice documentation for some of the methods, but some are missing. Can we add documentation for the ones that is not obvious what they are doing?

mndeveci · 2021-12-17T00:18:47Z

samcli/commands/init/init_templates.py

+        (app_template and app_template != template["appTemplate"])
+        or (package_type and package_type != template["packageType"])
+        or (dependency_manager and dependency_manager != template["dependencyManager"])


Suggested change

(app_template and app_template != template["appTemplate"])

or (package_type and package_type != template["packageType"])

or (dependency_manager and dependency_manager != template["dependencyManager"])

(app_template and app_template != template.get("appTemplate"))

or (package_type and package_type != template.get("packageType"))

or (dependency_manager and dependency_manager != template.get("dependencyManager"))

* (fix) stop resolve relative paths for function imageUri * fix unit testing

* SAM INIT Update * resolved conflict * test added and updated * block default prompt when Image packageType is provide and skip use case prompt when --app-template is provided * update default hello world prompt * filtering updates * added typing * Fixing failed canaries (aws#3532) * (fix) stop resolving relative paths for Function image URI (aws#3531) * (fix) stop resolve relative paths for function imageUri * fix unit testing * update template_does_not_meet_filter_criteria use <dict>.get Co-authored-by: Mehmet Nuri Deveci <[email protected]> Co-authored-by: Qingchuan Ma <[email protected]> Co-authored-by: Mohamed Elasmar <[email protected]>

jonife force-pushed the sam_init_revamp branch from fcee1b0 to 1113ae3 Compare October 26, 2021 17:54

mndeveci reviewed Nov 2, 2021

View reviewed changes

samcli/commands/init/__init__.py Show resolved Hide resolved

samcli/commands/init/__init__.py Show resolved Hide resolved

CoshUS added the pr/internal label Nov 8, 2021

aws-sam-cli-bot added the maintainer/need-followup label Nov 18, 2021

JadenSimon mentioned this pull request Nov 19, 2021

SAM: update create wizard flow for better UX aws/aws-toolkit-vscode#2314

Open

mndeveci suggested changes Nov 24, 2021

View reviewed changes

aws-sam-cli-bot removed the maintainer/need-followup label Nov 24, 2021

SAM INIT Update

61eba16

aws-sam-cli-bot added the maintainer/need-followup label Nov 29, 2021

resolved conflict

4388d47

jonife marked this pull request as draft November 29, 2021 19:42

jonife force-pushed the sam_init_revamp branch from 1113ae3 to 4388d47 Compare November 29, 2021 19:43

aws-sam-cli-bot removed the maintainer/need-followup label Nov 30, 2021

test added and updated

9a8058f

jonife marked this pull request as ready for review November 30, 2021 17:53

jonife requested review from mndeveci and hawflau November 30, 2021 17:53

hawflau reviewed Dec 8, 2021

View reviewed changes

hawflau approved these changes Dec 8, 2021

View reviewed changes

block default prompt when Image packageType is provide and skip use c…

c0927f7

…ase prompt when --app-template is provided

jonife force-pushed the sam_init_revamp branch from 12a77f1 to c0927f7 Compare December 9, 2021 22:08

jonife and others added 2 commits December 14, 2021 12:23

update default hello world prompt

5a0492a

Merge branch 'develop' into sam_init_revamp

05aecef

mndeveci approved these changes Dec 15, 2021

View reviewed changes

jonife and others added 3 commits December 14, 2021 20:39

filtering updates

b56e08a

added typing

fff8584

Merge branch 'develop' into sam_init_revamp

e088c5d

mndeveci reviewed Dec 17, 2021

View reviewed changes

qingchm and others added 2 commits December 17, 2021 15:07

Fixing failed canaries (aws#3532)

0f65d73

(fix) stop resolving relative paths for Function image URI (aws#3531)

d6a9e70

* (fix) stop resolve relative paths for function imageUri * fix unit testing

jonife and others added 2 commits December 17, 2021 15:07

update template_does_not_meet_filter_criteria use <dict>.get

5059f37

Merge branch 'develop' into sam_init_revamp

ad55a93

mndeveci approved these changes Dec 18, 2021

View reviewed changes

mndeveci merged commit 4d250ce into aws:develop Dec 18, 2021

HijadelUFO mentioned this pull request Jan 1, 2024

[Snyk] Security upgrade axios from 0.18.1 to 0.21.1 HijadelUFO/aws-sam-cli#85

Open

This was referenced Jan 2, 2024

[Snyk] Security upgrade axios from 0.18.1 to 0.21.1 Frameio/aws-sam-cli#10

Open

[Snyk] Security upgrade axios from 0.18.1 to 0.21.1 Frameio/aws-sam-cli#11

Open

dexterinbk mentioned this pull request Mar 15, 2024

[Snyk] Security upgrade axios from 0.18.1 to 0.21.1 Frameio/aws-sam-cli#14

Open

jpalomar mentioned this pull request Mar 16, 2024

[Snyk] Security upgrade axios from 0.18.1 to 0.21.1 Frameio/aws-sam-cli#15

Closed

HijadelUFO mentioned this pull request Mar 16, 2024

[Snyk] Security upgrade axios from 0.18.1 to 0.21.1 HijadelUFO/aws-sam-cli#89

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: SAM Init interactive flow redesign #3410

feat: SAM Init interactive flow redesign #3410

jonife commented Oct 26, 2021

mndeveci left a comment

mndeveci Nov 24, 2021

mndeveci Nov 24, 2021

mndeveci Nov 24, 2021

mndeveci Nov 24, 2021

hawflau Nov 25, 2021

mndeveci Dec 14, 2021

mndeveci Dec 17, 2021

		except KeyError as ex:
		raise InvalidInitOptionException(f"Lambda Runtime {runtime} is not supported for {use_case} examples.") from ex

		return (runtime, package_type, dependency_manager, pt_explicit)


		def _get_app_template_properties(preprocessed_options, use_case, base_image, template_properties):

feat: SAM Init interactive flow redesign #3410

feat: SAM Init interactive flow redesign #3410

Conversation

jonife commented Oct 26, 2021

Which issue(s) does this change fix?

Why is this change necessary?

How does it address the issue?

What side effects does this change have?

Checklist

mndeveci left a comment

Choose a reason for hiding this comment

mndeveci Nov 24, 2021

Choose a reason for hiding this comment

mndeveci Nov 24, 2021

Choose a reason for hiding this comment

mndeveci Nov 24, 2021

Choose a reason for hiding this comment

mndeveci Nov 24, 2021

Choose a reason for hiding this comment

hawflau Nov 25, 2021

Choose a reason for hiding this comment

mndeveci Dec 14, 2021

Choose a reason for hiding this comment

mndeveci Dec 17, 2021

Choose a reason for hiding this comment