-
Notifications
You must be signed in to change notification settings - Fork 343
fix(python): fixed sample code and created CI job to validate #266
Conversation
"### Pull Request Checklist * [ ] Testing - Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change) * [ ] Title and Description - Change type: title prefixed with fix, feat and module name in parens, which 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'" |
@@ -12,7 +12,7 @@ def __init__(self, scope: core.Construct, id: str, **kwargs) -> None: | |||
super().__init__(scope, id, **kwargs) | |||
|
|||
# Defines an AWS Lambda resource | |||
my_lambda = _lambda.Function( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In all the workshop guides this is named hello
so for sake of consistency I updated this name.
@@ -2,7 +2,7 @@ | |||
|
|||
from aws_cdk import core | |||
|
|||
from cdk-workshop.pipeline_stack import WorkshopPipelineStack |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
is not a supported char in python filenames, so I replaced them with _
.
@@ -1,8 +1,6 @@ | |||
{ | |||
"app": "python3 app.py", | |||
"context": { | |||
"@aws-cdk/core:enableStackNameDuplicates": "true", | |||
"aws-cdk:enableDiffNoFail": "true", | |||
"@aws-cdk/core:stackRelativeExports": "true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of these are required however @aws-cdk/core:newStyleStackSynthesis
was missing here but is required in order for this code to synth
@@ -3,7 +3,7 @@ | |||
aws_lambda as _lambda, | |||
aws_apigateway as apigw, | |||
) | |||
from cdk_dynamo_table_viewer import TableViewer | |||
from cdk_dynamo_table_view import TableViewer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why there are 2 libraries in pip that seem to be the same package by the same author however there are peer dependency differences.
cdk_dynamo_table_viewer
does not support a recent version of JSII and throws dependency conflict errors with aws-cdk
packages.
cdk_dynamo_table_view
is by the same author however it does support a recent/compatible version of JSII.
@@ -1,7 +1,7 @@ | |||
from aws_cdk import ( | |||
core | |||
) | |||
from pypipworkshop_stack import PypipworkshopStack |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a typo
aws-cdk.aws_codebuild | ||
aws-cdk.aws_codepipeline | ||
aws-cdk.aws_codepipeline_actions | ||
cdk_dynamo_table_viewer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main-workshop
lists dependencies in setup.py
instead of using requirements.txt
. So for sake of consistency I replicated main-workshop
's configuration.
@@ -33,7 +33,7 @@ Hopefully this command finished successfully and we can move on to deploy our ap | |||
Use `cdk deploy` to deploy a CDK app: | |||
|
|||
``` | |||
cdk deploy cdkworkshop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stack name is not required here and is not used in other workshop guides.
@@ -83,7 +80,7 @@ class CdkWorkshopStack(core.Stack): | |||
my_lambda = _lambda.Function( | |||
self, 'HelloHandler', | |||
runtime=_lambda.Runtime.PYTHON_3_7, | |||
code=_lambda.Code.asset('lambda'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecated API
…erencing code) or cdk-workshop(if AWS/CDK stack name)
Summary
This PR implements CI workflows that verify changes do not break builds.
cdk synth
Fixes #259, #263, #266, #152
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.