Conversation
|
@codex can you review this please? |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b8c70499c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _, _ = tracer.SetDataStreamsCheckpointWithParams(datastreams.ExtractFromBase64Carrier(ctx, &evt), options.CheckpointParams{ | ||
| ServiceOverride: "productservice-publiceventpublisher", | ||
| }, "direction:in", "type:sqs", "topic:"+evt.Type, "manual_checkpoint:true") | ||
| }, "direction:in", productcore.InternalPubSubName, "topic:"+evt.Type, "manual_checkpoint:true") |
There was a problem hiding this comment.
This handler is invoked from events.SQSEvent and tags spans as messaging.system=aws_sqs, but the new checkpoint now records ingress as type:sns; that misclassifies the transport leg and degrades Data Streams topology/latency attribution for the SNS→SQS path (and makes this consumer inconsistent with other SQS consumers in this repo that still checkpoint type:sqs).
Useful? React with 👍 / 👎.
...ntory-service/inventory-api/src/test/java/com/inventory/api/MockInventoryItemRepository.java
Show resolved
Hide resolved
The mock was storing and returning item references, so mutations from the parallel stock-check phase (processProductReservation) were immediately visible in stored state without a successful update() call. This caused the rollback assertion in reserve_stock_rolls_back_on_partial_write_failure to fail because productId3's reserved level was non-zero despite update() always throwing. Returning copies from withProductId() now matches real database read behaviour. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29fba2e55f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SetHeader, | ||
| scope.Span.Context, | ||
| "sns", | ||
| "eventbridge", |
There was a problem hiding this comment.
Changing InjectIncludingDsm to use "eventbridge" here updates only the producer side for orders.* events, but inventory ACL consumers still call setConsumeCheckpoint("sns", ...) in src/inventory-service/inventory-acl/src/main/java/com/inventory/acl/lambda/handleOrderCreatedLambda.java and handleOrderCompletedLambda.java (line 68 in both). For the order→inventory flow this creates mixed transport tags for the same pipeline, which splits/misattributes Data Streams topology and latency attribution; this should be migrated end-to-end in one change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d097c66b10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| for (InventoryItem inventoryItem : result.stockAddedFor()) { | ||
| reserveStockWithRetry(inventoryItem.getProductId(), orderNumber); | ||
| successfullyReserved.add(inventoryItem.getProductId()); |
There was a problem hiding this comment.
The rollback list is populated unconditionally after reserveStockWithRetry, but that method can return after an idempotent no-op when the same orderNumber was already reserved (for example, on an SQS retry of orderCreated). If a later product update fails, the rollback loop calls releaseStockWithRetry for these entries and can remove a reservation that existed before this attempt, leaving inventory under-reserved for an order that was previously successful.
Useful? React with 👍 / 👎.
1 participant
What does this PR do?
Motivation
Testing Guidelines
Additional Notes
Types of Changes
Check all that apply