Copilot commented on code in PR #332:
URL:
https://github.com/apache/incubator-hugegraph-computer/pull/332#discussion_r2123091485
##########
vermeer/config/worker.ini:
##########
@@ -19,4 +19,5 @@ debug_mode=release
http_peer=0.0.0.0:6788
grpc_peer=0.0.0.0:6789
master_peer=127.0.0.1:6689
-run_mode=worker
\ No newline at end of file
+run_mode=worker
+worker_group=default
Review Comment:
[nitpick] Consider adding an inline comment explaining the purpose of
`worker_group` and the valid values (e.g., `default`) to help users understand
how to configure different groups.
##########
vermeer/apps/master/workers/worker_manager.go:
##########
@@ -133,6 +138,11 @@ func (wm *workerManager) CreateWorker(workerPeer string,
ipAddr string, version
worker.InitTime = worker.LaunchTime
}
+ // if workerGroup in workerInDB is different from the one in worker,
give a warning to the user
+ if workerGroup != "$" && worker.Group != workerGroup {
+ logrus.Warnf("worker manager, worker group mismatch: given %s,
but found %s in db for worker %s", workerGroup, worker.Group, worker.Name)
Review Comment:
The mismatch check compares `workerGroup` to `worker.Group`, which was just
set to the same value; it should compare `workerGroup` to the existing group in
`workerInDB` (e.g., `workerInDB.Group`) to detect real mismatches.
```suggestion
if workerGroup != "$" && workerInDB != nil && workerInDB.Group !=
workerGroup {
logrus.Warnf("worker manager, worker group mismatch: given %s,
but found %s in db for worker %s", workerGroup, workerInDB.Group, worker.Name)
```
##########
vermeer/apps/master/bl/grpc_handlers.go:
##########
@@ -104,7 +104,7 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context,
req *pb.HelloMasterR
return &pb.HelloMasterResp{}, err
}
- logrus.Infof("worker say hello name: %s, client: %s", reqWorker.Name,
p.Addr.String())
+ logrus.Infof("worker say hello name: %s and set to workgroup: %s,
client: %s", reqWorker.Name, reqWorker.Group, p.Addr.String())
Review Comment:
[nitpick] Consider standardizing the log format for consistency and easier
parsing, e.g.: `worker registered name=%s group=%s client=%s`.
```suggestion
logrus.Infof("worker registered name=%s group=%s client=%s",
reqWorker.Name, reqWorker.Group, p.Addr.String())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
