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

[Spike] Use Flask-Caching for shared caching - prototype 1 #116

Closed
wants to merge 13 commits into from

Conversation

lingyielia
Copy link
Contributor

@lingyielia lingyielia commented Oct 17, 2023

Description

  • Allow no caching as default
  • Allow user to use other caching options when launching the dashboard
  • Caching is enabled on original data
  • Allow workers to share caching objects
  • a simple example on how to remove certain cache objects using custom actions @petar-qb

New features

  • a simple example on how to clear the entire cache @petar-qb
  • when caching is enabled, remove the original data if the same key exists in lazy data

Test cases:

  • test with redis
    to run with redis:
brew install redis
redis-server

then use Vizro._cache_config = {"CACHE_TYPE": "RedisCache", "CACHE_REDIS_URL": "redis://localhost:6379/0", "CACHE_DEFAULT_TIMEOUT": 3000,}

  • test with active loading vs. lazy loading
figure=px.box(
                df_gapminder,  # active loading
                x="continent",
                y="lifeExp",
                color="continent",
                title="Distribution per continent",
            ),

figure=px.box(
                "df_gapminder",  # lazy loading
                x="continent",
                y="lifeExp",
                color="continent",
                title="Distribution per continent",
            ),
  • test with gunicorn
    to run with gunicorn:
hatch shell
cd vizro/vizro-core/examples/default
gunicorn app:server -b 0.0.0.0:8000 -w 3 --preload

TODOs:

  • fix failing unit tests

Screenshot

Checklist

  • I have not referenced individuals, products or companies in any commits, directly or indirectly
  • I have not added data or restricted code in any commits, directly or indirectly
  • I have updated the docstring of any public function/class/model changed
  • I have added the PR number to the change description in the changelog fragment, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1)) (if applicable)
  • I have added tests to cover my changes (if applicable)

Types of changes

  • Docs/refactoring (non-breaking change which improves codebase)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

    • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
    • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
    • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

Copy link
Contributor

@antonymilne antonymilne left a comment

Choose a reason for hiding this comment

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

Looks really good I think 👍

I've thought some more about exactly what/where the user should set stuff and made some suggestions to swap things round a bit - would be great if you could give it a quick try and hopefully it still works fine. Other than that it would be nice to tidy up the data manager up a bit in preparation to discuss with @petar-qb tomorrow by removing the commented out code 🙂

vizro-core/hatch.toml Show resolved Hide resolved
vizro-core/src/vizro/_vizro.py Outdated Show resolved Hide resolved
@@ -21,12 +25,15 @@ class DataManager:

"""

_cache = Cache(config={"CACHE_TYPE": "NullCache"})
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to myself to think about whether we want NullCache or SimpleCache as default and whether we want some sort of development vs. production mode that switches cache automatically or gives a hint...

@@ -44,10 +44,13 @@ def build(self, dashboard: Dashboard):
Returns:
Vizro: App object
"""
data_manager._cache.init_app(self.dash.server, config=data_manager._cache.config)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
data_manager._cache.init_app(self.dash.server, config=data_manager._cache.config)
data_manager._cache.init_app(self.dash.server)

I guess that doesn't work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove config=data_manager._cache.config from here, as it's duplicate

component_data = self._get_original_data(dataset_name)

# clean up original data if the cache type is not NullCache
self._clean_original_data()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we want this here and called again in the Vizro.build? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch! I now dropped the one in Vizro.build as it's duplicate. We need to put it here so it can prune the original data in two cases:

  1. when the dashboard first build
  2. when the cache is invalid, and a reload happens

@antonymilne antonymilne changed the title [Spike] using Flask-Caching for shared caching [Spike] using Flask-Caching for shared caching - prototype 1 Oct 26, 2023
@huong-li-nguyen huong-li-nguyen changed the title [Spike] using Flask-Caching for shared caching - prototype 1 [Spike] Use Flask-Caching for shared caching - prototype 1 Feb 23, 2024
@antonymilne
Copy link
Contributor

Superseded by #398.

@antonymilne antonymilne closed this Apr 3, 2024
@huong-li-nguyen huong-li-nguyen deleted the dev/flask_caching branch April 12, 2024 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants