Skip to content

ESM/Pipes stream pollers: add shards to init params #12659

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

Jump to bottom
Merged
merged 2 commits into from
May 30, 2025
Merged

ESM/Pipes stream pollers: add shards to init params #12659

merged 2 commits into from
May 30, 2025

Conversation

tiurin
Copy link
Contributor

@tiurin tiurin commented May 26, 2025

Motivation

Changes needed for UpdatePipe operation. A companion PR to the main one in ext.

Changes

  • Initialize the new poller for updated pipe with shards info from the previous poller. When updating a pipe, its new poller needs to pick up where the old one left off.
  • Fix get current time inconsistency in DynamoDB poller - led to valid records being expired in bisect_events_by_record_age in stream poller. Use same helper function across poller instead of deprecated .utcnow() method.

@tiurin tiurin added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases aws:pipes Amazon EventBridge Pipes labels May 26, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented May 26, 2025 •
edited
Loading

Test Results - Preflight, Unit

21 579 tests  ±0   19 927 ✅ ±0   6m 12s ⏱️ -4s
     1 suites ±0    1 652 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 38620e3. ± Comparison against base commit 433aeff.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 26, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 21m 20s ⏱️ - 24m 4s
3 259 tests  - 1 209  2 973 ✅  - 1 107  286 💤  - 102  0 ❌ ±0 
3 261 runs   - 1 209  2 973 ✅  - 1 107  288 💤  - 102  0 ❌ ±0 

Results for commit 38620e3. ± Comparison against base commit 433aeff.

This pull request removes 1209 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.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 26, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files  ±0      5 suites  ±0   2h 23m 51s ⏱️ + 1m 1s
4 823 tests ±0  4 282 ✅ ±0  541 💤 ±0  0 ❌ ±0 
4 829 runs  ±0  4 282 ✅ ±0  547 💤 ±0  0 ❌ ±0 

Results for commit 38620e3. ± Comparison against base commit 433aeff.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 26, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

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

Results for commit 38620e3. ± Comparison against base commit 433aeff.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 26, 2025 •
edited
Loading

Test Results - Alternative Providers

597 tests  ±0   420 ✅ ±0   14m 41s ⏱️ -11s
  4 suites ±0   177 💤 ±0 
  4 files   ±0     0 ❌ ±0 

Results for commit 38620e3. ± Comparison against base commit 433aeff.

♻️ This comment has been updated with latest results.

tiurin added 2 commits May 28, 2025 09:59
@tiurin
Add init parameter for shards
9769022 
When updating a pipe, its new poller needs to pick up where the old one left off.
@tiurin
Use same util function to get current time
38620e3 
datetime.utcnow() is deprecated and was giving a 2-hour difference result than get_current_time() for a computer running in CEST timezone.
This led to valid records being expired in bisect_events_by_record_age in stream poller.

Same function needs to be used across the code, especially when a comparison is made within same logic.
@tiurin tiurin force-pushed the feature/update-pipe-source-parameters branch from 9078508 to 38620e3 Compare May 28, 2025 07:59
@tiurin tiurin added this to the 4.5 milestone May 29, 2025
gregfurman
gregfurman reviewed May 29, 2025
View reviewed changes
Copy link
Contributor

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

Neat addition! 😄 Once we have a green run in -ext (and against your pipes feature branch) I'm happy to merge.

@tiurin tiurin requested a review from gregfurman May 30, 2025 06:34
gregfurman
gregfurman approved these changes May 30, 2025
View reviewed changes
Copy link
Contributor

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

LGTM! Feel free to merge :shipit:

One small nit: can we change the title to be a bit more informative? Especially since we don't support UpdatePipe in this repo.

@tiurin tiurin changed the title UpdatePipe: source parameters ESM/Pipes stream pollers: add shards to init params May 30, 2025
@tiurin tiurin merged commit d5ddcab into master May 30, 2025
59 checks passed
@tiurin tiurin deleted the feature/update-pipe-source-parameters branch May 30, 2025 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregfurman gregfurman gregfurman approved these changes

@joe4dev joe4dev Awaiting requested review from joe4dev joe4dev is a code owner

Assignees
No one assigned
Labels
aws:pipes Amazon EventBridge Pipes semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Milestone
4.5
Development

Successfully merging this pull request may close these issues.
2 participants
@tiurin @gregfurman