Replaced zrangebylex method in the range method #521
Conversation
cheesecake100201
requested review from
ashwinb,
yanxi0830,
hardikjshah,
dltn and
raghotham
as code owners
facebook-github-bot
added
the
CLA Signed
|
|
||
| return await self.redis.zrangebylex(start_key, end_key) | ||
| for key in keys: | ||
| key_str = key.decode("utf-8") if isinstance(key, bytes) else key |
There was a problem hiding this comment.
why is the decode necessary? we already using a str as input when setting the key.
There was a problem hiding this comment.
So when we fetch the keys using .scan() method, the list of keys fetched is of instance bytes. I tested it on my local machine as well and I am getting the list of keys where each key is of type byte.
@dineshyv
| return await self.redis.zrangebylex(start_key, end_key) | ||
| for key in keys: | ||
| key_str = key.decode("utf-8") if isinstance(key, bytes) else key | ||
| if start_key <= key_str <= end_key: |
There was a problem hiding this comment.
can we use a more strict pattern like pattern = f"{namespace}[{start_key}-{end_key}]" to only fetch the keys in range instead of having to filter again here?
| if matching_keys: | ||
| values = await self.redis.mget(matching_keys) | ||
| return [ | ||
| value.decode("utf-8") if isinstance(value, bytes) else value |
There was a problem hiding this comment.
why do we need a decode here since we are taking only strings as input?
|
This PR passes the error for me |
|
Can you take a look at this PR again? |
cheesecake100201
requested review from
vladimirivic and
sixianyi0721
as code owners
What does this PR do?
In short, provide a summary of what this PR does and why. Usually, the relevant context should be present in a linked issue.
Test Plan
Please describe:
python llama_stack/apis/agents/client.py localhost 8001 tools_llama_3_1 meta-llama/Llama-3.1-70B-InstructHave used redis in the run.yaml file as well for the persistence_store. Also enable_session_persistence turned to True for this test.
Have also tested this in a jupyter notebook to make sure the current flow does not work through multiple turns in the same session.
Sources
Please link relevant resources if necessary.
Before submitting
Pull Request section?