feature(websocket): Add websocket client #508

Mohamedemad4 · 2020-09-02T01:40:44Z

Summary of Changes

added a simplified Barebones interface to Tiingo's websocket API
implemented #2

Just a couple of notes:

I didn't write tests cause i didn't know what to test for since this is a really simple enhancement

and question.
what should I write in the HISTORY.rst exactly? I didn't understand that part.

@hydrosquall

codecov · 2020-09-02T01:42:24Z

Codecov Report

Merging #508 (b162391) into master (db92c33) will decrease coverage by 4.16%.
The diff coverage is 69.23%.

@@            Coverage Diff             @@
##           master     #508      +/-   ##
==========================================
- Coverage   94.64%   90.47%   -4.17%     
==========================================
  Files           5        6       +1     
  Lines         224      294      +70     
==========================================
+ Hits          212      266      +54     
- Misses         12       28      +16

Impacted Files	Coverage Δ
tiingo/wsclient.py	`68.42% <68.42%> (ø)`
tiingo/__init__.py	`100.00% <100.00%> (ø)`
tiingo/api.py	`93.18% <0.00%> (-1.00%)`	⬇️

Continue to review full report at Codecov.

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

lgtm-com · 2020-09-02T01:52:37Z

This pull request introduces 9 alerts when merging 1910c4e into db92c33 - view on LGTM.com

new alerts:

4 for First parameter of a method is not named 'self'
2 for Testing equality to None
1 for Unused local variable
1 for Unused import
1 for Modification of parameter with default

hydrosquall

Hi @Mohamedemad4 , thanks for taking the time to make this contribution! I have 3 questions, since it's been a while since we've added a new dependency to the library, so I want to make sure we do this carefully.

What do you see are any pros/cons of choosing websocket-client over other choices?
Once someone has instantiated a TiingoWebsocketClient, will they ever do anything with the returned object, or does all of their logic have to be in the callback function?
For the purpose of documentation, could we try to describe the shape of the data returned in msg (or link to where someone can go to find out)?

hydrosquall · 2020-09-04T01:34:56Z

tiingo/wsclient.py

+            ws.send(json.dumps(GLOB_config))
+        thread.start_new_thread(run, ())
+    def __init__(self,config,on_msg_cb):
+        global GLOB_config


Hey, thanks for investigating this! I'm a little bit concerned by the use of global here, so I went and looked at the issue in the comment, and I'm not sure that I entirely follow why resolving this requires using globals. websocket-client/websocket-client#471

I was wondering if there were any existing Websocket clients that this implementation was inspired by that I could take a look at.

i have tracked this down to this line of code in the library.
which checks if the method is bounded before passing self to it which is needed for the number of arguments to match up. and ofc you can't have unbounded method access class data. I would love not to use global ,but i have no idea how to implement something like that without actually changing the websocket-client code. if you have an idea or if i am missing something, Please. let me know

I'm unclear about why genericWebsocketClient class is necessary in the first place, since we never actually even read the value in the ws_client variable. This is why I was curious if this is based on an existing production library / program that you saw, so I can evaluate whether that approach makes sense for us too.

If we're not actually going to read the value of ws_client, I'm not sure that I see the value of creating it in the first place.

hydrosquall · 2020-09-04T01:35:49Z

tiingo/wsclient.py

@@ -0,0 +1,93 @@
+import os
+import websocket
+try:


I was wondering, what is this try-except import for?

to import the correct version of thread regardless of python version

I see, that makes sense. Could we add a comment explaining this is the reason?

hydrosquall · 2020-09-04T01:39:33Z

tiingo/wsclient.py

+    while True:pass
+    '''
+
+    def __init__(self,config={},endpoint=None,on_msg_cb=None):


What do you think about documenting the expected.permitted options for the config dictionary + the expected values for endpoint + the function signature for on_msg_cb ? The example code you provided in the docstring is a good start for a documentation page, but we would want to aim for something closer to a descriptive docstring for a class comment. See https://realpython.com/python-pep8/#documentation-strings for more background.

hydrosquall · 2020-09-04T01:40:07Z

tiingo/wsclient.py

+        if not(api_key):
+            raise RuntimeError("Tiingo API Key not provided. Please provide"
+                               " via environment variable or config argument."
+                               "Notice that this config dict ticks the API Key as authorization ")


I think ticks might have actually be takes

hydrosquall · 2020-09-04T01:42:22Z

tiingo/wsclient.py

+
+        self.on_msg_cb = on_msg_cb
+        if self.on_msg_cb==None:
+            raise AttributeError("please define on_msg_cb ")


I think we can use a MissingRequiredArgument error here, and perhaps describe what "on_msg_cb" means, as a new user may not know that cb is shorthand for "callback".

https://github.com/hydrosquall/tiingo-python/blob/master/tiingo/exceptions.py

hydrosquall · 2020-09-04T01:56:21Z

I didn't write tests cause i didn't know what to test for since this is a really simple enhancement

I'll try this out on the weekend and let you know if anything comes to mind, but that's probably fine. We don't want to just be testing the underlying library, I suppose a test could be written for a function that validates the arguments passed into the constructor for the client if you'd like to practice getting that experience. It would look something like this:

tiingo-python/tests/test_tiingo.py

Lines 29 to 37 in db92c33

    
           class TestClient(TestCase): 
        
               def test_api_key_missing_error(self): 
        
                   config = { 
        
                       'api_key': "" 
        
                   } 
        
                   with self.assertRaises(RuntimeError): 
        
                       client = TiingoClient(config=config) 
        
                       assert client

what should I write in the HISTORY.rst exactly? I didn't understand that part.

This is a description of the changes that your PR is introducing - if you're not sure, you can also provide a suggestion directly as a PR comment, and I can update History.rst directly when I cut a new release.

…e error msg for missing on_msg_cb

Mohamedemad4 · 2020-09-06T13:35:27Z

Thank you @hydrosquall for taking the time to review my noob code.
as for your questions.

1- it appeared to be the most popular one. maintained and mature client
2- no they once the object is called all user logic should be written in the callback function
3- see commit febcd8a

lgtm-com · 2020-09-06T13:45:40Z

This pull request introduces 7 alerts when merging febcd8a into 4d9557a - view on LGTM.com

new alerts:

4 for First parameter of a method is not named 'self'
1 for Unused local variable
1 for Unused import
1 for Modification of parameter with default

lgtm-com · 2020-09-06T13:59:51Z

This pull request introduces 7 alerts when merging 111dcb0 into 4d9557a - view on LGTM.com

new alerts:

4 for First parameter of a method is not named 'self'
1 for Unused local variable
1 for Unused import
1 for Modification of parameter with default

hydrosquall

This is great progress, thanks for working with me on iterating on this. Could we try adding a fixture test to check for the API key + valid initialization arguments so that this client file file has some test coverage?

hydrosquall · 2020-09-08T00:00:24Z

README.rst

+        #   "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


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.

I don't think adding examples would work out for stylistic reasons. anyways the link provided gives a lot of in-depth details

hydrosquall · 2020-09-08T00:00:56Z

README.rst

+
+        # Example response 
+        # msg = {
+        #   "service":"iex" # An identifier telling you this is IEX data. The value returned by this will always be "iex".


Will this always be iex even if the endpoint provided to TiingoWebsocketClient is something different from iex?

hydrosquall · 2020-09-08T00:01:24Z

tiingo/wsclient.py

@@ -0,0 +1,93 @@
+import os
+import websocket
+try:


I see, that makes sense. Could we add a comment explaining this is the reason?

hydrosquall · 2020-09-08T17:51:13Z

tiingo/wsclient.py

+                               " via environment variable or config argument."
+                               "Notice that this config dict takes the API Key as authorization ")
+
+        try:


Instead of doing this try/except block here are raising a KeyError that gets turned into an Attribute error, what do you think about making a single if-statement which raises a single InvalidArgumentError (or just Attribute error if you don't want to create a new exception class)?

hydrosquall · 2020-09-08T17:52:18Z

tiingo/wsclient.py

+        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:"


Nice idea to put an example function in the error message for how to fix the bug, developers really appreciate that! :)

glad you liked it

…socketClient

lgtm-com · 2020-09-10T10:18:35Z

This pull request introduces 6 alerts when merging fa6b010 into 4d9557a - view on LGTM.com

new alerts:

4 for First parameter of a method is not named 'self'
1 for Unused local variable
1 for Modification of parameter with default

hydrosquall

I was reviewing the results on https://lgtm.com/projects/g/hydrosquall/tiingo-python/rev/pr-598152663398970811215859fcbae6dc034640ef , and realized that I'm not sure about the role that genericWebsocketClient is serving. Would removing this class remove the need for creating global variables?

hydrosquall · 2020-09-13T02:12:20Z

tiingo/wsclient.py

+            ws.send(json.dumps(GLOB_config))
+        thread.start_new_thread(run, ())
+    def __init__(self,config,on_msg_cb):
+        global GLOB_config


I'm unclear about why genericWebsocketClient class is necessary in the first place, since we never actually even read the value in the ws_client variable. This is why I was curious if this is based on an existing production library / program that you saw, so I can evaluate whether that approach makes sense for us too.

If we're not actually going to read the value of ws_client, I'm not sure that I see the value of creating it in the first place.

Mohamedemad4 · 2020-09-16T09:42:08Z

genericWebsocketClient is not necessary. BUT otherwise the functions inside it would have to be left in the global space of the file. I just thought it was neater to put it all in a class with an issue description.

hydrosquall · 2020-09-19T00:23:52Z

@Mohamedemad4 - thanks for the explanation. I see the intention behind using the class as a way to help keep the code organized. With that said, I see two advantages to avoiding the need creating a class:

We no longer need to create a class with methods where self is not the first argument, which is unconventional in the python world (as you already knew since you left a comment explaining why it was necessary to do something unusual)
We won't need to create globals: the callbacks and functions could be created just within scope of the TiingoWebsocketClient's __init__ method.

Given that avoiding the class would prevent the need for globals and unconventional use of "self", I think this code might be easier to maintain if we omit the abstraction layer for now. What do you think?

Mohamedemad4 · 2020-09-19T18:56:27Z

@hydrosquall
If i understand what you are saying correctly. By globals you are referring to GLOB_config and GLOB_on_msg_cb. right?
because in that case we would still need them simply because TiingoWebsocketClient CANNOT be passed to the callback functions via the self parameter.
However if you mean just putting the Callback methods in TiingoWebsocketClient scope,then i see no problems with that.

hydrosquall · 2020-09-19T19:39:44Z

By globals you are referring to GLOB_config and GLOB_on_msg_cb. right?

Yes - I think if we put those callbacks directly in TiingoWebsocketClient's init, it'll avoid the need for globals. I understand the original motivation to help keep the code organized by putting all the callbacks in a class, but in this case I'm not sure it helps us very much + risks confusing people with the conventional use of self + globals.

Mohamedemad4 · 2020-09-19T19:43:38Z

@hydrosquall
I am confused. do you mean the Callback methods should be put in TiingoWebsocketClient scope?
because I see why we can do that.
BUT the whole point of the Globals is because you can't pass TiingoWebsocketClient instance to the callback when calling them.
so that's why GLOB_config and GLOB_on_msg_cb need to be global.

hydrosquall · 2020-09-28T14:44:40Z

BUT the whole point of the Globals is because you can't pass TiingoWebsocketClient instance to the callback when calling them.

Hey, thanks for the additional explanation. Can you show me an example of where the callback would need access to the TiingoWebsocketClient instance? I don't think it's necessary if the callback only ever needs to know about the message item, and that can be done without creating a global.

I do see the one place where the global is used in the callback, however, it looks to me like that can be used without ever converting the local config into a global.

tiingo-python/tiingo/wsclient.py

Line 28 in fa6b010

ws.send(json.dumps(GLOB_config))

Mohamedemad4 · 2020-09-29T04:18:19Z

Well. I can see why you would want to get rid of the Globals.
However. the config dict needs to be passed to the on_open callback in order for Tiingo to know information about the client. and there is just no way of getting around to having to pass that config to the on_open callback (well maybe if you use something like time.sleep after opening the connection. but that's even worse.

And looking back at the code. You are correct. GLOB_on_msg_cb doesn't need to be global. strictly speaking at least.
But as for GLOB_config i really don't see it.
plus having everything related to the callbacks in one place, should really help with readability and maintainability.
What do you think?

hydrosquall · 2020-09-29T22:47:45Z

However. the config dict needs to be passed to the on_open callback in order for Tiingo to know information about the client. and there is just no way of getting around to having to pass that config to the on_open callback

How do you feel about using a higher order function? In case you haven't run into this before, it's a function that returns a function. This lets us "bake in" the config without needing to create a throwaway class (since we're never using that ws_client variable for anything.

def get_on_open(config):
          def on_open(ws):
               # ... everything the same as before but without globals
               def run(*args):
                   print(GLOB_config)
                   ws.send(json.dumps(GLOB_config))
              thread.start_new_thread(run, ())

         return on_open

def __init__(self,config={},endpoint=None,on_msg_cb=None):
     # ... same as before
    ws = websocket.WebSocketApp("ws://echo.websocket.org/",
                              on_message = on_message,
                              on_error = on_error,
                              on_close = get_on_open(config))

I really appreciate that you're thinking about maintainability in grouping these callbacks together under a class. However, I would say that I think the value of grouping all the callbacks together under 1 class is limited if the class isn't stateful/if the values of the class instance are never read themselves. I'd suggest waiting until we have a reason to actually read the value of the ws_client variable before creating a class to wrap all of those callbacks.

Thanks for sharing more about your thought process for this and sticking with this PR! Let me know if you have any other questions, it has been a good learning experience for me going through this architecture/design decision with you.

Mohamedemad4 · 2020-09-29T22:59:30Z

Thank you @hydrosquall for taking the time with me and rejecting it outright. Using Higher Order functions seems like a better solution.
will Commit the new code so you can review it

… the send thread on_open()

Mohamedemad4 · 2020-10-03T18:47:51Z

@hydrosquall sorry for the delay. Just pushed the new code with higher order functions. what do you think?

lgtm-com · 2020-10-03T19:04:06Z

This pull request introduces 5 alerts when merging d9cb894 into 4354b1c - view on LGTM.com

new alerts:

2 for First parameter of a method is not named 'self'
2 for Unused import
1 for Modification of parameter with default

lgtm-com · 2020-10-04T04:45:39Z

This pull request introduces 3 alerts when merging 716a5b6 into 4354b1c - view on LGTM.com

new alerts:

2 for First parameter of a method is not named 'self'
1 for Modification of parameter with default

hydrosquall

Almost there! Just 2 more warnings for the LGTM linter to address, and I think we'll be ready to bring this in. The new higher order function approach looks good to me.

hydrosquall · 2020-10-06T01:39:12Z

tiingo/wsclient.py

+            return 
+        return on_msg_cb_local
+
+    def on_error(ws, error):


Let's make the first argument to on_error and on_close self rather than ws, to address the warning provided by LGTM

https://lgtm.com/projects/g/hydrosquall/tiingo-python/rev/pr-ba63650c8fdd00a84b804ee3327857a1f001cbf6

that's just going to be confusing. since these functions don't get self when they are called. and will just receive a ws instance. i think a better approach would be to just use *args so as not to confuse future maintainers.BUT i doubt that would stop the LGTM linter from throwing a warning

I see your point about not wanting people to think that "self" refers to the Tiingo client instead of the ws instance - what do you think about:

Leaving the function as is, but leaving a comment to explain to future maintainers why the variable is called ws on this line. This won't affect LGTM but will help maintainers

Removing this function from the TiingoClient class and defining it as a standalone function above the class - this will solve the LGTM issue.

let's go with the first solution. i think it's best for this case

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.

hydrosquall · 2020-10-06T01:43:11Z

tiingo/wsclient.py

+    def __init__(self,config={},endpoint=None,on_msg_cb=None):
+
+        self._base_url = "wss://api.tiingo.com"
+        self.config=config


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/

Make the initial value None in the init constructor statement

self.config = {} if config is None else config

This should address the warning.

…() and on_close() don't get a self

lgtm-com · 2020-10-06T02:39:40Z

This pull request introduces 2 alerts when merging f157b0c into dd57795 - view on LGTM.com

new alerts:

2 for First parameter of a method is not named 'self'

hydrosquall · 2020-10-06T23:39:57Z

tiingo/wsclient.py

@@ -38,7 +38,7 @@ def cb_fn(msg):
    while True:pass
    '''

