Implementing the faiss vector store skill#40
Conversation
| @@ -0,0 +1,9 @@ | |||
| 2025-07-25 14:27:10,048 - FaissVectorStoreSkill - INFO - Running FaissVectorStoreSkill... | |||
There was a problem hiding this comment.
The log file/folder should not be part of the repository
There was a problem hiding this comment.
should I add the log file/folder to .gitignore ?
| @@ -0,0 +1,3368 @@ | |||
| 2025-07-09 16:56:07,000 - ScrollWorldExporterSkill - INFO - Running ScrollWorldExporter | |||
There was a problem hiding this comment.
Local execution logs should not be part of the repository
There was a problem hiding this comment.
should I add the logs to .gitignore ?
There was a problem hiding this comment.
This file should not be part of the repository
There was a problem hiding this comment.
Should I remove it or add it to the .gitignore ?
There was a problem hiding this comment.
yes, you can add to .gitignore, but be specific with the log file name
|
|
||
| def test_faiss_vector_store_skill() -> None: | ||
| db_path = Path("..\\..\\Documents\\docs2vec_ressources\\faissdb_test\\my_index_test.faiss") | ||
| files_path = Path("..\\..\\Documents\\MS teams credentials IT change mana.txt") |
There was a problem hiding this comment.
This variable is used only for setting the source_link property, so it needs to be a string, not a Path.
There was a problem hiding this comment.
I will update the db_path to string. Thank you Dorin.
| from pathlib import Path | ||
|
|
||
| def test_faiss_vector_store_skill() -> None: | ||
| db_path = Path("..\\..\\Documents\\docs2vec_ressources\\faissdb_test\\my_index_test.faiss") |
There was a problem hiding this comment.
As per my previous comment, please don't use local hardcoded paths. These unit tests will run in a GitHub runner, so, the path you set here will not be present. You should use local relative paths instead.
There was a problem hiding this comment.
My apologies, I understood that the path should be relative, if my understanding is correct now, I should add the path to the repository, so it is accessible within the repository, Am I right?
|
|
||
|
|
||
| def dummy_embedding_function(text: str) -> list[float]: | ||
| # This is a dummy implementation. Replace it with your actual embedding logic. |
There was a problem hiding this comment.
Please rephrase or remove this comment :)
There was a problem hiding this comment.
Yes I will update it.
| - package-ecosystem: "github-actions" | ||
| directory: "/" | ||
| schedule: | ||
| interval: "weekly" |
There was a problem hiding this comment.
My bad, these lines were not intended to be added into my code I will delete them.
|
Thank you Dorin, for the review, I will try fix the issues and submit a new Pull request. |
Driss-Lalami99
force-pushed
the
my-dev-branch
branch
5 times, most recently
from
73b47de to
f47650e
Compare
Driss-Lalami99
force-pushed
the
my-dev-branch
branch
from
f47650e to
e8c8099
Compare
Driss-Lalami99
force-pushed
the
my-dev-branch
branch
from
e8c8099 to
11e8361
Compare
|
I will close the old pull request and send a new one with the same changes . |
2 participants
Implementing the faiss vector store as an alternative to chromadb vector store