-
-
Notifications
You must be signed in to change notification settings - Fork 302
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: add universal cancel all orders api helper #1545
Conversation
Welcome back! @c9s, This pull request may get 341 BBG. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1545 +/- ##
==========================================
- Coverage 21.77% 21.73% -0.04%
==========================================
Files 606 607 +1
Lines 43939 44004 +65
==========================================
Hits 9566 9566
- Misses 33691 33756 +65
Partials 682 682
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
s.logger.WithError(err).Errorf("CancelOrdersByGroupID api call error") | ||
werr = multierr.Append(werr, err) | ||
s.logger.WithError(err).Errorf("unable to query open orders") | ||
continue |
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.
Will this hit api rate limit?
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 think we have the protection in the exchange api layer
return symbols | ||
} | ||
|
||
func CollectOpenOrders(ctx context.Context, ex types.Exchange, symbols ...string) ([]types.Order, error) { |
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.
Where does this method get called?
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 will be used from xtri
pkg/util/tradingutil/cancel.go
Outdated
} | ||
} | ||
|
||
return fmt.Errorf("unable to cancel all orders: %+v", openOrders) |
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.
How about adding anyErr
?
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.
Sure
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.
Updated
Re-estimated karma: this pull request may get 404 BBG |
Hi @c9s, Well done! 429 BBG has been sent to your polygon wallet. Please check the following tx: https://polygonscan.com/tx/0xaf64b84ca7175ed6fe1fb23fec39481c8be51c2a4c9fa02984150db6fdf85071 Thank you for your contribution! |
No description provided.