Support serializable toolbar #1432

tim-schilling · 2021-01-03T23:28:04Z

In order to support Channels' non-django request possibilities, we need to start making the toolbar more flexible.In order to support Django Channels out of the box we need to support multiple processes running. This implies serializing the toolbar's data into some type of backend (database, memory, redis, etc). Currently the toolbar stores everything in memory as non-primitives.

My assumption is that we need to serialize the panel's template context into primitives so that it can be stored in any medium between requests and then rendered at some future time (or even stored elsewhere). I could be wrong here, so feel free to push back. I suspect this is going to change the toolbar's internals dramatically.

This should not be merged as it's not complete, but I wanted to get it out there for feedback and to solicit for ideas/alternative options. Currently this PR does the following:

Creates a global store for the toolbar and the panels' stats (debug_toolbar.store.MemoryStore)
Reworks all the existing panels to function with their context being serialized to JSON and back (this should be pulled out for the MemoryStore, but I'm doing it to prove that it does work).

Future work:

Create ~~LocMemCacheStore~~ DBCacheStore
Verify compatibility with Django Channels sync Django views
Determine what's needed to support async Django Channels views.

This relates to #1430 and I hope we can structure it in a way such that it also works for async views in Channels and #1413.

codecov · 2021-01-03T23:30:10Z

Codecov Report

Merging #1432 (3fb2745) into main (e20ac73) will decrease coverage by 0.80%.
The diff coverage is 75.94%.

@@            Coverage Diff             @@
##             main    #1432      +/-   ##
==========================================
- Coverage   85.73%   84.93%   -0.81%     
==========================================
  Files          35       38       +3     
  Lines        1893     2004     +111     
  Branches      273      280       +7     
==========================================
+ Hits         1623     1702      +79     
- Misses        190      219      +29     
- Partials       80       83       +3

Impacted Files	Coverage Δ
debug_toolbar/db_store.py	`0.00% <0.00%> (ø)`
debug_toolbar/panels/timer.py	`78.94% <0.00%> (-0.72%)`	⬇️
debug_toolbar/settings.py	`100.00% <ø> (ø)`
debug_toolbar/panels/templates/panel.py	`87.50% <60.00%> (+0.40%)`	⬆️
debug_toolbar/toolbar.py	`92.59% <85.71%> (+0.54%)`	⬆️
debug_toolbar/store.py	`87.09% <87.09%> (ø)`
debug_toolbar/panels/history/panel.py	`89.09% <87.50%> (-1.30%)`	⬇️
debug_toolbar/models.py	`100.00% <100.00%> (ø)`
debug_toolbar/panels/__init__.py	`96.00% <100.00%> (+0.61%)`	⬆️
debug_toolbar/panels/cache.py	`72.72% <100.00%> (+0.16%)`	⬆️
... and 11 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 e20ac73...3fb2745. Read the comment docs.

…lity.

In order to support the toolbar persisting stats, we need to move away from calling record_stats with non-primitives.

Removes storing the entire toolbar in the Store.

tim-schilling · 2022-04-19T14:25:36Z

@tapaswenipathak tagging you here. Feel free to create a new PR or continue working off of this one.

tapaswenipathak · 2022-06-26T04:31:50Z

debug_toolbar/panels/__init__.py

+
+        Provided to support future store mechanisms overriding a panel's content.
+        """
+        return stats


are these two methods complete?

deserialize_stats

serialize_stats

tapaswenipathak · 2022-06-26T04:37:07Z

debug_toolbar/store.py

+
+def deserialize(data):
+    return json.loads(data)
+


are these two methods complete?

serialize()

deserialize()

tapaswenipathak · 2022-06-26T04:39:06Z

debug_toolbar/store.py

+
+    @classmethod
+    def delete(cls, store_id):
+        raise NotImplementedError


should these four be implemented?

ids()

exists()

set()

delete()

Not here. BaseStore is the interface for the API. The subclasses of BaseStore will have different implementations.

tapaswenipathak · 2022-06-26T04:40:49Z

debug_toolbar/store.py

+        except KeyError:
+            data = None
+        return {} if data is None else deserialize(data)
+


are these two method definitions complete?

save_panel()

panel()

tapaswenipathak · 2022-06-26T04:42:49Z

debug_toolbar/templates/debug_toolbar/panels/history_tr.html

@@ -43,7 +43,7 @@
    </td>
    <td class="djdt-actions">
        <form method="get" action="{% url 'djdt:history_sidebar' %}">
-            {{ store_context.form }}
+            {{ history.form }}


store_context is functionally history after this PR?

No, this is likely due to the context for the template changing.

tapaswenipathak · 2022-06-26T04:50:05Z

debug_toolbar/toolbar.py

-    @classmethod
-    def fetch(cls, store_id):
-        return cls._store.get(store_id)
+        get_store().set(self.store_id)


now i would:

return import_string(dt_settings.get_config()["TOOLBAR_STORE_CLASS"])

are these edits/changes functional and complete?

The function store here is more the verb case of "store". What's happening is that we want to store this version of the toolbar into the storage mechanism. The naming could be improved.

tim-schilling · 2023-01-22T17:32:01Z

This is going to be replaced by the work associated with https://github.com/orgs/jazzband/projects/9/

@matthiask can you review those high level tasks and let me know what context I'm missing? When these parts are picked up, they should be converted to issues. Though perhaps I should do that now?

matthiask · 2023-01-23T08:12:25Z

@tim-schilling Thanks for starting this!

I think we should check if we can use the cache framework instead of writing our own store classes; we should document that you're supposed to use a cache which supports cross-process caching and maybe warn if users use e.g. the local memory or dummy cache backend.

I'm sure that the point could be made that this data doesn't belong in a cache but it would mean much less work for us and many many options immediately available for our users.

I also think Django Channels is nice to have, not essential -- Django itself comes with async views, middleware and ORM methods. I have used Channels in the past with success and I like it, but supporting Django's own async machinery is surely more important.

Re. the serialization framework: Do you think it would be feasible to enforce usage of these methods so that we do not have to support two code paths in the future? I think it would be great if we also added a pending deprecation warning to using current-style panels which do not use serialization at all.

(I haven't thought long and hard about these points so I'm not all that sure that we should do what I'm proposing.)

tim-schilling · 2023-02-04T16:25:17Z

I think we should check if we can use the cache framework instead of writing our own store classes; we should document that you're supposed to use a cache which supports cross-process caching and maybe warn if users use e.g. the local memory or dummy cache backend

@matthiask Good point. I'm not sure why I went into specific backends. In reality, it should have been MemoryStore, CacheStore and DatabaseStore. However, I'm happy to start with using the cache, then extending if folks want to.

I also think Django Channels is nice to have, not essential

Another good point. But let's reach for it still. Especially if we get help from GSoC.

Re. the serialization framework: Do you think it would be feasible to enforce usage of these methods so that we do not have to support two code paths in the future? I think it would be great if we also added a pending deprecation warning to using current-style panels which do not use serialization at all.

Yes, I think we'll drop the double code paths in the future. I don't have a timeline for you, but if you feel we should define one, I'll say 1 year from release we'll drop the deprecated path.

matthiask · 2023-02-04T16:36:38Z

Another good point. But let's reach for it still. Especially if we get help from GSoC.

I totally agree.

Yes, I think we'll drop the double code paths in the future. I don't have a timeline for you, but if you feel we should define one, I'll say 1 year from release we'll drop the deprecated path.

I don't think we need a timeline for this, but we should be intentional about it, and communicate clearly that we don't expect to keep both paths indefinitely.

Thanks!

tim-schilling · 2023-08-21T02:12:28Z

I'm closing this as this approach is largely being dropped. It can still be referred to for inspiration, but it won't be merged in.

Base automatically changed from master to main February 11, 2021 15:01

tim-schilling added 4 commits September 11, 2021 09:40

Create basic memory store for toolbar to reproduce existing functiona…

3cb0970

…lity.

Support storing serialized panel stats directly into the store.

9cf5bfd

In order to support the toolbar persisting stats, we need to move away from calling record_stats with non-primitives.

Support stats only version of toolbar.

54ff730

Removes storing the entire toolbar in the Store.

WIP

3fb2745

tim-schilling force-pushed the serializable-toolbar branch from 220373f to 3fb2745 Compare September 11, 2021 14:52

tapaswenipathak reviewed Jun 26, 2022

View reviewed changes

This was referenced Aug 29, 2022

Django Channels and SPA support #981

Closed

Allow to reload rendered panel separately #894

Closed

matthiask force-pushed the main branch from a5aaf50 to b543e67 Compare December 18, 2022 10:21

matthiask mentioned this pull request Apr 14, 2023

add Redis history #1763

Closed

2 tasks

tim-schilling mentioned this pull request May 17, 2023

Add the Store API and initial documentation. #1784

Merged

2 tasks

tim-schilling closed this Aug 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support serializable toolbar #1432

Support serializable toolbar #1432

tim-schilling commented Jan 3, 2021 •

edited

Loading

codecov bot commented Jan 3, 2021 •

edited

Loading

tim-schilling commented Apr 19, 2022

tapaswenipathak Jun 26, 2022

tapaswenipathak Jun 26, 2022

tim-schilling Jun 27, 2022

tapaswenipathak Jun 26, 2022

tim-schilling Jun 27, 2022

tapaswenipathak Jun 26, 2022

tim-schilling Jun 27, 2022

tapaswenipathak Jun 26, 2022

tim-schilling Jun 27, 2022

tapaswenipathak Jun 26, 2022

tim-schilling Jun 27, 2022

tim-schilling commented Jan 22, 2023

matthiask commented Jan 23, 2023

tim-schilling commented Feb 4, 2023

matthiask commented Feb 4, 2023

tim-schilling commented Aug 21, 2023

Support serializable toolbar #1432

Support serializable toolbar #1432

Conversation

tim-schilling commented Jan 3, 2021 • edited Loading

codecov bot commented Jan 3, 2021 • edited Loading

Codecov Report

tim-schilling commented Apr 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tim-schilling commented Jan 22, 2023

matthiask commented Jan 23, 2023

tim-schilling commented Feb 4, 2023

matthiask commented Feb 4, 2023

tim-schilling commented Aug 21, 2023

tim-schilling commented Jan 3, 2021 •

edited

Loading

codecov bot commented Jan 3, 2021 •

edited

Loading