-
Notifications
You must be signed in to change notification settings - Fork 26
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
local cache for port-target-cache between session (RDT-501) #221
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR. Overall LGTM.
Instead of upload the local cache every time we run set_port_target_cache
or drop_port_target_cache
, we may implement init
and close
for Meta
instances. Then we will only load/dump cache once per session.
Besides, please cleanup your commit history a bit. We're using the commit messages to generate the changelog. so it's quite important.
For the PR title, you may write a short summary. Inside the PR description (the first comment), you may add closes #xx
. The issue will be closed automatically after this PR is merged.
9a709a9
to
7739198
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Could you help add a test to check if the local cache works? You may refer to test_detect_port_with_cache
for an example
oh, and one last comment, please follow the contributions guide. The pre-commit check is failing |
Implement local cache for port-target-cache between session with use shelve.
Change function port_target_cache to yield fixtures type.
closes #182