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

Pass self._streams as a list to subscribe() #363

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Shakespeare0
Copy link

@Shakespeare0 Shakespeare0 commented Jan 13, 2021

If the connection drops and it tries to reconnect, it fails because the channels are saved in self._streams, which is a set, which is not serializable in JSON, so convert it back to a list when passing the channels to _StreamConn.subscribe().

Pass self._streams as a list (rather than a set) to _StreamConn.subscribe() because JSON can not encode a set
@shlomiku
Copy link
Contributor

This PR was closed last week: #352
with the same solution but @medusa-trade came to the conclusion it doesn't work. could you elaborate on why?

@medusa-trade
Copy link

@shlomikushchi / @Shakespeare0 - My implementation was a bit different. I was casting to list from within the json.dumps method within the send. In my case, it would just hang and not respond to messages. I didn't have time to fix, so I deleted the PR. By casting at the time of passing the arguments, it may work. Also, this bug seems to be in Python 3.8.7, and it works for me locally in Python 3.8.5. So this PR should be tested against both versions, IMO.

@Shakespeare0
Copy link
Author

I'm not sure why @medusa-trade 's fix did not work, but self._streams is defined to be a set here and JSON does not support encoding sets, so when self._streams is used to reconnect if the connection drops here, it has to be converted back to a list at some point. In my opinion it should be before it is passed to subscribe(), since subscribe() tries to make sure it's argument is converted to a list (like here) if it's not a list already.

@medusa-trade
Copy link

I agree with @Shakespeare0. This seems like a better solution. I just haven't been able to test it. If it works across Python versions, I think it would be best to merge it. I honestly did dig into why my connections were hanging with my solution. I just know that when I removed those changes, it was still broke on my Ubuntu system with version 3.8.7, but it worked for my local 3.8.5. So I moved on.

@Shakespeare0
Copy link
Author

I think the connection hanging may be a different issue. Without this fix, rather than the connection hanging, I get the error 2021-01-15 22:27:28,344 error while consuming ws messages: Object of type set is not JSON serializable.

@nickvertucci
Copy link

I get 1006 errors ALLLLLLL THE TIME. Same thing Object of type set is not JSON serializable. Would love to see this fix merged in ASAP

@nickvertucci
Copy link

Merge?

This is because Alpaca API gets disconnected midway. So the API tries to reconnect. But when it does so, the channels are not reinitialized, hence it remains a set that is not JSON serializable. Hence, in alpaca_trade_api > stream2.py change 'streams': channels to 'streams': list(channels) in line 132.
That should solve the reconnecting to api and 1006 problem.

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.

4 participants