Conversation
Omarabdul3ziz
force-pushed
the
main_myceliumMessaging
branch
from
9a88038 to
a450ec6
Compare
- 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
force-pushed
the
main_myceliumMessaging
branch
2 times, most recently
from
460a672 to
a29e3d2
Compare
Omarabdul3ziz
force-pushed
the
main_myceliumMessaging
branch
from
a29e3d2 to
32a694e
Compare
Omarabdul3ziz
force-pushed
the
main_myceliumMessaging
branch
from
bbba65d to
60345b9
Compare
Omarabdul3ziz
requested review from
Eslam-Nawara,
ashraffouda,
muhamadazmy,
rawdaGastan and
xmonader
as code owners
There was a problem hiding this comment.
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
apipackage with typed methods and JSON-RPC handlers underpkg/api - Enhance performance monitoring: updated interface, stub methods, and Redis cache utilities in
pkg/perf - Add a
nodeclientmodule 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 |
pkg/api/system.go
Outdated
| // 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 |
There was a problem hiding this comment.
Dereferencing the returned DMI pointer before checking err can cause a panic if dmi is nil; check err first and handle it before dereferencing.
| return *dmi, err | |
| if err != nil { | |
| return dmi.DMI{}, err | |
| } | |
| return *dmi, nil |
nodeclient/nodeclient.go
Outdated
| 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 |
There was a problem hiding this comment.
The return type pkg.Interfaces does not match the API method, which returns []pkg.Interface; this mismatch will cause compilation errors.
| 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 |
pkg/api/system.go
Outdated
| return version, nil | ||
| } | ||
|
|
||
| // TODO: we should provide a better parsing for this to sent over api |
There was a problem hiding this comment.
Typo in comment: 'sent' should be 'send'.
| // 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 |
| return | ||
| } | ||
|
|
||
| func (s *PerformanceMonitorStub) GetAllTaskResult(ctx context.Context) (ret0 pkg.AllTaskResult, ret1 error) { |
There was a problem hiding this comment.
[nitpick] The stub methods for each task result share nearly identical logic; consider extracting a helper to reduce duplication.
| for _, item := range ips { | ||
| ipNet := net.IPNet{ | ||
| IP: item, | ||
| Mask: nil, |
There was a problem hiding this comment.
[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.
| Mask: nil, | |
| Mask: net.CIDRMask(32, 32), |
| inMemCache *cache.Cache | ||
| } | ||
|
|
||
| func NewAPI(client zbus.Client, msgBrokerCon string, mode string) (*API, error) { |
There was a problem hiding this comment.
[nitpick] Exported function NewAPI lacks a doc comment; adding a Go‐style comment helps with code readability and tooling (godoc).
1 participant
apipkg handling both modes normal/light handlers with defined param/return type, decoupled from the messaging partapi/jsonrpcpkg handling the messaging over mycelium binary with handlers mapped to api pkgwaiting: