LNK-2801 Add Property to Kafka Messages #419
Conversation
WalkthroughThe changes introduce a new boolean property, Changes
Assessment against linked issues
Possibly related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- DotNet/DataAcquisition/Application/Models/Kafka/ResourceAcquired.cs (1 hunks)
- DotNet/DataAcquisition/Application/Services/PatientDataService.cs (1 hunks)
- DotNet/Normalization/Application/Models/Messages/ResourceAcquiredMessage.cs (1 hunks)
- DotNet/Normalization/Application/Models/Messages/ResourceNormalizedMessage.cs (1 hunks)
- DotNet/Normalization/Listeners/ResourceAcquiredListener.cs (3 hunks)
Additional comments not posted (9)
DotNet/Normalization/Application/Models/Messages/ResourceAcquiredMessage.cs (1)
7-7: Addition of new propertyAcquisitionCompletelooks good.The new property
AcquisitionCompleteis a boolean initialized tofalse. This enhances the class by allowing it to track the completion status of resource acquisition.DotNet/DataAcquisition/Application/Models/Kafka/ResourceAcquired.cs (1)
7-7: Addition of new propertyAcquisitionCompletelooks good.The new property
AcquisitionCompleteis a boolean initialized tofalse. This enhances the class by allowing it to track the completion status of resource acquisition.DotNet/Normalization/Application/Models/Messages/ResourceNormalizedMessage.cs (1)
8-8: Addition of new propertyAcquisitionCompletelooks good.The new property
AcquisitionCompleteis a boolean initialized tofalse. This enhances the class by allowing it to track the completion status of resource acquisition.DotNet/DataAcquisition/Application/Services/PatientDataService.cs (3)
202-204: Ensure proper error handling in theGetmethod.The addition of the
ProduceTailingMessagecall within the catch block is a good practice to log the context of the failure. Ensure that all relevant identifiers are correctly passed to the method.
213-214: Ensure the completion status is correctly logged.The addition of the
ProduceTailingMessagecall after the try-catch block ensures that the completion status is logged regardless of the outcome. This enhances the traceability of the data acquisition process.
217-235: Verify the correctness of theProduceTailingMessagemethod.The method correctly constructs and sends a
ResourceAcquiredmessage to the Kafka topic. Ensure that all necessary properties are included and correctly assigned.DotNet/Normalization/Listeners/ResourceAcquiredListener.cs (3)
107-113: Enhance error handling conditions inStartConsumerLoop.The new condition ensures that a
DeadLetterExceptionis only thrown ifAcquisitionCompleteis false. This allows for more nuanced control over message validity.
128-150: Handle completed acquisitions with null resources.The new logic handles scenarios where
AcquisitionCompleteis true but the resource is null, allowing for the production of messages for further processing. This enhances the robustness of the message processing logic.
310-313: Ensure correct assignment of new properties inResourceNormalizedMessage.The new properties are correctly assigned based on the updated message structure. This ensures consistency in the way the message data is handled throughout the method.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- DotNet/Normalization/Listeners/ResourceAcquiredListener.cs (3 hunks)
Additional comments not posted (4)
DotNet/Normalization/Listeners/ResourceAcquiredListener.cs (4)
107-113: Verify the correctness of the new condition.The new condition refines error handling by allowing messages with
AcquisitionCompleteset to true to bypass certain checks. Ensure that this change aligns with the intended logic and does not introduce any unintended side effects.
146-167: Verify the accuracy of logging and message construction.The new logic allows for the production of messages even when resources are missing. Ensure that the informational log and the construction of
ResourceNormalizedMessageare accurate and align with the intended functionality.
169-171: LGTM!The code correctly checks if the operation sequence is null or empty and handles it appropriately.
310-313: LGTM!The code correctly updates the construction of
ResourceNormalizedMessageto include theAcquisitionCompleteproperty.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- DotNet/Normalization/Listeners/ResourceAcquiredListener.cs (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- DotNet/Normalization/Listeners/ResourceAcquiredListener.cs
…dicate-if-the-resource-acquisition-process-has-completed-in-Data-Acquisition-and-Normalization
seanmcilvenna
deleted the
LNK-2801-Add-a-property-to-event-models-to-indicate-if-the-resource-acquisition-process-has-completed-in-Data-Acquisition-and-Normalization
branch
Summary by CodeRabbit
New Features
AcquisitionComplete, to improve tracking of resource acquisition completion across multiple classes.Getmethod to include better error handling and logging for successful and failed data retrieval processes.Bug Fixes
AcquisitionCompletestatus.