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

[caching] Using request context rather than globals #9715

Merged

Conversation

john-bodley
Copy link
Member

CATEGORY

Choose one

  • Bug Fix
  • Enhancement (new features, refinement)
  • Refactor
  • Add tests
  • Build / Development Environment
  • Documentation

SUMMARY

@villebro I was thinking some more about the cache warmup and the form-data problem, and for testing we were leveraging test_request_context which creates a RequestContext and I wondered if using that was cleaner than temporarily leveraging g.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

Tested locally.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

REVIEWERS

to: @villebro

@john-bodley
Copy link
Member Author

@villebro what are your thoughts regarding this approach?

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

I think this is a cleaner approach. Would be nice if there was a linter that made it possible to blacklist flask.g and others we want to avoid in the future (or is there one?)

@john-bodley
Copy link
Member Author

@villebro there are times we want to use flask.g, i.e., for storing the current user so I don't think it's a pattern we always wish to strictly avoid.

@john-bodley john-bodley merged commit 04e3a4a into apache:master May 11, 2020
@john-bodley john-bodley deleted the john-bodley--warm-up-cache-with-context branch May 11, 2020 22:55
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.37.0 labels Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 0.37.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants