feat: Add etag caching to dashboard APIs #14357
Conversation
superset/dashboards/dao.py
Outdated
| @@ -54,6 +55,21 @@ def get_datasets_for_dashboard(id_or_slug: str) -> List[Any]: | |||
| def get_charts_for_dashboard(id_or_slug: str) -> List[Slice]: | |||
| return DashboardDAO.get_by_id_or_slug(id_or_slug).slices | |||
|
|
|||
| @staticmethod | |||
| def get_dashboard_changed_on(id_or_slug: str) -> datetime: | |||
There was a problem hiding this comment.
I believe this could also be affected by datasets changed_on, but adding that would make this more expensive to check. The slices changed_on check could also be skipped for /dashboard/:id_or_slug. Maybe it would be better to have a separate method per endpoint?
Codecov Report
@@ Coverage Diff @@
## master #14357 +/- ##
==========================================
+ Coverage 76.70% 76.88% +0.17%
==========================================
Files 955 955
Lines 48256 48664 +408
Branches 6035 6035
==========================================
+ Hits 37015 37415 +400
- Misses 11046 11054 +8
Partials 195 195
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
etr2460
force-pushed
the
erik-ritter--dashboard-etag-caching
branch
7 times, most recently
from
4e6b5d2
to
8186a35
Compare
| max_age=0, | ||
| raise_for_access=lambda _self, id_or_slug: DashboardDAO.get_by_id_or_slug( | ||
| id_or_slug | ||
| ), |
There was a problem hiding this comment.
So it seems DashboardDAO.get_by_id_or_slug will be called at least 3 times on first visit (no etag). I'm starting to wonder whether it's worth it to move everything out of the view handler. Maybe some direct function calls would be both more performant and easier to understand.
There was a problem hiding this comment.
Yeah, i noticed that too. The db call should be very fast since it's indexed, so i didn't think it was too big of a deal. Maybe it's worth caching for the request lifespan though. Honestly, I wonder if sqlalchemy could do something like that for us
etr2460
force-pushed
the
erik-ritter--dashboard-etag-caching
branch
from
8186a35
to
7113a5c
Compare
etr2460
force-pushed
the
erik-ritter--dashboard-etag-caching
branch
from
7113a5c
to
091f5a6
Compare
|
:'( why'd you close this @mistercrunch ? I assume a misclick or a rogue script :P |
* master: (38 commits) refactor(native-filters): allow cascading only for filter_select (apache#14441) test(maximize-chart): Add tests to maximize chart action (apache#14371) fix: fixing mysql error message (apache#14416) feat: Logic added to limiting factor column in Query model (apache#13521) change relationship (apache#14435) fix: bootstrap data permissions (apache#14348) fix: parse simple string error message values (apache#14360) chore: add stack trace to all calls of logger.error (apache#14382) update README with new docs and recordings (apache#14432) Renamed impyla from implya in impala.mdx and Renamed PIP package impyla from impala in index.mdx (apache#14425) fix(native-filters): fix filter scope error (apache#14426) feat: Adding limiting_factor column to Query model (apache#14234) feat: Add etag caching to dashboard APIs (apache#14357) chore: Moves Card to the components folder (apache#14139) feat: Dynamic imports for the Icons component (apache#14318) feat: Support env vars configuration for WebSocket server (apache#14398) fix: SQLLab role permissions (apache#14372) fix(native-filters): always show filters without dataset (apache#14409) fix error getting partitionQuery from table.partition (apache#14369) refactor: Boostrap to AntD - Tabs (apache#14048) ...
mistercrunch
added
🏷️ bot
SUMMARY
Reimplementation of #10963 and #11137 now that APIs aren't user specific
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Non etag cached:
Etag cached:
TEST PLAN
Unit tests, validate that the proper headers are set on responses, ensure that non-first loads of dashboards load faster, and that dashboard loads after edits load slower and with the fresh data
ADDITIONAL INFORMATION
to: @ktmud @suddjian @graceguo-supercat