-
Notifications
You must be signed in to change notification settings - Fork 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
Distinguish between Mezmo error vs no logs for specified time period #180
Conversation
Signed-off-by: Dave Thaler <[email protected]>
WalkthroughThe changes in the Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
OrcanodeMonitor/Core/MezmoFetcher.cs (1)
85-86
: Consider adding more descriptive error logging.
While the error handling is correct, consider adding more context to help diagnose issues when GetMezmoDataAsync
returns null. This could include the URL being queried and the time range.
- if (jsonString == null)
- {
- // Error.
- return null;
- }
+ if (jsonString == null)
+ {
+ logger.LogDebug($"Failed to fetch Mezmo logs for {node.S3NodeName} between {from} and {to}");
+ return null;
+ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- OrcanodeMonitor/Core/MezmoFetcher.cs (6 hunks)
🔇 Additional comments (2)
OrcanodeMonitor/Core/MezmoFetcher.cs (2)
36-36
: LGTM: Improved error handling with nullable return type.
The changes enhance error handling by clearly distinguishing between Mezmo errors (returning null
) and empty responses (returning empty string). This aligns with C# best practices and the PR's objective.
Also applies to: 44-44, 63-63
261-261
: LGTM: Improved error handling for views without hosts.
Good change to not log an error when a view doesn't have hosts, as this is an expected case for certain view types.
Signed-off-by: Dave Thaler <[email protected]>
Summary by CodeRabbit
Bug Fixes
null
instead of empty strings when data retrieval fails.New Features