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

Replace table.getn with length operator #63

Merged
merged 1 commit into from
Nov 1, 2019
Merged

Replace table.getn with length operator #63

merged 1 commit into from
Nov 1, 2019

Conversation

kmontag
Copy link

@kmontag kmontag commented Nov 1, 2019

table.getn was removed in Lua 5.2, in favor of the # operator - see e.g. https://stackoverflow.com/questions/11890125/lua-table-library-removed.

While Redis itself still seems to support the old syntax, this change allows compatibility with mock libraries which may rely on newer Lua versions. In my case I ran into issues with https://github.com/stipsan/ioredis-mock.

In the meantime, if anyone is looking a solution to use redlock with ioredis-mock or similar, this has worked for me:

const replaceGetn = (lua: string) => {
  // table.getn(KEYS) --> #KEYS
  return lua.replace(/table\.getn\(([A-Za-z0-9]+)\)/g, '#$1');
}
const redlock = new Redlock(clients, {
  lockScript: replaceGetn,
  extendScript: replaceGetn,
});

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 107d986 on kmontag:patch-1 into 75c0772 on mike-marcacci:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 107d986 on kmontag:patch-1 into 75c0772 on mike-marcacci:master.

@coveralls
Copy link

coveralls commented Nov 1, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 63d8037 on kmontag:patch-1 into 75c0772 on mike-marcacci:master.

@mike-marcacci
Copy link
Owner

👍This looks great – it appears that redis has always supported lua 5.1, so this doesn't introduce any compatibility issues. I'll release this as a minor shortly.

@mike-marcacci mike-marcacci merged commit 955719d into mike-marcacci:master Nov 1, 2019
@kmontag kmontag deleted the patch-1 branch November 2, 2019 14:29
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