Skip to content

UpdatePipe: source parameters #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
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

UpdatePipe: source parameters #12659

wants to merge 2 commits into from

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@gregfurman gregfurman Awaiting requested review from gregfurman gregfurman 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
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@tiurin