added stream support #5
Conversation
There was a problem hiding this comment.
Love how simple the new input is. Just a boolean. changing this input would also update the table, not only create. Is that correct? I think so from the code, but wanted to double check.
Just wanna make sure the table state is idempotent. Another edge case is, if the user changed the stream config using the console, after deployment, whatever in the yaml file should still be applied.
|
@dodgeblaster @eahefnawy Can I chip in and ask for making |
There was a problem hiding this comment.
Thanks @dodgeblaster ... looks good. Could you just update the README with this new input?
|
Any chance that there's an ETA for this? |
|
| const validate = { | ||
| streamViewType: (inputs) => { | ||
| if (!inputs.streamViewType) { | ||
| return | ||
| } | ||
|
|
||
| const validStreamTypes = ['NEW_IMAGE', 'OLD_IMAGE', 'NEW_AND_OLD_IMAGES', 'KEYS_ONLY'] | ||
| if (!validStreamTypes.includes(inputs.streamViewType)) { | ||
| throw Error(`${inputs.streamViewType} is not a valid streamViewType.`) | ||
| } | ||
| }, | ||
|
|
||
| streamViewTypeUpdate: (comp, previousTable, inputs) => { | ||
| if (!previousTable.streamArn || !inputs.streamViewType) { | ||
| return | ||
| } | ||
|
|
||
| if (comp.state.stream && inputs.stream && comp.state.streamViewType !== inputs.streamViewType) { | ||
| throw Error(`You cannot change the view type of an existing DynamoDB stream.`) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I added validation around stream types for creating and updating tables.
streamViewType just confirms they are using a correct view type, and will throw an error if they are not
streamViewTypeUpdate is a bit more subtle. If you change the viewType on an table that already has a stream, it wont do anything. Its possible people might assume it all worked unless they see a message explaining nothing actually happened. So I made an error for it.
@eahefnawy is throwing errors the best way to give messages like this in the CLI?
| async function getStreamArn ({dynamodb, name}) { | ||
| const maxTries = 5 | ||
| let tries = 0 | ||
|
|
||
| const getStreamArn = async () => { | ||
| if (tries > maxTries) { | ||
| throw Error(`There was a problem getting the arn for your DynamoDB stream. Please try again.`) | ||
| } | ||
|
|
||
| const {streamArn } = await describeTable({ dynamodb, name}) | ||
| if (!streamArn && tries <= maxTries) { | ||
| tries++ | ||
| const sleep = ms => new Promise(r => setTimeout(r,ms)) | ||
| await sleep(3000) | ||
| return await getStreamArn() | ||
| } | ||
| return streamArn | ||
| } | ||
|
|
||
| return await getStreamArn() | ||
| } |
There was a problem hiding this comment.
This function is used in the update table function below.
I found that if I created a table with a stream, I would get returnValueFromCreateTable.TableDescription.LatestStreamArn successfully. But if I created a table with no stream, then set the stream to true in an update, the return value of the updateTable sdk call woud have returnValueFromUpdatTable.TableDescription.LatestStreamArn set to undefined (I am assuming that is because it takes a bit of time to register?). Running sls a second time will correctly give you the stream arn.
Anyways, that is a problem, because users will likely depend on getting a stream arn back from the outputs of this component. So the above function will try to get the stream arn, if it doesnt, it will wait 3 seconds and try again. It will do this 5 more times until it gets a stream arn. If it does not get it in 5 tries, it will throw an error.
The error does not mean the stream was not created, it just means we did not get the stream arn, which might be really important if the user is depending on the stream arn in other components.
| ...(disableStream && { | ||
| StreamSpecification: { | ||
| StreamEnabled: false | ||
| } | ||
| }), |
There was a problem hiding this comment.
Ive added this disableStream flag here because the SDK will error if you try to disable a non existing stream. So this flag insures we only add this to the object if the inputs are disabling a stream that exists.
|
Hello, will this PR be merged ? Or is it too far from current git history ? @dodgeblaster @eahefnawy ? |
No description provided.