added lua scripts support in node. #775
Conversation
adanWattad
force-pushed
the
node/scripts
branch
2 times, most recently
from
ff7e60e
to
ea6cb39
Compare
node/src/BaseClient.ts
Outdated
| /** | ||
| * @internal | ||
| */ | ||
| protected createScriptWritePromise<T>( |
There was a problem hiding this comment.
why not use createWritePromise and just pass message to the function?
node/src/BaseClient.ts
Outdated
| @@ -888,6 +935,16 @@ export class BaseClient { | |||
| return this.createWritePromise(createTTL(key)); | |||
| } | |||
|
|
|||
| /** Invoke the given script with its keys and arguments. | |||
There was a problem hiding this comment.
please fully flesh out the comment - what does this do?
remember, the user expects us to have something that is mappable to Redis commands. You need to explain, at least roughly, what happens internally.
adanWattad
force-pushed
the
node/scripts
branch
3 times, most recently
from
8c15405
to
919c797
Compare
node/src/BaseClient.ts
Outdated
| @@ -297,8 +309,9 @@ export class BaseClient { | |||
| * @internal | |||
| */ | |||
| protected createWritePromise<T>( | |||
| command: redis_request.Command | redis_request.Command[], | |||
| route?: redis_request.Routes | |||
| command?: redis_request.Command | redis_request.Command[], | |||
There was a problem hiding this comment.
the way you implemented this, this function can be called without any argument. That doesn't make sense to me.
why not make command be either redis_request.Command | redis_request.Command[] | redis_request.RedisRequest?
Or, wouldn't it make even more sense to pass redis_request.ScriptInvocation instead of redis_request.RedisRequest, and skip the repetition of the callback index handling?
node/src/BaseClient.ts
Outdated
| @@ -888,6 +913,40 @@ export class BaseClient { | |||
| return this.createWritePromise(createTTL(key)); | |||
| } | |||
|
|
|||
| /** Invokes a Lua script with its keys and arguments. | |||
| * This method simplifies the process of invoking scripts on a Redis server by using an object that represents a Lua script. | |||
| * The Script object is immutable and can be shared. | |||
There was a problem hiding this comment.
Why does this line belong here? shouldn't it be on the script object?
node/tests/SharedTests.ts
Outdated
|
|
||
| expect( | ||
| await client.invokeScript(script, { keys: [key2] }) | ||
| ).toEqual("bar"); |
There was a problem hiding this comment.
if you give both keys the same value, how do you know that you actually got the value from the second key?
adanWattad
force-pushed
the
node/scripts
branch
from
919c797
to
8a0675e
Compare
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.