Ignore privilege count object in provider stream handler #1249
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughThe provider update ingest handler now skips privilege-count DynamoDB counter records, logs and records SQS message IDs for partial failures (missing images or missing essential fields), and consolidates top-level batchItemFailures reporting after processing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
landonshumway-ia
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py:
- Line 819: The test's eventSourceARN uses a real AWS account ID and production
stack name; update the 'eventSourceARN' value to use the fake account ID used
elsewhere in the test (123456789012) and a generic test table name (e.g.,
Test-ProviderTable or ProviderTable) so the ARN is consistent and non-production
(e.g., arn:aws:dynamodb:us-east-1:123456789012:table/ProviderTable/stream/...),
ensuring the change targets the 'eventSourceARN' entry in this test fixture.
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py (1)
72-77: Privilege count skip logic correctly implements the fix.The detection logic correctly identifies privilege count tracker records by checking for 'PRIVILEGE_COUNT' in the pk field, then skips them with an informational log. Importantly, skipped records are not added to
batch_item_failuressince skipping is expected behavior, not a failure.♻️ Optional: Extract magic string to constant
For improved maintainability, consider extracting the magic string to a module-level constant:
# At module level (near top of file) PRIVILEGE_COUNT_PK_MARKER = 'PRIVILEGE_COUNT' # In the handler (lines 75-76) if pk and PRIVILEGE_COUNT_PK_MARKER in pk: logger.info('Skipping privilege count record', message_id=message_id, pk=pk)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.pybackend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T19:05:36.255Z
Learning: In CompactConnect PR #848, the user landonshumway-ia decided to leave timezone handling code in _should_allow_reregistration function as-is after testing in sandbox environment confirmed it works correctly. The user's reasoning was that reregistration is an edge case, the code has been tested and verified, and AWS is unlikely to change behavior that would break many clients. This represents a pragmatic engineering decision based on testing and risk assessment.
📚 Learning: 2025-12-01T17:57:35.454Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/tests/app/test_search_persistent_stack.py:104-130
Timestamp: 2025-12-01T17:57:35.454Z
Learning: In backend/compact-connect/tests/app/test_search_persistent_stack.py, the OpenSearch Domain's EncryptionAtRestOptions.KmsKeyId assertion correctly uses {"Ref": encryption_key_logical_id} format. This pattern has been validated to work with CDK's opensearchservice.Domain construct.
Applied to files:
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.pybackend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py
📚 Learning: 2026-01-05T22:50:09.696Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/tests/smoke/smoke_common.py:503-521
Timestamp: 2026-01-05T22:50:09.696Z
Learning: Enforce the rule: there is only one privilege record per provider per jurisdiction, and do not paginate when querying privilege records by jurisdiction. This applies across the codebase wherever privilege records are queried or tested (implementation and tests). Note that privilege update records can have multiple rows and require pagination. Implement checks and queries to assume a unique constraint for (provider, jurisdiction) on privilege records, and ensure any list endpoints or test smoke checks reflect no pagination for jurisdiction-based privilege queries while preserving pagination for privilege updates.
Applied to files:
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.pybackend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py
📚 Learning: 2025-06-30T17:30:31.252Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 864
File: backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py:866-866
Timestamp: 2025-06-30T17:30:31.252Z
Learning: In the CompactConnect project, the `sqs_handler` decorator transforms Lambda functions to accept `(event, context)` parameters and automatically handles iteration over `event['Records']`, parsing JSON from each record's body, and managing SQS batch item failures. Functions decorated with `sqs_handler` have their underlying implementation accept a single `message: dict` parameter, but the decorator makes them callable with standard Lambda `(event, context)` signature. Tests should call these decorated functions with `(event, context)` parameters.
Applied to files:
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py
📚 Learning: 2025-12-11T17:30:34.483Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py:15-20
Timestamp: 2025-12-11T17:30:34.483Z
Learning: In this repository, CDK PythonFunction bundling may allow Python handlers to import modules from a parent directory without an __init__.py (e.g., from opensearch_client import OpenSearchClient) and still resolve at runtime. Treat this as runtime-specific behavior to rely on rather than static analysis; ensure tests validate the import path and document this expectation for the search-level modules. When reviewing similar files, confirm that the bundling setup indeed supports these bare imports and that any changes do not introduce static-analysis false positives.
Applied to files:
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py
🧬 Code graph analysis (2)
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py (1)
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py (1)
provider_update_ingest_handler(28-218)
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (1)
pk(42-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestApp
🔇 Additional comments (6)
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py (2)
793-820: Test helper correctly models privilege count records.The helper creates a realistic DynamoDB stream record for the privilege count tracker with pk/sk both containing 'PRIVILEGE_COUNT', matching the production case described in issue #1247.
822-848: Comprehensive test coverage for privilege count skip behavior.The test correctly verifies that:
- Privilege count records do not trigger OpenSearch indexing or deletion
- No batch item failures are reported (skipping is not a failure)
- The handler processes the record without errors
This test effectively validates the fix for issue #1247.
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py (4)
53-53: Top-level batch failure tracking enables partial success handling.Initializing
batch_item_failuresat the top level allows the handler to accumulate failures throughout processing and return them at the end. This is a solid improvement over halting on first error.
65-67: Missing image data now handled gracefully.Changing from error-raising to warning-and-continue allows the handler to process other records in the batch even when some records lack image data. This aligns with the PR objective to enhance error handling for partial success scenarios.
84-90: Missing required fields now reported as batch failures.The change from potentially raising an error to logging a warning and adding to batch failures enables partial success. Records with missing compact/providerId are now properly tracked in
batch_item_failuresfor SQS retry, while other valid records in the batch can still be processed.
205-213: Batch failures correctly mapped back to SQS message IDs.The logic correctly maps failed provider IDs back to their originating SQS message IDs using the
record_mappingdict. This ensures SQS receives the properitemIdentifiervalues for partial batch failure reporting.
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
Outdated
Show resolved
Hide resolved
This counter object is not provider specific, so it can be ignored when processing events from the provider table stream. This also enhances the error handling so we place these failed objects in the DLQ.
landonshumway-ia
force-pushed
the
fix/ignore-counter-record
branch
from
83b58c9 to
86d812c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py (1)
793-820: Good production-repro test; consider minimizing the counter stream record shape to reduce coupling.The new test accurately locks in the desired behavior (privilege-count records don’t trigger OpenSearch writes and don’t produce
batchItemFailures). The helper record currently includes several DynamoDB stream fields that the handler doesn’t read; you could slim it down to justdynamodb.NewImage.pk(and maybeOldImage/Keysif you want) to make the test less brittle to future event-shape tweaks while still reproducing the production issue.Also applies to: 822-848
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T19:05:36.255Z
Learning: In CompactConnect PR #848, the user landonshumway-ia decided to leave timezone handling code in _should_allow_reregistration function as-is after testing in sandbox environment confirmed it works correctly. The user's reasoning was that reregistration is an edge case, the code has been tested and verified, and AWS is unlikely to change behavior that would break many clients. This represents a pragmatic engineering decision based on testing and risk assessment.
📚 Learning: 2025-12-01T17:57:35.454Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/tests/app/test_search_persistent_stack.py:104-130
Timestamp: 2025-12-01T17:57:35.454Z
Learning: In backend/compact-connect/tests/app/test_search_persistent_stack.py, the OpenSearch Domain's EncryptionAtRestOptions.KmsKeyId assertion correctly uses {"Ref": encryption_key_logical_id} format. This pattern has been validated to work with CDK's opensearchservice.Domain construct.
Applied to files:
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
📚 Learning: 2025-10-15T21:52:47.392Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1135
File: backend/compact-connect/stacks/api_lambda_stack/provider_management.py:100-0
Timestamp: 2025-10-15T21:52:47.392Z
Learning: In the CompactConnect repository, the team prefers to keep cdk-nag suppressions explicit with hard-coded logical IDs (e.g., 'Resource::<ProviderTableEC5D0597.Arn>/index/*') rather than using generic suppressions. This intentional brittleness ensures that when underlying resources change, the suppressions break and force manual review of the IAM policies.
Applied to files:
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
📚 Learning: 2026-01-05T22:50:09.696Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/tests/smoke/smoke_common.py:503-521
Timestamp: 2026-01-05T22:50:09.696Z
Learning: Enforce the rule: there is only one privilege record per provider per jurisdiction, and do not paginate when querying privilege records by jurisdiction. This applies across the codebase wherever privilege records are queried or tested (implementation and tests). Note that privilege update records can have multiple rows and require pagination. Implement checks and queries to assume a unique constraint for (provider, jurisdiction) on privilege records, and ensure any list endpoints or test smoke checks reflect no pagination for jurisdiction-based privilege queries while preserving pagination for privilege updates.
Applied to files:
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
🧬 Code graph analysis (1)
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py (2)
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py (1)
provider_update_ingest_handler(28-218)backend/compact-connect/lambdas/python/search/opensearch_client.py (2)
bulk_index(211-233)bulk_delete(235-275)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestApp
Collaborator
Author
|
The linter is failing due to dependencies that need to be updated, which is handled in this PR #1243 To keep this hotfix small, we will not be adding those updates here. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py (1)
71-78: Make privilege-counter detection more robust (and cheaper) by checkingdynamodb.Keys.pkbefore deserializing.
Today you only detect the counter via the deserialized image’spk; usingKeysavoids relying onNewImage/OldImageshape and skips theTypeDeserializer()work for those records.Proposed diff
@sqs_batch_handler def provider_update_ingest_handler(records: list[dict]) -> dict: @@ - # Extract compact and providerId from each record + deserializer = TypeDeserializer() + + # Extract compact and providerId from each record for record in records: message_id = record['messageId'] # The body contains the DynamoDB stream record sent via EventBridge Pipe stream_record = record['body'] - # Try to get the data from NewImage first, fall back to OldImage for deletes - image = stream_record.get('dynamodb', {}).get('NewImage') or stream_record.get('dynamodb', {}).get('OldImage') + dynamodb = stream_record.get('dynamodb', {}) + + # Skip privilege counter records early (keys are always present on stream records) + keys_pk = (dynamodb.get('Keys', {}).get('pk', {}) or {}).get('S', '') + if keys_pk and 'PRIVILEGE_COUNT' in keys_pk: + logger.info('Skipping privilege count record', message_id=message_id, pk=keys_pk) + continue + + # Try to get the data from NewImage first, fall back to OldImage for deletes + image = dynamodb.get('NewImage') or dynamodb.get('OldImage') @@ - deserialized_image = TypeDeserializer().deserialize(value={'M': image}) - - # Check if this is a privilege counter record (used to track the next privilege number) - pk = deserialized_image.get('pk', '') - if pk and 'PRIVILEGE_COUNT' in pk: - logger.info('Skipping privilege count record', message_id=message_id, pk=pk) - continue + deserialized_image = deserializer.deserialize(value={'M': image})
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.pybackend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 848
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/registration.py:111-117
Timestamp: 2025-06-17T19:05:36.255Z
Learning: In CompactConnect PR #848, the user landonshumway-ia decided to leave timezone handling code in _should_allow_reregistration function as-is after testing in sandbox environment confirmed it works correctly. The user's reasoning was that reregistration is an edge case, the code has been tested and verified, and AWS is unlikely to change behavior that would break many clients. This represents a pragmatic engineering decision based on testing and risk assessment.
📚 Learning: 2025-12-16T21:43:07.408Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/search.py:131-140
Timestamp: 2025-12-16T21:43:07.408Z
Learning: In backend/compact-connect/lambdas/python/search/handlers/search.py, avoid logging the full request body. Do not log sensitive content by default. If logging is required for security investigations, redact or mask sensitive fields (e.g., credentials, tokens, PII) and log only safe metadata (method, path, status, user identifier). Use a secure, access-controlled audit log or feature flag to enable such logs, ensuring minimal exposure and compliance with security policies. This guideline targets Python backend handlers handling external requests and should be considered for similar files with request processing.
Applied to files:
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.pybackend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
📚 Learning: 2025-06-30T17:30:31.252Z
Learnt from: jusdino
Repo: csg-org/CompactConnect PR: 864
File: backend/compact-connect/lambdas/python/data-events/tests/function/test_encumbrance_events.py:866-866
Timestamp: 2025-06-30T17:30:31.252Z
Learning: In the CompactConnect project, the `sqs_handler` decorator transforms Lambda functions to accept `(event, context)` parameters and automatically handles iteration over `event['Records']`, parsing JSON from each record's body, and managing SQS batch item failures. Functions decorated with `sqs_handler` have their underlying implementation accept a single `message: dict` parameter, but the decorator makes them callable with standard Lambda `(event, context)` signature. Tests should call these decorated functions with `(event, context)` parameters.
Applied to files:
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py
📚 Learning: 2025-12-11T17:30:34.483Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py:15-20
Timestamp: 2025-12-11T17:30:34.483Z
Learning: In this repository, CDK PythonFunction bundling may allow Python handlers to import modules from a parent directory without an __init__.py (e.g., from opensearch_client import OpenSearchClient) and still resolve at runtime. Treat this as runtime-specific behavior to rely on rather than static analysis; ensure tests validate the import path and document this expectation for the search-level modules. When reviewing similar files, confirm that the bundling setup indeed supports these bare imports and that any changes do not introduce static-analysis false positives.
Applied to files:
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py
📚 Learning: 2026-01-05T22:50:09.696Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1243
File: backend/compact-connect/tests/smoke/smoke_common.py:503-521
Timestamp: 2026-01-05T22:50:09.696Z
Learning: Enforce the rule: there is only one privilege record per provider per jurisdiction, and do not paginate when querying privilege records by jurisdiction. This applies across the codebase wherever privilege records are queried or tested (implementation and tests). Note that privilege update records can have multiple rows and require pagination. Implement checks and queries to assume a unique constraint for (provider, jurisdiction) on privilege records, and ensure any list endpoints or test smoke checks reflect no pagination for jurisdiction-based privilege queries while preserving pagination for privilege updates.
Applied to files:
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.pybackend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
📚 Learning: 2025-12-01T17:57:35.454Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1219
File: backend/compact-connect/tests/app/test_search_persistent_stack.py:104-130
Timestamp: 2025-12-01T17:57:35.454Z
Learning: In backend/compact-connect/tests/app/test_search_persistent_stack.py, the OpenSearch Domain's EncryptionAtRestOptions.KmsKeyId assertion correctly uses {"Ref": encryption_key_logical_id} format. This pattern has been validated to work with CDK's opensearchservice.Domain construct.
Applied to files:
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
📚 Learning: 2025-10-15T21:52:47.392Z
Learnt from: landonshumway-ia
Repo: csg-org/CompactConnect PR: 1135
File: backend/compact-connect/stacks/api_lambda_stack/provider_management.py:100-0
Timestamp: 2025-10-15T21:52:47.392Z
Learning: In the CompactConnect repository, the team prefers to keep cdk-nag suppressions explicit with hard-coded logical IDs (e.g., 'Resource::<ProviderTableEC5D0597.Arn>/index/*') rather than using generic suppressions. This intentional brittleness ensures that when underlying resources change, the suppressions break and force manual review of the IAM policies.
Applied to files:
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py
🧬 Code graph analysis (1)
backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/investigation/__init__.py (1)
pk(42-48)backend/compact-connect/lambdas/python/common/cc_common/utils.py (1)
get(86-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestApp
🔇 Additional comments (4)
backend/compact-connect/lambdas/python/search/tests/function/test_provider_update_ingest.py (2)
793-821: Test fixture for privilege counter record looks realistic and focused.
The record shape (Keys + NewImage/OldImage, numericprivilegeCount, pk/sk containingPRIVILEGE_COUNT) is a good regression fixture for the production case.
822-848: Good regression test: skip counter record without indexing and without batch failure.
This directly protects the intended behavior (ignore non-provider counter updates) and ensures the batch isn’t partially failed unnecessarily.backend/compact-connect/lambdas/python/search/handlers/provider_update_ingest.py (2)
53-67: Batch failure handling for “no image data” is the right behavior for DLQ routing.
Returning themessageIdviabatchItemFailuresshould prevent silent drops and aligns with the “malformed events should end up in DLQ” objective.
83-90: Good: missing essential fields now produces batchItemFailures (instead of throwing / silently skipping).
Also good that you don’t log the full record payload here (consistent with prior logging guidance). Based on learnings, avoid logging full request/body content.
Collaborator
Author
|
@jlkravitz This is ready for your review. Thanks |
jlkravitz
approved these changes
Jan 12, 2026
Collaborator
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
@isabeleliassen Good to merge!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This counter object is not provider specific, so it can be ignored when processing events from the provider table stream. This also enhances the error handling so we place these failed objects in the DLQ.
Testing List
Closes #1247
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.