-    def __init__(self,config={},endpoint=None,on_msg_cb=None):
+    def __init__(self,config,endpoint=None,on_msg_cb=None):


Thanks for updating this line, to complete the change we should also add the following ternary

self.config = {} if config is None else config

Otherwise, we'll get an error when we try to run self.config.update as update isn't available on a None type.

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

but like. that should only happen if someone called TiingowebsocketClient without the config param. python would raise an error. because the param doesn't have a default argument. so it won't even reach the self.config.update.
am i getting this right?

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 === undefined. I supposed the value of doing the check would have been to guard against someone doing TiingoWebsocketClient(config=None).

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 delenv example here:

monkeypatch.delenv

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

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.

the link you provided is broken

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?

Hi @Mohamededmad4, thanks for looking into this. The CI environment does have a TIINGO_API_KEY. The test is failing because the TIINGO_API_KEY is provided, which is why test_missing_api_key doesn't work.

tiingo-python/.github/workflows/python-package.yml

Lines 42 to 43 in f6ef473

env:

TIINGO_API_KEY: 0000000000000000000000000000000000000000

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

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 ")

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

lgtm-com · 2021-01-03T11:12:53Z

This pull request introduces 2 alerts when merging be25aa2 into f6ef473 - view on LGTM.com

new alerts:

