-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Hotfix/weird logs #5439
Hotfix/weird logs #5439
Conversation
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.
Thanks @hjoaquim
I was seeing this beyond ipython terminal and notebooks. Let me check the wheel
15 mins later...
Ok, the wheel does not give this error when pip installed
@@ -816,4 +816,7 @@ def get_spread_for_crypto_pair( | |||
return df | |||
|
|||
|
|||
POSSIBLE_CRYPTOS = list(get_erc20_tokens()["symbol"].unique()) |
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.
yeah, i also thought about this. i didn't like how it was implemented as a constant but it's actually loading stuff from disk
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.
Yes, it is - my issue with it was that this file gets "loaded" before the loggers are set up, and since get_erc20_tokens
gets called and will try to log something, might lead to an unexpected behavior.
@hjoaquim you fix mypy yourself or you need a hand? |
Regarding 3cf7751, @montezdesousa needed to go back and define |
Description
On the SDK, we were getting logs messages printed on notebooks - this was because notebooks come with handlers by default (see Notebook has a root logging handler installed by default ipython/ipython#8282 for more context).
The solution proposed on this PR is to clean the root logger handler, if any, during
setup_handlers()
.Also, took the opportunity to:
filter empty credentials incoming from the hub
encapsulated the usage of a variable that is created of the cost of calling a function inside a function so we don't have unnecessary executions during initialization
Link # issue, if applicable.
NA
Screenshot of the feature or the bug before/after fix, if applicable.
Before
After
Relevant motivation and context.
🐛 ☠️
List any dependencies that are required for this change.
NA
How has this been tested?
Just initialize the SDK:
from openbb_terminal.sdk import openbb
Checklist:
feature/feature-name
orhotfix/hotfix-name
.Others