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

Added support for caching results #97

Merged

Conversation

AkashD-Developer
Copy link
Contributor

Checklist

  • My code is properly formatted using the latest black
  • I have added/updated tests if needed
  • I have tried running my code manually
  • I have checked for spelling errors

Description
Resolves #61
Added support to cache the responses to a JSON file which will get created when the user first runs the program and also added it to .gitignore so that it will not be tracked by git.

The cache file looks something like this after few requests
cached_results

Those are the keys, which contains the content of the responses.

@hedyhli
Copy link
Owner

hedyhli commented Feb 16, 2021

hello, thank you for your pr! I will take a look when I have time, hopefully before the end of next week.

@AkashD-Developer
Copy link
Contributor Author

Thanks, Hedy. Glad to hear back from you.
Will be waiting for the review.

Copy link
Owner

@hedyhli hedyhli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

There are some minor improvements that could be made, but since I might not be able to review them quickly after your response, I will do them myself. Thanks again!


from datetime import datetime, timedelta
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be with the other standard library imports

@@ -12,6 +15,8 @@
status_actions,
)

CACHED_RESULT_FP = os.path.dirname(os.path.dirname(__file__)) + "/.cached_result.json"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think FP is a good constant name, how about:

Suggested change
CACHED_RESULT_FP = os.path.dirname(os.path.dirname(__file__)) + "/.cached_result.json"
CACHED_RESULT_PATH = os.path.dirname(os.path.dirname(__file__)) + "/.cached_result.json"

hedyhli pushed a commit that referenced this pull request Feb 21, 2021
@hedyhli hedyhli merged commit 66cd701 into hedyhli:main Feb 21, 2021
@hedyhli
Copy link
Owner

hedyhli commented Feb 21, 2021

@allcontributors please add @AkashD-Developer for code

@allcontributors
Copy link
Contributor

@hedythedev

I've put up a pull request to add @AkashD-Developer! 🎉

@AkashD-Developer
Copy link
Contributor Author

Hey Hedy, thank you so much. I would love to contribute more if there’s anything else you require help for.

@hedyhli
Copy link
Owner

hedyhli commented Feb 28, 2021

No problem, feel free to do the tests ^

@AkashD-Developer
Copy link
Contributor Author

Hey @hedythedev, sure I will take up the new issue. I was actually thinking of some tests for it but was unable to come up with anything. Thanks for the ideas 👍

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.

Caching request responses to prevent hitting rate limits
2 participants