-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
StreamFetcher POC #6459
base: develop
Are you sure you want to change the base?
StreamFetcher POC #6459
Conversation
# import asyncio | ||
|
||
from openbb_core.app.model.command_context import CommandContext |
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.
# import asyncio | |
from openbb_core.app.model.command_context import CommandContext | |
from openbb_core.app.model.command_context import CommandContext |
Thoughts:
|
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.
@router.command(methods=["GET"]) | ||
async def live( | ||
symbol: str = "ethbtc", lifetime: int = 10 | ||
) -> BinanceCryptoHistoricalData: |
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 can we make this a Provider Interface method? There could be any number of providers that have a WS connection, like here - https://site.financialmodelingprep.com/developer/docs#crypto-websocket
) | ||
|
||
|
||
class BinanceCryptoHistoricalFetcher(Fetcher): |
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 this model name and Fetcher class is deceiving. It is not at all like the other provider/standard models with this name. This is not historical data, and the pattern itself is significantly different. It needs to be differentiated.
Like how you have added an additional atransform
method, there should be something like wsconnect
that accepts a URL and **kwargs where the core logic can be easily reused, like a standard model.
|
||
https://binance.p.rapidapi.com/ | ||
|
||
If you're not a developer but would still like to use Biztoc outside of the main website, |
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 description is half BizToc and half maybe Binance.
@@ -0,0 +1,19 @@ | |||
[tool.poetry] | |||
name = "openbb-binance" | |||
version = "1.2.1" |
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.
Version should be 1.0.0
@@ -0,0 +1 @@ | |||
"""Biztoc Provider tests.""" |
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.
Biztoc -> Binance
|
||
@router.stream(methods=["GET"]) | ||
async def live(symbol: str = "ethbtc", lifetime: int = 10, tld: str = "us") -> OBBject: | ||
"""Connect to Binance WebSocket Crypto Price data feed.""" |
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 endpoint will not generate any documentation or descriptions because it is not using the ProviderInterface.
Why? (1-3 sentences or a bullet point list):
What? (1-3 sentences or a bullet point list):
atransform_data
method.Impact (1-2 sentences or a bullet point list):
Testing Done:
To test on the API interface do:
And on the Python interface do:
Reviewer Notes (optional):