-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement database backed store #2073
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
Comments
From Djangonaut Space meeting:
|
Team Neptune: I'll stop fearing the 'async' label and start working on this. :) |
@tim-schilling I want to make sure I'm not going down a rabbit hole on this issue and that I understand the goal and purpose of this task. I've explored the following solutions and would greatly welcome your feedback. I know I'm throwing a lot at you hoping that the right approach is more obvious to you than it is to me. I will scrap everything I've done and start from scratch if I have just completely misinterpreted the intention of this issue, so feel free to tell me so.
class DebugToolbarEntry(models.Model):
uuid = ...
request_id = ...
data = models.JSONField()
created_at = ... Which can then be used in DatabaseStore, for example: class DatabaseStore(BaseStore):
@classmethod
def request_ids(cls):
# Return all stored request ids
return list(DebugToolbarEntry.objects.values_list("request_id", flat=True)) As simple (and easy to understand for Django developers) as this approach would be, implemented with the power of Django's ORM and migrations, it just doesn't seem to be the right solution for the debug toolbar. But if I'm wrong, this would be so much easier than either of the solutions below. So, next, I wondered what creating a JSON file (as you had suggested) or a SQLite store (without the Django ORM) might look like. The complete implementation of the class SQLiteStore(BaseStore):
"""
A SQLite-backed store for the debug toolbar data.
This implementation uses SQLite directly without requiring Django models or migrations.
It creates the database file and tables at runtime if they don't exist.
"""
# Default database location in user's temp directory
_db_path = ...
_connection = None
_initialized = False
@classmethod
def _get_connection(cls):
"""Get or create a SQLite connection with proper settings for concurrency"""
if cls._connection is None:
...
cls._connection = ...
...
return cls._connection
@classmethod
def _init_db(cls):
"""Initialize the database schema if it doesn't exist"""
conn = cls._get_connection()
cursor = conn.cursor()
# Create the main table for debug toolbar entries
cursor.execute('''
CREATE TABLE...
''')
# Create table for panel data
cursor.execute('''
CREATE TABLE...
''')
# Create index for faster retrieval
cursor.execute('''
CREATE INDEX...
''')
conn.commit()
@classmethod
def _cleanup_old_entries(cls):
"""Remove entries that are older than the configured retention period"""
...
try:
cursor.execute(
"DELETE FROM ...",
(cutoff_timestamp,)
)
conn.commit()
except sqlite3.Error as e:
logger.error(...)
conn.rollback()
@classmethod
def request_ids(cls) -> Iterable:
"""Return all stored request ids"""
try:
conn = cls._get_connection()
cursor = conn.cursor()
cursor.execute(
"SELECT request_id FROM debug_toolbar_entries ORDER BY created_at DESC LIMIT ?",
(dt_settings.get_config()["RESULTS_CACHE_SIZE"],)
)
return [row[0] for row in cursor.fetchall()]
except sqlite3.Error as e:
logger.error(...)
return [] class JSONFileStore(BaseStore):
"""
A JSON file-backed store for the debug toolbar data.
This implementation stores data in a JSON file with file locking for concurrency.
It's simpler than the SQLite store but may have more limitations with high concurrency.
"""
# Default file location in user's temp directory
_file_path = ...
_data = None
_last_load_time = 0
@classmethod
def _get_data(cls, reload=False):
"""Get the data from the JSON file with a file lock to prevent race conditions"""
# Use cached data if recently loaded (within 2 seconds) and not forced to reload
current_time = time.time()
...
cls._last_load_time = current_time
cls._cleanup_old_entries()
return cls._data
@classmethod
def _save_data(cls):
"""Save the data to the JSON file with a file lock"""
...
@classmethod
def _cleanup_old_entries(cls):
"""Remove entries that are older than the configured retention period"""
...
@classmethod
def request_ids(cls) -> Iterable:
"""The stored request ids"""
data = cls._get_data()
# Return a copy to prevent modification of the internal list
return list(data["request_ids"]) I have removed so much code that I may have abstracted the implementation a bit too much. Let me know if you would prefer to see it all in a PR (including the tests and settings) or if you have any high-level thoughts about the different approaches and which one(s) I should explore the most. Thank you and I apologize for such a long post! |
That's exactly right -- the database store shouldn't be that complicated to implement! You do not have to write your own database backend or go low level, unless we discover a reason why using the ORM in a straightforward manner wouldn't work (but I don't expect that to happen). |
Thank you for the feedback. That's great. I will continue exploring the Django ORM option in more detail. |
- Introduced `DatabaseStore` to store debug toolbar data in the database. - Added `DebugToolbarEntry` model and migrations for persistent storage. - Updated documentation to include configuration for `DatabaseStore`. - Added tests for `DatabaseStore` functionality, including CRUD operations and cache size enforcement. Fixes django-commons#2073
This work should occur off of the serializable branch.
The goal here is to implement a
DatabaseStore
that inherits fromtoolbar.store.BaseStore
which is similar toMemoryStore
.Some considerations:
DatabaseStore
easilymake example
, we should be able to kill the process, start it up and see requests from the firstmake example
run. This proves we're able to fetch data from the database.AppConfig.ready()
. We don't need to retain data from forever. This should be exposed as a setting and documented.Sub-issues should be created from this to track functonality
The text was updated successfully, but these errors were encountered: