-
Notifications
You must be signed in to change notification settings - Fork 3
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
Implement progressive call results #69
Implement progressive call results #69
Conversation
864150b
to
2c47c93
Compare
messages/validator.go
Outdated
@@ -302,7 +306,10 @@ func AsInt64(i interface{}) (int64, bool) { | |||
case int32: | |||
return int64(v), true | |||
case uint: | |||
return int64(v), true | |||
if v > math.MaxInt64 { |
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.
why is this check needed?
2c47c93
to
f5ab68f
Compare
f5ab68f
to
9d6a7a1
Compare
dealer.go
Outdated
@@ -108,19 +113,28 @@ func (d *Dealer) ReceiveMessage(sessionID int64, msg messages.Message) (*Message | |||
break | |||
} | |||
|
|||
receiveProgress, ok := call.Options()[OptionReceiveProgress].(bool) | |||
if !ok { |
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.
this check can be removed and the value of receiveProgress
maybe used directly without reassining.
if !ok { | |
if !ok { |
dealer.go
Outdated
@@ -131,13 +145,23 @@ func (d *Dealer) ReceiveMessage(sessionID int64, msg messages.Message) (*Message | |||
return nil, fmt.Errorf("yield: not pending calls for session %d", sessionID) | |||
} | |||
|
|||
delete(d.pendingCalls, yield.RequestID()) | |||
progress, ok := yield.Options()[OptionProgress].(bool) | |||
if !ok { |
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.
same here, we can avoid the reassigning
session.go
Outdated
@@ -60,7 +60,10 @@ func (w *Session) SendMessage(msg messages.Message) ([]byte, error) { | |||
return data, nil | |||
case messages.MessageTypeYield: | |||
yield := msg.(*messages.Yield) | |||
w.invocationRequests.Delete(yield.RequestID()) | |||
progress, ok := yield.Options()[OptionProgress].(bool) | |||
if !ok || !progress { |
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.
if !ok || !progress { | |
if !progress { |
should also work
session.go
Outdated
if !exists { | ||
return nil, fmt.Errorf("received RESULT for invalid requestID") | ||
} | ||
|
||
progress, ok := result.Details()[OptionProgress].(bool) | ||
if !ok || !progress { |
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.
if !ok || !progress { | |
if !progress { |
try
9d6a7a1
to
6c573ce
Compare
6c573ce
to
a8b8455
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.
Change looks good. But we need to add tests now. I am using wampproto-go in production in a product and landing changes without tests is worrying.
No description provided.