feat(stepfunctions-tasks): assign boolean value in DynamoDB from state input (Json path) #9088
Conversation
…lling dynamodb is now possible to map a boolean. closes #9007
|
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
michaelwiles
changed the title
There was a problem hiding this comment.
@michaelwiles I believe your build is failing because you have some missing trailing commas (in integ.call-dynamodb.ts and put-item.test.ts
I was about to make this change for you, but I don't have permissions to push to your fork.
packages/@aws-cdk/aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts
Outdated
Show resolved
Hide resolved
shivlaks
changed the title
shivlaks
added
the
pr-linter/exempt-readme
|
I've addressed those issues. @shivlaks I've added you as a committer on that repo. It'd probably be easier for you to make changes you need. I'm not sure if you want for example the validate method to throw an exception or not and it'd be easier if you just made any changes yourself. |
thanks! I think you've addressed my feedback. might update the doc string for the method slightly, but other than that this is fantastic! |
There was a problem hiding this comment.
@michaelwiles I wasn't able to push, but had one small change to the doc string (added as a suggestion in the PR).
After that, I think this is ready to merge!
packages/@aws-cdk/aws-stepfunctions-tasks/lib/dynamodb/shared-types.ts
Outdated
Show resolved
Hide resolved
…types.ts upgrading docstring for booleanFromJsonPath Co-authored-by: Shiv Lakshminarayan <[email protected]>
|
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). |
|
@michaelwiles you might need to give maintainers permissions so mergify can update and merge the PR. here is the GitHub documentation on how you can provide maintainers permissions to the PR |
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). |
@shivlaks: Yeah... being new to this I made the mistake of creating the pull request repo in an organisation account and that "allow edits from maintainers" check box is thus not available. |
|
Yeah, like I said I'm new to this so I naively used an organisation repo
and with an organisation repo you can't allow changes to be pushed from the
pull request
<https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork>
.
So unless I create a whole new pull request from a user repository (I don't
think that would be a good idea as I'd assume we'd lose the pull request
history).
I am sorry, will know for next time (let's hope there is a next time).
…On Fri, 17 Jul 2020 at 10:21, Romain Marcadier ***@***.***> wrote:
This PR will not auto-merge because it's behind master and we don't have
permissions to update the PR branch. @michaelwiles
<https://github.com/michaelwiles>, would you be able to edit the PR to
allow us to make changes there (it's a per-PR setting)? Otherwise, we'll
just have to manually merge this :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#9088 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAODX5EDYN7HXBZF7L2PDEDR4ACZBANCNFSM4O2ZMQPA>
.
|
|
@michaelwiles - it's not a big deal if you want to create another PR. You can always mention that in the description that it supersedes this one so we have a breadcrumb back to the PR history. Alternatively, I could merge this although I think that's the least desirable option as it might not pick up an in-flight change that might conflict. Let me know if it's possible to re-publish and we'll go from there to work with the best option in front of us. We really appreciate the contribution and would love to see more! |
|
@michaelwiles - I merged it this time since it's less painful. I'll add a note re: pull requests from organization repos since I was not aware that they could not delegate permissions to maintainers. At least it's on your radar for the next time! thanks a bunch 🎉 |
…e input (Json path) (aws#9088) It is now possible to map a boolean value from state input using a Json path. function booleanFromJsonPath added to DynamoAttributeValue which accepts a Json path Closes aws#9007 Co-authored-by: Michael Wiles <[email protected]> Co-authored-by: Shiv Lakshminarayan <[email protected]>
It is now possible to map a boolean value from state input using a Json path.
function
booleanFromJsonPathadded toDynamoAttributeValuewhich acceptsa Json path
Closes #9007
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license