-
Notifications
You must be signed in to change notification settings - Fork 949
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
[phrasePlugin][Stale Private Key]: #3755
[phrasePlugin][Stale Private Key]: #3755
Conversation
|
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.
LGTM
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.
Do add connected Jira ticket in the PR title and description
@@ -34,11 +34,12 @@ export class Phrase { | |||
} | |||
|
|||
async request(path: string, config?: RequestInit, search = {}) { | |||
let privateKey = await appState.globalState.getPluginPrivateKey(pkg.name); |
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 would be expensive, since it would hit firestore everytime instead of once per load. I think that's the reason why it is cached.
Could we look at how many times this URL is called daily to understand impact?
Co-authored-by: Sanyam Kamat <[email protected]>
@sanyamkamat since Jira is our internal project and this is open source, I thought we were not adding jira ids here |
…v2023/builder into ENG-7441-phrase-private-key-issue
Description
The Private key in phrase plugin gets updated only in constructor i.e. when the plugin loads. This happens on initial set-up or when we refresh builder web-app. This stale private key causes authorization failure when we switch b/w environments.
The solution is a small fix which gets the latest private key everytime before making api-call.
Loom showing before after
https://www.loom.com/share/63c07935720749b7800aaf422b771ae4