2 for First parameter of a method is not named 'self'

lgtm-com · 2021-01-12T13:59:06Z

This pull request introduces 2 alerts when merging a15986f into f6ef473 - view on LGTM.com

new alerts:

2 for First parameter of a method is not named 'self'

lgtm-com · 2021-07-01T12:30:56Z

This pull request introduces 2 alerts when merging 0be8171 into 7ef43c5 - view on LGTM.com

new alerts:

2 for First parameter of a method is not named 'self'

Mohamedemad4 · 2021-07-02T18:16:36Z

@hydrosquall I give up. I tried setting up the lgtm[py/not-named-self] but they aren't working correctly (or i just used the wrong flags) could you take a look?

hydrosquall · 2021-07-07T00:54:55Z

Hi @Mohamedemad4 , thanks for picking this up again.

@hydrosquall I give up. I tried setting up the lgtm[py/not-named-self] but they aren't working correctly (or i just used the wrong flags) could you take a look?

I think the syntax you used is right, from my reading of the docs

alert suppression changes made in a pull request are only applied when that pull request is merged. So, if you suppress an alert in a pull request, LGTM will continue to report the related alert for as long as the pull request is open.

the alert won't go away until the PR is actually merged.

The more pressing thing to fix is the unit test (see https://github.com/hydrosquall/tiingo-python/runs/3004371828 )

Mohamedemad4 · 2021-07-07T01:49:54Z

tests/test_wsclient.py

+    def test_missing_api_key(self):
+        del os.environ['TIINGO_API_KEY']
+        with self.assertRaises(RuntimeError) as ex:


I just realized something. This test might actually hurt other devs writing their own tests on the future if they depend on TIINGO_API_KEY being in the env. Correct?
@hydrosquall

Also, This test by deleting the TIINGO_API_KEY also fails the test previous to it in the same file. I have no clue how that works.

and I might also add the removing it fixes everything. So I just want to go ahead and delete this test.
is that fine?

hydrosquall · 2021-07-07T02:14:19Z

As mentioned in a previous comment, you don’t want to modify the environment directly, you probably want to monkeypatch the environment variable with delenv. This ensures your patch only modifies the state of this 1 test, and doesn’t have a global effect for other devs or other tests. https://docs.pytest.org/en/6.2.x/monkeypatch.html

…

On Tue, Jul 6, 2021 at 10:03 PM Mohamed Emad ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In tests/test_wsclient.py <#508 (comment)> : > + def test_missing_api_key(self): + del os.environ['TIINGO_API_KEY'] + with self.assertRaises(RuntimeError) as ex: Also, This test by deleting the TIINGO_API_KEY also fails the test previous to it in the same file. I have no clue how that works. and I might also add the removing it fixes everything. So I just want to go ahead and delete this test. is that fine? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#508 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACE2MM2CEET272OQNKJH6ELTWOYVVANCNFSM4QSOMYIQ> .

lgtm-com · 2021-07-07T11:31:23Z

This pull request introduces 2 alerts when merging b162391 into 7f40176 - view on LGTM.com

new alerts:

2 for First parameter of a method is not named 'self'

Mohamedemad4 · 2021-07-19T10:15:42Z

!ping @hydrosquall. tests are all passing locally now

hydrosquall · 2021-07-19T22:20:31Z

This looks great! Thank you for your efforts here @Mohamedemad4 :)!

