redis-om-python icon indicating copy to clipboard operation
redis-om-python copied to clipboard

RediSearch and RedisJson checks do not fire against redis:latest

Open moznuy opened this issue 3 years ago • 2 comments

When testing with make test_oss we get TypeError: 'NoneType' object is not subscriptable with Traceback in redis-py not in redis-om:

tests_sync/test_json_model.py:29: in <module>
    if not has_redis_json():
redis_om/checks.py:17: in has_redis_json
    command_exists = check_for_command(conn, "json.set")
redis_om/checks.py:9: in check_for_command
    cmd_info = conn.execute_command("command", "info", cmd)
../../.cache/pypoetry/virtualenvs/redis-om-2-PyqfHp-py3.8/lib/python3.8/site-packages/redis/client.py:1218: in execute_command
    return conn.retry.call_with_retry(
../../.cache/pypoetry/virtualenvs/redis-om-2-PyqfHp-py3.8/lib/python3.8/site-packages/redis/retry.py:45: in call_with_retry
    return do()
../../.cache/pypoetry/virtualenvs/redis-om-2-PyqfHp-py3.8/lib/python3.8/site-packages/redis/client.py:1219: in <lambda>
    lambda: self._send_command_parse_response(
../../.cache/pypoetry/virtualenvs/redis-om-2-PyqfHp-py3.8/lib/python3.8/site-packages/redis/client.py:1195: in _send_command_parse_response
    return self.parse_response(conn, command_name, **options)
../../.cache/pypoetry/virtualenvs/redis-om-2-PyqfHp-py3.8/lib/python3.8/site-packages/redis/client.py:1240: in parse_response
    return self.response_callbacks[command_name](response, **options)
../../.cache/pypoetry/virtualenvs/redis-om-2-PyqfHp-py3.8/lib/python3.8/site-packages/redis/client.py:553: in parse_command
    cmd_name = str_if_bytes(command[0])
    TypeError: 'NoneType' object is not subscriptable

You can replicate this against redis:latest with

import redis

redis.StrictRedis().execute_command("command", "info", "json.set")

There is a problem in redis-py with parsing the response from redis. (It's nill) Maybe it's related to: https://github.com/redis/redis-py/blob/09a52dba48221353eafa8188d73ab97e8f4ccc49/redis/commands/core.py#L740-L743

    def command_info(self, **kwargs) -> None:
        raise NotImplementedError(
            "COMMAND INFO is intentionally not implemented in the client."
        )

So when I try to use redis-om against redis:latest:

from redis_om import HashModel, Migrator, get_redis_connection, Field


class TestModel(HashModel):
    test_field: int = Field(index=True)

    class Meta:
        database = get_redis_connection(port=6381)  # redis:latest


Migrator().run()
TestModel.find()

I get this:

Traceback (most recent call last):
  File "/home/lamar/PycharmProjects/redis-om-python-flask-skeleton-app/check.py", line 12, in <module>
    TestModel.find()
  File "/home/lamar/.env/redis-om-python-flask-skeleton-app/lib/python3.8/site-packages/redis_om/model/model.py", line 1168, in find
    return FindQuery(expressions=expressions, model=cls)
  File "/home/lamar/.env/redis-om-python-flask-skeleton-app/lib/python3.8/site-packages/redis_om/model/model.py", line 347, in __init__
    if not has_redisearch(model.db()):
  File "/home/lamar/.env/redis-om-python-flask-skeleton-app/lib/python3.8/site-packages/redis_om/checks.py", line 25, in has_redisearch
    if has_redis_json(conn):
  File "/home/lamar/.env/redis-om-python-flask-skeleton-app/lib/python3.8/site-packages/redis_om/checks.py", line 17, in has_redis_json
    command_exists = check_for_command(conn, "json.set")
...
  File "/home/lamar/.env/redis-om-python-flask-skeleton-app/lib/python3.8/site-packages/redis/client.py", line 530, in parse_command
    cmd_name = str_if_bytes(command[0])
TypeError: 'NoneType' object is not subscriptable

instead of https://github.com/redis/redis-om-python/blob/00ed537fcaf43d93ea26e0332d5cb2f1a4c1c4a1/aredis_om/model/model.py#L347-L352

So maybe we can test with

try:
    db = redis.StrictRedis()
    db.json().set("tmp_key", ".", {})
    db.delete("tmp_key")
except redis.ResponseError:
    raise RedisModelError("...")

here https://github.com/redis/redis-om-python/blob/00ed537fcaf43d93ea26e0332d5cb2f1a4c1c4a1/aredis_om/checks.py#L14 and something similar for RediSearch here https://github.com/redis/redis-om-python/blob/00ed537fcaf43d93ea26e0332d5cb2f1a4c1c4a1/aredis_om/checks.py#L22

moznuy avatar May 02 '22 21:05 moznuy

OK, maybe there is more important problem here: https://github.com/redis/redis-om-python/blob/00ed537fcaf43d93ea26e0332d5cb2f1a4c1c4a1/aredis_om/checks.py#L13-L14

I think lru_cache caches coroutine creation, doesn't it? So it is always not <coroutine object> here https://github.com/redis/redis-om-python/blob/00ed537fcaf43d93ea26e0332d5cb2f1a4c1c4a1/aredis_om/model/model.py#L1468-L1474

and more over, this:

from functools import lru_cache

@lru_cache(maxsize=None)
async def has_redis_json():
    return False

print(type(has_redis_json()))
print(not has_redis_json())

returns:

<class 'coroutine'>
False
sys:1: RuntimeWarning: coroutine 'has_redis_json' was never awaited

Which is exactly expected (I spent a lot of time to get this warning from aredis_om.JsonModel().__init__() but could not for some reason) So you won't see logging.error from redis-om ever I think it should always be sync check in init (with lru_cache). Or somewhere else entirely? Or I simply missing something between aredis_om <-> redis_om import connections or some magic?

moznuy avatar May 04 '22 14:05 moznuy

And what if user uses async API: Model would get async connection, then the check that should be sync(because in __init__) gets async connection... Either we create async and sync Redis instance in metaclass instead of only 1. (which is bad?) Or maybe check might go to the place where async and sync is a part of protocol instead of __init__, which is always sync, like execute() in FindQuery or save(), get() in JsonModel

moznuy avatar May 04 '22 15:05 moznuy