feat: Add python support to cdk init #2130

garnaat · 2019-04-01T13:09:38Z

Introduces an init template for Python projects, thus enabling cdk init --language python.

There are (at least) two potentially controversial approaches to this template.

The template creates a virtualenv in the directory and provides instructions for how to use it. There is currently no way to disable this behavior. I feel that the vast majority of use cases would benefit from a virtualenv but it could complicate some other use cases. Of course, you could always just delete the virtualenv or ignore it.
The template assumes you want to build a Python package. So, there is a setup.py and the README instructs the user to run python setup.py develop to build the package in a way to makes it easy to develop locally. I think this better aligns with where a developer would eventually want to go but does so at the expense of more initial complexity.

Pull Request Checklist

Testing
- Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
- CLI change?: coordinate update of integration tests with team
- cdk-init template change?: coordinated update of integration tests with team
Docs
- jsdocs: All public APIs documented
- README: README and/or documentation topic updated
Title and Description
- Change type: title prefixed with fix, feat will appear in changelog
- Title: use lower-case and doesn't end with a period
- Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
- Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
Sensitive Modules (requires 2 PR approvers)
- IAM Policy Document (in @aws-cdk/aws-iam)
- EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
- Grant APIs (only if not based on official documentation with a reference)

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

…emplate

RomainMuller

This looks good to me overall, but I'm not enough of a Python practitioner to have an opinion on the "possibly controversial" decisions here... I'll defer that to people with more experience on the subject, who can understand the tradeoffs & such.

packages/aws-cdk/lib/init-templates/app/python/.gitignore

eladb · 2019-04-01T13:14:24Z

packages/aws-cdk/lib/init-templates/app/python/README.md

@@ -0,0 +1,42 @@
+
+Welcome to your CDK Python project!


This template should actually be called "sample-app". The "app" template should result in an empty stack (so you don't have to go delete things). See typescript as an example.

I'm confused by this comment. I was following the existing examples for csharp, java, etc. Is this inconsistent with them?

Well.... those should be changed to ;-)

Ah, I see. Yeah, its kind of backwards now. Do you want to fix all of them now?

So do you want to change all of those now?

Let's start by renaming this specific template to sample-app and then follow up with a PR for an empty app in python.

packages/aws-cdk/lib/init-templates/app/python/README.md

packages/aws-cdk/lib/init-templates/app/python/hello/hello_stack.py

eladb · 2019-04-01T13:18:37Z

packages/aws-cdk/lib/init-templates/app/python/setup.py