Mohamedemad4 added 2 commits September 2, 2020 03:17

Added barebone support for websockets API

4ce17b8

added docs to README

1910c4e

hydrosquall reviewed Sep 4, 2020

View reviewed changes

added a more informative docstring with response examples;more verbos…

febcd8a

…e error msg for missing on_msg_cb

ammends to the last commit

111dcb0

hydrosquall requested changes Sep 8, 2020

View reviewed changes

Mohamedemad4 added 3 commits September 10, 2020 11:25

minor comments and doc changes

a76fa92

Added tests to check for valid initialization arguments for TiingoWeb…

e5b89b3

…socketClient

Added tests to check for valid initialization arguments for TiingoWeb…

fa6b010

…socketClient

hydrosquall reviewed Sep 13, 2020

View reviewed changes

ditched the Globals and used a higher order func instead;also ditched…

d9cb894

… the send thread on_open()

removed uneeded import

716a5b6

hydrosquall requested changes Oct 6, 2020

View reviewed changes

removed config as a default arg;added comment to explain why on_error…

f157b0c

…() and on_close() don't get a self

hydrosquall reviewed Oct 6, 2020

View reviewed changes

better error handling for config option

be25aa2

test of missing API key in os env and config dict

a15986f

ignore lgtm [py/not-named-self]

