-
Notifications
You must be signed in to change notification settings - Fork 216
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
Update binance_provider.dart #2502
Conversation
WalkthroughThe pull request modifies the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/komodo_cex_market_data/lib/src/binance/data/binance_provider.dart (1)
Line range hint
59-65
: Consider providing more context when returning an empty response for HTTP 451.While returning an empty response object for status code 451 helps avoid throwing an exception, it may be beneficial to log or otherwise expose the reason (i.e., service restricted) so callers can react accordingly, rather than receiving a seemingly valid—but empty—response. This can prevent confusion for consumers of the API.
} else if (response.statusCode == 451) { + // Consider adding a log or debug message here, e.g.: + // debugPrint('Binance service unavailable due to legal reasons (451). Returning empty response'); return BinanceExchangeInfoResponseReduced( timezone: '', serverTime: 0, symbols: List.empty(), ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/komodo_cex_market_data/lib/src/binance/data/binance_provider.dart
(1 hunks)
🔇 Additional comments (1)
packages/komodo_cex_market_data/lib/src/binance/data/binance_provider.dart (1)
11-11
: Change from.com
to.us
domain is aligned with PR objective.Switching the default base URL to
https://api.binance.us/api/v3
aligns well with the stated goal of enabling access for international users outside mainland China. This should simplify the flow for users who previously needed alternative endpoints.
.com is accessible internationally (https://developers.binance.com/docs/binance-spot-api-docs/web-socket-api/general-api-information) if you face restrictions please use VPN or locally ran wallet with your desired API endpoint - we are subscribed to the .com API |
update to .us instead of .com restrictions for binance.com api usage
for international use worldwide
the .com api is restricted only to China mainland
.us should be open internationally (except for China mainland or something)
one API for all? sure why not
see changes please
Summary by CodeRabbit
New Features
Bug Fixes