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

fix: remove cache when getProjectById #3067

Merged
merged 4 commits into from
May 15, 2020
Merged

fix: remove cache when getProjectById #3067

merged 4 commits into from
May 15, 2020

Conversation

zhixzhan
Copy link
Contributor

@zhixzhan zhixzhan commented May 15, 2020

Description

Remove use cache at getProjectById introduced by #2786 to prevent high frequency server index.
nowadays, benefit from LeiLei's work, client do not fetch project at every route change, so there is no need to use cache at getProjectById anymore.

When use want a fresh from server, ctrl-f refresh page would work as expected.

Appends:
Also fixed a skill validating ui.

Task Item

close #3068

Screenshots

Untitled1

skill url validating spinner
Untitled1

@github-actions
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling 035743d on zhixzhan/cache-fix into 6cd92ee on master.

Copy link
Contributor

@boydc2014 boydc2014 left a comment

Choose a reason for hiding this comment

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

LGTM cc @cwhitten

@boydc2014 boydc2014 changed the title fix: cache at getProjectById fix: remove cache when getProjectById May 15, 2020
@boydc2014 boydc2014 added the Approved to merge approved, waiting to be merged label May 15, 2020
@cwhitten cwhitten merged commit f11a464 into master May 15, 2020
@cwhitten cwhitten deleted the zhixzhan/cache-fix branch May 15, 2020 15:24
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
* do not use cache at getProjectById

* change skill manifest to warning

* code style

Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved to merge approved, waiting to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants