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

feat: cleanup mechanism for conversation history #369

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

Conversation

tanweersalah
Copy link
Contributor

Description

Changes proposed in this pull request:

  • Added TTL for each data point

Related issue(s)

#283

Copy link

github-actions bot commented Feb 6, 2025

Note(s) for PR Auther:

  • The integration test will be skipped for the PR. You can trigger it manually after adding the label: run-integration-test.
  • The evaluation test will be skipped for the PR. You can trigger it manually after adding the label: evaluation requested.
  • If any changes are made to the evaluation tests data, make sure that the integration tests are working as expected.
  • If any changes are made to how to run the unit tests, make sure to update the steps for unit-tests in the create-release.yml workflow as well.

Note(s) for PR Reviewer(s):

  • Make sure that the integration and evaluation tests are working as expected.

@tanweersalah tanweersalah marked this pull request as ready for review February 6, 2025 13:18
@tanweersalah tanweersalah requested a review from a team as a code owner February 6, 2025 13:18
@tanweersalah tanweersalah requested a review from muralov February 6, 2025 13:18
@tanweersalah tanweersalah linked an issue Feb 6, 2025 that may be closed by this pull request
1 task
@tanweersalah tanweersalah removed the request for review from friedrichwilken February 6, 2025 13:31
Copy link
Collaborator

@muralov muralov left a comment

Choose a reason for hiding this comment

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

good job, it looks good.

Can you please add integration tests? You can use fakeredis for that.
You can just assert in the test if a key is deleted after certain seconds, by faking REDIS_TTL

@muralov muralov added run-integration-test evaluation requested A PR with this label will trigger the validation workflow. labels Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes evaluation requested A PR with this label will trigger the validation workflow. run-integration-test size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Develop cleanup mechanism for conversation history
3 participants