Skip to content

add small fixes/improvements to Firehose.CreateDeliveryStream #12656

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pinzon
Copy link
Member

@pinzon pinzon commented May 22, 2025

Motivation

This PR adds small fixes for issues reported by @kostas-localstack internally.

1.- The operation allows the user to attempt to create a second DeliveryStream with a name already in use.
2.- The DirectPut is the default value for DeliveryStreamType when is not defined.
3.- When creating a DeliveryStream with Kinesis as destination and it fails. The resource is still created.

Changes

  • Add assertion that the name is not in use before attempting to create a DeliveryStream.
  • Add DirectPut as default value.
  • Move the storage of the resource to the end of the operation.

@pinzon pinzon added aws:firehose Amazon Data Firehose semver: patch Non-breaking changes which can be included in patch releases labels May 22, 2025
Copy link

github-actions bot commented May 22, 2025

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   6m 56s ⏱️ - 1h 37m 2s
522 tests  - 3 941  504 ✅  - 3 570  18 💤  - 371  0 ❌ ±0 
524 runs   - 3 941  504 ✅  - 3 570  20 💤  - 371  0 ❌ ±0 

Results for commit ca8214f. ± Comparison against base commit f6ddc00.

This pull request removes 3941 tests.
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented May 22, 2025

Test Results - Preflight, Unit

21 601 tests  ±0   19 955 ✅ ±0   6m 8s ⏱️ -4s
     1 suites ±0    1 646 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit ca8214f. ± Comparison against base commit f6ddc00.

♻️ This comment has been updated with latest results.

Copy link

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 12s ⏱️ +4s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit ca8214f. ± Comparison against base commit f6ddc00.

Copy link

Test Results - Alternative Providers

597 tests   419 ✅  14m 56s ⏱️
  4 suites  178 💤
  4 files      0 ❌

Results for commit ca8214f.

Copy link

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 24m 4s ⏱️
4 818 tests 4 276 ✅ 542 💤 0 ❌
4 824 runs  4 276 ✅ 548 💤 0 ❌

Results for commit ca8214f.

@pinzon pinzon marked this pull request as ready for review May 22, 2025 22:43
@pinzon pinzon requested a review from viren-nadkarni May 23, 2025 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:firehose Amazon Data Firehose 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.

2 participants