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

Core: Allow creation of RESP3 connections. #669

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

shachlanAmazon
Copy link
Contributor

depends on amazon-contributing/redis-rs#74.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@shachlanAmazon shachlanAmazon requested a review from a team as a code owner December 11, 2023 11:49
@shachlanAmazon shachlanAmazon force-pushed the enable-resp3 branch 3 times, most recently from 3d4d093 to 8e98ea1 Compare December 12, 2023 12:07
@shachlanAmazon
Copy link
Contributor Author

@barshaul I added another commit, with test fixes that were required by the changes to redis-rs.

@shachlanAmazon shachlanAmazon force-pushed the enable-resp3 branch 2 times, most recently from 12e182d to 98c6a59 Compare December 12, 2023 12:20
@@ -9,7 +9,7 @@
# Typing
T = TypeVar("T")
TOK = Literal["OK"]
TResult = Union[TOK, str, List[str], List[List[str]], int, None]
TResult = Union[TOK, str, List[str], List[List[str]], int, None, dict[str, str]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change dict to Dict (import from Typing)

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be Dict[str, T], as we can get different response types

Copy link
Collaborator

@barshaul barshaul left a comment

Choose a reason for hiding this comment

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

only need to change to Dict[str, T]

These tests were broken by clusters returning maps instead of arrays.
@shachlanAmazon shachlanAmazon merged commit eac0e64 into valkey-io:main Dec 13, 2023
19 checks passed
@shachlanAmazon shachlanAmazon deleted the enable-resp3 branch December 13, 2023 10:16
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.

3 participants