redis / redis-om-python

Object mapping, and more, for Redis and Python
MIT License
1.13k stars 116 forks source link

RediSearch and RedisJson checks do not fire against redis:latest #228

Open moznuy opened 2 years ago

moznuy commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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