Skip to content

Update all schemas to use the key_separator #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Update all schemas to use the key_separator #16

wants to merge 1 commit into from

Conversation

@tylerhutcherson
Copy link
Contributor

@tylerhutcherson tylerhutcherson commented Mar 7, 2025 •
edited
Loading

Small update for better key creation safety through RedisVL. Looks like the parent_checkpoint_id issue still remains. Very strange that this small tweak wreaks havoc here.

@tylerhutcherson tylerhutcherson added the bug Something isn't working label Mar 7, 2025
@bsbodden
Copy link
Contributor

bsbodden commented May 13, 2025

  1. The PR Update all schemas to use the key_separator #16 changes have already been implemented in the current codebase. Specifically:
    • The key parsing method in base.py now correctly handles keys with extra components using slicing (parts[:6]) instead of direct unpacking.
    • The error message has been updated to say "Expected at least 6 parts" instead of "Expected exactly 6 parts".
    • The fix correctly handles complex keys from subgraphs and semantic search operations.
  2. Comprehensive test coverage exists:
    - test_key_parsing.py includes basic tests for the key parsing functionality.
    - test_key_parsing_focus.py and test_key_parsing_suite.py specifically test the fix for handling keys with extra components.
    - test_subgraph_key_parsing.py tests the fix in the context of subgraphs, which was one of the main use cases affected by the issue.
    - test_semantic_search_keys.py tests the fix with semantic search operations, another key use case.

@bsbodden bsbodden closed this May 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bsbodden bsbodden Awaiting requested review from bsbodden

@abrookins abrookins Awaiting requested review from abrookins

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tylerhutcherson @bsbodden