-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Implemented automatic caching for the discovery documents. #127
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
Conversation
|
@nathanielmanistaatgoogle I'd appreciate it if you could take a look. |
|
@dhermes FYI |
|
Wow. So happy this is coming (3 years later)! /cc @wescpy This is what we talked about at SFPython |
googleapiclient/discovery.py
Outdated
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Please squash commits. |
|
Thanks for the comment @nathanielmanistaatgoogle All addressed, PTAL |
|
Let me write some tests. I'll update here once done. |
|
I added some tests in test_discovery. PTAL |
googleapiclient/discovery.py
Outdated
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@dhermes are you also reviewing this or just excited about it happening? |
|
I am not reviewing. |
|
@nathanielmanistaatgoogle |
|
Looks good. Are there more unit tests still to come or is this what you'd like to have merged? |
|
@nathanielmanistaatgoogle Thanks! |
|
Great; I'll hold steady until I hear more from you. |
782ba62 to
1e10bfa
Compare
|
I have just added a unit test for the file cache, which discovered several bugs in my code! @nathanielmanistaatgoogle Now I'm confident to say "let's merge this". PTAL |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Implemented automatic caching for discovery documents.
|
Thanks for the contribution! |
|
Thanks for the review! I'm wondering when it goes to pypi distribution so that everyone can benefit from it. |
|
Likely not this week; we're swamped with other things. Perhaps toward the end of next week? Would that be acceptable? Otherwise "sometime in September"? |
|
Yeah, sounds good, thanks |
|
Ok, will you be able to do the release this week? If you still have other things, maybe I can do the release, especially if there's how-to doc. Let me know if you want me to do. |
|
To clarify, I have uploaded a small library to pypi before, so I know the basic. My username on pypi is tmatsuo, so maybe you can add me to the owners list of this package on pypi so that I can do the release? My current guess for how to release is that:
|
|
JFYI it's released. |
|
@dhermes I haven't seen any issues with the new release for a week, so I removed your original article about cache on App Engine. Thanks! |
|
You could have just marked it deprecated. Now I have one less imprint on Google Developers 😞 |
|
Yeah, in that sense, I initially tried to mark it as deprecated, but tech writers don't like it. You're doing a greater work on gcloud-python, so I thought you're ok, but let me know if you want it back strongly. |
|
Ha no worries Takashi! I was just giving you a hard time. Thanks! |
Added 2 keyword arguments; cache_discovery and cache. You can suppress caching by setting cache_discovery=False. You can inject your own caching module via the cache kwarg.
It automatically uses app engine memcache if it's detected. It's not a hard dependency. Then fall back to file based caching.
fixes #110