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

Enable caching for the synchronous context #1751

Merged
merged 1 commit into from Feb 8, 2022

Conversation

lubosmj
Copy link
Member

@lubosmj lubosmj commented Dec 3, 2021

ref #8806

lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 3, 2021
@pulpbot
Copy link
Member

pulpbot commented Dec 3, 2021

Attached issue: https://pulp.plan.io/issues/8806

lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 3, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 3, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 3, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 3, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 6, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 6, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 6, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 6, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 7, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 8, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 8, 2021
@ipanova ipanova requested a review from gerrod3 December 8, 2021 12:18
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 8, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 8, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 8, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 9, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 9, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 9, 2021
@lubosmj lubosmj force-pushed the redis-caching-for-pc-tt branch 2 times, most recently from 367b107 to b36da97 Compare December 10, 2021 12:30
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 10, 2021
@lubosmj lubosmj force-pushed the redis-caching-for-pc-tt branch 2 times, most recently from 0880fe4 to d755870 Compare December 21, 2021 23:46
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Dec 21, 2021
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Jan 21, 2022
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Jan 25, 2022
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Jan 25, 2022
@lubosmj lubosmj force-pushed the redis-caching-for-pc-tt branch 3 times, most recently from de5be8e to d59101a Compare January 31, 2022 12:54
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Jan 31, 2022
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Jan 31, 2022
@lubosmj lubosmj marked this pull request as ready for review January 31, 2022 19:49
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Feb 1, 2022
Copy link
Contributor

@gerrod3 gerrod3 left a comment

Choose a reason for hiding this comment

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

Looks good, I still want to know what is up with the hex call.

pulpcore/cache/cache.py Outdated Show resolved Hide resolved
Comment on lines 355 to 363
if binary := entry.pop("body", None):
entry["body"] = bytes.fromhex(binary)

Copy link
Contributor

Choose a reason for hiding this comment

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

Still confused by this.

Copy link
Member

Choose a reason for hiding this comment

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

leaving a comment in the code would be good.

In this commit, the existing method used for submitting cache entries was altered to enable caching binary data as well.

closes pulp#2003
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Feb 8, 2022
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Feb 8, 2022
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Feb 8, 2022
Copy link
Contributor

@gerrod3 gerrod3 left a comment

Choose a reason for hiding this comment

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

I've read the comment on the hex calls and it makes sense. If the response contains bytes we can't assume those bytes are 'utf-8' encoded, so converting to hex in order json encode the value for the cache makes sense.

@ipanova ipanova merged commit 53fa750 into pulp:main Feb 8, 2022
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Feb 8, 2022
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Feb 8, 2022
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Feb 8, 2022
lubosmj added a commit to lubosmj/pulp_container that referenced this pull request Feb 9, 2022
lubosmj added a commit to pulp/pulp_container that referenced this pull request Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants