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

Fixed an issue where different cache key is generated for the same message content. #26

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

barthc
Copy link
Contributor

@barthc barthc commented Apr 17, 2024

Context

⛑️ Ticket(s): https://secure.helpscout.net/conversation/2560473810/64343

Summary

The issue here is the message string that is used to generate the cache key. Though both values(first the value used to populate the live merge tag, and the one used to run the feed on form submission) look the same, but strlen shows they have different lengths.

Copy link
Contributor

@claygriffiths claygriffiths left a comment

Choose a reason for hiding this comment

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

LGTM pending the addition of a code comment.

@spivurno This may be something we want to do with LMTs in GPPA, but that will likely be more complex to handle.

When you Squash & Merge, your PR title is mostly fine, but make sure it ends in a full stop (period or exclamation). Here's what I'd use:

Fixed an issue where different cache keys could be generated with the same input resulting in different responses.

class-gwiz-gf-openai.php Outdated Show resolved Hide resolved
@barthc barthc changed the title Fixed an issue where different cache key is generated for the same message content Fixed an issue where different cache key is generated for the same message content. Apr 18, 2024
@barthc barthc merged commit 01ba1c2 into master Apr 18, 2024
1 check passed
@barthc barthc deleted the barth/fix/64343-sha1-cache-key branch April 18, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants