Skip to content

Conversation

@martinslota
Copy link
Contributor

@martinslota martinslota commented May 21, 2025

Update: The fix in this PR has been combined with another (very likely unnecessary) change in #34 and merged to main.

This PR addresses the immediate error reported in #31 by fixing an oversight from #5.

However, this fix may not be sufficient to address #31 overall.

@gurgunday
Copy link
Contributor

You should also add it to getNodes, it also changed behavior after #5

@gurgunday
Copy link
Contributor

@martinslota
Copy link
Contributor Author

martinslota commented May 27, 2025

See https://github.com/valkey-io/iovalkey/pull/34/files#diff-482d55b63b803659d1c76ee835e5d49e0ea771921be69380cf0f971928aa37d3R32

@gurgunday I considered making that change as well, but I believe it is unnecessary because in getNodes the keys are guaranteed to be the existing ones - a synchronous iteration over the keys is happening directly in that method.

Were you able to provoke a similar error from getNodes? If you did, it would be worth a separate regression test.

@martinslota martinslota deleted the martinslota/tolerate-undefined-in-getSampleInstance branch May 27, 2025 14:45
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.

2 participants