Use random name for RAG namespace#675
Conversation
📝 WalkthroughWalkthroughAdded an optional Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)utilities/infra.py (1)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/hold', '/cherry-pick', '/wip', '/build-push-pr-image', '/lgtm', '/verified'} |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/llama_stack/rag/test_rag.py (1)
21-21: Consider increasing the random suffix length for better uniqueness.The change correctly uses
generate_random_nameto create a random namespace for each test run. However, withlength=4, there are only 65,536 possible namespace names (16^4 hex combinations). This might lead to collisions if tests run in parallel or if namespace cleanup is delayed.Consider using a longer suffix (e.g.,
length=8for 4.3 billion combinations) to reduce collision probability:- {"name": generate_random_name("test-rag", length=4)}, + {"name": generate_random_name("test-rag", length=8)},Note: The function is called at test collection time, so all test methods in the
TestLlamaStackRagclass will share the same namespace name within a single test run, which appears to be the intended behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/llama_stack/rag/test_rag.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/llama_stack/rag/test_rag.py (1)
utilities/general.py (1)
generate_random_name(314-339)
🔇 Additional comments (1)
tests/llama_stack/rag/test_rag.py (1)
12-12: LGTM!The import is correctly added and the function is used appropriately in the test parametrization.
b3c997f to
8c21dc4
Compare
|
@jgarciao |
for more information, see https://pre-commit.ci
|
/cherry-pick 2.25 |
* Use random name for RAG namespace * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jorge <jgarciao@users.noreply.github.com>
|
Cherry pick action created PR #677 successfully 🎉! |
|
Status of building tag latest: success. |
* Use random name for RAG namespace * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Jiri Petrlik <jiripetrlik@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jorge <jgarciao@users.noreply.github.com>
* Use random name for RAG namespace * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jorge <jgarciao@users.noreply.github.com>
Based on https://issues.redhat.com/browse/RHAIENG-999 it will be good to use random namespace name.
Summary by CodeRabbit