Skip to content

Conversation

@dfangl
Copy link
Member

@dfangl dfangl commented Dec 15, 2023

Motivation

Using the AWS CLI, it is easily accomplished to have surrounding double quotes around the Shard Iterator of the get_records request.

Even though this is not technically correct, both the CLI and AWS allow this.

This PR will add a test for the behavior on LS side

Kinesis-mock issue: etspaceman/kinesis-mock#682
Upstream PR: etspaceman/kinesis-mock#689

Changes

  • Add test for double quotes within shard iterator

@dfangl dfangl force-pushed the kinesis-shard-iterator-surrounding-quotes branch from a4ff172 to c7777e5 Compare December 19, 2023 14:06
@dfangl dfangl requested a review from simonrw December 19, 2023 14:07
Copy link
Contributor

@simonrw simonrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this regression test, nice and clear. Only really a question about receiving any records, but otherwise 🎉

StreamName=stream_name, ShardIteratorType="LATEST", ShardId=shard_id
)["ShardIterator"]

aws_client.kinesis.get_records(ShardIterator=f'"{shard_iterator}"')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a scenario where there might be single quotes? Do we need to cover both cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not check whether single quotes would be allowed, but the main issue of the customer was that the aws cli returns it wrapped in double quotes, so this is the most important case for now :)

@github-actions
Copy link

github-actions bot commented Dec 19, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 11m 13s ⏱️
2 411 tests 2 183 ✔️ 228 💤 0
2 412 runs  2 183 ✔️ 229 💤 0

Results for commit dbe07e9.

♻️ This comment has been updated with latest results.

@dfangl dfangl merged commit cea1705 into master Dec 19, 2023
@dfangl dfangl deleted the kinesis-shard-iterator-surrounding-quotes branch December 19, 2023 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants