-
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/Frontend sync #6327
base: develop
Are you sure you want to change the base?
hotfix/Frontend sync #6327
Conversation
This reverts commit 79f1dba.
@@ -144,3 +149,27 @@ def get_interval(value: str) -> str: | |||
} | |||
|
|||
return f"{value[:-1]}{intervals[value[-1]]}" | |||
|
|||
|
|||
# some fmp endpoint return date in EST without a timezone, this function will parse it |
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.
They actually return the date in the timezone of the exchange, without a timezone. For example:
https://financialmodelingprep.com/api/v3/historical-chart/5min/MNDI.L?
{
"date": "2024-04-08 08:05:00",
"open": 1400.5,
"low": 1396.5,
"high": 1400.5,
"close": 1398,
"volume": 5298
},
{
"date": "2024-04-08 08:00:00",
"open": 1395,
"low": 1395,
"high": 1407.5,
"close": 1399,
"volume": 4883
}
The London Stock Exchange is open Monday through Friday from 8:00 am to 4:30 pm British Summer Time (GMT+01:00).
You are localizing that as 8:00 AM EST, which is not a correct TZ conversion.
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.
smh they really are the best 💀
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.
This is exactly the reason why it's not localized, it's better to leave it alone than to make it incorrectly tz-aware.
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.
then I need to figure out the current timezone per symbol, or else all dates will be treated as UTC on the frontend. RIP
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.
Seems like something frontend needs to deal with independently of the fetcher. These should stay tz-unaware from the fetcher.
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.
@deeleeramone if we have to do this on FE can you get us a list of all exchanges and the time zone they in
Ty
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.
Not from FMP directly, at least AFAICT. They also don't use the ISO codes for exchanges.
return [FMPFinancialRatiosData.model_validate(d) for d in results] | ||
results: List[FMPFinancialRatiosData] = [] | ||
for item in data: | ||
new_item = {to_snake_case(k).replace("ttm", ""): v for k, v in item.items()} |
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.
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.
nice yup done!
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.
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.
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.
also have no idea why this one keeps failing on the test runs
I think this is because of way the requests are structured - the nested async requests within the callback.
Frontend sync