+    package_dir={"": "hello"},
+    packages=setuptools.find_packages(where="hello"),
+
+    install_requires=[


Does it make sense in python-land to have this duplicated with requirements.txt?

Many packages use both but I have simplified this to put the requirements in the setup.py and then tell pip to use those rather than list them explicitly in requirements.txt.

eladb · 2019-04-01T13:19:41Z

packages/aws-cdk/lib/init.ts

+
+  print(`Executing ${colors.green('python -m venv .env')}`);
+  try {
+    await execute('python3', '-m venv', '.env');


Would it make sense to also do the pip install stuff here so people can get started quicker?

rix0rrr · 2019-04-01T13:33:21Z

packages/aws-cdk/lib/init-templates/app/python/README.md

+```
+$ source .env/bin/activate
+$ pip install -r requirements.txt
+$ python setup.py develop


Isn't develop for libraries (as opposed to apps)?

develop is just a variant of install that allows you to edit your code and run it without having to re-install. This example really consists of both an app and a library. I have restructured the code slightly to make that more clear, I hope.

rix0rrr · 2019-04-01T13:34:12Z

packages/aws-cdk/lib/init-templates/app/python/cdk.json

@@ -0,0 +1,3 @@
+{
+    "app": "python3 hello/hello_stack.py"


Could this include running via the venv? Isn't that a better experience than requiring users to have pre-actived their shell and THEN running cdk synth?

Yes, that's a good idea. We should be able to just point to the python3 executable in the virtualenv.

The EXE on Windows (for 3.7 at least as I just installed it last week) is just python.

This gets ugly. If we are using a virtualenv that has been created using Python3 then there is no real need to say python3 here. However, that does mean that during the init process we have done a python3 -m venv .env so this, too, will fail on Windows.

We could try python3 for the virtualenv and then try python if that fails but this would cause problems on a machine that does not have Python3 installed at all because we would end up with a virtualenv for Python2.

Maybe this is an argument against trying to create the virtualenv during the init process.

It is possible to actually have the app entrypoint an executable file that takes care of all these details, so eventually cdk.json would be:

{ "app": "bin/my-awesome-app" }

…ctory. Also some formatting changes.

jfuss · 2019-04-01T16:26:40Z

Capturing/continuing a conversation with @garnaat from Gitter.

The current example creates a venv and a requirements.txt for customers. This is a great starting point for new customers or customers that follow these patterns. Some customers (including myself), prefer to use Pipfile and pipenv when working with Python projects. Only supporting venv would mean customers would need to generate a Pipfile from requirements.txt (you may need to configure pipenv to use the generated venv, but depends on where the venv was created). Instead, we could provide a way for a customer to tell cdk init which flavor of the example they want.

Other languages might have similar stories, so there may be a case for cdk init to generate different 'flavors' of examples (gradle vs maven for java might be a similar case).

garnaat · 2019-04-01T16:39:28Z

Capturing/continuing a conversation with @garnaat from Gitter.

The current example creates a venv and a requirements.txt for customers. This is a great starting point for new customers or customers that follow these patterns. Some customers (including myself), prefer to use Pipfile and pipenv when working with Python projects. Only supporting venv would mean customers would need to generate a Pipfile from requirements.txt (you may need to configure pipenv to use the generated venv, but depends on where the venv was created). Instead, we could provide a way for a customer to tell cdk init which flavor of the example they want.

Other languages might have similar stories, so there may be a case for cdk init to generate different 'flavors' of examples (gradle vs maven for java might be a similar case).

I agree that there will always be multiple styles of development for any given language. We could have options like python-venv and python-pipenv to allow different starting projects.

…, not an empty project. Will need to create a separate issue to move the others over and create templates for empty projects.

garnaat · 2019-04-01T20:22:17Z

I have moved the original template over to sample-app and create a new, blank template for app. I will create a ticket to move the other incorrectly placed sample-app examples over to the right place.

Running cdk init will now produce:

Available templates:
* app: Template for a CDK Application
   └─ cdk init app --language=[csharp|fsharp|java|python|typescript]
* lib: Template for a CDK Construct Library
   └─ cdk init lib --language=typescript
* sample-app: Example CDK Application with some constructs
   └─ cdk init sample-app --language=[python|typescript]

The default choice is app so if you want the full example you need to run:

$ cdk init --language python sample-app

eladb

I've had a few previous comments unanswered

eladb · 2019-04-01T20:37:15Z

packages/aws-cdk/lib/init-templates/app/python/app.py

+
+from aws_cdk import cdk
+
+from hello.hello_stack import PyStack


We have some templating foo that you can use to generate a name for this stack and files based on the directory name in which the project was initialized (see typescript as an example). Otherwise, this means users will need to modify this after they've initialized the app and ideally they shouldn't have to.

Latest commit uses templating.

eladb · 2019-04-01T20:38:45Z

packages/aws-cdk/lib/init-templates/sample-app/python/app.py

+
+
+app = cdk.App()
+MyStack(app, "hello-cdk-1")


Maybe put each stack in a different region (would be a nicer example)

… use those and automatically run the python setup.py develop step. Also use templating feature for blank app.

eladb · 2019-04-02T14:17:30Z

packages/aws-cdk/lib/init-templates/app/python/%name%/%name%_stack.py

+from aws_cdk import cdk
+
+
+class PyStack(cdk.Stack):


Probably want to template this guy too.... Sorry

eladb · 2019-04-02T14:17:58Z

packages/aws-cdk/lib/init-templates/sample-app/python/hello/hello_stack.py

@@ -10,8 +10,8 @@

 class MyStack(cdk.Stack):

-    def __init__(self, app: cdk.App, id: str) -> None:
-        super().__init__(app, id)
+    def __init__(self, app: cdk.App, id: str, **kwargs) -> None:


eladb · 2019-04-02T14:18:11Z

packages/aws-cdk/lib/init-templates/sample-app/python/requirements.txt

-aws-cdk.aws_sqs
-aws-cdk.aws_sns
-aws-cdk.aws_s3
+-e .


eladb

Looks great!

Mitch Garnaat added 2 commits March 31, 2019 13:56

First take on an init template for a Python CDK project.

c6bfc0a

Merge remote-tracking branch 'upstream/master' into add_python_init_t…

269f1fd

…emplate

garnaat requested a review from a team as a code owner April 1, 2019 13:09

garnaat requested a review from dstufft April 1, 2019 13:09

RomainMuller approved these changes Apr 1, 2019

View reviewed changes

eladb approved these changes Apr 1, 2019

View reviewed changes

rix0rrr reviewed Apr 1, 2019

View reviewed changes

Mitch Garnaat added 3 commits April 1, 2019 06:41

The cdk.context.json should not be ignored.

8f23fa7

Fixing some review issues

713f4a2

Restructured to move app into a separate file outside of package dire…

406d846

…ctory. Also some formatting changes.

garnaat added the language/python Related to Python bindings label Apr 1, 2019

garnaat self-assigned this Apr 1, 2019

Mitch Garnaat added 4 commits April 1, 2019 10:13

Slight change to code formatting.

23b413c

Remove reference to app.sh

55c5639

Moving this over to sample-app since it actually creates a sample app…

46de258

…, not an empty project. Will need to create a separate issue to move the others over and create templates for empty projects.

Adding an app template for Python.

9d30dcc

eladb reviewed Apr 1, 2019

View reviewed changes

Keep all dependencies in setup.py and use -e . in requirements.txt to…

85ccb5b

… use those and automatically run the python setup.py develop step. Also use templating feature for blank app.

eladb approved these changes Apr 2, 2019

View reviewed changes

Templatize the class name.

f36ae06

eladb approved these changes Apr 2, 2019

View reviewed changes

garnaat merged commit 997dbcc into aws:master Apr 2, 2019

garnaat deleted the add_python_init_template branch April 2, 2019 15:53

This was referenced Dec 12, 2019

[Snyk] Upgrade @aws-cdk/cdk from 0.22.0 to 0.35.0 MechanicalRock/scdk#3

Closed

[Snyk] Upgrade @aws-cdk/cdk from 0.24.1 to 0.35.0 MechanicalRock/cdk-constructs#8

Open

[Snyk] Upgrade @aws-cdk/cdk from 0.24.1 to 0.35.0 MechanicalRock/account-reaper#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add python support to cdk init #2130

feat: Add python support to cdk init #2130

garnaat commented Apr 1, 2019

RomainMuller left a comment

eladb Apr 1, 2019

garnaat Apr 1, 2019

eladb Apr 1, 2019

garnaat Apr 1, 2019

garnaat Apr 1, 2019

eladb Apr 1, 2019

eladb Apr 1, 2019

garnaat Apr 2, 2019

eladb Apr 1, 2019

rix0rrr Apr 1, 2019

garnaat Apr 1, 2019

rix0rrr Apr 1, 2019

garnaat Apr 1, 2019

Doug-AWS Apr 1, 2019

garnaat Apr 1, 2019

eladb Apr 1, 2019

jfuss commented Apr 1, 2019

garnaat commented Apr 1, 2019

garnaat commented Apr 1, 2019

eladb left a comment

eladb Apr 1, 2019

garnaat Apr 2, 2019

eladb Apr 1, 2019

garnaat Apr 2, 2019

eladb Apr 2, 2019

eladb Apr 2, 2019

eladb Apr 2, 2019

eladb left a comment


		from aws_cdk import cdk

		from hello.hello_stack import PyStack

feat: Add python support to cdk init #2130

feat: Add python support to cdk init #2130

Conversation

garnaat commented Apr 1, 2019

Pull Request Checklist

RomainMuller left a 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

jfuss commented Apr 1, 2019

garnaat commented Apr 1, 2019

garnaat commented Apr 1, 2019

eladb left a 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

eladb left a comment

Choose a reason for hiding this comment