-
Notifications
You must be signed in to change notification settings - Fork 55
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(websocket): Add websocket client #508
Changes from 10 commits
4ce17b8
1910c4e
febcd8a
111dcb0
a76fa92
e5b89b3
fa6b010
d9cb894
716a5b6
f157b0c
be25aa2
a15986f
0be8171
b162391
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
from unittest import TestCase | ||
from tiingo.wsclient import TiingoWebsocketClient | ||
from tiingo.exceptions import MissingRequiredArgumentError | ||
|
||
class TestRestClientWithSession(TestCase): | ||
def setUp(self): | ||
|
||
def msg_cb(msg): | ||
print(msg) | ||
|
||
self.cb=msg_cb | ||
|
||
self.config = { | ||
'eventName':'subscribe', | ||
'authorization':'API_KEY_GOES_HERE', | ||
#see https://api.tiingo.com/documentation/websockets/iex > Request for more info | ||
'eventData': { | ||
'thresholdLevel':5 | ||
} | ||
} | ||
|
||
# test for missing or incorrectly supplied endpoints | ||
def test_missing_or_wrong_endpoint(self): | ||
with self.assertRaises(AttributeError) as ex: | ||
TiingoWebsocketClient(config=self.config,on_msg_cb=self.cb) | ||
self.assertTrue(type(ex.exception)==AttributeError) | ||
|
||
with self.assertRaises(AttributeError) as ex: | ||
TiingoWebsocketClient(config=self.config,endpoint='wq',on_msg_cb=self.cb) | ||
self.assertTrue(type(ex.exception)==AttributeError) | ||
|
||
# test for missing API keys in config dict | ||
def test_missing_api_key(self): | ||
with self.assertRaises(RuntimeError) as ex: | ||
TiingoWebsocketClient(config={},endpoint='iex',on_msg_cb=self.cb) | ||
self.assertTrue(type(ex.exception)==RuntimeError) | ||
|
||
# test for missing callback argument | ||
def test_missing_msg_cb(self): | ||
with self.assertRaises(MissingRequiredArgumentError) as ex: | ||
TiingoWebsocketClient(config=self.config,endpoint='iex') | ||
self.assertTrue(type(ex.exception)==MissingRequiredArgumentError) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
# -*- coding: utf-8 -*- | ||
from tiingo.api import TiingoClient | ||
from tiingo.wsclient import TiingoWebsocketClient | ||
|
||
__author__ = """Cameron Yick""" | ||
__email__ = '[email protected]' |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,96 @@ | ||||||||||||||||||||||||||||
import os | ||||||||||||||||||||||||||||
import websocket | ||||||||||||||||||||||||||||
import json | ||||||||||||||||||||||||||||
from tiingo.exceptions import MissingRequiredArgumentError | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
class TiingoWebsocketClient: | ||||||||||||||||||||||||||||
''' | ||||||||||||||||||||||||||||
from tiingo import TiingoWebsocketClient | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def cb_fn(msg): | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# Example response | ||||||||||||||||||||||||||||
# msg = { | ||||||||||||||||||||||||||||
# "service":"iex" # An identifier telling you this is IEX data. | ||||||||||||||||||||||||||||
# The value returned by this will correspond to the endpoint argument. | ||||||||||||||||||||||||||||
# | ||||||||||||||||||||||||||||
# # Will always return "A" meaning new price quotes. There are also H type Heartbeat msgs used to keep the connection alive | ||||||||||||||||||||||||||||
# "messageType":"A" # A value telling you what kind of data packet this is from our IEX feed. | ||||||||||||||||||||||||||||
# | ||||||||||||||||||||||||||||
# # see https://api.tiingo.com/documentation/websockets/iex > Response for more info | ||||||||||||||||||||||||||||
# "data":[] # an array containing trade information and a timestamp | ||||||||||||||||||||||||||||
# | ||||||||||||||||||||||||||||
# } | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
print(msg) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
subscribe = { | ||||||||||||||||||||||||||||
'eventName':'subscribe', | ||||||||||||||||||||||||||||
'authorization':'API_KEY_GOES_HERE', | ||||||||||||||||||||||||||||
#see https://api.tiingo.com/documentation/websockets/iex > Request for more info | ||||||||||||||||||||||||||||
'eventData': { | ||||||||||||||||||||||||||||
'thresholdLevel':5 | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
# notice how the object isn't needed after using it | ||||||||||||||||||||||||||||
# any logic should be implemented in the callback function | ||||||||||||||||||||||||||||
TiingoWebsocketClient(subscribe,endpoint="iex",on_msg_cb=cb_fn) | ||||||||||||||||||||||||||||
while True:pass | ||||||||||||||||||||||||||||
''' | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def __init__(self,config,endpoint=None,on_msg_cb=None): | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for updating this line, to complete the change we should also add the following ternary
Otherwise, we'll get an error when we try to run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why the travis checks aren't showing in the Github UI, but you can check the test results from the Travis ui here: https://travis-ci.org/github/hydrosquall/tiingo-python/jobs/733171252 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but like. that should only happen if someone called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, you're right - I have been working mostly in JS recently, and forgot that in Python, if you don't pass in an argument that you'll get an error at runtime (in JS, if you don't pass in an argument, the function runs but assumes that the value of config === Anyways, I think that having a named argument is helpful to callers so they know the meaning of the position, so I guess the full change would look like: def __init__(config=None, # other args)
self.config = {} if config is None else config Regarding the failing test, it appears that the error wasn't raised because the test environment contained the TIINGO_API_KEY environment variable. In order to complete the change, you can follow the
https://docs.pytest.org/en/4.6.1/monkeypatch.html#monkeypatching-environment-variables In case you haven't run the unit tests locally before, that can be done by running the following in your terminal when in the root directory, this will be faster than waiting for the tests to run in CI. python setup.py test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @Mohamedemad4, just wanted to check if you might have time to take a look at fixing this test - I think it's the last blocking item that we'd need before we can include your changes in the new release. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the link you provided is broken There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also i don't get why the CI tests are failing? shouldn't the CI be providing it's own TINGO_API_KEY as an env var? or am i missing something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @Mohamededmad4, thanks for looking into this. The CI environment does have a tiingo-python/.github/workflows/python-package.yml Lines 42 to 43 in f6ef473
As a result, the code uses that environment variable and we never get to test the "RuntimeError" check for when the config is empty AND nothing is specified in the environment variables. To fix it, you can either change the logic of the client code, or delete the environment variable just inside of that test. tiingo-python/tiingo/wsclient.py Lines 46 to 56 in be25aa2
Let me know if that's still confusing. The updated link for how to patch environment variables is here: https://docs.pytest.org/en/stable/monkeypatch.html#monkeypatching-environment-variables |
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
self._base_url = "wss://api.tiingo.com" | ||||||||||||||||||||||||||||
self.config=config | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're getting a warning from LGTM about mutating a basic object. I think we can solve it by doing https://lgtm.com/rules/4840097/
self.config = {} if config is None else config This should address the warning. |
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||
api_key = self.config['authorization'] | ||||||||||||||||||||||||||||
except KeyError: | ||||||||||||||||||||||||||||
api_key = os.environ.get('TIINGO_API_KEY') | ||||||||||||||||||||||||||||
self.config.update({"authorization":api_key}) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
self._api_key = api_key | ||||||||||||||||||||||||||||
if not(api_key): | ||||||||||||||||||||||||||||
raise RuntimeError("Tiingo API Key not provided. Please provide" | ||||||||||||||||||||||||||||
" via environment variable or config argument." | ||||||||||||||||||||||||||||
"Notice that this config dict takes the API Key as authorization ") | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
self.endpoint = endpoint | ||||||||||||||||||||||||||||
if not (self.endpoint=="iex" or self.endpoint=="fx" or self.endpoint=="crypto"): | ||||||||||||||||||||||||||||
raise AttributeError("Endpoint must be defined as either (iex,fx,crypto) ") | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
self.on_msg_cb = on_msg_cb | ||||||||||||||||||||||||||||
if not self.on_msg_cb: | ||||||||||||||||||||||||||||
raise MissingRequiredArgumentError("please define on_msg_cb It's a callback that gets called when new messages arrive " | ||||||||||||||||||||||||||||
"Example:" | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice idea to put an example function in the error message for how to fix the bug, developers really appreciate that! :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. glad you liked it |
||||||||||||||||||||||||||||
"def cb_fn(msg):" | ||||||||||||||||||||||||||||
" print(msg)") | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
websocket.enableTrace(False) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
ws = websocket.WebSocketApp("{0}/{1}".format(self._base_url,self.endpoint), | ||||||||||||||||||||||||||||
on_message = self.get_on_msg_cb(), | ||||||||||||||||||||||||||||
on_error = self.on_error, | ||||||||||||||||||||||||||||
on_close = self.on_close, | ||||||||||||||||||||||||||||
on_open = self.get_on_open(self.config)) | ||||||||||||||||||||||||||||
ws.run_forever() | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def get_on_open(self,config): | ||||||||||||||||||||||||||||
# the methods passed to websocketClient have to be unbounded if we want WebSocketApp to pass everything correctly | ||||||||||||||||||||||||||||
# see websocket-client/#471 | ||||||||||||||||||||||||||||
def on_open(ws): | ||||||||||||||||||||||||||||
ws.send(json.dumps(config)) | ||||||||||||||||||||||||||||
return on_open | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def get_on_msg_cb(self): | ||||||||||||||||||||||||||||
def on_msg_cb_local(ws,msg): | ||||||||||||||||||||||||||||
self.on_msg_cb(msg) | ||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||
return on_msg_cb_local | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
# since methods need to be unbound in order for websocketClient these methods don't have a self as their first parameter | ||||||||||||||||||||||||||||
def on_error(ws, error): | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's make the first argument to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's just going to be confusing. since these functions don't get There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see your point about not wanting people to think that "self" refers to the Tiingo client instead of the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's go with the first solution. i think it's best for this case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you see this in time, can we also add a comment to suppress the warning? https://lgtm.com/help/lgtm/alert-suppression#python If not no worries, I'll add this before releasing to PyPi on Friday - this is to avoid future contributors from getting spammed by a warning they can ignore. |
||||||||||||||||||||||||||||
print(error) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
def on_close(ws): | ||||||||||||||||||||||||||||
pass |
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.
Nice work on the documentation for "service" and "messageType"!
Can we provide a sample of what one of these objects would look like? 1 or 2 would be plenty.
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 don't think adding examples would work out for stylistic reasons. anyways the link provided gives a lot of in-depth details