-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(stepfunctions-tasks): task constructs to call DynamoDB APIs #8466
Conversation
packages/@aws-cdk/aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
1 similar comment
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
copy @RomainMuller - added you as a reviewer as you will likely have other suggestions for cleaning this integration up while we're in here from both jsii and Dynamo perspective |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
packages/@aws-cdk/aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts
Outdated
Show resolved
Hide resolved
@RomainMuller - took a stab at your suggestion(s) - have not done the READMEs yet, but let me know if you had any other thoughts or ideas before I do that. PS: I'm not super delighted with the naming ( |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
packages/@aws-cdk/aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
/** | ||
* Determines the level of detail about provisioned throughput consumption that is returned. | ||
*/ | ||
export enum DynamoConsumedCapacity { |
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 wonder if those should be directly in the dynamodb
package? I don't feel strongly either way, but it sounds like something that could be useful in other contexts using DDB tables?
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 think that makes sense. I'm not strongly opinionated here either.
I'll put together another PR moving them over and let's see what that looks like in a draft to continue with this idea.
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.
Looks much better IMO. I'm wondering if we should move the enums
directly in the DynamoDB package, since those are applicable beyond SFN... But I'm not sure it's the right move...
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Replaces the implementation to call DynamoDb from within a task to
merge state level properties as a construct.
Notable changes:
ITable
to be supplied instead oftableName
partitionKey
andsortKey
are now represented askey
DynamoAttributeValue
is now a struct rather than a classwithX
methods don't translate to every language andwith a struct a fluent builder will be generated with a more idiomatic syntax
for the language being used
Refactored where it seemed sensible. Test expectations have not changed with
the exception of the tableName being a reference. Verified the integration test.
Closes #8108
BREAKING CHANGE:
Dynamo*
tasks no longer implementIStepFunctionsTask
and have been replaced byconstructs that can be instantiated directly. See README for examplesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license