Skip to content

feat: info resolver refactor#1594

Merged
elibosley merged 8 commits intofeat/cpu-utilization-queryfrom
feat/info-resolver-refactor
Aug 19, 2025
Merged

feat: info resolver refactor#1594
elibosley merged 8 commits intofeat/cpu-utilization-queryfrom
feat/info-resolver-refactor

Conversation

@elibosley
Copy link
Member

@elibosley elibosley commented Aug 19, 2025

No description provided.

@elibosley
feat(api): enhance GraphQL schema with new types and subscriptions fo…
fac605a 
…r system metrics

- Introduced new types for `ContainerPort`, `CpuLoad`, `MemoryUtilization`, and `InfoDisplay` to expand system metrics capabilities.
- Added subscriptions for `systemMetricsCpu` and `systemMetricsMemory` to provide real-time updates on CPU and memory utilization.
- Refactored existing resolvers to utilize the new types and improve data handling.
- Consolidated imports and removed deprecated services to streamline the codebase.
- Updated tests to cover new functionality and ensure reliability.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 19, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/info-resolver-refactor
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@claude
Copy link

claude bot commented Aug 19, 2025 •
edited
Loading

Claude finished @elibosley's task —— View job

Code Review Progress

Analyzing the diff for critical issues in the info resolver refactor changes:

  • Analyze core resolver changes for critical bugs
  • Check service implementations for security issues
  • Review data handling for potential data loss risks
  • Examine GraphQL schema changes for breaking changes
  • Provide final review feedback

Review Results

✅ No critical issues found in changes

This is a well-structured refactor that breaks down a monolithic info resolver into modular services. Key observations from the diff analysis:

What was changed:

  • Refactored monolithic InfoService into specialized services (CpuService, MemoryService, DevicesService, etc.)
  • Moved subscription logic from InfoResolver to new MetricsResolver
  • Added proper race condition protection in MetricsResolver polling methods (lines 24-25, 60-84)
  • Reorganized file structure into domain-specific subdirectories

Security & Safety:

  • All external command executions use execa safely with proper error handling
  • File system operations use proper async/await with error catching
  • No hardcoded credentials or exposed secrets
  • Input sanitization maintained for vendor/product names

API Compatibility:

  • GraphQL schema maintains backward compatibility
  • New MEMORY_UTILIZATION pubsub channel added without breaking existing channels
  • Resolver field signatures preserved

The refactor follows good practices with proper error handling, modular design, and maintains API compatibility.

@elibosley
refactor(api): reorganize and enhance GraphQL schema for system and m…
cd0d763 
…otherboard information

- Reintroduced `InfoSystem` and `InfoBaseboard` types in the GraphQL schema with additional fields for memory capacity and slots.
- Updated the `CachedResolverBase` to improve promise caching and data merging logic, ensuring thread-safe operations.
- Added comprehensive unit tests for `CachedResolverBase` to validate caching behavior, error handling, and concurrent access scenarios.
- Enhanced memory management with WeakMap to prevent memory leaks during data fetching.
- Cleaned up promise cache key handling to avoid data corruption during concurrent resolver access.
@elibosley elibosley requested a review from pujitm August 19, 2025 13:51
@elibosley
fix(api): update type declaration for parent in CachedResolverBase tests
61d1db4 
- Changed the type declaration of `parent` from an empty object to `any` in two test cases within `CachedResolverBase` to improve type safety and clarity in the tests.
@elibosley
refactor(api): simplify data fetching and caching logic in CachedReso…
5f045c2 
…lverBase

- Removed the data lock mechanism to streamline the promise caching process.
- Enhanced the data merging logic to directly merge fetched data into the parent object, improving performance and reducing complexity.
- Ensured thread-safe operations by leveraging the promise itself for concurrency handling.
pujitm
pujitm reviewed Aug 19, 2025
View reviewed changes
Copy link
Member

@pujitm pujitm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally looks good, just some thoughts & q's

@elibosley
feat(api): introduce Metrics type and resolver for system metrics
7eb80b3 
- Added a new `Metrics` type to the GraphQL schema to encapsulate CPU and memory utilization metrics.
- Implemented `MetricsResolver` to handle queries and subscriptions for real-time CPU and memory data.
- Updated existing resolvers to remove deprecated fields and improve data handling.
- Enhanced the GraphQL query structure to include the new `metrics` field for accessing system metrics.
- Added integration and unit tests to ensure the reliability of the new metrics functionality.
@elibosley
feat(api): update Metrics type and memory service for accurate utiliz…
46bf348 
…ation metrics

- Reintroduced the `Metrics` type in the GraphQL schema to include CPU and memory utilization metrics.
- Modified the calculation of `usedPercent` in the `MemoryService` to accurately reflect memory usage based on available and total memory.
- Removed deprecated `Metrics` type definition from the schema to streamline the codebase.
@elibosley
fix(api): ensure accurate memory usage calculation in MemoryService
7a2feef 
- Updated the `usedPercent` calculation in the `MemoryService` to handle cases where total memory is zero, preventing potential division by zero errors and ensuring accurate memory utilization metrics.
@elibosley
test(api): skip versions data test in CI environment
c947abd 
- Updated the integration test for `InfoResolver` to skip the versions data test when running in a CI environment, ensuring compatibility and preventing potential failures during automated testing.
@elibosley elibosley merged commit 0fadb6f into feat/cpu-utilization-query Aug 19, 2025
9 checks passed
@elibosley elibosley deleted the feat/info-resolver-refactor branch August 19, 2025 16:26
@github-actions
Copy link
Contributor

github-actions bot commented Aug 19, 2025

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1594/dynamix.unraid.net.plg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pujitm pujitm pujitm left review comments

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@elibosley @pujitm