feat: validate basket products against Catalog — architecture impact demo - #6
ivanmilevtues wants to merge 1 commit into
Conversation
Introduce a Basket API to Catalog Service dependency for architecture review. Amp-Thread-ID: https://ampcode.com/threads/T-01a0d0bb-dbb1-77fc-9712-4aeb9f411d79 Co-authored-by: Amp <amp@ampcode.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Additional verification: AppHost builds successfully with |
CodeBoarding reviewStatus: 1 changed component See the full change in CodeBoarding. graph LR
n_Ordering["Ordering"]
n_ClientApp["ClientApp"]
n_Webhooks["Webhooks"]
n_Identity["Identity"]
n_WebApp["WebApp"]
n_EventBus["EventBus"]
n_js["js"]
n_Loose_files_in_src_WebApp["Loose files in src.WebApp"]
n_Ordering -- "calls" --> n_Webhooks
n_Ordering -- "calls" --> n_EventBus
n_ClientApp -- "calls" --> n_Webhooks
n_Webhooks -- "calls" --> n_ClientApp
n_WebApp -- "calls" --> n_Webhooks
classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;
class n_Webhooks modified;
|
![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: c068b520e5
ℹ️ 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".
| catch (OperationCanceledException) when (!context.CancellationToken.IsCancellationRequested) | ||
| { | ||
| throw new RpcException(new Status(StatusCode.Unavailable, "Catalog validation timed out. Please retry.")); | ||
| } |
There was a problem hiding this comment.
When the outgoing Catalog request reaches the timeout configured by the newly enabled AddStandardResilienceHandler, Polly throws TimeoutRejectedException, which does not derive from OperationCanceledException. Consequently this catch is skipped and the gRPC call surfaces as an unhandled Unknown/Internal error rather than the promised retryable Unavailable status. Catch the resilience timeout exception explicitly while continuing to let caller-initiated cancellation propagate.
Useful? React with 👍 / 👎.
1 participant
Architecture demo
Introduce a real new runtime dependency: Basket API → Catalog Service. Basket updates now check that each distinct product exists in Catalog before writing to Redis. This demonstrates an architectural change rather than a cosmetic code change.
Baseline and expected analysis
I inspected
.codeboarding/analysis.jsonon the latest fork main before implementing this change. Its Basket API component (8) has no outgoing relation to Catalog Service (5), including their child components. The baseline is intentionally unchanged in this PR: the review must infer the new relationship from source.Expected highlights:
The architecture tradeoff is visible at a glance: nonempty basket writes now depend on Catalog availability. This is an expected review result, not a claim that the generated graph has already been verified.
Behavior
Verification
dotnet test --project tests/Basket.UnitTests/Basket.UnitTests.csproj "-p:DefaultItemExcludes=**/obj/**"(excludes stale local obj files).git diff --checkpasses.Leave open for the CodeBoarding architecture review demo.