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.

Sign up for GitHub

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

Jump to bottom

Add Caching to Thread summaries #8717

Merged
merged 1 commit into from
Aug 12, 2023
Merged

Add Caching to Thread summaries #8717

merged 1 commit into from
Aug 12, 2023

Conversation

hamza221
Copy link
Contributor

Ref #8508

@hamza221 hamza221 added enhancement 3. to review skill:backend Issues and PRs that require backend development skills labels Aug 10, 2023
ChristophWurst
ChristophWurst approved these changes Aug 11, 2023
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Bonus points for unit tests that verify that thread summaries are not fetched from the text APIs if cached

lib/Service/AiIntegrations/Cache.php Show resolved Hide resolved
lib/Service/AiIntegrations/Cache.php Outdated Show resolved Hide resolved
@hamza221
Copy link
Contributor Author

hamza221 commented Aug 11, 2023 •
edited
Loading

I'll add the test to #8716 cause it makes sense to add it the service's Tests.
so let's get this merged first?

@ChristophWurst ChristophWurst merged commit 02c21cb into main Aug 12, 2023
27 of 28 checks passed
@ChristophWurst ChristophWurst deleted the enh/caching-thread-summary branch August 12, 2023 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@kesselb kesselb Awaiting requested review from kesselb kesselb is a code owner

Assignees
No one assigned
Labels
3. to review enhancement skill:backend Issues and PRs that require backend development skills
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hamza221 @ChristophWurst