Skip to content
This repository was archived by the owner on Nov 16, 2023. It is now read-only.

Caching design doc update#25

Merged
samiyaakhtar merged 10 commits intomicrosoft:masterfrom
samiyaakhtar:cache-design-doc-update
Apr 27, 2020
Merged

Caching design doc update#25
samiyaakhtar merged 10 commits intomicrosoft:masterfrom
samiyaakhtar:cache-design-doc-update

Conversation

@samiyaakhtar
Copy link
Contributor

Just updating the caching design doc with slight changes

  • To account for PR fetch APIs
  • Logic that determines whether a deployment is to be considered a changed deployment or not
  • Added simple flowchart

Will make a draft PR against Spektate shortly, to start getting feedback for this implementation.

Related to microsoft/bedrock#1318

@samiyaakhtar samiyaakhtar requested a review from dennisseah April 24, 2020 17:59
How to change the refresh duration
How to change the refresh duration: Check the deployment environment variables
passed in through the docker image or helm chart, and add a variable
`REACT_APP_CACHE_REFRESH_IN_SEC` set it to let's say `60` for a 60 seconds
Copy link
Contributor

Choose a reason for hiding this comment

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

Once this is added, this should also set the default refreshRate property in the frontend dashboard component state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evanlouie When the dashboard launches, should we apply this env setting by default to the refreshRate property in the component state?

Copy link
Collaborator

Choose a reason for hiding this comment

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

there are two refresh rate settings. One for backend and one for frontend. REACT_APP_CACHE_REFRESH_IN_SEC is for backend.

@andrebriggs andrebriggs marked this pull request as draft April 25, 2020 20:30
Copy link
Member

@andrebriggs andrebriggs left a comment

Choose a reason for hiding this comment

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

Pausing until other changes are complete

@andrebriggs andrebriggs self-requested a review April 26, 2020 02:03
@andrebriggs andrebriggs marked this pull request as ready for review April 26, 2020 02:03
3. Check if there are deleted deployed instances, if yes, remove them from the
cache.
4. one API call to get Manifest Repo Sync State.
4. Check if there are new or updated instances, and if yes, one API call to get
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@dennisseah dennisseah left a comment

Choose a reason for hiding this comment

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

LGTM. This doc can be set to version 1.0 :-)

@samiyaakhtar samiyaakhtar merged commit 67d3d0b into microsoft:master Apr 27, 2020
@samiyaakhtar samiyaakhtar deleted the cache-design-doc-update branch April 27, 2020 22:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants