fix: gradle init support #1040

sriram-mv · 2019-03-05T00:23:54Z

Add ability in sam init to specify dependency manager and generate
appropriate sam templates.
Write Design Document (Do I need to write a design document?)
Write unit tests
Write/update functional tests
Write/update integration tests
make pr passes
Write documentation

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

- Add ability in sam init to specify dependency manager and generate appropraite sam templates.

jfuss · 2019-03-05T14:03:25Z

samcli/local/init/__init__.py

+    "dotnetcore1.0_cli-package": os.path.join(_templates, "cookiecutter-aws-sam-hello-dotnet"),
+    "dotnetcore_cli_package": os.path.join(_templates, "cookiecutter-aws-sam-hello-dotnet"),
+    "dotnet_cli-package": os.path.join(_templates, "cookiecutter-aws-sam-hello-dotnet"),
+    "go1.x_go-get": os.path.join(_templates, "cookiecutter-aws-sam-hello-golang"),


go_get? Shouldn't this be dep vs modules to match our builders?

Yeah, you are right, I just went by the go inits.

jfuss · 2019-03-05T14:05:04Z

samcli/local/init/__init__.py

+    "java_gradle": os.path.join(_templates, "cookiecutter-aws-sam-hello-java-gradle")
+}
+
+RUNTIME_DEP_MAPPING = {


Why not encapsulate this into the Runtimes enum to keep all of this into one place?

Maybe, I need a double mapping though. Language -> Dep manager and language, dep combo to template. will think about it.

jfuss · 2019-03-05T14:05:16Z

samcli/local/init/__init__.py

+    "dotnetcore1.0": ["cli-package"],
+    "dotnetcore": ["cli-package"],
+    "dotnet": ["cli-package"],
+    "go1.x": ["go-get"],


dep or modules?

Yes, will change it.

jfuss · 2019-03-05T14:10:53Z

samcli/local/init/__init__.py

-    "go": os.path.join(_templates, "cookiecutter-aws-sam-hello-golang"),
-    "java8": os.path.join(_templates, "cookiecutter-aws-sam-hello-java"),
-    "java": os.path.join(_templates, "cookiecutter-aws-sam-hello-java")
+RUNTIME_DEP_TEMPLATE_MAPPING = {


There is a lot of mappings to keep updated all together. Can we condense these into one?

Will see how to condense this.

jfuss · 2019-03-05T14:16:46Z

samcli/local/init/__init__.py

+    "go1.x_go-get": os.path.join(_templates, "cookiecutter-aws-sam-hello-golang"),
+    "go_go-get": os.path.join(_templates, "cookiecutter-aws-sam-hello-golang"),
+    "java8_maven": os.path.join(_templates, "cookiecutter-aws-sam-hello-java-maven"),
+    "java_maven": os.path.join(_templates, "cookiecutter-aws-sam-hello-java-maven"),


Do we need a java_maven and a java8_maven? This questions is a general question as we have shortcuts for every language which makes sam init --help super long and is only going to get worse if we have to shortcut all language + dependency manager combinations.

The code for maven and gradle look the same. Are they? If so, do we need to maintain two copies directly? Can we combine them so our java examples are the same but can have different dependency managers?

Not really the same, slightly different source files, but the core java files are the same. We can look at de-duping that bit of source code.

jfuss · 2019-03-05T14:20:53Z

tests/unit/local/init/test_init.py



 class TestInit(TestCase):

    def setUp(self):
        self.location = None
        self.runtime = "python3.6"
+        self.dependency_manager = "pip"


What about the None case?

Will add a test for this, if a dependency manager is not given, we default to our choice.

- address comments

sanathkr · 2019-03-07T18:48:02Z

samcli/local/common/runtime_template.py

+# The config mutates based on the workflow selector, so they  are not to be imported and used from the template mapping.
+# They are an indication of metadata associated with a specific runtime.
+
+RUNTIME_DEP_TEMPLATE_MAPPING = {


This does not make sense. Why would you depend on the build workflow configs within init. They should be treated as isolated modules. Otherwise you will end up with a tangled mess of dependencies one year from now.

@jfuss filled me in on the intention of this mapping. The config here makes sense (as a true configuration data), but I am not excited about importing workflow_config which is an implementation detail. Can we just remove the workflow configs because they are not really used? That would make me more happy :)

Will remove that. 👍

sanathkr · 2019-03-07T18:59:08Z

samcli/local/common/runtime_template.py

+# The config mutates based on the workflow selector, so they  are not to be imported and used from the template mapping.
+# They are an indication of metadata associated with a specific runtime.
+
+RUNTIME_DEP_TEMPLATE_MAPPING = {


@jfuss filled me in on the intention of this mapping. The config here makes sense (as a true configuration data), but I am not excited about importing workflow_config which is an implementation detail. Can we just remove the workflow configs because they are not really used? That would make me more happy :)

sanathkr · 2019-03-07T19:00:31Z

samcli/local/common/runtime_template.py

+    ]
+}
+
+SUPPORTED_DEP_MANAGERS = set([c['dependency_manager'] for c in list(


I understand what this line is doing, but boy there are so many brackets!

Yes, Having a single config, makes extraction a bit more dense.

- fix README of Gradle init.

jfuss approved

sriram-mv added 2 commits March 4, 2019 16:22

fix: gradle init support

1cc4fe0

- Add ability in sam init to specify dependency manager and generate appropraite sam templates.

fix: click option short hand for dependency manager

e2b02d8

jfuss requested changes Mar 5, 2019

View reviewed changes

sriram-mv added 2 commits March 7, 2019 09:16

fix: central runtime template mapping

c7e4fe6

- address comments

fix: py2

9a9dead

sanathkr suggested changes Mar 7, 2019

View reviewed changes

sanathkr previously requested changes Mar 7, 2019

View reviewed changes

sriram-mv added 2 commits March 7, 2019 11:38

remove: remove builder config information from runtime templates

5c16091

fix: init template structure

a05aa78

- fix README of Gradle init.

sriram-mv force-pushed the gradle_init branch from 6e3f1c5 to a05aa78 Compare March 7, 2019 22:42

jfuss approved these changes Mar 8, 2019

View reviewed changes

Merge branch 'develop' into gradle_init

182154d

jfuss merged commit 5deff3c into aws:develop Mar 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: gradle init support #1040

fix: gradle init support #1040

sriram-mv commented Mar 5, 2019 •

edited

Loading

jfuss Mar 5, 2019

sriram-mv Mar 5, 2019

jfuss Mar 5, 2019

sriram-mv Mar 5, 2019

jfuss Mar 5, 2019

sriram-mv Mar 5, 2019

jfuss Mar 5, 2019

sriram-mv Mar 5, 2019

jfuss Mar 5, 2019

sriram-mv Mar 5, 2019

jfuss Mar 5, 2019

sriram-mv Mar 5, 2019

sanathkr Mar 7, 2019

sanathkr Mar 7, 2019

sriram-mv Mar 7, 2019

sanathkr Mar 7, 2019

sanathkr Mar 7, 2019

sriram-mv Mar 7, 2019

fix: gradle init support #1040

fix: gradle init support #1040

Conversation

sriram-mv commented Mar 5, 2019 • 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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sriram-mv commented Mar 5, 2019 •

edited

Loading