Skip to content
This repository has been archived by the owner on Mar 13, 2022. It is now read-only.

fix: load cache error when CacheDecoder object is not callable #226

Merged
merged 3 commits into from
Feb 2, 2021

Conversation

jamesgetx
Copy link
Contributor

fix the code snippet self._cache = json.load(f, cls=CacheDecoder(self.client)) raise 'CacheDecoder' object is not callable

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 29, 2021
@fejta-bot
Copy link

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/check-cla

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 29, 2021
@yliaog
Copy link
Contributor

yliaog commented Jan 31, 2021

thanks for the pr, could you please add a test case?

@jamesgetx
Copy link
Contributor Author

thanks for the pr, could you please add a test case?

Ok, I will add it later:)

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 1, 2021
@jamesgetx
Copy link
Contributor Author

jamesgetx commented Feb 1, 2021

hi @yliaog. I have pushed the test case, but i'm not sure there is any other better solutions to test the code

@fabianvf
Copy link
Contributor

fabianvf commented Feb 1, 2021

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2021
def setUpClass(cls):
cls.config = base.get_e2e_configuration()

def test_init_cache_from_file(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

so if you run the test without the fix, the test will fail, and there will be exception, right?

Copy link
Contributor Author

@jamesgetx jamesgetx Feb 2, 2021

Choose a reason for hiding this comment

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

yes. without the fix, Discoverer.__init_cache will raise 'CacheDecoder' object is not callable then call self.invalidate_cache(), finally self._write_cache() will be called to modify Discoverer.__cache_file

@yliaog
Copy link
Contributor

yliaog commented Feb 2, 2021

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jamesgetx, yliaog

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 2, 2021
@jamesgetx jamesgetx requested a review from yliaog February 2, 2021 03:13
@k8s-ci-robot k8s-ci-robot merged commit 6e2e494 into kubernetes-client:master Feb 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants