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

Node: Add binary variant to server management commands. #2179

Merged
merged 9 commits into from
Aug 23, 2024

Conversation

Yury-Fridlyand
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand commented Aug 21, 2024

https://redis.io/docs/latest/commands/?group=server

  • ConfigGet
  • ConfigResetStat
  • ConfigRewrite
  • ConfigSet
  • DbSize
  • FlushAll
  • FlushDb
  • Info
  • LastSave
  • Lolwut
  • Time

@Yury-Fridlyand Yury-Fridlyand added the node Node.js wrapper label Aug 21, 2024
): Promise<Record<string, string>> {
return this.createWritePromise(createConfigGet(parameters));
decoder?: Decoder,
): Promise<Record<string, GlideString>> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I assume that config parameter names are defined by the server, so all of them are simple strings.
Can I assume that parameter values are simple strings as well?

public async configSet(parameters: Record<string, string>): Promise<"OK"> {
return this.createWritePromise(createConfigSet(parameters));
public async configSet(
parameters: Record<string, GlideString>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same here

@Yury-Fridlyand Yury-Fridlyand marked this pull request as ready for review August 22, 2024 20:31
@Yury-Fridlyand Yury-Fridlyand requested a review from a team as a code owner August 22, 2024 20:31
Copy link
Collaborator

@acarbonetto acarbonetto left a comment

Choose a reason for hiding this comment

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

some minor comments to clean up

@@ -98,6 +98,15 @@ export type DecoderOption = {
decoder?: Decoder;
};

/** An extension to command option types with {@link Routes}. */
export type RouteOption = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this used in this file?
Maybe RouteOption should be defined in GlideClusterClient because it's not ever used here.

Thoughts?

node/src/GlideClient.ts Show resolved Hide resolved
node/src/GlideClient.ts Outdated Show resolved Hide resolved
node/src/GlideClient.ts Outdated Show resolved Hide resolved
node/src/GlideClient.ts Outdated Show resolved Hide resolved
node/src/GlideClusterClient.ts Outdated Show resolved Hide resolved
node/src/GlideClusterClient.ts Outdated Show resolved Hide resolved
node/src/Transaction.ts Outdated Show resolved Hide resolved
node/src/Transaction.ts Outdated Show resolved Hide resolved
node/src/Transaction.ts Outdated Show resolved Hide resolved
@Yury-Fridlyand Yury-Fridlyand merged commit 82cbd33 into valkey-io:main Aug 23, 2024
9 checks passed
@Yury-Fridlyand Yury-Fridlyand deleted the node/yuryf-valkey-271 branch August 23, 2024 20:56
liorsve pushed a commit to liorsve/valkey-glide that referenced this pull request Aug 25, 2024
* Add binary variant to server management commands.

Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Andrew Carbonetto <[email protected]>
liorsve pushed a commit to liorsve/valkey-glide that referenced this pull request Aug 25, 2024
* Add binary variant to server management commands.

Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Andrew Carbonetto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node Node.js wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants