Skip to content
This repository was archived by the owner on Apr 23, 2026. It is now read-only.

fix(RAG): Better error handling for missing RAG db or model in chat - #3048

Closed
jwm4 wants to merge 1 commit into
instructlab:mainfrom
jwm4:jwm4-improve-error-missing-db
Closed

fix(RAG): Better error handling for missing RAG db or model in chat#3048
jwm4 wants to merge 1 commit into
instructlab:mainfrom
jwm4:jwm4-improve-error-missing-db

Conversation

@jwm4

@jwm4 jwm4 commented Jan 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Issue resolved by this Pull Request:
Resolves #2973

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the
    conventional commits.
  • Changelog updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Functional tests have been added, if necessary.
  • E2E Workflow tests have been added, if necessary.

@jwm4
jwm4 requested a review from a team January 28, 2025 23:45
nathan-weinberg
nathan-weinberg reviewed Jan 29, 2025
View reviewed changes
Comment thread src/instructlab/model/chat.py Outdated
@jwm4

jwm4 commented Jan 29, 2025

Copy link
Copy Markdown
Contributor Author

Addressed review comment and I finally tracked down the correct issue to attach this PR too so I updated the description too.

RobotSail
RobotSail reviewed Jan 29, 2025
View reviewed changes
Comment thread src/instructlab/model/chat.py Outdated
RobotSail
RobotSail reviewed Jan 29, 2025
View reviewed changes
Comment thread src/instructlab/model/chat.py Outdated
RobotSail
RobotSail reviewed Jan 29, 2025
View reviewed changes
Comment thread src/instructlab/model/chat.py Outdated
@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 29, 2025
@jwm4

jwm4 commented Jan 29, 2025

Copy link
Copy Markdown
Contributor Author

@RobotSail Can you re-review? I believe I have addressed the main substance of your concern, but I wound up taking it a different direction than the one you had recommended.

@jwm4 jwm4 changed the title bug(RAG): Better error handling for missing RAG db in chat fix(RAG): Better error handling for missing RAG db in chat Jan 29, 2025
@jwm4 jwm4 added RAG RAG specific issues and removed RAG RAG specific issues labels Jan 30, 2025
anastasds
anastasds reviewed Jan 30, 2025
View reviewed changes
Comment thread src/instructlab/model/chat.py Outdated
@mergify mergify Bot added the ci-failure PR has at least one CI failure label Jan 30, 2025
@jwm4 jwm4 added the hold In-progress PR. Tag should be removed before merge. label Jan 30, 2025
@mergify mergify Bot removed the ci-failure PR has at least one CI failure label Jan 30, 2025
@jwm4

jwm4 commented Jan 30, 2025

Copy link
Copy Markdown
Contributor Author

Added hold tag so I can squash commits before this merges.

@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Jan 30, 2025
RobotSail
RobotSail approved these changes Jan 31, 2025
View reviewed changes

@RobotSail RobotSail left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mergify mergify Bot added the one-approval PR has one approval from a maintainer label Jan 31, 2025
dmartinol
dmartinol reviewed Feb 3, 2025
View reviewed changes
Comment thread src/instructlab/model/chat.py
dmartinol
dmartinol reviewed Feb 3, 2025
View reviewed changes
Comment thread src/instructlab/model/chat.py
@mergify mergify Bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Feb 3, 2025
@jwm4 jwm4 changed the title fix(RAG): Better error handling for missing RAG db in chat fix(RAG): Better error handling for missing RAG db or model in chat Feb 3, 2025
@jwm4
jwm4 force-pushed the jwm4-improve-error-missing-db branch from 36b54ff to 4f7fbfd Compare February 3, 2025 19:44
@mergify mergify Bot added CI/CD Affects CI/CD configuration container Affects containization aspects documentation Improvements or additions to documentation testing Relates to testing dependencies Relates to dependencies and removed ci-failure PR has at least one CI failure labels Feb 3, 2025
@jwm4 jwm4 removed the hold In-progress PR. Tag should be removed before merge. label Feb 3, 2025
@mergify mergify Bot added the ci-failure PR has at least one CI failure label Feb 3, 2025
@mergify

mergify Bot commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be merged. @jwm4 please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase This Pull Request needs to be rebased label Feb 4, 2025
@jwm4
fix(RAG): Better error handling for missing RAG db or model in chat
68a5af3 
Signed-off-by: Bill Murdock <bmurdock@redhat.com>
@jwm4
jwm4 force-pushed the jwm4-improve-error-missing-db branch from 4f7fbfd to 68a5af3 Compare February 6, 2025 16:18
@mergify mergify Bot removed needs-rebase This Pull Request needs to be rebased ci-failure PR has at least one CI failure labels Feb 6, 2025
@jwm4 jwm4 added the hold In-progress PR. Tag should be removed before merge. label Feb 6, 2025
@jwm4

jwm4 commented Feb 6, 2025

Copy link
Copy Markdown
Contributor Author

This PR has become severely entangled with merge conflicts due to multiple rounds of squashing and rebasing (and probably a fair amount of user error on my part). I am closing it for now. If we decide that it is a priority, I will start over with the updates to the chat code in a new PR on a new branch.

@jwm4 jwm4 closed this Feb 6, 2025
@mergify mergify Bot added the ci-failure PR has at least one CI failure label Feb 6, 2025
@jwm4
jwm4 deleted the jwm4-improve-error-missing-db branch February 12, 2025 13:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

4 more reviewers
@anastasds anastasds anastasds left review comments
@nathan-weinberg nathan-weinberg nathan-weinberg left review comments
@dmartinol dmartinol dmartinol left review comments
@RobotSail RobotSail RobotSail approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CI/CD Affects CI/CD configuration ci-failure PR has at least one CI failure container Affects containization aspects dependencies Relates to dependencies documentation Improvements or additions to documentation hold In-progress PR. Tag should be removed before merge. one-approval PR has one approval from a maintainer testing Relates to testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[RAG][Bug] Running ilab model chat --rag before ingesting anything causes a FileNotFoundError

5 participants

@jwm4 @anastasds @nathan-weinberg @dmartinol @RobotSail