-
Notifications
You must be signed in to change notification settings - Fork 1
Fix null handling in ingestion and simulation services #1
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.
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR improves null and configuration validation by ensuring the EventGrid connection string is present in the ingestion service and that simulation results are not null in the Mycosoft integration service. Class diagram for updated MycosoftIntegrationService null handlingclassDiagram
class MycosoftIntegrationService {
+Task<HplSimulationResult> ProcessHplSimulationAsync(HplSimulationRequest request, CancellationToken cancellationToken)
}
class MyceliumSimResult
MycosoftIntegrationService --> MyceliumSimResult : uses
%% Highlight new null check
note for MycosoftIntegrationService "Throws InvalidOperationException if simResult is null after deserialization"
Class diagram for updated EventGridPublisherClient initialization in ingestion serviceclassDiagram
class Program {
+EventGridPublisherClient EventGridPublisherClientFactory()
}
class EventGridPublisherClient
class AzureKeyCredential
Program --> EventGridPublisherClient : creates
EventGridPublisherClient o-- AzureKeyCredential : uses
%% Highlight new configuration check
note for Program "Throws InvalidOperationException if EventGrid connection string is missing"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @nodefather - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/ingestion/Program.cs:41` </location>
<code_context>
+ throw new InvalidOperationException("EventGrid connection string is not configured");
+ }
+
+ var key = configuration["EventGridKey"] ?? string.Empty;
return new EventGridPublisherClient(new Uri(endpoint), new AzureKeyCredential(key));
});
</code_context>
<issue_to_address>
Defaulting to an empty string for the EventGrid key may mask misconfiguration.
Constructing EventGridPublisherClient with an empty key may cause runtime authentication errors. Instead, throw an exception if the key is missing, as is done for the endpoint.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
var key = configuration["EventGridKey"] ?? string.Empty;
return new EventGridPublisherClient(new Uri(endpoint), new AzureKeyCredential(key));
=======
var key = configuration["EventGridKey"];
if (string.IsNullOrWhiteSpace(key))
{
throw new InvalidOperationException("EventGrid key is not configured");
}
return new EventGridPublisherClient(new Uri(endpoint), new AzureKeyCredential(key));
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| var key = configuration["EventGridKey"] ?? string.Empty; | ||
| return new EventGridPublisherClient(new Uri(endpoint), new AzureKeyCredential(key)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): Defaulting to an empty string for the EventGrid key may mask misconfiguration.
Constructing EventGridPublisherClient with an empty key may cause runtime authentication errors. Instead, throw an exception if the key is missing, as is done for the endpoint.
| var key = configuration["EventGridKey"] ?? string.Empty; | |
| return new EventGridPublisherClient(new Uri(endpoint), new AzureKeyCredential(key)); | |
| var key = configuration["EventGridKey"]; | |
| if (string.IsNullOrWhiteSpace(key)) | |
| { | |
| throw new InvalidOperationException("EventGrid key is not configured"); | |
| } | |
| return new EventGridPublisherClient(new Uri(endpoint), new AzureKeyCredential(key)); |
Summary
Testing
dotnet build NatureOS.slnnpm test -- --passWithNoTestshttps://chatgpt.com/codex/tasks/task_e_6885c85015c8832a8073b3c2312d787a
Summary by Sourcery
Improve null handling and validation in the ingestion and simulation services
Bug Fixes: