Skip to content

mycelium messaging integration #26

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Omarabdul3ziz
Copy link
Contributor

@Omarabdul3ziz Omarabdul3ziz commented May 11, 2025

  • new api pkg handling both modes normal/light handlers with defined param/return type, decoupled from the messaging part
  • new api/jsonrpc pkg handling the messaging over mycelium binary with handlers mapped to api pkg
  • update monitor pkg/stub for better defined-typed results
  • generated openrpc file with all methods/types for api
  • backward compatible: old rmb api still valid
  • identity management, (map mycelium -> twin on chain)
  • new node-client over mycelium

waiting:

@Omarabdul3ziz Omarabdul3ziz force-pushed the main_myceliumMessaging branch from 9a88038 to a450ec6 Compare May 24, 2025 12:14
- generated openrpc file with all methods/types for api
- new `api` pkg handling both modes normal/light handlers with defined
  param/return type, decoupled from the messaging part
- new `reciever` pkg handling the messaging over mycelium binary with
  handlers mapped to api pkg
- update monitor pkg/stub for better defined-typed results
@Omarabdul3ziz Omarabdul3ziz force-pushed the main_myceliumMessaging branch 2 times, most recently from 460a672 to a29e3d2 Compare May 26, 2025 11:15
@Omarabdul3ziz Omarabdul3ziz force-pushed the main_myceliumMessaging branch from a29e3d2 to 32a694e Compare May 26, 2025 11:25
@Omarabdul3ziz Omarabdul3ziz force-pushed the main_myceliumMessaging branch from bbba65d to 60345b9 Compare June 3, 2025 11:09
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new transport-agnostic API layer backed by Mycelium-based messaging, updates performance monitoring stubs and cache layers, and adds a Go node-client for JSON-RPC interactions.

  • Define a new api package with typed methods and JSON-RPC handlers under pkg/api
  • Enhance performance monitoring: updated interface, stub methods, and Redis cache utilities in pkg/perf
  • Add a nodeclient module wrapping JSON-RPC calls for external consumers

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
pkg/stubs/performance_monitor_stub.go Added RPC stub methods returning typed task results
pkg/performance_monitor.go Extended interface with typed task-result methods
pkg/perf/cache_utils.go Introduced helper functions for Redis key management
pkg/perf/cache.go Implemented typed cache getters for each task result
pkg/api/system.go Added system version, DMI, hypervisor, diagnostics methods
pkg/api/network.go Added light/full-mode network endpoints
pkg/api/api.go API constructor and mode handling
pkg/api/jsonrpc/handlers.go Registered JSON-RPC handlers mapping to api methods
nodeclient/nodeclient.go New NodeClient wrapper for JSON-RPC calls

// not all the fields are needed, and some of them are not even used
func (a *API) SystemDMI(ctx context.Context) (dmi.DMI, error) {
dmi, err := a.oracle.DMI()
return *dmi, err
Copy link
Preview

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

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

Dereferencing the returned DMI pointer before checking err can cause a panic if dmi is nil; check err first and handle it before dereferencing.

Suggested change
return *dmi, err
if err != nil {
return dmi.DMI{}, err
}
return *dmi, nil

Copilot uses AI. Check for mistakes.

Comment on lines 156 to 159
func (c *NodeClient) GetNetworkInterfaces(ctx context.Context) (pkg.Interfaces, error) {
var interfaces pkg.Interfaces
if err := c.rpcClient.Call(ctx, c.destination, "network.interfaces", nil, &interfaces); err != nil {
return pkg.Interfaces{}, err
Copy link
Preview

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

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

The return type pkg.Interfaces does not match the API method, which returns []pkg.Interface; this mismatch will cause compilation errors.

Suggested change
func (c *NodeClient) GetNetworkInterfaces(ctx context.Context) (pkg.Interfaces, error) {
var interfaces pkg.Interfaces
if err := c.rpcClient.Call(ctx, c.destination, "network.interfaces", nil, &interfaces); err != nil {
return pkg.Interfaces{}, err
func (c *NodeClient) GetNetworkInterfaces(ctx context.Context) ([]pkg.Interface, error) {
var interfaces []pkg.Interface
if err := c.rpcClient.Call(ctx, c.destination, "network.interfaces", nil, &interfaces); err != nil {
return nil, err

Copilot uses AI. Check for mistakes.

return version, nil
}

// TODO: we should provide a better parsing for this to sent over api
Copy link
Preview

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

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

Typo in comment: 'sent' should be 'send'.

Suggested change
// TODO: we should provide a better parsing for this to sent over api
// TODO: we should provide a better parsing for this to send over api

Copilot uses AI. Check for mistakes.

@@ -60,3 +60,88 @@ func (s *PerformanceMonitorStub) GetAll(ctx context.Context) (ret0 []pkg.TaskRes
}
return
}

func (s *PerformanceMonitorStub) GetAllTaskResult(ctx context.Context) (ret0 pkg.AllTaskResult, ret1 error) {
Copy link
Preview

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

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

[nitpick] The stub methods for each task result share nearly identical logic; consider extracting a helper to reduce duplication.

Copilot uses AI. Check for mistakes.

for _, item := range ips {
ipNet := net.IPNet{
IP: item,
Mask: nil,
Copy link
Preview

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

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

[nitpick] Constructing net.IPNet with a nil Mask may be ambiguous; consider setting an explicit mask (e.g., /32 or based on returned data) for correctness.

Suggested change
Mask: nil,
Mask: net.CIDRMask(32, 32),

Copilot uses AI. Check for mistakes.

inMemCache *cache.Cache
}

func NewAPI(client zbus.Client, msgBrokerCon string, mode string) (*API, error) {
Copy link
Preview

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

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

[nitpick] Exported function NewAPI lacks a doc comment; adding a Go‐style comment helps with code readability and tooling (godoc).

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant