From 84c2eef42b1ef38f15c6cabf8833686f3eab2d28 Mon Sep 17 00:00:00 2001 From: Chris Clark Date: Wed, 21 Aug 2024 16:33:10 -0400 Subject: [PATCH] All connections are db-backed (#660) * make all connections db-backed --- .gitignore | 1 + HISTORY.rst | 10 + docs/development.rst | 4 +- docs/install.rst | 27 +-- docs/settings.rst | 15 +- explorer/app_settings.py | 3 +- explorer/apps.py | 47 ++--- explorer/assistant/utils.py | 27 ++- explorer/assistant/views.py | 10 +- explorer/connections.py | 62 ------ explorer/ee/db_connections/models.py | 110 ++++++++--- explorer/ee/db_connections/utils.py | 60 ++---- explorer/ee/db_connections/views.py | 24 +-- explorer/forms.py | 36 +++- ...0023_query_database_connection_and_more.py | 49 +++++ .../migrations/0024_auto_20240803_1135.py | 75 +++++++ ...tion_alter_querylog_database_connection.py | 24 +++ explorer/models.py | 18 +- explorer/schema.py | 44 ++--- explorer/src/js/assistant.js | 2 +- explorer/src/js/codemirror-config.js | 2 +- explorer/src/js/explorer.js | 2 +- explorer/src/js/main.js | 3 +- explorer/src/js/schemaService.js | 2 +- explorer/tasks.py | 27 ++- explorer/telemetry.py | 2 +- .../templates/connections/connections.html | 2 +- .../connections/database_connection_form.html | 4 +- explorer/templates/explorer/play.html | 8 +- explorer/templates/explorer/query.html | 7 +- explorer/templates/explorer/query_list.html | 2 +- .../templates/explorer/querylog_list.html | 4 +- .../templates/explorer/schema_building.html | 9 - explorer/templates/explorer/schema_error.html | 11 ++ explorer/tests/factories.py | 6 +- explorer/tests/settings.py | 13 ++ explorer/tests/test_apps.py | 16 -- explorer/tests/test_assistant.py | 112 +++++------ explorer/tests/test_connections.py | 71 ------- explorer/tests/test_db_connection_utils.py | 60 ++---- explorer/tests/test_exporters.py | 13 +- explorer/tests/test_forms.py | 35 ++++ explorer/tests/test_models.py | 48 ++--- explorer/tests/test_schema.py | 32 ++- explorer/tests/test_tasks.py | 148 ++++---------- explorer/tests/test_utils.py | 11 -- explorer/tests/test_views.py | 187 +++++++++++------- explorer/utils.py | 14 -- explorer/views/download.py | 5 +- explorer/views/list.py | 1 + explorer/views/query.py | 9 +- explorer/views/schema.py | 28 +-- explorer/views/utils.py | 2 +- test_project/celery_config.py | 4 + test_project/settings.py | 13 +- 55 files changed, 755 insertions(+), 806 deletions(-) delete mode 100644 explorer/connections.py create mode 100644 explorer/migrations/0023_query_database_connection_and_more.py create mode 100644 explorer/migrations/0024_auto_20240803_1135.py create mode 100644 explorer/migrations/0025_alter_query_database_connection_alter_querylog_database_connection.py delete mode 100644 explorer/templates/explorer/schema_building.html create mode 100644 explorer/templates/explorer/schema_error.html delete mode 100644 explorer/tests/test_connections.py diff --git a/.gitignore b/.gitignore index 80fb1467..6703b59a 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,4 @@ chinook.sqlite model_data.json tst1 tst1-journal +tst2 diff --git a/HISTORY.rst b/HISTORY.rst index 6bd26534..21e6d4bb 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -5,6 +5,16 @@ Change Log This document records all notable changes to `SQL Explorer `_. This project adheres to `Semantic Versioning `_. +vNext +=========================== +* `#660`_: Userspace connection migration. This should be an invisible change, but represents a significant refactor of how connections function. +Instead of a weird blend of DatabaseConnection models and underlying Django models (which were the original Explorer connections), +this migrates all connections to DatabaseConnection models and implements proper foreign keys to them on the Query and QueryLog models. +A data migration creates new DatabaseConnection models based on the configured settings.EXPLORER_CONNECTIONS. +Going forward, admins can create new Django-backed DatabaseConnection models by registering the connection in EXPLORER_CONNECTIONS, and then creating a +DatabaseConnection model using the Django admin or the user-facing /connections/new/ form, and entering the Django DB alias and setting the connection type to "Django Connection" + + `5.2.0`_ (2024-08-19) =========================== * `#651`_: Ability to append an upload to a previously uploaded file/sqlite DB as a new table diff --git a/docs/development.rst b/docs/development.rst index ff9939eb..0b4de453 100644 --- a/docs/development.rst +++ b/docs/development.rst @@ -36,10 +36,10 @@ Install the dev requirements: And then: -``python manage.py test --settings=tests.settings`` +``python manage.py test --settings=explorer.tests.settings`` Or with coverage: -``coverage run --source='.' manage.py test --settings=tests.settings`` +``coverage run --source='.' manage.py test --settings=explorer.tests.settings`` ``coverage combine`` ``coverage report`` diff --git a/docs/install.rst b/docs/install.rst index f16fa5ea..a49ea757 100644 --- a/docs/install.rst +++ b/docs/install.rst @@ -30,20 +30,6 @@ Add to your ``INSTALLED_APPS``, located in the ``settings.py`` file in your proj 'explorer', ) -Configure your settings to something like: - -.. code-block:: python - - EXPLORER_CONNECTIONS = { 'Default': 'readonly' } - EXPLORER_DEFAULT_CONNECTION = 'readonly' - -The first setting lists the connections you want to allow Explorer to -use. The keys of the connections dictionary are friendly names to show -Explorer users, and the values are the actual database aliases used in -``settings.DATABASES``. It is highly recommended to setup read-only roles -in your database, add them in your project's ``DATABASES`` setting and -use these read-only connections in the ``EXPLORER_CONNECTIONS``. - Add the following to your urls.py (all Explorer URLs are restricted via the ``EXPLORER_PERMISSION_VIEW`` and ``EXPLORER_PERMISSION_CHANGE`` settings. See Settings section below for further documentation.): @@ -57,15 +43,6 @@ settings. See Settings section below for further documentation.): path('explorer/', include('explorer.urls')), ] -If you want to quickly use django-sql-explorer with the existing default -connection **and know what you are doing** (or you are on development), you -can use the following settings: - -.. code-block:: python - - EXPLORER_CONNECTIONS = { 'Default': 'default' } - EXPLORER_DEFAULT_CONNECTION = 'default' - Run migrate to create the tables: ``python manage.py migrate`` @@ -78,7 +55,9 @@ And run the server: ``python manage.py runserver`` -You can now browse to http://127.0.0.1:8000/explorer/ and get exploring! +You can now browse to http://127.0.0.1:8000/explorer/. Add a database connection at /explorer/connections/new/, and you +are ready to start exploring! If you have a database in your settings.DATABASES you would like to query, you can create +a connection with the same alias and name and set the Engine to "Django Database". Note that Explorer expects STATIC_URL to be set appropriately. This isn't a problem with vanilla Django setups, but if you are using e.g. Django Storages with S3, you diff --git a/docs/settings.rst b/docs/settings.rst index 8386cefd..9d069fbc 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -93,24 +93,17 @@ Generate DB schema asynchronously. Requires Celery and ``EXPLORER_TASKS_ENABLED` EXPLORER_ASYNC_SCHEMA = False -Default connection -****************** - -The name of the Django database connection to use. Ideally set this to a connection with read only permissions - -.. code-block:: python - - EXPLORER_DEFAULT_CONNECTION = None # Must be set for the app to work, as this is required - - Database connections ******************** A dictionary of ``{'Friendly Name': 'django_db_alias'}``. +If you want to create a DatabaseConnection to a DB that is registered in settings.DATABASES, then add the alias to this +dictionary (with a friendly name of your choice), and then create a new connection (../connections/new/) with the alias +and name set to the alias of the Django database, and the Engine set to "Django Database". .. code-block:: python - EXPLORER_CONNECTIONS = {} # At a minimum, should be set to something like { 'Default': 'readonly' } or similar. See connections.py for more documentation. + EXPLORER_CONNECTIONS = {} Permission view diff --git a/explorer/app_settings.py b/explorer/app_settings.py index c558e3f9..a93257b3 100644 --- a/explorer/app_settings.py +++ b/explorer/app_settings.py @@ -4,6 +4,8 @@ EXPLORER_CONNECTIONS = getattr(settings, "EXPLORER_CONNECTIONS", {}) + +# Deprecated as of 6.0. Will be removed in a future version. EXPLORER_DEFAULT_CONNECTION = getattr( settings, "EXPLORER_DEFAULT_CONNECTION", None ) @@ -75,7 +77,6 @@ EXPLORER_RECENT_QUERY_COUNT = getattr( settings, "EXPLORER_RECENT_QUERY_COUNT", 5 ) -EXPLORER_ASYNC_SCHEMA = getattr(settings, "EXPLORER_ASYNC_SCHEMA", False) DEFAULT_EXPORTERS = [ ("csv", "explorer.exporters.CSVExporter"), diff --git a/explorer/apps.py b/explorer/apps.py index 522d9ada..a5eabe7a 100644 --- a/explorer/apps.py +++ b/explorer/apps.py @@ -1,7 +1,6 @@ from django.apps import AppConfig -from django.core.exceptions import ImproperlyConfigured -from django.db import connections as djcs from django.utils.translation import gettext_lazy as _ +from django.db import transaction, DEFAULT_DB_ALIAS, connections class ExplorerAppConfig(AppConfig): @@ -10,36 +9,26 @@ class ExplorerAppConfig(AppConfig): verbose_name = _("SQL Explorer") default_auto_field = "django.db.models.AutoField" - def ready(self): - from explorer.schema import build_async_schemas - _validate_connections() - build_async_schemas() +# SQL Explorer DatabaseConnection models store connection info but are always translated into "django-style" connections +# before use, because we use Django's DB engine to run queries, gather schema information, etc. -def _get_default(): - from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION - return EXPLORER_DEFAULT_CONNECTION +# In general this isn't a problem; we cough up a django-style connection and all is well. The exception is when using +# the `with transaction.atomic(using=...):` context manager. The atomic() function takes a connection *alias* argument +# and the retrieves the connection from settings.DATABASES. But of course if we are providing a user-created connection +# alias, Django doesn't find it. +# The solution is to monkey-patch the `get_connection` function within transaction.atomic to make it aware of the +# user-created connections. -def _get_explorer_connections(): - from explorer.app_settings import EXPLORER_CONNECTIONS - return EXPLORER_CONNECTIONS +# This code should be double-checked against new versions of Django to make sure the original logic is still correct. +def new_get_connection(using=None): + from explorer.ee.db_connections.models import DatabaseConnection + if using is None: + using = DEFAULT_DB_ALIAS + if using in connections: + return connections[using] + return DatabaseConnection.objects.get(alias=using).as_django_connection() -def _validate_connections(): - - # Validate connections, when using settings.EXPLORER_CONNECTIONS - # Skip if none are configured, as the app will use user-configured connections (DatabaseConnection models) - if _get_explorer_connections().values() and _get_default() not in _get_explorer_connections().values(): - raise ImproperlyConfigured( - f"EXPLORER_DEFAULT_CONNECTION is {_get_default()}, " - f"but that alias is not present in the values of " - f"EXPLORER_CONNECTIONS" - ) - - for name, conn_name in _get_explorer_connections().items(): - if conn_name not in djcs: - raise ImproperlyConfigured( - f"EXPLORER_CONNECTIONS contains ({name}, {conn_name}), " - f"but {conn_name} is not a valid Django DB connection." - ) +transaction.get_connection = new_get_connection diff --git a/explorer/assistant/utils.py b/explorer/assistant/utils.py index 448d78fe..3dd3cdfb 100644 --- a/explorer/assistant/utils.py +++ b/explorer/assistant/utils.py @@ -1,7 +1,6 @@ from explorer import app_settings from explorer.schema import schema_info from explorer.models import ExplorerValue -from explorer.utils import get_valid_connection from django.db.utils import OperationalError @@ -35,8 +34,8 @@ def extract_response(r): return r[-1].content -def tables_from_schema_info(connection, table_names): - schema = schema_info(connection) +def tables_from_schema_info(db_connection, table_names): + schema = schema_info(db_connection) return [table for table in schema if table[0] in set(table_names)] @@ -65,8 +64,7 @@ def sample_rows_from_table(connection, table_name): Returns: A list of rows with field values truncated if they exceed 500 characters/bytes. """ - conn = get_valid_connection(connection) - cursor = conn.cursor() + cursor = connection.cursor() try: cursor.execute(f"SELECT * FROM {table_name} LIMIT {ROW_SAMPLE_SIZE}") ret = [[header[0] for header in cursor.description]] @@ -125,30 +123,27 @@ def fits_in_window(string: str) -> bool: return num_tokens_from_string(string) < (app_settings.EXPLORER_ASSISTANT_MODEL["max_tokens"] * 0.95) -def build_prompt(request_data, included_tables): +def build_prompt(db_connection, assistant_request, included_tables, query_error=None, sql=None): user_prompt = "" + djc = db_connection.as_django_connection() + user_prompt += f"## Database Vendor / SQL Flavor is {djc.vendor}\n\n" - db_vendor = get_valid_connection(request_data.get("connection")).vendor - user_prompt += f"## Database Vendor / SQL Flavor is {db_vendor}\n\n" + if query_error: + user_prompt += f"## Query Error ##\n\n{query_error}\n\n" - db_error = request_data.get("db_error") - if db_error: - user_prompt += f"## Query Error ##\n\n{db_error}\n\n" - - sql = request_data.get("sql") if sql: user_prompt += f"## Existing SQL ##\n\n{sql}\n\n" - results_sample = sample_rows_from_tables(request_data["connection"], + results_sample = sample_rows_from_tables(djc, included_tables) if fits_in_window(user_prompt + results_sample): user_prompt += f"## Table Structure with Sampled Data ##\n\n{results_sample}\n\n" else: # If it's too large with sampling, then provide *just* the structure - table_struct = tables_from_schema_info(request_data["connection"], + table_struct = tables_from_schema_info(db_connection, included_tables) user_prompt += f"## Table Structure ##\n\n{table_struct}\n\n" - user_prompt += f"## User's Request to Assistant ##\n\n{request_data['assistant_request']}\n\n" + user_prompt += f"## User's Request to Assistant ##\n\n{assistant_request}\n\n" prompt = { "system": ExplorerValue.objects.get_item(ExplorerValue.ASSISTANT_SYSTEM_PROMPT).value, diff --git a/explorer/assistant/views.py b/explorer/assistant/views.py index ff557619..f5d6f2aa 100644 --- a/explorer/assistant/views.py +++ b/explorer/assistant/views.py @@ -4,6 +4,7 @@ import json from explorer.telemetry import Stat, StatNames +from explorer.ee.db_connections.models import DatabaseConnection from explorer.assistant.models import PromptLog from explorer.assistant.utils import ( do_req, extract_response, @@ -18,7 +19,14 @@ def run_assistant(request_data, user): extra_tables = request_data.get("selected_tables", []) included_tables = get_table_names_from_query(sql) + extra_tables - prompt = build_prompt(request_data, included_tables) + connection_id = request_data.get("connection_id") + try: + conn = DatabaseConnection.objects.get(id=connection_id) + except DatabaseConnection.DoesNotExist: + return "Error: Connection not found" + + prompt = build_prompt(conn, request_data.get("assistant_request"), + included_tables, request_data.get("db_error"), request_data.get("sql")) start = timezone.now() pl = PromptLog( diff --git a/explorer/connections.py b/explorer/connections.py deleted file mode 100644 index e1921bf9..00000000 --- a/explorer/connections.py +++ /dev/null @@ -1,62 +0,0 @@ -from django.db import connections as djcs -from django.db import transaction, DEFAULT_DB_ALIAS - -from explorer.ee.db_connections.utils import create_django_style_connection -from explorer import app_settings -from explorer.models import DatabaseConnection - -# To support user-configured database connections that can be managed through the Explorer UI, *as well* as the -# 'legacy' connections that are configured in Django's normal settings.DATABASES config, we stitch together the two. - -# We allow queries to be associated with either type of connection, seamlessly. - -# The approach is to allow users to create connections with approximately the same parameters that a settings.DATABASE -# would expect. We then stitch them together into one list. When Explorer needs to access a connection, it coughs up a -# Django DatabaseWrapper connection in either case (natively, if it's coming from settings.DATABASES, or by taking the -# user-created connection and running it through the create_django_style_connection() function in this file). - -# In general, amazingly, this "just works" and the entire application is perfectly happy to use either type as a -# connection. The exception to this is that there are a few bits of code that ultimately (or directly) use the -# django.db.transaction.atomic context manager. For some reason that particular Django innard takes an *alias*, not a -# proper connection. Then it retrieves the connection based on that alias. But of course if we are providing a -# user-created connection alias, Django doesn't find it (because it is looking in settings.DATABASES). - -# The solution is to monkey-patch the get_connection function that transaction.atomic uses, to make it aware of the -# user-created connections. - - -def new_get_connection(using=None): - if using is None: - using = DEFAULT_DB_ALIAS - if using in djcs: - return djcs[using] - return create_django_style_connection(DatabaseConnection.objects.get(alias=using)) - - -# Monkey patch -transaction.get_connection = new_get_connection - - -# We export valid SQL connections here so that consuming code never has to -# deal with django.db.connections directly, and risk accessing a connection -# that hasn't been registered to Explorer. - -# Django insists that connections that are created in a thread are only accessed -# by that thread, so here we create a dictionary-like collection of the valid -# connections, but does a 'live' lookup of the connection on each item access. -class ExplorerConnections(dict): - - def __getitem__(self, item): - if item in djcs: - return djcs[item] - else: - return create_django_style_connection(DatabaseConnection.objects.get(alias=item)) - - -def connections(): - _connections = [c for c in djcs if c in app_settings.EXPLORER_CONNECTIONS.values()] - db_connections = DatabaseConnection.objects.all() - _connections += [c.alias for c in db_connections] - return ExplorerConnections(zip(_connections, _connections)) - - diff --git a/explorer/ee/db_connections/models.py b/explorer/ee/db_connections/models.py index 55e4ccaf..ea81a8f8 100644 --- a/explorer/ee/db_connections/models.py +++ b/explorer/ee/db_connections/models.py @@ -1,17 +1,31 @@ import os -from django.conf import settings -from django.core.exceptions import ValidationError -from django.db import models -from django.db.models.signals import pre_save -from django.dispatch import receiver -from explorer.ee.db_connections.utils import uploaded_db_local_path, quick_hash +import json +from django.db import models, DatabaseError, connections, transaction +from django.db.utils import load_backend +from explorer.app_settings import EXPLORER_CONNECTIONS +from explorer.ee.db_connections.utils import quick_hash, uploaded_db_local_path from django_cryptography.fields import encrypt +class DatabaseConnectionManager(models.Manager): + + def uploads(self): + return self.filter(engine=DatabaseConnection.SQLITE, host__isnull=False) + + def non_uploads(self): + return self.exclude(engine=DatabaseConnection.SQLITE, host__isnull=False) + + def default(self): + return self.filter(default=True).first() + + class DatabaseConnection(models.Model): + objects = DatabaseConnectionManager() + SQLITE = "django.db.backends.sqlite3" + DJANGO = "django_connection" DATABASE_ENGINES = ( (SQLITE, "SQLite3"), @@ -22,20 +36,22 @@ class DatabaseConnection(models.Model): ("django_cockroachdb", "CockroachDB"), ("mssql", "SQL Server (mssql-django)"), ("django_snowflake", "Snowflake"), + (DJANGO, "Django Connection"), ) alias = models.CharField(max_length=255, unique=True) engine = models.CharField(max_length=255, choices=DATABASE_ENGINES) - name = models.CharField(max_length=255) + name = models.CharField(max_length=255, blank=True, null=True) user = encrypt(models.CharField(max_length=255, blank=True, null=True)) password = encrypt(models.CharField(max_length=255, blank=True, null=True)) host = encrypt(models.CharField(max_length=255, blank=True)) - port = models.CharField(max_length=255, blank=True) + port = models.CharField(max_length=255, blank=True, null=True) extras = models.JSONField(blank=True, null=True) upload_fingerprint = models.CharField(max_length=255, blank=True, null=True) + default = models.BooleanField(default=False) def __str__(self): - return f"{self.name} ({self.alias})" + return f"{self.alias}" def update_fingerprint(self): self.upload_fingerprint = self.local_fingerprint() @@ -57,11 +73,14 @@ def download_sqlite_if_needed(self): self._download_sqlite() self.update_fingerprint() - @property def is_upload(self): return self.engine == self.SQLITE and self.host + @property + def is_django_alias(self): + return self.engine == DatabaseConnection.DJANGO + @property def local_name(self): if self.is_upload: @@ -71,22 +90,55 @@ def delete_local_sqlite(self): if self.is_upload and os.path.exists(self.local_name): os.remove(self.local_name) - @classmethod - def from_django_connection(cls, connection_alias): - conn = settings.DATABASES.get(connection_alias) - if conn: - return DatabaseConnection( - alias=connection_alias, - engine=conn.get("ENGINE"), - name=conn.get("NAME"), - user=conn.get("USER"), - password=conn.get("PASSWORD"), - host=conn.get("HOST"), - port=conn.get("PORT"), - ) - - -@receiver(pre_save, sender=DatabaseConnection) -def validate_database_connection(sender, instance, **kwargs): - if instance.name in settings.DATABASES.keys(): - raise ValidationError(f"Database name '{instance.name}' already exists.") + # See the comment in apps.py for a more in-depth explanation of what's going on here. + def as_django_connection(self): + if self.is_upload: + self.download_sqlite_if_needed() + + # You can't access a Django-backed connection unless it has been registered in EXPLORER_CONNECTIONS. + # Otherwise, users with userspace DatabaseConnection rights could connect to underlying Django DB connections. + if self.is_django_alias: + if self.alias in EXPLORER_CONNECTIONS.values(): + return connections[self.alias] + else: + raise DatabaseError("Django alias connections must be registered in EXPLORER_CONNECTIONS.") + + connection_settings = { + "ENGINE": self.engine, + "NAME": self.name if not self.is_upload else self.local_name, + "USER": self.user, + "PASSWORD": self.password, + "HOST": self.host if not self.is_upload else None, + "PORT": self.port, + "TIME_ZONE": None, + "CONN_MAX_AGE": 0, + "CONN_HEALTH_CHECKS": False, + "OPTIONS": {}, + "TEST": {}, + "AUTOCOMMIT": True, + "ATOMIC_REQUESTS": False, + } + + if self.extras: + extras_dict = json.loads(self.extras) if isinstance(self.extras, str) else self.extras + connection_settings.update(extras_dict) + + try: + backend = load_backend(self.engine) + return backend.DatabaseWrapper(connection_settings, self.alias) + except DatabaseError as e: + raise DatabaseError(f"Failed to create explorer connection: {e}") from e + + def save(self, *args, **kwargs): + # If this instance is marked as default, unset the default on all other instances + if self.default: + with transaction.atomic(): + DatabaseConnection.objects.filter(default=True).update(default=False) + else: + # If there is no default set yet, make this newly created one the default. + # Effectively this is for first-time installations. + has_default = DatabaseConnection.objects.filter(default=True).exists() + if not has_default: + self.default = True + + super().save(*args, **kwargs) diff --git a/explorer/ee/db_connections/utils.py b/explorer/ee/db_connections/utils.py index 7acae8b1..069102ea 100644 --- a/explorer/ee/db_connections/utils.py +++ b/explorer/ee/db_connections/utils.py @@ -1,12 +1,19 @@ -from django.db import DatabaseError -from django.db.utils import load_backend import os -import json + import hashlib import sqlite3 import io +def default_db_connection(): + from explorer.ee.db_connections.models import DatabaseConnection + return DatabaseConnection.objects.default() + + +def default_db_connection_id(): + return default_db_connection().id + + # Uploading the same filename twice (from the same user) will overwrite the 'old' DB on S3 def upload_sqlite(db_bytes, path): from explorer.utils import get_s3_bucket @@ -19,10 +26,8 @@ def upload_sqlite(db_bytes, path): # the DBs go into user-specific folders on s3), but the *aliases* would be the same. So one user # could (innocently) upload a file with the same name, and any existing queries would be suddenly pointing # to this new database connection. Oops! -# TODO: In the future, queries should probably be FK'ed to the ID of the connection, rather than simply -# storing the alias of the connection as a string. def create_connection_for_uploaded_sqlite(filename, s3_path): - from explorer.models import DatabaseConnection + from explorer.ee.db_connections.models import DatabaseConnection return DatabaseConnection.objects.create( alias=filename, engine=DatabaseConnection.SQLITE, @@ -31,16 +36,6 @@ def create_connection_for_uploaded_sqlite(filename, s3_path): ) -def get_sqlite_for_connection(explorer_connection): - # Get the database from s3, then modify the connection to work with the downloaded file. - # E.g. "host" should not be set, and we need to get the full path to the file - explorer_connection.download_sqlite_if_needed() - # Note the order here is important; .local_name checked "is_upload" which relies on .host being set - explorer_connection.name = explorer_connection.local_name - explorer_connection.host = None - return explorer_connection - - def user_dbs_local_dir(): d = os.path.normpath(os.path.join(os.getcwd(), "user_dbs")) if not os.path.exists(d): @@ -52,39 +47,6 @@ def uploaded_db_local_path(name): return os.path.join(user_dbs_local_dir(), name) -def create_django_style_connection(explorer_connection): - - if explorer_connection.is_upload: - explorer_connection = get_sqlite_for_connection(explorer_connection) - - connection_settings = { - "ENGINE": explorer_connection.engine, - "NAME": explorer_connection.name, - "USER": explorer_connection.user, - "PASSWORD": explorer_connection.password, - "HOST": explorer_connection.host, - "PORT": explorer_connection.port, - "TIME_ZONE": None, - "CONN_MAX_AGE": 0, - "CONN_HEALTH_CHECKS": False, - "OPTIONS": {}, - "TEST": {}, - "AUTOCOMMIT": True, - "ATOMIC_REQUESTS": False, - } - - if explorer_connection.extras: - extras_dict = json.loads(explorer_connection.extras) if isinstance(explorer_connection.extras, - str) else explorer_connection.extras - connection_settings.update(extras_dict) - - try: - backend = load_backend(explorer_connection.engine) - return backend.DatabaseWrapper(connection_settings, explorer_connection.alias) - except DatabaseError as e: - raise DatabaseError(f"Failed to create explorer connection: {e}") from e - - def sqlite_to_bytesio(local_path): # Write the file to disk. It'll be uploaded to s3, and left here locally for querying db_file = io.BytesIO() diff --git a/explorer/ee/db_connections/views.py b/explorer/ee/db_connections/views.py index ed1e5ed5..3f5c904e 100644 --- a/explorer/ee/db_connections/views.py +++ b/explorer/ee/db_connections/views.py @@ -3,21 +3,19 @@ from django.views import View from django.http import JsonResponse, HttpResponse from django.urls import reverse_lazy -from django.db.utils import OperationalError +from django.db.utils import OperationalError, DatabaseError from explorer.models import DatabaseConnection from explorer.ee.db_connections.utils import ( upload_sqlite, create_connection_for_uploaded_sqlite ) from explorer.ee.db_connections.create_sqlite import parse_to_sqlite -from explorer import app_settings from explorer.schema import clear_schema_cache from explorer.app_settings import EXPLORER_MAX_UPLOAD_SIZE from explorer.ee.db_connections.forms import DatabaseConnectionForm from explorer.utils import delete_from_s3 from explorer.views.auth import PermissionRequiredMixin from explorer.views.mixins import ExplorerContextMixin -from explorer.ee.db_connections.utils import create_django_style_connection from explorer.ee.db_connections.mime import is_sqlite @@ -47,7 +45,9 @@ def post(self, request): # noqa # You can't double stramp a triple stamp! if append_path and is_sqlite(file): - raise TypeError("Can't append a SQLite file to a SQLite file. Only CSV and JSON.") + msg = "Can't append a SQLite file to a SQLite file. Only CSV and JSON." + logger.error(msg) + return JsonResponse({"error": msg}, status=400) try: f_bytes, f_name = parse_to_sqlite(file, conn, request.user.id) @@ -73,7 +73,7 @@ def post(self, request): # noqa if not append_path: conn = create_connection_for_uploaded_sqlite(f_name, s3_path) - clear_schema_cache(conn.alias) + clear_schema_cache(conn) conn.update_fingerprint() return JsonResponse({"success": True}) else: @@ -87,14 +87,6 @@ class DatabaseConnectionsListView(PermissionRequiredMixin, ExplorerContextMixin, template_name = "connections/connections.html" model = DatabaseConnection - def get_queryset(self): - qs = list(DatabaseConnection.objects.all()) - for _, alias in app_settings.EXPLORER_CONNECTIONS.items(): - django_conn = DatabaseConnection.from_django_connection(alias) - if django_conn: - qs.append(django_conn) - return qs - class DatabaseConnectionDetailView(PermissionRequiredMixin, ExplorerContextMixin, DetailView): permission_required = "connections_permission" @@ -149,7 +141,7 @@ class DatabaseConnectionRefreshView(PermissionRequiredMixin, View): def get(self, request, pk): # noqa conn = DatabaseConnection.objects.get(id=pk) conn.delete_local_sqlite() - clear_schema_cache(conn.alias) + clear_schema_cache(conn) message = f"Deleted schema cache for {conn.alias}. Schema will be regenerated on next use." if conn.is_upload: message += "\nRemoved local SQLite DB. Will be re-downloaded from S3 on next use." @@ -181,11 +173,13 @@ def post(self, request, pk=None): # noqa extras=connection_data["extras"] ) try: - conn = create_django_style_connection(explorer_connection) + conn = explorer_connection.as_django_connection() with conn.cursor() as cursor: cursor.execute("SELECT 1") return JsonResponse({"success": True}) except OperationalError as e: return JsonResponse({"success": False, "error": str(e)}) + except DatabaseError as e: + return JsonResponse({"success": False, "error": str(e)}) else: return JsonResponse({"success": False, "error": "Invalid form data"}) diff --git a/explorer/forms.py b/explorer/forms.py index cd9dd2f7..fb6511c2 100644 --- a/explorer/forms.py +++ b/explorer/forms.py @@ -1,8 +1,10 @@ from django.forms import BooleanField, CharField, ModelForm, ValidationError from django.forms.widgets import CheckboxInput, Select +from django.db.models import Value, IntegerField, When, Case -from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION from explorer.models import MSG_FAILED_BLACKLIST, Query +from explorer.ee.db_connections.models import DatabaseConnection +from explorer.ee.db_connections.utils import default_db_connection class SqlField(CharField): @@ -32,14 +34,14 @@ class QueryForm(ModelForm): sql = SqlField() snapshot = BooleanField(widget=CheckboxInput, required=False) - connection = CharField(widget=Select, required=False) + database_connection = CharField(widget=Select, required=False) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.fields["connection"].widget.choices = self.connections - if not self.instance.connection: - self.initial["connection"] = EXPLORER_DEFAULT_CONNECTION - self.fields["connection"].widget.attrs["class"] = "form-select" + self.fields["database_connection"].widget.choices = self.connections + if not self.instance.database_connection: + self.initial["database_connection"] = default_db_connection().alias + self.fields["database_connection"].widget.attrs["class"] = "form-select" def clean(self): # Don't overwrite created_by_user @@ -48,15 +50,31 @@ def clean(self): self.instance.created_by_user return super().clean() + def clean_database_connection(self): + connection_id = self.cleaned_data.get("database_connection") + if connection_id: + try: + return DatabaseConnection.objects.get(id=connection_id) + except DatabaseConnection.DoesNotExist as e: + raise ValidationError("Invalid database connection selected.") from e + return None + @property def created_at_time(self): return self.instance.created_at.strftime("%Y-%m-%d") @property def connections(self): - from explorer.connections import connections - return [(c, c) for c in connections()] + # Ensure the default connection appears first in the dropdown in the form + result = DatabaseConnection.objects.annotate( + custom_order=Case( + When(id=default_db_connection().id, then=Value(0)), + default=Value(1), + output_field=IntegerField(), + ) + ).order_by("custom_order", "id") + return [(c.id, c.alias) for c in result.all()] class Meta: model = Query - fields = ["title", "sql", "description", "snapshot", "connection"] + fields = ["title", "sql", "description", "snapshot", "database_connection"] diff --git a/explorer/migrations/0023_query_database_connection_and_more.py b/explorer/migrations/0023_query_database_connection_and_more.py new file mode 100644 index 00000000..865e4802 --- /dev/null +++ b/explorer/migrations/0023_query_database_connection_and_more.py @@ -0,0 +1,49 @@ +# Generated by Django 5.0.4 on 2024-08-03 16:54 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('explorer', '0022_databaseconnection_upload_fingerprint'), + ] + + operations = [ + migrations.AddField( + model_name='query', + name='database_connection', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='explorer.databaseconnection'), + ), + migrations.AddField( + model_name='querylog', + name='database_connection', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='explorer.databaseconnection'), + ), + migrations.AlterField( + model_name='databaseconnection', + name='engine', + field=models.CharField( + choices=[('django.db.backends.sqlite3', 'SQLite3'), ('django.db.backends.postgresql', 'PostgreSQL'), + ('django.db.backends.mysql', 'MySQL'), ('django.db.backends.oracle', 'Oracle'), + ('django.db.backends.mysql', 'MariaDB'), ('django_cockroachdb', 'CockroachDB'), + ('mssql', 'SQL Server (mssql-django)'), ('django_snowflake', 'Snowflake'), + ('django_connection', 'Django Connection')], max_length=255), + ), + migrations.AlterField( + model_name='databaseconnection', + name='name', + field=models.CharField(blank=True, max_length=255, null=True), + ), + migrations.AlterField( + model_name='databaseconnection', + name='port', + field=models.CharField(blank=True, max_length=255, null=True), + ), + migrations.AddField( + model_name='databaseconnection', + name='default', + field=models.BooleanField(default=False), + ), + ] diff --git a/explorer/migrations/0024_auto_20240803_1135.py b/explorer/migrations/0024_auto_20240803_1135.py new file mode 100644 index 00000000..2e01ab56 --- /dev/null +++ b/explorer/migrations/0024_auto_20240803_1135.py @@ -0,0 +1,75 @@ +# Generated by Django 5.0.4 on 2024-08-03 16:35 + +from django.db import migrations, connection +from django.db import connections as djcs +from explorer import app_settings + + +def populate_new_foreign_key(apps, _): + from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION + DatabaseConnection = apps.get_model('explorer', 'DatabaseConnection') + + # Create new connection for each based on EXPLORER_CONNECTIONS + connections = [c for c in djcs if c in app_settings.EXPLORER_CONNECTIONS.values()] + for c in connections: + if not DatabaseConnection.objects.filter(alias=c).exists(): + is_default = c == EXPLORER_DEFAULT_CONNECTION + obj = DatabaseConnection( + alias=c, + name=c, + engine="django_connection", + default=is_default + ) + obj.save() + print(f"created Connection ID {obj.id} for {obj.alias}") + + has_default = DatabaseConnection.objects.filter(default=True).exists() + if not has_default: + dbc = DatabaseConnection.objects.all().first() + if dbc: + dbc.default = True + dbc.save() + + # These are written as raw SQL rather than queryset updates because e.g. Query.connection is no longer + # referencable as it was removed from the codebase. + with connection.cursor() as cursor: + for c in DatabaseConnection.objects.all(): + # Update Query table + cursor.execute(""" + UPDATE explorer_query + SET database_connection_id = %s + WHERE connection = %s + """, [c.id, c.alias]) + + # Update QueryLog table + cursor.execute(""" + UPDATE explorer_querylog + SET database_connection_id = %s + WHERE connection = %s + """, [c.id, c.alias]) + + if EXPLORER_DEFAULT_CONNECTION: + default_conn = DatabaseConnection.objects.filter(alias=EXPLORER_DEFAULT_CONNECTION).first() + if default_conn: + cursor.execute(""" + UPDATE explorer_query + SET database_connection_id = %s + WHERE connection = '' + """, [default_conn.id]) + + cursor.execute(""" + UPDATE explorer_querylog + SET database_connection_id = %s + WHERE connection = '' + """, [default_conn.id]) + + +class Migration(migrations.Migration): + + dependencies = [ + ('explorer', '0023_query_database_connection_and_more'), + ] + + operations = [ + migrations.RunPython(populate_new_foreign_key), + ] diff --git a/explorer/migrations/0025_alter_query_database_connection_alter_querylog_database_connection.py b/explorer/migrations/0025_alter_query_database_connection_alter_querylog_database_connection.py new file mode 100644 index 00000000..8c08b76b --- /dev/null +++ b/explorer/migrations/0025_alter_query_database_connection_alter_querylog_database_connection.py @@ -0,0 +1,24 @@ +# Generated by Django 5.0.4 on 2024-08-13 13:45 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('explorer', '0024_auto_20240803_1135'), + ] + + operations = [ + migrations.AlterField( + model_name='query', + name='database_connection', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='explorer.databaseconnection'), + ), + migrations.AlterField( + model_name='querylog', + name='database_connection', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='explorer.databaseconnection'), + ), + ] diff --git a/explorer/models.py b/explorer/models.py index 604f9dc5..9fe8ef11 100644 --- a/explorer/models.py +++ b/explorer/models.py @@ -13,9 +13,11 @@ from explorer.assistant import models as assistant_models # noqa from explorer.telemetry import Stat, StatNames from explorer.utils import ( - extract_params, get_params_for_url, get_s3_bucket, get_valid_connection, passes_blacklist, s3_url, + extract_params, get_params_for_url, get_s3_bucket, passes_blacklist, s3_url, shared_dict_update, swap_params, ) +from explorer.ee.db_connections.utils import default_db_connection + # Issue #618. All models must be imported so that Django understands how to manage migrations for the app from explorer.ee.db_connections.models import DatabaseConnection # noqa @@ -42,6 +44,9 @@ class Query(models.Model): default=False, help_text=_("Include in snapshot task (if enabled)") ) + # NOTE this field is deprecated in favor of database_connection and no longer in use. + # It is present in the 6.0 release to preserve backwards compatibility in case there is need for a rollback. + # It will be removed in a future release (e.g. v6.1) connection = models.CharField( blank=True, max_length=128, @@ -52,6 +57,7 @@ class Query(models.Model): "Will use EXPLORER_DEFAULT_CONNECTION if left blank" ) ) + database_connection = models.ForeignKey(to=DatabaseConnection, on_delete=models.SET_NULL, null=True) def __init__(self, *args, **kwargs): self.params = kwargs.get("params") @@ -102,9 +108,9 @@ def execute_query_only(self): error, code="InvalidSql" ) - + conn = self.database_connection or default_db_connection() return QueryResult( - self.final_sql(), get_valid_connection(self.connection) + self.final_sql(), conn.as_django_connection() ) def execute_with_logging(self, executing_user): @@ -174,7 +180,7 @@ def log(self, user=None): sql=self.final_sql(), query_id=self.id, run_by_user=user, - connection=self.connection + database_connection=self.database_connection, ) ql.save() return ql @@ -228,7 +234,11 @@ class QueryLog(models.Model): ) run_at = models.DateTimeField(auto_now_add=True) duration = models.FloatField(blank=True, null=True) # milliseconds + # NOTE this field is deprecated in favor of database_connection and no longer in use. + # It is present in the 6.0 release to preserve backwards compatibility in case there is need for a rollback. + # It will be removed in a future release (e.g. v6.1) connection = models.CharField(blank=True, max_length=128, default="") + database_connection = models.ForeignKey(to=DatabaseConnection, on_delete=models.SET_NULL, null=True) success = models.BooleanField(default=True) error = models.TextField(blank=True, null=True) diff --git a/explorer/schema.py b/explorer/schema.py index 6609cc81..07a8cf2c 100644 --- a/explorer/schema.py +++ b/explorer/schema.py @@ -1,11 +1,11 @@ from django.core.cache import cache from explorer.app_settings import ( - ENABLE_TASKS, EXPLORER_ASYNC_SCHEMA, EXPLORER_CONNECTIONS, EXPLORER_SCHEMA_EXCLUDE_TABLE_PREFIXES, + EXPLORER_SCHEMA_EXCLUDE_TABLE_PREFIXES, EXPLORER_SCHEMA_INCLUDE_TABLE_PREFIXES, EXPLORER_SCHEMA_INCLUDE_VIEWS, ) from explorer.tasks import build_schema_cache_async -from explorer.utils import get_valid_connection, InvalidExplorerConnectionException +from explorer.utils import InvalidExplorerConnectionException # These wrappers make it easy to mock and test @@ -21,22 +21,18 @@ def _include_views(): return EXPLORER_SCHEMA_INCLUDE_VIEWS is True -def do_async(): - return ENABLE_TASKS and EXPLORER_ASYNC_SCHEMA - - def _include_table(t): if _get_includes() is not None: return any([t.startswith(p) for p in _get_includes()]) return not any([t.startswith(p) for p in _get_excludes()]) -def connection_schema_cache_key(connection_alias): - return f"_explorer_cache_key_{connection_alias}" +def connection_schema_cache_key(connection_id): + return f"_explorer_cache_key_{connection_id}" -def connection_schema_json_cache_key(connection_alias): - return f"_explorer_cache_key_json_{connection_alias}" +def connection_schema_json_cache_key(connection_id): + return f"_explorer_cache_key_json_{connection_id}" def transform_to_json_schema(schema_info): @@ -48,13 +44,13 @@ def transform_to_json_schema(schema_info): return json_schema -def schema_json_info(connection_alias): - key = connection_schema_json_cache_key(connection_alias) +def schema_json_info(db_connection): + key = connection_schema_json_cache_key(db_connection.id) ret = cache.get(key) if ret: return ret try: - si = schema_info(connection_alias) or [] + si = schema_info(db_connection) or [] except InvalidExplorerConnectionException: return [] json_schema = transform_to_json_schema(si) @@ -62,26 +58,24 @@ def schema_json_info(connection_alias): return json_schema -def schema_info(connection_alias): - key = connection_schema_cache_key(connection_alias) +def schema_info(db_connection): + key = connection_schema_cache_key(db_connection.id) ret = cache.get(key) if ret: return ret - if do_async(): - build_schema_cache_async.delay(connection_alias) else: - return build_schema_cache_async(connection_alias) + return build_schema_cache_async(db_connection.id) -def clear_schema_cache(connection_alias): - key = connection_schema_cache_key(connection_alias) +def clear_schema_cache(db_connection): + key = connection_schema_cache_key(db_connection.id) cache.delete(key) - key = connection_schema_json_cache_key(connection_alias) + key = connection_schema_json_cache_key(db_connection.id) cache.delete(key) -def build_schema_info(connection_alias): +def build_schema_info(db_connection): """ Construct schema information via engine-specific queries of the tables in the DB. @@ -98,7 +92,7 @@ def build_schema_info(connection_alias): ] """ - connection = get_valid_connection(connection_alias) + connection = db_connection.as_django_connection() ret = [] with connection.cursor() as cursor: tables_to_introspect = connection.introspection.table_names( @@ -125,7 +119,3 @@ def build_schema_info(connection_alias): return ret -def build_async_schemas(): - if do_async(): - for c in EXPLORER_CONNECTIONS: - schema_info(c) diff --git a/explorer/src/js/assistant.js b/explorer/src/js/assistant.js index e424b6b4..9e7c4690 100644 --- a/explorer/src/js/assistant.js +++ b/explorer/src/js/assistant.js @@ -103,7 +103,7 @@ function submitAssistantAsk() { const data = { sql: window.editor?.state.doc.toString() ?? null, - connection: document.getElementById("id_connection")?.value ?? null, + connection_id: document.getElementById("id_database_connection")?.value ?? null, assistant_request: document.getElementById("id_assistant_input")?.value ?? null, selected_tables: selectedTables, db_error: getErrorMessage() diff --git a/explorer/src/js/codemirror-config.js b/explorer/src/js/codemirror-config.js index 4343eb9c..a4539e49 100644 --- a/explorer/src/js/codemirror-config.js +++ b/explorer/src/js/codemirror-config.js @@ -55,7 +55,7 @@ function fetchAndShowSchema(view) { if (schema.hasOwnProperty(tableName)) { formattedSchema = JSON.stringify(schema[tableName], null, 2); } else { - formattedSchema = `Table '${tableName}' not found in schema for connection '${conn}'`; + formattedSchema = `Table '${tableName}' not found in schema for connection`; } displaySchemaTooltip(view, formattedSchema); }); diff --git a/explorer/src/js/explorer.js b/explorer/src/js/explorer.js index 69f3fb10..ad113de1 100644 --- a/explorer/src/js/explorer.js +++ b/explorer/src/js/explorer.js @@ -49,7 +49,7 @@ function selectConnection() { var connectionId = urlParams.get('connection'); if (connectionId) { - var connectionSelect = document.getElementById('id_connection'); + var connectionSelect = document.getElementById('id_database_connection'); if (connectionSelect) { connectionSelect.value = connectionId; } diff --git a/explorer/src/js/main.js b/explorer/src/js/main.js index 32c79947..4d698497 100644 --- a/explorer/src/js/main.js +++ b/explorer/src/js/main.js @@ -17,7 +17,8 @@ const route_initializers = { explorer_playground: () => import('./explorer').then(({ExplorerEditor}) => new ExplorerEditor('new')), explorer_schema: () => import('./schema').then(({setupSchema}) => setupSchema()), explorer_upload_create: () => import('./uploads').then(({setupUploads}) => setupUploads()), - explorer_connection_update: () => import('./uploads').then(({setupUploads}) => setupUploads()) + explorer_connection_update: () => import('./uploads').then(({setupUploads}) => setupUploads()), + explorer_connection_create: () => import('./uploads').then(({setupUploads}) => setupUploads()) }; document.addEventListener('DOMContentLoaded', function() { diff --git a/explorer/src/js/schemaService.js b/explorer/src/js/schemaService.js index 7f5e49bf..84e9e577 100644 --- a/explorer/src/js/schemaService.js +++ b/explorer/src/js/schemaService.js @@ -27,5 +27,5 @@ export const SchemaSvc = { }; export function getConnElement() { - return document.querySelector('#id_connection'); + return document.querySelector('#id_database_connection'); } diff --git a/explorer/tasks.py b/explorer/tasks.py index 0d4292c2..e694a22b 100644 --- a/explorer/tasks.py +++ b/explorer/tasks.py @@ -96,26 +96,33 @@ def truncate_querylogs(days): @shared_task -def build_schema_cache_async(connection_alias): +def build_schema_cache_async(db_connection_id): from .schema import ( build_schema_info, connection_schema_cache_key, connection_schema_json_cache_key, transform_to_json_schema, ) - - ret = build_schema_info(connection_alias) - cache.set(connection_schema_cache_key(connection_alias), ret) - cache.set(connection_schema_json_cache_key(connection_alias), transform_to_json_schema(ret)) + db_connection = DatabaseConnection.objects.get(id=db_connection_id) + ret = build_schema_info(db_connection) + cache.set(connection_schema_cache_key(db_connection.id), ret) + cache.set(connection_schema_json_cache_key(db_connection.id), transform_to_json_schema(ret)) return ret @shared_task def remove_unused_sqlite_dbs(): - uploaded_dbs = DatabaseConnection.objects.filter(engine=DatabaseConnection.SQLITE, host__isnull=False) - for db in uploaded_dbs: + days = app_settings.EXPLORER_PRUNE_LOCAL_UPLOAD_COPY_DAYS_INACTIVITY + t = timezone.make_aware(datetime.now() - timedelta(days=days), timezone.get_default_timezone()) + for db in DatabaseConnection.objects.uploads(): if os.path.exists(db.local_name): - recent_run = QueryLog.objects.filter(connection=db.alias).first() - days = app_settings.EXPLORER_PRUNE_LOCAL_UPLOAD_COPY_DAYS_INACTIVITY - if recent_run and (datetime.now() - timedelta(days=days)) > recent_run.run_at: + recent_run = QueryLog.objects.filter(database_connection=db).first() + if recent_run and t > recent_run.run_at: os.remove(db.local_name) + + +@shared_task +def build_async_schemas(): + from explorer.schema import schema_info + for c in DatabaseConnection.objects.non_uploads().all(): + schema_info(c.alias) diff --git a/explorer/telemetry.py b/explorer/telemetry.py index 80e3decd..7d818bf3 100644 --- a/explorer/telemetry.py +++ b/explorer/telemetry.py @@ -131,7 +131,7 @@ def _gather_summary_stats(): q_stats = Query.objects.aggregate( total_count=Count("*"), - unique_connection_count=Count("connection", distinct=True) + unique_connection_count=Count("database_connection_id", distinct=True) ) # Round the counts to provide additional anonymity diff --git a/explorer/templates/connections/connections.html b/explorer/templates/connections/connections.html index f0e350cb..beaffc73 100644 --- a/explorer/templates/connections/connections.html +++ b/explorer/templates/connections/connections.html @@ -29,7 +29,7 @@

Connections

{{ connection.name }} {{ connection.get_engine_display }}{% if connection.is_upload %} (uploaded){% endif %} - Query + Query {% if connection.id %} diff --git a/explorer/templates/connections/database_connection_form.html b/explorer/templates/connections/database_connection_form.html index 92e54a68..ef6d9399 100644 --- a/explorer/templates/connections/database_connection_form.html +++ b/explorer/templates/connections/database_connection_form.html @@ -19,7 +19,7 @@

{% if object %}Edit{% else %}Create New{% endif %} Connection

{{ form.name }} - + Required. The name of the database itself.
@@ -33,12 +33,10 @@

{% if object %}Edit{% else %}Create New{% endif %} Connection

{{ form.host }} - Required.
{{ form.port }} - Required.
{{ form.extras }} diff --git a/explorer/templates/explorer/play.html b/explorer/templates/explorer/play.html index 33aa700f..a9f1572b 100644 --- a/explorer/templates/explorer/play.html +++ b/explorer/templates/explorer/play.html @@ -18,13 +18,13 @@

{% translate "Playground" %}

{{ form.non_field_errors }} {% if form.connections|length > 1 and can_change %}
- {{ form.connection }} - + {{ form.database_connection }} +
{% else %} {# still need to submit the connection, just hide the UI element #} -