Skip to content
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: [bitget] integrate QueryMarkets, QueryTicker and QueryAccount api #1280

Merged
merged 5 commits into from Oct 20, 2023

Conversation

c9s
Copy link
Owner

@c9s c9s commented Aug 9, 2023

No description provided.

@bbgokarma-bot
Copy link

Welcome back! @c9s, This pull request may get 366 BBG.

@c9s c9s changed the title WIP: FEATURE: bitget exchange FEATURE: [bitget] integrate QueryMarkets, QueryTicker and QueryAccount api Aug 15, 2023
@c9s c9s requested a review from zenixls2 August 15, 2023 08:34
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 371 BBG

@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #1280 (874b647) into main (d43c358) will not change coverage.
Report is 4 commits behind head on main.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1280   +/-   ##
=======================================
  Coverage   21.58%   21.58%           
=======================================
  Files         540      540           
  Lines       37435    37435           
=======================================
  Hits         8082     8082           
  Misses      28769    28769           
  Partials      584      584           
Files Changed Coverage Δ
...g/exchange/bitget/bitgetapi/get_symbols_request.go 0.00% <ø> (ø)
pkg/types/market.go 49.56% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d43c358...874b647. Read the comment docs.

@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 389 BBG

Copy link
Collaborator

@bailantaotao bailantaotao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else LGTM

}

func (e *Exchange) QueryMarkets(ctx context.Context) (types.MarketMap, error) {
// TODO implement me
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm comments

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or write "TODO: " for color highlight


func (e *Exchange) QueryMarkets(ctx context.Context) (types.MarketMap, error) {
// TODO implement me
req := e.client.NewGetSymbolsRequest()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add rate limiter?

@@ -86,8 +86,23 @@ func (e *Exchange) QueryMarkets(ctx context.Context) (types.MarketMap, error) {
}

func (e *Exchange) QueryTicker(ctx context.Context, symbol string) (*types.Ticker, error) {
// TODO implement me
panic("implement me")
req := e.client.NewGetTickerRequest()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rate limiter?

@@ -116,8 +116,21 @@ func (e *Exchange) QueryKLines(ctx context.Context, symbol string, interval type
}

func (e *Exchange) QueryAccount(ctx context.Context) (*types.Account, error) {
// TODO implement me
panic("implement me")
req := e.client.NewGetAccountAssetsRequest()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rate limiter?

Comment on lines +120 to +129
resp, err := req.Do(ctx)
if err != nil {
return nil, err
}

bals := types.BalanceMap{}
for _, asset := range resp {
b := toGlobalBalance(asset)
bals[asset.CoinName] = b
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you can move this implementation to QueryAccountBalances, and then QueryAccount can utilize it.


func (e *Exchange) QueryKLines(ctx context.Context, symbol string, interval types.Interval, options types.KLineQueryOptions) ([]types.KLine, error) {
// TODO implement me
panic("implement me")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please mark in document that bitget is not ready for backtesting

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just suggestion. Users should also be able to find this on their own

@c9s c9s merged commit eb404a5 into main Oct 20, 2023
4 checks passed
@c9s c9s deleted the feature/bitget branch October 20, 2023 05:36
@bbgokarma-bot
Copy link

Hi @c9s,

Well done! 474 BBG has been sent to your polygon wallet. Please check the following tx:

https://polygonscan.com/tx/0x367f00eb843429a531152d6874ba18c33f3db716272603d25cd8e9d59216d5ec

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants