Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added very first spike for NOT filtering #689

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

alkampfergit
Copy link
Contributor

This is NOT ready to be merged, because I've not updated all the Memories, but it is a possible implementation for NOT filter as for discussion #688.

We need to discuss if the interface for memory filter is ok (I've tried to maintain binary compatibility in the case someone serialized the structure somewhere). I've added a simple function to grab all the filters that are configured, you can see how you can use the helper in Redis Memory that was the only memory I've updated in this PR (if everything is good I'll proceed in implementing in other basic providers as well.

I've also modified the configuration for Redis test adding an optional setting that allows using Azure Openai for testing. Actually it is more common for organizations to have Azure OpenAI than Direct OpenAI

@alkampfergit alkampfergit requested a review from dluc as a code owner July 9, 2024 07:56
@alkampfergit alkampfergit marked this pull request as draft July 15, 2024 16:09
@alkampfergit
Copy link
Contributor Author

@dluc I've implemented for almost all provider, except elasticsearch. The elasticsesarch provider has some test that fails outside the scope of this PR, but it uses an approac to save tag into nested object where, if we have tags with multiple values, like multiple users, it saves multiple nested object with the same name but different value. This makes quite difficult to generate the query, I'll need probably more work.

In the meanwhile you can validate the overall PR, after all elasticsearch is the only provider missing up to now.

@alkampfergit alkampfergit force-pushed the feature/not_filter branch 3 times, most recently from ae85a29 to 888345f Compare July 16, 2024 07:42
@alkampfergit alkampfergit force-pushed the feature/not_filter branch 3 times, most recently from 0d86bda to 49dad5b Compare August 2, 2024 15:21
@alkampfergit alkampfergit marked this pull request as ready for review August 2, 2024 15:22
@alkampfergit
Copy link
Contributor Author

@dluc if you want this PR is ready for review.

@dluc
Copy link
Collaborator

dluc commented Nov 19, 2024

hi @alkampfergit would you have time to merge the latest changes from main? I was trying to fix SqlServerMemory conflicts but I'm not 100% confident about my changes. Please let me know if there's anything I can do to help. This feature would be really useful, hope we can find a way to complete it.

@alkampfergit
Copy link
Contributor Author

hi @alkampfergit would you have time to merge the latest changes from main? I was trying to fix SqlServerMemory conflicts but I'm not 100% confident about my changes. Please let me know if there's anything I can do to help. This feature would be really useful, hope we can find a way to complete it.

I'll retry in the weekend, I've tried this morning but I saw that the implementation of SQL memory changed, so I carefully need to understand what is really changed and replay my modifications.

I'll keep you informed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants