Skip to content

Update Kinesis Mock to 0.3.5 #7639

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.

Sign up for GitHub

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

Closed
etspaceman wants to merge 6 commits into from
Closed

Update Kinesis Mock to 0.3.5 #7639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
83d7667
Update Kinesis Mock to 0.3.5
etspaceman Feb 7, 2023
5d429d0
Remove assertion for FailedRecordCount
etspaceman Feb 7, 2023
22f2eed
More test fixes
etspaceman Feb 7, 2023
abbeb89
Linting
etspaceman Feb 7, 2023
22b089d
Fix
etspaceman Feb 7, 2023
f0b90a7
Merge remote-tracking branch 'localstack/master' into patch-10
etspaceman Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion localstack/services/kinesis/packages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from localstack.packages import GitHubReleaseInstaller, Package, PackageInstaller
from localstack.utils.platform import get_arch, get_os

_KINESIS_MOCK_VERSION = os.environ.get("KINESIS_MOCK_VERSION") or "0.3.4"
_KINESIS_MOCK_VERSION = os.environ.get("KINESIS_MOCK_VERSION") or "0.3.5"


class KinesisMockPackage(Package):
Expand Down
7 changes: 1 addition & 6 deletions tests/integration/cloudformation/resources/test_kinesis.snapshot.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@
"stream_description": {
"StreamDescription": {
"EncryptionType": "NONE",
"EnhancedMonitoring": [
Copy link
Contributor Author

@etspaceman etspaceman Feb 7, 2023

Choose a reason for hiding this comment

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

These are not returned when empty now. According to the Kinesis API, ShardLevelMetrics must be non-empty if this is returned, otherwise it is null. In this case, it is null.

Copy link
Member

@dominikschubert dominikschubert Feb 8, 2023

Choose a reason for hiding this comment

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

Hi @etspaceman !

These are actually validated against AWS. Our *.snapshot.json files are all automatically generated when running against AWS and we generally don't update them manually for this reason. To update them we run the test with the following additional environment variables.

AWS_PROFILE=<your-profile>
TEST_TARGET=AWS_CLOUD
SNAPSHOT_UPDATE=1  # or set it to `0` if you just want to verify that the existing snapshot still validates against AWS

This could've changed since we recorded it, but I just updated to have the newest specs in boto3, re-ran the test and this seems to still be the observed result unfortunately.

Additionally, please note that these are not the raw HTTP responses you're seeing, but the objects generated by the SDK.

Copy link
Contributor Author

@etspaceman etspaceman Feb 8, 2023

Choose a reason for hiding this comment

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

That's very interesting. Here is section in the smithy documents about this:

https://github.com/aws/aws-sdk-js-v3/blob/main/codegen/sdk-codegen/aws-models/kinesis.json#L5072

You can see that the min length here is 1. I've confirmed this in the Java SDK as well.

But also, if I run a describe-stream in the aws-cli, I see empty lists being returned. So I wonder if this is a bug in Smithy.

Copy link
Contributor Author

@etspaceman etspaceman Feb 8, 2023

Choose a reason for hiding this comment

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

@dominikschubert I am going to open a support ticket with AWS on this one to get some clarity. For now I'll close this PR and revert my change in kinesis-mock

{
"ShardLevelMetrics": []
}
],
"HasMoreShards": false,
"RetentionPeriodHours": 24,
"Shards": [
Expand Down Expand Up @@ -63,4 +58,4 @@
}
}
}
}
}
5 changes: 0 additions & 5 deletions tests/integration/cloudformation/resources/test_lambda.snapshot.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1228,11 +1228,6 @@
"describe_stream_result": {
"StreamDescription": {
"EncryptionType": "NONE",
"EnhancedMonitoring": [
Copy link
Contributor Author

@etspaceman etspaceman Feb 7, 2023

Choose a reason for hiding this comment

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

Ditto

{
"ShardLevelMetrics": []
}
],
"HasMoreShards": false,
"RetentionPeriodHours": 24,
"Shards": [
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_apigateway.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ def test_api_gateway_kinesis_integration(self):
# post test data to Kinesis via API Gateway
result = requests.post(url, data=json.dumps(test_data))
result = json.loads(to_str(result.content))
assert 0 == result["FailedRecordCount"]
assert result.get("FailedRecordCount") is None
assert len(test_data["records"]) == len(result["Records"])

# clean up
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_error_injection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def test_kinesis_error_injection(self, monkeypatch, kinesis_client, wait_for_str

# by default, no errors
test_no_errors = kinesis.put_records(StreamName=stream_name, Records=records)
assert test_no_errors["FailedRecordCount"] == 0
assert test_no_errors.get("FailedRecordCount") is None

# with a probability of 1, always throw errors
monkeypatch.setattr(config, "KINESIS_ERROR_PROBABILITY", 1.0)
Expand Down