-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Streaming changes according to Schwab_Trader_API_-_Streamer_Guide.pdf #127
Conversation
Found out that ADD functions need fields to be sent in the request to the websocket so fixed that. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #127 +/- ##
==========================================
- Coverage 98.87% 98.55% -0.32%
==========================================
Files 18 18
Lines 2216 2214 -2
Branches 226 229 +3
==========================================
- Hits 2191 2182 -9
- Misses 17 21 +4
- Partials 8 11 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Test cases related to the streaming module has been updated and added to reflect all the changes. |
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.
Initial round
Also it looks like you're causing a test coverage regression here: https://app.codecov.io/gh/alexgolec/schwab-py/pull/127?dropdown=coverage&src=pr&el=continue#8172eb34b156f7f11dfb8fa1008c98e5-L330 |
… services removed because aren't supported by Schwab
Thanks for this PR! |
#: UNKNOWN | ||
FIELD_15 = 15 | ||
#: Current percent change | ||
FUTURE_CHANGE_PERCENT = 20 |
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 believe this may have been done accidentally since other fields were modified where it does make sense to have the FUTURE
prefix. Would it make more sense to keep this consistent with other contract/ticker types?
MARK_CHANGE_PERCENT
instead of FUTURE_CHANGE_PERCENT
Field names were changed to match the Streamer Guide document. Timesales and News services were removed and Screener Service added. Implemented LOGOUT functionality and for each service ADD feature is implemented as well.