-
Notifications
You must be signed in to change notification settings - Fork 75
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
Feature: extended deal info #386
Conversation
c4bd427
to
90e48df
Compare
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.
Minor issues, sorry for long review.
insonmnia/hub/server.go
Outdated
h.deleteTask(task.ID) | ||
err = h.deleteTask(task.ID) | ||
if err != nil { | ||
log.G(ctx).Warn("cannot delete task", zap.Error(err)) |
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.
Error, because we break the request on this.
insonmnia/hub/server.go
Outdated
taskDetails, err := mctx.Client.TaskDetails(ctx, &pb.ID{Id: t.ID}) | ||
if err != nil { | ||
log.G(h.ctx).Warn("cannot get task status", | ||
zap.String("worker_id", t.MinerId), zap.String("task_id", t.ID), zap.Error(err)) |
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.
I thought we use camelCase in log attribute names.
90e48df
to
1e64826
Compare
1e64826
to
946a868
Compare
The
deal status
handle now can show extended info, that fetched from the Hub.If a Deal was not accepted (there is no info about the Hub) - show only a Deal info (previous behavior)
If the Hub is associated with a Deal - the Node will try to connect to the Hub and retrieve the details (resources from the Order, running tasks, finished tasks)
BTW:
dealInfo
struct became unused and was removed;hub.deleteTask(id)
was added;