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

[20.01] Prevent caching of bootstrapped client data #9294

Merged

Conversation

dannon
Copy link
Member

@dannon dannon commented Jan 31, 2020

Attempting to address the last bits of #7249

The javascript itself can/will still be cached, and that's the brunt of the data transmitted when filling out the app. The majority of this particular response is the bootstrapped data related primarily to current user that we don't really want to cache.

Copy link
Member

@mvdbeek mvdbeek left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @dannon !

@@ -276,6 +276,10 @@ def client(self, trans, **kwd):
return self._bootstrapped_client(trans, **kwd)

def _bootstrapped_client(self, trans, app_name='analysis', **kwd):
# This includes contextualized user options in the bootstrapped data; we don't want to cache it.
trans.response.headers['Cache-Control'] = ['no-cache', 'no-store', 'must-revalidate']
Copy link
Member

Choose a reason for hiding this comment

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

Might make sense to consolidate this in a decorator (in dev) ? We do this in a couple of places.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I actually started doing exactly that yesterday, but for a separate PR for dev

@mvdbeek mvdbeek added this to the 20.01 milestone Feb 1, 2020
@jmchilton jmchilton merged commit bd96d8d into galaxyproject:release_20.01 Feb 3, 2020
@nsoranzo
Copy link
Member

nsoranzo commented Feb 3, 2020

Merged forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants