[DRAFT] fix: Change url in cache-warmup celery task

[duynguyenhoang]

SUMMARY

Currently, there is cache-warmup task allows us to warm up dashboards/slices. But this feature is not working at all.
There is also available endpoint to warmup cache, this pull request will use that endpoint.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

• Add cache task: 'cache-warmup' to CELERYBEAT_SCHEDULE
• Very that the selected slice in dashboard is refreshed after every warmup

Example Beat schedule config

CELERYBEAT_SCHEDULE = {
	'cache-warmup-3-minute': {
		'task': 'cache-warmup',
		'schedule': crontab(minute='*/3'),  # Every 3 minutes for testing
		'kwargs': {
			'strategy_name': 'top_n_dashboards',
			'top_n': 1,
			'since': '7 days ago',
		},
	},
}

ADDITIONAL INFORMATION

• Has associated issue:
  • superset cache warmup not working #10150,
  • Cache warm-ups never succeed #9597
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

Merging #15713 (1814f11) into master (02032ee) will decrease coverage by 0.25%.
The diff coverage is 42.42%.

@@            Coverage Diff             @@
##           master   #15713      +/-   ##
==========================================
- Coverage   76.94%   76.68%   -0.26%     
==========================================
  Files         983      983              
  Lines       51700    51736      +36     
  Branches     6983     6983              
==========================================
- Hits        39780    39676     -104     
- Misses      11696    11836     +140     
  Partials      224      224              

Flag	Coverage Δ
hive	?
mysql	81.49% <42.42%> (-0.06%)	⬇️
postgres	81.52% <42.42%> (-0.06%)	⬇️
presto	?
python	81.61% <42.42%> (-0.51%)	⬇️
sqlite	81.13% <42.42%> (-0.06%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/config.py	91.21% <ø> (+0.05%)	⬆️
superset/utils/webdriver.py	68.47% <31.57%> (-10.19%)	⬇️
superset/tasks/cache.py	72.03% <57.14%> (-1.88%)	⬇️
superset/db_engines/hive.py	0.00% <0.00%> (-82.15%)	⬇️
superset/db_engine_specs/hive.py	69.44% <0.00%> (-17.07%)	⬇️
superset/db_engine_specs/presto.py	83.36% <0.00%> (-6.95%)	⬇️
superset/views/database/mixins.py	81.03% <0.00%> (-1.73%)	⬇️
superset/connectors/sqla/models.py	88.26% <0.00%> (-1.65%)	⬇️
superset/utils/core.py	88.12% <0.00%> (-0.78%)	⬇️
superset/db_engine_specs/base.py	88.28% <0.00%> (-0.40%)	⬇️
... and 6 more

---

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 02032ee...1814f11. Read the comment docs.

[nytai]

@etr2460 @john-bodley @graceguo-supercat I believe y'all make use of the cache warmup feature, do these changes make sense?

[nytai]

Can we add an example (can be commented out) to the default Celery beat config in config.py so other users know how to enable cache warmup scheduling?

[graceguo-supercat]

@nytai thanks for the alert :)
This PR updates cache_warmup celery tasks, while in airbnb we used airflow job and call superset/warm_up_cache/?slice_id= directly. I don't think this change will impact airbnb, @betodealmeida is the best person for this feature.

[duynguyenhoang]

Having more test and I encounter that superset/warm_up_cache/?slice_id= doesn't work as expected. The root cause is the way cache keys are build. They are different in places.

[low-on-mana]

@duynguyenhoang were you able to get this to work ?
Also does this solve the issue with authentication.

[duynguyenhoang]

Hi @t0il3ts0ap,
It does fix issue with authentication, but for some chart types it doesn't work correct. For new charts using new API, they use different way to build cache key and why the current API doesn't work.

[b1de0]

Any updates?

[duynguyenhoang]

Hi @b1de0 unfortunately I am quite busy recently. I couldn't make it work. The cache API has been changed alot after version 1.0. I need to spend more time to understand the new implementation in order to fix this issue properly.

[villebro]

For charts that are using the new chart data endpoint the cache key should be calculated based on the query-context payload in the slice metadata (this is populated automatically when the chart is saved). In the long term we will probably introduce a Node based "sidecar" for generating the query context payload on the fly (they have to be rendered in JavaScript by the buildQuery callback in the plugin, hence can't be generated by the Python-based backend), but this will probably not happen in the near future.

@duynguyenhoang let me know if you have time to work on this and need help.

[duynguyenhoang]

@villebro thank you for your input, I might have some free time next week. I will check your suggestion and will ask you if I have any further questions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

[musicmuthu]

this issue not solved yet #bug

[musicmuthu]

Hi @duynguyenhoang this cache refresh is not working in 2.0 also can you please reopen this?

[rajniyadavashish]

Its working in 3.0.2 version but not in 3.1.0