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

Add testing for query caching #1

Closed
xtrojak opened this issue Jun 15, 2021 · 6 comments · Fixed by #23 or #27
Closed

Add testing for query caching #1

xtrojak opened this issue Jun 15, 2021 · 6 comments · Fixed by #23 or #27
Assignees

Comments

@xtrojak
Copy link
Collaborator

xtrojak commented Jun 15, 2021

To avoid multiple execution of the same API queries, the Converter class has cache implemented. This needs to be properly tested.

@xtrojak
Copy link
Collaborator Author

xtrojak commented Jul 16, 2021

Make sure lru_cache works correctly with self attribute, as pointed in this PR.

@xtrojak xtrojak mentioned this issue Jul 16, 2021
@martenson
Copy link
Member

Even if the session is hashable and a singleton I am not sure it makes sense to include it in the cache fingerprint. The response body should be independent of session unless the API does weird things I think?

@martenson
Copy link
Member

Should this really be closed?

@martenson martenson reopened this Jul 27, 2021
@xtrojak
Copy link
Collaborator Author

xtrojak commented Jul 28, 2021

I guess you are right. Any suggestions how to solve this? I would like to keep query_the_service as a method of the class Converter.

@martenson
Copy link
Member

We could run the test with actual API calls, CI can handle that. Or are you asking re: my session comment above?

@xtrojak
Copy link
Collaborator Author

xtrojak commented Jul 29, 2021

Test could be improved using stored info in cach_info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants