-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add tweet news in terminal toolbar #3757
Conversation
Same thing is happening to me that was happening on the mstarpy branch.
@Chavithra @piiq Do we need to add setuptools to poetry? Although running
|
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.
Please:
- Make sure that there is exception handling in place for situations when data is queried for non-popular instruments
- Make sure that timeouts for people who don't have twitter keys or are in networks where twitter is blocked are 1 second and less
- Add unit tests for every function that you have added
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.
Looks ok to me.
There is a API_TWITTER_BEARER_TOKEN loading issue : this feature requires the credentials to be loaded sooner.
But I think we should try to resolve that on the credentials refactoring that is planned.
For now it works if we put the credentials on config_terminal.py
.
Dependencies were updated.
Let me know if there's anything else you want me to do!