Oraclevs integration #28464
Oraclevs integration #28464
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
dosubot
bot
added
size:L
|
I had a lint failure that I have fixed. @baskaryan, @efriis. Everything passes now; please take a look and approve this PR. |
There was a problem hiding this comment.
hey there! This is a difficult PR to review because it modifies a lot of lines unnecessarily. Could you resubmit as a minimal change?
I believe there is also some work at oracle to launch a langchain-oracle integration package, so might make sense to collaborate with that team to do so, such that PRs like this can be reviewed by oracle folks instead of us!
| @@ -1,3 +1,4 @@ | |||
| # OopCompanion:suppressRename | |||
There was a problem hiding this comment.
Hi @efriis: Thanks for the feedback. I will remove the offending line and resubmit.
You see a lot of changes because in this version, a client can be a connection or a database connection pool. We have thus changed what used to be a connection to a client that is dynamically resolved based on whether a connection or connection pool is being passed. This rename has resulted in a no. of trivial changes which is contributing to the diffs. In addition, the transaction also supports DSL, which is passed through kwargs that can act as a pre/in-filter based on the way the SQL optimizer costs the query. Finally, to the best of my knowledge, I don't know of another Oracle team - we work for the Oracle database group and are responsible for the langchain integration. I will try to inquire and find out, though. Hope this helps.
There was a problem hiding this comment.
great thanks! You're also welcome to publish as separate packages. Here's the docs if you wanted to pursue separately: https://python.langchain.com/docs/contributing/how_to/integrations/
I just emailed Kohei to see if they can reach out to you!
There was a problem hiding this comment.
Hi @efriis: I have made the change to oraclevs_integration of the offending line and pushed it to remote. Kindly review.
There was a problem hiding this comment.
@efriis - Thanks.
|
still not a minimal change - going to mark as needs support, and can review when that's through! in general would recommend doing refactors like renaming client -> connection as a separate PR because it makes the PR unnecessarily long to review |
|
going to close for now due to the potential for vulnerabilities getting passed in those kwargs, and would highly recommend publishing a separate https://python.langchain.com/docs/contributing/how_to/integrations/ |
|
Ok. I will bifurcate the transaction to make it easier for you to review. |
Thank you for contributing to LangChain!
This change adds support for the Oracle Connection pool. In addition, it also adds support for dsl where a dsl is specified as follows:
db_filter = {
"_and": [
{"key": "id", "oper": "EQ", "value": "101"},
{
"_or": [
{"key": "status", "oper": "EQ", "value": "approved" },
{"key": "link", "oper": "EQ", "value": "Document Example Test 2"},
{
"_and": [
{"key": "status", "oper": "EQ", "value": "approved"},
{"key": "link", "oper": "EQ", "value": "Document Example Test 2"}
]
}
]
}
]
}
This can be used in calls such as similarity_search_with_score, where, along with other arguments, one can specify db_filter = db_filter.
@baskaryan, @efriis - can you please approve this PR.