Fix processing cluster response. #421
Conversation
Signed-off-by: Yury-Fridlyand <[email protected]>
Yury-Fridlyand
force-pushed
the
node/return-record-with-glidestring-part-2
branch
from
99c42be to
9da36d2
Compare
| @@ -943,6 +999,8 @@ export class GlideClusterClient extends BaseClient { | |||
| /** | |||
| * Returns information about the functions and libraries. | |||
| * | |||
| * The command will be routed to a random node, unless `route` is provided. | |||
There was a problem hiding this comment.
Add @remakrs and move it right after @see
There was a problem hiding this comment.
Other commands have the same - I'll update all of them in bulk later
There was a problem hiding this comment.
I added a task reminder to do that when we will work on docs.
| } | ||
|
|
||
| /** | ||
| * Returns information about the function that's currently running and information about the | ||
| * available execution engines. | ||
| * | ||
| * The command will be routed to all primary nodes, unless `route` is provided. |
There was a problem hiding this comment.
Add @remarks and move this line after @see line.
node/src/BaseClient.ts
Outdated
| @@ -348,20 +348,63 @@ function convertGlideRecordForZSet( | |||
|
|
|||
| /** | |||
| * @internal | |||
| * Downcast `GlideRecord` to `Record`. Use if you are 146% aware that `data` keys are always strings. | |||
| * Recursevly downcast `GlideRecord` to `Record`. Use if `data` keys are always strings. | |||
There was a problem hiding this comment.
| * Recursevly downcast `GlideRecord` to `Record`. Use if `data` keys are always strings. | |
| * Recursively downcast `GlideRecord` to `Record`. Use if `data` keys are always strings. |
There was a problem hiding this comment.
I don't understand that second part?
There was a problem hiding this comment.
That will crash if a key is a Buffer. I use it in tests and while converting data for streams (stream IDs are strings)
There was a problem hiding this comment.
Oh - because it's not safe.
Only use if the key attribute in `data` will always be if string type.
Maybe specifically indicate that it should only be used for stream data containing stream ids?
| */ | ||
| export function glideRecordToRecord<T>( | ||
| data: GlideRecord<T>, | ||
| ): Record<string, T> { | ||
| const res: Record<string, T> = {}; | ||
|
|
||
| for (const pair of data) { | ||
| res[pair.key as string] = pair.value; | ||
| let newVal = pair.value; |
There was a problem hiding this comment.
consider:
const value = pair.value;
let newVal =
isGlideRecord(value) ? glideRecordToRecord(value as GlideRecord<unknown>) as T :
isGlideRecordArray(value) ? (value as GlideRecord<unknown>[]).map(glideRecordToRecord) as T :
value;
There was a problem hiding this comment.
you could also use a
const res = data.map(...)
There was a problem hiding this comment.
Let me avoid that. linter makes nested ternary operators unreadable
| { key: "bar", value: "baz" }, | ||
| { key: "foo", value: "bar" }, |
There was a problem hiding this comment.
A flaky test there, sometimes it receives elements reordered. Will fix it later.
Co-authored-by: Andrew Carbonetto <[email protected]>
Yury-Fridlyand
merged commit 6d832a4
into
node/integ-valkey-293-return-record-with-glidestring
Yury-Fridlyand
deleted the
node/return-record-with-glidestring-part-2
branch
ClusterValueglideRecordToRecordgreat againtraverse and update data struct recursivlyNote: return type for updated functions weren't changed.