Serialize the panels #1826
Closed
Serialize the panels #1826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This matches the new naming defined in the store module and will make things eaiser to change moving forward. This will break anything using the store internally causing issues for third party packages.
The remainder of the work is to fix the individual panels' serialization errors.
This avoids needing to create an instance of a panel to get its panel ID.
tim-schilling
force-pushed
the
serialize-panels
branch
from
c728270 to
14a5e0c
Compare
|
I think this is at a point where we can add some async tests to confirm that this approach will work or not. |
matthiask
reviewed
Aug 21, 2023
There was a problem hiding this comment.
Thanks!
- I like the change from store IDs to request IDs.
- It's nice that we no longer accept SQL strings from the client and execute them (even with signing)
I'm not totally sure, is this (somewhat) backwards compatible for third party panels? This wouldn't be an argument against it, just something to clarify.
While it doesn't break any documented APIs for the panel to function itself, this version is not backwards compatible since it loads the content for the panel from the store ( |
|
@matthiask I'm back to forcing everything that can't be serialized to JSON to a str. Otherwise we're trying to do it manually per panel which will involve a function that's likely going to do similar object and collection iteration as |
The alternative here is to inspect and iterate over every collection and object passed around. This avoids having to reinvent the wheel in that scenario.
* Add note on lack of async support to Installation docs * Add note on lack of async support to the Readme #1828 (comment)
updates: - [github.com/adamchainz/django-upgrade: 1.14.0 → 1.14.1](adamchainz/django-upgrade@1.14.0...1.14.1) - [github.com/pre-commit/mirrors-prettier: v3.0.1 → v3.0.2](pre-commit/mirrors-prettier@v3.0.1...v3.0.2) - [github.com/pre-commit/mirrors-eslint: v8.47.0 → v8.48.0](pre-commit/mirrors-eslint@v8.47.0...v8.48.0) - [github.com/astral-sh/ruff-pre-commit: v0.0.284 → v0.0.286](astral-sh/ruff-pre-commit@v0.0.284...v0.0.286) - [github.com/tox-dev/pyproject-fmt: 0.13.1 → 1.1.0](tox-dev/pyproject-fmt@0.13.1...1.1.0) - [github.com/abravalheri/validate-pyproject: v0.13 → v0.14](abravalheri/validate-pyproject@v0.13...v0.14)
Any instance attributes shouldn't be used because they can't be relied upon for historical purposes. Especially when it comes to the titles and nav titles.
|
This assumes we'll have some shared memory store for the cache hence the change to |
* panels(templates): postpone context processing - this makes it show evaluated querysets which were used in the template - removes completely context processing when SHOW_TEMPLATE_CONTEXT is disabled * panels(templates): avoid evaluating LazyObject LazyObject is typically used for something expensive to evaluate, so avoid evaluating it just for showing it in the debug toolbar.
* Add Python 3.12 to the test matrix. * Allow pre-releases for setup actions with GitHub actions. * Skip unnecessary test for python 3.12 * Update docs/changes.rst Co-authored-by: Matthias Kestenholz <[email protected]> --------- Co-authored-by: Matthias Kestenholz <[email protected]>
Only when the user is customizing both the show toolbar callback setting and the URLs aren't installed will the underlying NoReverseMatch error occur.
While this isn't a huge improvement, it provides a hook for us to more easily introduce library level controls for when urls are installed. It also eliminates the user from having to write an if statement for the most basic of installations.
…ng (#1933) * Fixed issue #1682 -- alert user when using file field without proper encoding * Changed issues to alerts, added docstring to check_invalid_... function, changed call in nav_subtitle to get_stats * Update AlertsPanel documentation to list pre-defined alert cases * added check for file inputs that directly reference a form, including tests; also added tests for setting encoding in submit input type * Update the alert messages to be on the panel as a map. This also explicitly mentions what attribute the form needs in the message. * Expose a page in the example app that triggers the alerts panel. --------- Co-authored-by: Tim Schilling <[email protected]>
* Use css properties for height and width
…1946) * Check for for StreamingHttpResponse when generating stats in Alert panel.
- Add jinja2 template to example app. - Switch to the render function to include context. It instruments the single template render, but not the inherited templates and I'm guessing not the included templates either. I suspect we're going to have to patch jinja templates more robustly than relying on the django jinja backend template class. Co-Authored-By: Tim Schilling <[email protected]>
Now that we're using the actual jinja template backend, it only instruments a single template.
* fixed order and grammatical number of panels in documentation * updated changes.rst to reflect change to docs
updates: - [github.com/astral-sh/ruff-pre-commit: v0.5.0 → v0.5.1](astral-sh/ruff-pre-commit@v0.5.0...v0.5.1) - [github.com/tox-dev/pyproject-fmt: 2.1.3 → 2.1.4](tox-dev/pyproject-fmt@2.1.3...2.1.4)
The alerts panel may eventually have other types of alerts that don't depend on the response. Such as Django's check system.
|
I messed this PR up. I meant to do the merge into serializable then rebase this. If the tests pass here, I'll push it to serializable. |
The stats must be stored as JSON, otherwise it'll be converted to a string.
This causes problems with tests and changing the settings via override_settings. Since we're using the lru_cache decorator on get_config, there's very little benefit to caching within the store too.
The majority were difficulities with caching and settings.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a draft PR. I'm keeping my on-going work in this branch. I found that my efforts to close #1807 and #1808 quickly spiraled into a major refactor of core logic.
This currently handles the SQLPanel which required the following changes:
raw_paramsentirely. We'll need to serialize the parameters every time now so we can't rely on the original paramsdjdt_query_idwhich uniquely identifies each query. The SQL forms now reference this id and avoid passing SQL to be executed.In addition this also changed our design for
Panelload_stats_from_store- Generates aPanelinstance with the passed in data.Panel.panel_idis now a class member to avoid needing to instantiate a panel to programatically get the panel id. With the new focus on fetching data for a specific panel given its ID, this became more of a need