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

12 fetching data twice #13

Conversation

jaapterwoerds
Copy link
Contributor

@jaapterwoerds jaapterwoerds commented Jul 25, 2021

I changed the way retries work by using urllib3 Retry mechanism which has as as a sideeffect the looping is not necessary anymore and data will be fetched only once if there are no errrors

@jaapterwoerds
Copy link
Contributor Author

Probably also explains why I couldn't get any data in, maybe a form of rate limiting if your hitting the API twice consequently with the same parameters

@jrdi
Copy link
Collaborator

jrdi commented Jul 26, 2021

Thank you for this PR @jaapterwoerds! The change makes completely sense and it's true we are calling twice the API for every single ticker. What a mistake!

Despite that I'm not fully convinced by the retry refactor, especially because sometimes the Yahoo API is responding with 200 on some errors 🤷‍♂️ and the parser failing is triggering the retry itself.

Can you just submit the changes removing the fetching twice in this PR? You can open another one refactoring the retry if you like.

@jaapterwoerds
Copy link
Contributor Author

Hi @jrdi, no problem I'll do that

@jaapterwoerds
Copy link
Contributor Author

@jrdi see this one #14. I'll close this one for now, I'll see if it's possible use the retry mechanism and ensure JSON parsing is only done once. If that's possible I''l reopen this PR

@jaapterwoerds jaapterwoerds deleted the 12-fetching-data-twice branch August 1, 2021 18:24
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.

2 participants