Skip to content

Implemented automatic caching for the discovery documents.#127

Merged
nathanielmanistaatgoogle merged 1 commit into
googleapis:masterfrom
tmatsuo:master
Aug 24, 2015
Merged

Implemented automatic caching for the discovery documents.#127
nathanielmanistaatgoogle merged 1 commit into
googleapis:masterfrom
tmatsuo:master

Conversation

@tmatsuo

@tmatsuo tmatsuo commented Aug 19, 2015

Copy link
Copy Markdown
Contributor

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

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 19, 2015
@tmatsuo

tmatsuo commented Aug 19, 2015

Copy link
Copy Markdown
Contributor Author

@nathanielmanistaatgoogle I'd appreciate it if you could take a look.

@tmatsuo

tmatsuo commented Aug 20, 2015

Copy link
Copy Markdown
Contributor Author

@dhermes FYI

@dhermes

dhermes commented Aug 20, 2015

Copy link
Copy Markdown
Contributor

Wow. So happy this is coming (3 years later)!

/cc @wescpy This is what we talked about at SFPython

Comment thread googleapiclient/discovery.py Outdated

This comment was marked as spam.

This comment was marked as spam.

@nathanielmanistaatgoogle

Copy link
Copy Markdown
Contributor

Please squash commits.

@tmatsuo

tmatsuo commented Aug 20, 2015

Copy link
Copy Markdown
Contributor Author

Thanks for the comment @nathanielmanistaatgoogle

All addressed, PTAL

@tmatsuo

tmatsuo commented Aug 20, 2015

Copy link
Copy Markdown
Contributor Author

Let me write some tests. I'll update here once done.

@tmatsuo

tmatsuo commented Aug 21, 2015

Copy link
Copy Markdown
Contributor Author

I added some tests in test_discovery. PTAL

Comment thread googleapiclient/discovery.py Outdated

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@nathanielmanistaatgoogle

Copy link
Copy Markdown
Contributor

@dhermes are you also reviewing this or just excited about it happening?

@dhermes

dhermes commented Aug 23, 2015

Copy link
Copy Markdown
Contributor

I am not reviewing.

@tmatsuo

tmatsuo commented Aug 23, 2015

Copy link
Copy Markdown
Contributor Author

@nathanielmanistaatgoogle
I addressed your comments. PTAL

@nathanielmanistaatgoogle

Copy link
Copy Markdown
Contributor

Looks good. Are there more unit tests still to come or is this what you'd like to have merged?

@tmatsuo

tmatsuo commented Aug 23, 2015

Copy link
Copy Markdown
Contributor Author

@nathanielmanistaatgoogle Thanks!
I would like to add some unit tests for the cache module. I will likely write those early next week.

@nathanielmanistaatgoogle

Copy link
Copy Markdown
Contributor

Great; I'll hold steady until I hear more from you.

@tmatsuo tmatsuo force-pushed the master branch 2 times, most recently from 782ba62 to 1e10bfa Compare August 24, 2015 17:33
@tmatsuo

tmatsuo commented Aug 24, 2015

Copy link
Copy Markdown
Contributor Author

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.

nathanielmanistaatgoogle added a commit that referenced this pull request Aug 24, 2015
Implemented automatic caching for discovery documents.
@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit ed3d5a6 into googleapis:master Aug 24, 2015
@nathanielmanistaatgoogle

Copy link
Copy Markdown
Contributor

Thanks for the contribution!

@tmatsuo

tmatsuo commented Aug 24, 2015

Copy link
Copy Markdown
Contributor Author

Thanks for the review! I'm wondering when it goes to pypi distribution so that everyone can benefit from it.

@nathanielmanistaatgoogle

Copy link
Copy Markdown
Contributor

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"?

@tmatsuo

tmatsuo commented Aug 24, 2015

Copy link
Copy Markdown
Contributor Author

Yeah, sounds good, thanks

@tmatsuo

tmatsuo commented Sep 3, 2015

Copy link
Copy Markdown
Contributor Author

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.

@tmatsuo

tmatsuo commented Sep 10, 2015

Copy link
Copy Markdown
Contributor Author

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:

  1. Create a release on github
  2. upload the package to testpypi first and do the sanity check
  3. upload it to pypi

@tmatsuo

tmatsuo commented Sep 14, 2015

Copy link
Copy Markdown
Contributor Author

@tmatsuo

tmatsuo commented Sep 24, 2015

Copy link
Copy Markdown
Contributor Author

@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!

@dhermes

dhermes commented Sep 24, 2015

Copy link
Copy Markdown
Contributor

You could have just marked it deprecated. Now I have one less imprint on Google Developers 😞

@tmatsuo

tmatsuo commented Sep 24, 2015

Copy link
Copy Markdown
Contributor Author

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.

@dhermes

dhermes commented Sep 24, 2015

Copy link
Copy Markdown
Contributor

Ha no worries Takashi! I was just giving you a hard time. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discussion: discovery doc caching

4 participants