add automatic caching for discovery requests, refreshing on a miss #238
Conversation
openshift-ci-robot
added
the
size/L
| if not self.__cache.get('version'): | ||
| self.__cache['version'] = {'kubernetes': load_json(self.request('get', '/version'))} | ||
| try: | ||
| self.__cache['version']['openshift'] = load_json(self.request('get', '/version/openshift')) |
There was a problem hiding this comment.
I believe /version/openshift no longer exists, is this just to maintain legacy support?
There was a problem hiding this comment.
Yeah, I imagine this is usually run against older versions of openshift so I figured I would just leave it in.
|
@willthames any thoughts on this? |
…ond attempt succeeds
openshift/dynamic/client.py
Outdated
| if PY3: | ||
| default_cache_id = default_cache_id.encode('utf-8') | ||
| self.__resources = ResourceContainer({}, client=self) | ||
| self.__cache_file = cache_file or '/tmp/osrcp-{0}.json'.format(base64.b64encode(default_cache_id).decode('utf-8')) |
There was a problem hiding this comment.
should probably use $TMPDIR rather than defaulting to /tmp
|
@fabianvf I'll run this in on my test suite next time I have some spare time, but I like the concept. |
|
Speed up of tests from Aus to us-west-2 is 58 seconds down to 31 seconds This is similar performance to #220 - would this get down to 15 seconds with both? :) |
|
I should be able to merge this with my changes without too much headache, just have to be mindful when rebasing or vice versa depending which merges first. |
|
@willthames added better tempfile stuff, ready to merge unless you have an objection. |
|
I think this is a great change @fabianvf, merge away! |
…penshift#238) * add automatic caching for discovery requests, refreshing on a miss * fix base64 encode in python3 * Don't replace ResourceContainer on cache invalidation so that the second attempt succeeds * Use more generic temp directory and path operations
|
/cherrypick release-0.8 |
|
@fabianvf: new pull request created: #257 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
…penshift#238) * add automatic caching for discovery requests, refreshing on a miss * fix base64 encode in python3 * Don't replace ResourceContainer on cache invalidation so that the second attempt succeeds * Use more generic temp directory and path operations (cherry picked from commit c6f2168)
) * add automatic caching for discovery requests, refreshing on a miss * fix base64 encode in python3 * Don't replace ResourceContainer on cache invalidation so that the second attempt succeeds * Use more generic temp directory and path operations (cherry picked from commit c6f2168)
…penshift#238) * add automatic caching for discovery requests, refreshing on a miss * fix base64 encode in python3 * Don't replace ResourceContainer on cache invalidation so that the second attempt succeeds * Use more generic temp directory and path operations
This adds an automatic cache of discovery requests that is written to
/tmp/osrcp-${BASE_64_ENCODED_OPENSHIFT_HOST).json.This cache will persist until
invalidate_cacheis called manually, or until a resource is not found in the cache (making misses fairly expensive).By necessity, also adds a JSON encoder and decoder for the resources.
May conflict with changes in #220 , so @asetty would you mind taking a look and seeing if there's something I should do to make this play more nicely with your work?
For the Ansible modules, this should cut the number of requests per task down significantly.
I don't know much about caching so please be brutal in your feedback.