Leverage the autodiscovery indexID feature#44
Conversation
Akrog
left a comment
Akrog
left a comment
There was a problem hiding this comment.
Please mention how the autodiscovery mechanism works (implicit directory for the DB in the container image):
- In the commit message
- In the code when creating the OLSConfig, explaining why we don't explicitly set thee index ID.
This patch leverage the autodiscovery indexID feature from OLS. Prior to this patch, our code started a temporary container to extract the indexID value from an env variable, this is no longer needed for OLS and the code can be removed. Now, OLS levarages the rag of the RAG images as the indexID for the vector database. Since our images already matches the tag with the IndexID that was used during build time, we no longer need to set it in OLSConfig. Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
That makes sense. Thank @Akrog, I added a comment in the code and also updated the commit message with that information. |
lpiwowar
left a comment
lpiwowar
left a comment
There was a problem hiding this comment.
LGTM 👍, just one comment.
Since we no longer are required to start a pod to extract the indexID, these RBAC permissions for jobs and pods can be removed. Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
|
/hold testing new rbac permissions |
|
/hold cancel Tested locally with the new RBAC permissions removed and everything worked as expected: [lucas@giant28 devsetup]$ oc get pods -n openshift-lightspeed |
Akrog
left a comment
Akrog
left a comment
There was a problem hiding this comment.
Looks good to me, a small nit though in the commit message there's a typo: levarages the rag of the RAG images says rag where it should be tag.
The comment in the code is correct, so I'm ok to merge it.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Akrog, umago The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit f5efbc1
into
openstack-lightspeed:main
3 participants
This patch leverage the autodiscovery indexID feature from OLS.
Prior to this patch, our code started a temporary container to extract the indexID value from an env variable, this is no longer needed for OLS and the code can be removed.