cunla / fakeredis-py

Implementation of Redis in python without having a Redis server running. Fully compatible with using redis-py.
https://fakeredis.moransoftware.ca/
BSD 3-Clause "New" or "Revised" License
298 stars 48 forks source link

Add support for specifying specific Lua runtime version #287

Closed serozhenka closed 7 months ago

serozhenka commented 9 months ago

Is your feature request related to a problem? Please describe. As stated in Redis documentation, the only supported Lua runtime is Lua 5.1. Logically, I would also expect it to work with Lua 5.x, because the new minor versions should be backward-compatible with previous minor versions. However, that is not the case. In Lua 5.2 unpack was completely removed and replaced with table.unpack. So when using Lua with fakeredis, the only available version is 5.4 and in the real-world environment it's 5.1. Being unable to fix the Lua version and use one of somebody's choice forces to do such a fix to make it working:

table.unpack = table.unpack or unpack

Describe the solution you'd like Probably, a good solution will be to pass Lua version with an environment variable like FAKEREDIS_LUA_VERSION and default it to the latest one in case it's not present or lupa doesn't support runtime for this version.

Upvote & Fund

Fund with Polar

cunla commented 9 months ago

Oh, so redis supports only LUA 5.1 while fakeredis supports LUA 5.4. I think it would be better to default it to 5.1 on fakeredis.

serozhenka commented 9 months ago

@cunla yeah, this is also doable and valid. I just thought it might break other users' scripts, so it might be better to leave a default as 5.4 for this minor version with an ability to select a custom one, and then make 5.1 a default in the next major version.

cunla commented 9 months ago

The issue is with lupa2.0. If you install lupa 1.14.1 lua 5.1 works fine

serozhenka commented 9 months ago

Sure, but can it be the feature of the fakeredis to have the ability to set a specific Lua version or at least have 5.1 as a default one? I don't think lupa maintainers will ever update 1.14.x version.

cunla commented 9 months ago

A bug should be opened on lupa package. They claim 5.1 is supported in v2.

serozhenka commented 9 months ago

Sorry, but I don't get it. How does opening an issue ticket in lupa package fix this problem? And what even is the exact problem with lupa?

As stated in lupa documentation:

The binary wheels include different Lua versions as well as LuaJIT, if supported. By default, import lupa uses the latest Lua version, but you can choose a specific one via import.

At the time of writing, the latest version of Lua is 5.4. So when you do

from lupa import LuaRuntime

it's the 5.4 that gets used. But in this specific Redis scenario, the Lua version is fixed to 5.1, so importing the LuaRuntime should be as follows:

from lupa.lua51 import LuaRuntime
cunla commented 9 months ago

So with lupa2 - from lupa.lua51 import LuaRuntime does not exist

serozhenka commented 9 months ago

ok, my bad for not checking it, I see what you mean. For some strange reason lupa ships only with 5.[234] and not 5.1.

cunla commented 7 months ago

I created an issue on lupa, closing this issue.

serozhenka commented 7 months ago

I also opened the issue some time ago right here. Basically, we'll have it with the next release, so I wouldn't close this issue as you'll still need to import from lupa.lua51 and not lupa

cunla commented 7 months ago

Good point...

serozhenka commented 7 months ago

thank you!