0be8171

Mohamedemad4 commented Jul 7, 2021

View reviewed changes

use unittest.mock to remove TIINGO_API_KEY

b162391

hydrosquall changed the title ~~Added websocket client~~ feature(websocket): Add websocket client Jul 19, 2021

hydrosquall merged commit 6793bbb into hydrosquall:master Jul 19, 2021

hydrosquall mentioned this pull request Jul 19, 2021

Support Websockets Endpoints #2

Closed

hydrosquall mentioned this pull request May 26, 2024

Release/0.15 #986

Merged

	env:
	TIINGO_API_KEY: 0000000000000000000000000000000000000000

	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 ")

feature(websocket): Add websocket client #508

feature(websocket): Add websocket client #508

Conversation

Mohamedemad4 commented Sep 2, 2020

Summary of Changes

codecov bot commented Sep 2, 2020 • edited Loading

Codecov Report

lgtm-com bot commented Sep 2, 2020

hydrosquall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hydrosquall Sep 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hydrosquall commented Sep 4, 2020

Mohamedemad4 commented Sep 6, 2020

lgtm-com bot commented Sep 6, 2020

lgtm-com bot commented Sep 6, 2020

hydrosquall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Sep 10, 2020

hydrosquall left a comment

Choose a reason for hiding this comment

hydrosquall Sep 13, 2020 • edited Loading

Choose a reason for hiding this comment

Mohamedemad4 commented Sep 16, 2020

hydrosquall commented Sep 19, 2020

Mohamedemad4 commented Sep 19, 2020

hydrosquall commented Sep 19, 2020

Mohamedemad4 commented Sep 19, 2020

hydrosquall commented Sep 28, 2020

Mohamedemad4 commented Sep 29, 2020

hydrosquall commented Sep 29, 2020

Mohamedemad4 commented Sep 29, 2020

Mohamedemad4 commented Oct 3, 2020

lgtm-com bot commented Oct 3, 2020

lgtm-com bot commented Oct 4, 2020

hydrosquall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Oct 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Jan 3, 2021

lgtm-com bot commented Jan 12, 2021

lgtm-com bot commented Jul 1, 2021

Mohamedemad4 commented Jul 2, 2021

hydrosquall commented Jul 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hydrosquall commented Jul 7, 2021 via email

lgtm-com bot commented Jul 7, 2021

Mohamedemad4 commented Jul 19, 2021

hydrosquall commented Jul 19, 2021

codecov bot commented Sep 2, 2020 •

edited

Loading

hydrosquall Sep 13, 2020 •

edited

Loading

hydrosquall Sep 13, 2020 •

edited

Loading