feat: Add path_utils.py with method resolve_repo_path#1278
Merged
dbasunag merged 5 commits intoopendatahub-io:mainfrom Mar 24, 2026
Merged
feat: Add path_utils.py with method resolve_repo_path#1278dbasunag merged 5 commits intoopendatahub-io:mainfrom
dbasunag merged 5 commits intoopendatahub-io:mainfrom
Conversation
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/wip', '/lgtm', '/hold', '/cherry-pick', '/build-push-pr-image', '/verified'} |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCentralizes repository-bound path resolution by adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/llama_stack/utils.py (1)
282-287: Type hint fordoc_sourcesis too permissive.
doc_sources: Anyloses type safety. Since the function expects a list of strings and validates this at runtime (line 304), tighten the signature.def vector_store_upload_doc_sources( - doc_sources: Any, + doc_sources: list[str], llama_stack_client: LlamaStackClient, vector_store: Any, vector_io_provider: str, ) -> None:This makes the runtime
TypeErrorcheck at line 304-305 redundant (static analysis catches misuse), or keep the check for defense-in-depth but update the type hint regardless.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/llama_stack/utils.py` around lines 282 - 287, The doc_sources parameter on vector_store_upload_doc_sources is too permissive (Any); change its type hint to a stricter collection type such as Sequence[str] or List[str] (e.g., doc_sources: Sequence[str]) and add the necessary import from typing, while optionally keeping the existing runtime TypeError check in the function for defense-in-depth; update references in the function body to rely on the stronger static type and adjust tests/type annotations accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONSTITUTION.md`:
- Line 82: The term "parametrized" in the CONSTITUTION.md line referencing
utilities.path_utils.resolve_repo_path is inconsistent with the project's
spelling "parameterized"; update that occurrence to "parameterized" so it
matches other uses (e.g., "parameterized fixtures") and ensure wording around
utilities.path_utils.resolve_repo_path remains unchanged except for the
corrected spelling.
In `@utilities/path_utils.py`:
- Line 37: The trailing noqa comment "FCN001" on the Path(source) assignment is
invalid for Ruff; remove the invalid "# noqa: FCN001" from the raw =
Path(source) line, or if FCN001 is intended from another linter, add "FCN001" to
the lint.external list in pyproject.toml so Ruff won't flag it; update the raw =
Path(source) line (or pyproject.toml) accordingly and run the linter to confirm
the warning is resolved.
---
Nitpick comments:
In `@tests/llama_stack/utils.py`:
- Around line 282-287: The doc_sources parameter on
vector_store_upload_doc_sources is too permissive (Any); change its type hint to
a stricter collection type such as Sequence[str] or List[str] (e.g.,
doc_sources: Sequence[str]) and add the necessary import from typing, while
optionally keeping the existing runtime TypeError check in the function for
defense-in-depth; update references in the function body to rely on the stronger
static type and adjust tests/type annotations accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 8262c5b1-effa-4809-8e5e-aa084e79298f
📒 Files selected for processing (4)
CONSTITUTION.mdtests/llama_stack/conftest.pytests/llama_stack/utils.pyutilities/path_utils.py
💤 Files with no reviewable changes (1)
- tests/llama_stack/conftest.py
Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com>
jgarciao
force-pushed
the
add-resolve_repo_path
branch
from
2d1de22 to
5cee8ca
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
utilities/path_utils.py (1)
37-37:⚠️ Potential issue | 🟡 MinorRemove or register invalid
# noqacode on Line 37.
# noqa: FCN001is not recognized by Ruff (RUF102), so this suppression is ineffective. Either remove the inlinenoqaor addFCN001to Rufflint.externalif another linter truly owns that code.Proposed fix (file-local)
- raw = Path(source) # noqa: FCN001 + raw = Path(source)#!/bin/bash set -euo pipefail echo "== Find invalid noqa usage ==" rg -n 'noqa:\s*FCN001' --type py echo echo "== Check whether Ruff external codes include FCN001 ==" fd pyproject.toml --exec sh -c 'echo "--- {} ---"; rg -n "external|FCN001" "{}" || true'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@utilities/path_utils.py` at line 37, The inline suppression "# noqa: FCN001" on the statement "raw = Path(source)" is invalid for Ruff (RUF102); either remove the inline noqa or register FCN001 as an external code in your Ruff config. Fix by deleting the trailing " # noqa: FCN001" from the "raw = Path(source)" line in utilities/path_utils.py, or alternatively add "FCN001" under lint.external in pyproject.toml so Ruff recognizes the suppression; reference the "raw" variable and the "Path(source)" expression when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@utilities/path_utils.py`:
- Line 37: The inline suppression "# noqa: FCN001" on the statement "raw =
Path(source)" is invalid for Ruff (RUF102); either remove the inline noqa or
register FCN001 as an external code in your Ruff config. Fix by deleting the
trailing " # noqa: FCN001" from the "raw = Path(source)" line in
utilities/path_utils.py, or alternatively add "FCN001" under lint.external in
pyproject.toml so Ruff recognizes the suppression; reference the "raw" variable
and the "Path(source)" expression when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: a5dde62c-0219-4b11-9b27-4fac286c3a5f
📒 Files selected for processing (4)
CONSTITUTION.mdtests/llama_stack/conftest.pytests/llama_stack/utils.pyutilities/path_utils.py
💤 Files with no reviewable changes (1)
- tests/llama_stack/conftest.py
✅ Files skipped from review due to trivial changes (1)
- CONSTITUTION.md
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/llama_stack/utils.py
Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com>
Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com>
dbasunag
requested changes
Mar 23, 2026
jiripetrlik
approved these changes
Mar 24, 2026
dbasunag
approved these changes
Mar 24, 2026
|
Status of building tag latest: success. |
mwaykole
pushed a commit
to mwaykole/opendatahub-tests
that referenced
this pull request
Mar 24, 2026
…#1278) * feat: Add path_utils.py with method resolve_repo_path Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com> * fix: add FCN001 as external linter error Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com> * fix: modify doc_sources type to be list Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com> --------- Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com> Signed-off-by: Milind waykole <mwaykole@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes