Skip to content

[Caching] Ensure cache is always created#9020

Merged
dpgaspar merged 3 commits into
masterfrom
erik-ritter--always-create-cache
Jan 25, 2020
Merged

[Caching] Ensure cache is always created#9020
dpgaspar merged 3 commits into
masterfrom
erik-ritter--always-create-cache

Conversation

@etr2460
Copy link
Copy Markdown
Member

@etr2460 etr2460 commented Jan 25, 2020

CATEGORY

Choose one

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

SUMMARY

Flask-Caching supports passing in "null" as the cache type, which resolves to no caching at all (https://flask-caching.readthedocs.io/en/latest/#nullcache). This is preferred to setting the cache to None so that we can assume a cache is always initialized and calls to cache.memoize() will never fail.

TEST PLAN

CI

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: @john-bodley @craig-rueda @villebro @mistercrunch

@john-bodley john-bodley force-pushed the erik-ritter--always-create-cache branch from 302f3d2 to 1e18818 Compare January 25, 2020 02:22
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 25, 2020

Codecov Report

Merging #9020 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9020   +/-   ##
=======================================
  Coverage   59.15%   59.15%           
=======================================
  Files         367      367           
  Lines       11686    11686           
  Branches     2866     2866           
=======================================
  Hits         6913     6913           
  Misses       4594     4594           
  Partials      179      179

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c552c85...640dccb. Read the comment docs.

Comment thread superset/utils/cache_manager.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, this was confusing logic

@john-bodley john-bodley force-pushed the erik-ritter--always-create-cache branch from 1e18818 to 640dccb Compare January 25, 2020 06:31
Comment thread superset/config.py
SMTP_PASSWORD = "superset"
SMTP_MAIL_FROM = "superset@superset.com"

if not CACHE_DEFAULT_TIMEOUT:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This logic is in the same block as line #313 and always evaluates to True and thus isn't required (hence the deletion).

@dpgaspar dpgaspar mentioned this pull request Jan 25, 2020
12 tasks
@dpgaspar dpgaspar merged commit 68e85ab into master Jan 25, 2020
john-bodley pushed a commit to airbnb/superset-fork that referenced this pull request Jan 26, 2020
* [Caching] Ensure cache is always created

* Update cache_manager.py

* Refactoring cache typing

(cherry picked from commit 68e85ab)
@etr2460 etr2460 deleted the erik-ritter--always-create-cache branch July 1, 2020 15:15
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.36.0 First shipped in 0.36.0 labels Feb 28, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
* [Caching] Ensure cache is always created

* Update cache_manager.py

* Refactoring cache typing
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/XS 🚢 0.36.0 First shipped in 0.36.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants