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

wrap read_key in a ResponseMeta so callers can access the index #48

Merged
merged 1 commit into from
Jul 20, 2024

Conversation

badalex
Copy link
Contributor

@badalex badalex commented Jul 18, 2024

when a delete occurs we need this index as the ModifyIndex of all the ReadKeyResponses will still be the same (and lower). which causes blocking requests to be "broken" as future read_key request immediatly returns until either a new key, or a update occuries to a key contained in the ReadKeyResponse Vec.

per #30

Checks

Please check these off before promoting the pull request to non-draft status.

  • All CI checks are green.
  • I have reviewed the proposed changes myself.

when a delete occurs we need this index as the ModifyIndex of all the
ReadKeyResponses will still be the same (and lower). which causes blocking
requests to be "broken" as future read_key request immediatly returns until
either a new key, or a update occuries to a key contained in the
ReadKeyResponse Vec.

per Roblox#30
Copy link

github-actions bot commented Jul 18, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@badalex
Copy link
Contributor Author

badalex commented Jul 18, 2024

I have read the CLA Document and I hereby sign the CLA

@kushudai
Copy link
Contributor

kushudai commented Jul 19, 2024

Hi @badalex Thank you for explaining the problem better in #30 and this change!
I was hoping you could update a test as well to verify the index is correctly returned? i.e. do a repeated read and verify index does not change. And read, update read and verify the index does increase?

EDIT: Nvm, I can take care of this.

@kushudai kushudai merged commit c756d9b into Roblox:main Jul 20, 2024
6 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 20, 2024
@badalex badalex deleted the read_key_return_index branch July 21, 2024 05:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants