Skip to content
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

How to use cdk to create step function to write to dynamodb? #8108

Closed
michaelwiles opened this issue May 20, 2020 · 4 comments · Fixed by #8466
Closed

How to use cdk to create step function to write to dynamodb? #8108

michaelwiles opened this issue May 20, 2020 · 4 comments · Fixed by #8466
Assignees
Labels

Comments

@michaelwiles
Copy link
Contributor

So I'm really struggling with how to deploy a step function to put an item into a dynamodb table with the python sdk. I eventually want to provide the item via event bridge.

When I get errors I'm not sure if they are real errors or not:

I've tried this:

    event_table = dynamodb.Table(self, 'events',
        partition_key=dynamodb.Attribute(name='id', type=dynamodb.AttributeType.STRING),
        billing_mode=dynamodb.BillingMode.PAY_PER_REQUEST, removal_policy=RemovalPolicy.DESTROY)

    map = {'id': {'S.$': '$.id'}}
    dynamo_put_task = CallDynamoDB.put_item(item=map, table_name=event_table.table_name)

    task = Task(self, 'invoke-notification-service', task=dynamo_put_task)
    state_machine = StateMachine(self, 'save-event', definition=task)
    Rule(self, 'rule_name', targets=[(SfnStateMachine(state_machine))], event_bus=event_bus, event_pattern=EventPattern(account=[Stack.of(self).account]))

I really don't know how to use a DynamoAttributeValueMap as this is just a shell in python which is why I'm not using one.

And even for this to run I've had to remove the following line from DynamoPutItemProps:

if isinstance(item, dict): item = DynamoAttributeValueMap(**item)

otherwise I get an "unknown keyword argument id" when I try to run this. Which kinda makes sense as it's trying to unwrap a map and pass it into a constructor that takes no parameters (just self).

A simple example of creating a step function with cdk to write to dynamodb would be very helpful.

I really don't know how to instantiate that map.

The code that I've got - in the example - at least generates the cloudformation template but the item is empty. It is just {}.

When I try to do something like this cause I think that's what I'm supposed to do...

map = DynamoAttributeValueMap()
map._values = {'id': DynamoAttributeValue().with_s('$.id')}

To try and force values into that map I get error:

TypeError: Don't know how to convert object to JSON: DynamoAttributeValueMap(id=<aws_cdk.aws_stepfunctions_tasks.DynamoAttributeValue object at 0x7f7b17e64150>)

Just a plain worked example would be useful - though there might also be a bug here...

@michaelwiles michaelwiles added the needs-triage This issue or PR still needs to be triaged. label May 20, 2020
@michaelwiles
Copy link
Contributor Author

michaelwiles commented May 20, 2020

hmmm so having had a look at the tests for dynamodb step functions it looks like this should work:

dynamo_put_task = CallDynamoDB.put_item(item={'id': DynamoAttributeValue().with_s('1234')},
            table_name=event_table.table_name, return_values=DynamoReturnValues.ALL_NEW)

But generates template thus:

 Type: AWS::StepFunctions::StateMachine
    Properties:
      DefinitionString:
        Fn::Join:
          - ""
          - - '{"StartAt":"invoke-notification-service","States":{"invoke-notification-service":{"End":true,"Parameters":{"Item":{},"TableName":"'
            - Ref: events26E65764
            - '","ReturnValues":"ALL_NEW"},"Type":"Task","Resource":"arn:'
            - Ref: AWS::Partition
            - :states:::dynamodb:putItem"}}}
      RoleArn:
        Fn::GetAtt:
          - saveeventRole24A1910F
          - Arn

Notice there is a blank Item.

Of course this is with the

if isinstance(item, dict): item = DynamoAttributeValueMap(**item)

from DynamoPutItemsProps removed.

So I'm beginning to think this is an actual bug in the pyhon sdk...

@SomayaB SomayaB added @aws-cdk/aws-dynamodb Related to Amazon DynamoDB @aws-cdk/aws-stepfunctions Related to AWS StepFunctions guidance Question that needs advice or information. labels May 26, 2020
@michaelwiles
Copy link
Contributor Author

I've created a repo with a very basic example demonstrates my issue...

https://github.com/Lumkani/cdk-python-step-function-aws-cdk-8108

@shivlaks
Copy link
Contributor

@michaelwiles - thank you for taking the time to fill in this thorough report and repro!
you're absolutely right. our current modeling is not compatible with Python, Java, or C#.

I'm in the middle of rewriting the task implementations for all service integrations and DynamoDB is the next one up. I'll be making some changes to address this problem. They will be breaking changes to the current implementation.

@shivlaks shivlaks added bug This issue is a bug. p1 @aws-cdk/aws-stepfunctions-tasks and removed guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. @aws-cdk/aws-dynamodb Related to Amazon DynamoDB labels Jun 10, 2020
@michaelwiles
Copy link
Contributor Author

Thank you very much

@mergify mergify bot closed this as completed in #8466 Jun 24, 2020
mergify bot pushed a commit that referenced this issue Jun 24, 2020
Replaces the implementation to call DynamoDb from within a task to 
merge state level properties as a construct.

Notable changes:
* APIs require an `ITable` to be supplied instead of `tableName`
* `partitionKey` and `sortKey` are now represented as `key`
  * Rationale: aligning with the SDK for making these calls to Dynamo
* `DynamoAttributeValue` is now a struct rather than a class
  * Rationale: the `withX` methods don't translate to every language and
    with 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 implement`IStepFunctionsTask` and have been replaced byconstructs that can be instantiated directly. See README for examples 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants