-
Notifications
You must be signed in to change notification settings - Fork 0
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: implement cache on getPrompt and getPromptById methods #86
base: main
Are you sure you want to change the base?
Conversation
2a95ead
to
15c1428
Compare
f04386c
to
9625e9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks that's great, I'm being a bit cautious with adding core layers such as caching as we will need to reuse and maintain them.
src/api.ts
Outdated
if (id) { | ||
return id; | ||
} else if (name && version) { | ||
} else if (name && (version || version === 0)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is actually a way to do that explicitly:
} else if (name && (version || version === 0)) { | |
} else if (name && typeof version === "number") { |
src/api.ts
Outdated
id?: string, | ||
name?: string, | ||
version?: number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When handling only optional params in js it's recommended to use an object because it's not great ux to do:
instance.getPromptCacheKey(undefined, undefined, version)
// vs
instance.getPromptCacheKey({ version })
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now I understand why auto-complete suggested a object thx
src/api.ts
Outdated
return SharedCache.instance; | ||
} | ||
|
||
public getPromptCacheKey( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that the cache is not prompt only, let's not leak that business logic in the cache layer.
src/api.ts
Outdated
return this.get(key); | ||
} | ||
|
||
public putPrompt(prompt: Prompt): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
src/api.ts
Outdated
@@ -325,6 +325,63 @@ type CreateAttachmentParams = { | |||
metadata?: Maybe<Record<string, any>>; | |||
}; | |||
|
|||
export class SharedCache { | |||
private static instance: SharedCache | null = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's simplify even more:
- have a
sharedCache.ts
file - and apply the singleton pattern https://www.patterns.dev/vanilla/singleton-pattern/
src/api.ts
Outdated
@@ -340,6 +397,8 @@ type CreateAttachmentParams = { | |||
* Then you can use the `api` object to make calls to the Literal service. | |||
*/ | |||
export class API { | |||
/** @ignore */ | |||
public cache: SharedCache; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be internal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used it to simplify my tests implementation and forgot to change it later
2cae4a1
to
856baf6
Compare
856baf6
to
f1902e7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there, 1 blocking and 2 nits.
I'm going to wait for the PR on the |
Add Prompt Caching Functionality
Changes
SharedCachePrompt
Implementation Details
Cache keys are generated in three formats:
id
name
name:version
Caching logic implemented on both
getPrompt
andgetPromptById
On each successful get prompt we will cache the prompt with keys
id
,name
,name:version
Added fallback to cached prompts when API calls fail, with warning logs
To keep in mind this cache will be cleared each time the application Is restarted