redis / redis-om-python

Object mapping, and more, for Redis and Python
MIT License
1.12k stars 112 forks source link

Fix None instance when querying #368

Closed tonibofarull closed 2 years ago

tonibofarull commented 2 years ago

Context I have an application that is constantly adding instance into redis with a TTL and another application that reads queries it.

Bug Sometimes, the application that queries gets the following error:

  File "/home/worker/.local/lib/python3.10/site-packages/aredis_om/model/model.py", line 760, in all
    return await query.execute()
  File "/home/worker/.local/lib/python3.10/site-packages/aredis_om/model/model.py", line 727, in execute
    results = self.model.from_redis(raw_result)
  File "/home/worker/.local/lib/python3.10/site-packages/aredis_om/model/model.py", line 1204, in from_redis
    map(to_string, res[i + fields_offset][::2]),
TypeError: 'NoneType' object is not subscriptable

If this application queries again, the error disappear for a while.

Reason (?) I think that the main problem is the interaction between the TTL and the query. If I examine the res variable in this loop: https://github.com/redis/redis-om-python/blob/c6fb00bfc3a32666abf502b003bfb031712e20b9/aredis_om/model/model.py#L1207-L1222

I see that the reason of the exception is that the variable has the following structure:

[
    3,
    ':src.model.MySchema:01GBZF2X6NZZQQ8M519J9B0NR9', None,
    ':src.model.MySchema:01GBZF2XSF7EKFB3AEA6EW85P2', ['instance_id', 'e86a35a9-0d5f-409c-a1ca-0d84832d2585', 'status', 'success'],
    ':src.model.MySchema:01GBZF2WHPNDE1K917NJ6TBV5N', ['instance_id', '9f61a920-cc58-4fdc-93c8-6bba872c4132', 'status', 'success'],
]

which means trying to access a position in a None.

I'm assuming that the reason of having a None is because the instance has died in the redis database but the key is still in the redis index.

tonibofarull commented 2 years ago

Hi @dvora-h and @chayim, can anyone review this PR? 🙇‍♂️ Another question. When is it planned to release a new version in pypi? The last one was 3 months ago.

codecov-commenter commented 2 years ago

Codecov Report

Merging #368 (3ece203) into main (788d3df) will decrease coverage by 0.04%. The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #368      +/-   ##
==========================================
- Coverage   77.64%   77.59%   -0.05%     
==========================================
  Files          14       14              
  Lines        1154     1156       +2     
==========================================
+ Hits          896      897       +1     
- Misses        258      259       +1     
Flag Coverage Δ
unit 77.59% <50.00%> (-0.05%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aredis_om/model/model.py 86.42% <50.00%> (-0.09%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

dvora-h commented 2 years ago

@tonibofarull Thanks for this fix!

I think we will release a new version soon.