search

redis / redis-om-python

Object mapping, and more, for Redis and Python
MIT License
1.06k stars 108 forks source link

Consolidate updates to dependencies #597

Closed gaby closed 3 months ago

gaby commented 4 months ago

Closes #577 #588 #548 #567 #556 #554 #564 #563 #560 #495 #518

Tests fail with: #497 Related to #596

image

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 77.89%. Comparing base (1213ca7) to head (ea59eea).

Files Patch % Lines
aredis_om/model/query_resolver.py 0.00% 2 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #597 +/- ## ========================================== + Coverage 77.87% 77.89% +0.01% ========================================== Files 15 15 Lines 1279 1280 +1 ========================================== + Hits 996 997 +1 Misses 283 283 ``` | [Flag](https://app.codecov.io/gh/redis/redis-om-python/pull/597/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) | Coverage Δ | | |---|---|---| | [unit](https://app.codecov.io/gh/redis/redis-om-python/pull/597/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis) | `77.89% <75.00%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redis#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

slorello89 commented 3 months ago

There's something very odd going on in the sync versions for python 3.12, looks like it's improperly escaping whitespace & punctuation:

"FT.SEARCH" "redis-om:testing:0.05308182803634398:member:index" "@first_name:{'Andrew\\\\,\\\\ the\\\\ Michael'} " "LIMIT" "0" "1"
slorello89 commented 3 months ago

Looks like the issue was caused by some updates to fstrings in python 3.12, to match the redis tag syntax, we naturally needed to escape braces in interpolated strings, see an example here, this is all well and good, and it still works in python 3.12, however we regenerate model.py using unasync, and the std_tokenizer changed in 3.12 so that f"@{field_name}:{{{value}}}" is yielded as a series of tokens @, {, field_name, }, :{, {, value, }, }, which from a code-interpretion perspective, is correct, however when the tokens are being reorganized they come out as `f"@{field_name}:{ {value}}", notice the space and missing braces.

It does not appear that there is a way to work around this with the code as-is, and changing those f-strings to the non-condensed format methods seems to resolve the tokenization issues:

so:

result += f"@{field_name}:{{{value}}}"

becomes:

result += "@{field_name}:{{{value}}}".format(
  field_name=field_name, value=value
)

This is a bit of hack, but it gets the job done and is the shortest and simplest way to the result we're looking for here, thoughts @banker, @gaby, @uglide?

slorello89 commented 3 months ago

Spoke with @banker we're good to go (when version update tests pass!)