From 69da9754bfe4eb26e77fb4383c48028f8cab6d54 Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 00:07:53 -0400 Subject: [PATCH 01/16] Problem importing celery to enable tasks Issue #493 --- explorer/exporters.py | 2 +- explorer/models.py | 2 +- explorer/tasks.py | 33 ++++++++++++++++++++------------ explorer/tests/test_exporters.py | 4 ++-- explorer/tests/test_models.py | 8 ++++---- explorer/tests/test_tasks.py | 2 +- explorer/tests/test_views.py | 4 ++-- explorer/utils.py | 24 +++++++++++------------ requirements/optional.txt | 5 ++--- test_project/__init__.py | 3 +++ test_project/celery.py | 17 ++++++++++++++++ test_project/settings.py | 13 +++++-------- 12 files changed, 71 insertions(+), 46 deletions(-) create mode 100644 test_project/celery.py diff --git a/explorer/exporters.py b/explorer/exporters.py index 6e1480e7..3f06a2f2 100644 --- a/explorer/exporters.py +++ b/explorer/exporters.py @@ -83,7 +83,7 @@ def _get_output(self, res, **kwargs): ) json_data = json.dumps(data, cls=DjangoJSONEncoder) - return StringIO(json_data) + return BytesIO(bytes(json_data, 'utf-8')) class ExcelExporter(BaseExporter): diff --git a/explorer/models.py b/explorer/models.py index c1375094..e82d8d3f 100644 --- a/explorer/models.py +++ b/explorer/models.py @@ -137,7 +137,7 @@ def shared(self): def snapshots(self): if app_settings.ENABLE_TASKS: b = get_s3_bucket() - keys = b.list(prefix=f'query-{self.id}/snap-') + keys = b.objects.filter(Prefix=f'query-{self.id}/snap-') keys_s = sorted(keys, key=lambda k: k.last_modified) return [ SnapShot( diff --git a/explorer/tasks.py b/explorer/tasks.py index 21dc0413..c2405728 100644 --- a/explorer/tasks.py +++ b/explorer/tasks.py @@ -4,24 +4,24 @@ from django.core.mail import send_mail from django.core.cache import cache -from django.db import DatabaseError from explorer import app_settings from explorer.exporters import get_exporter_class from explorer.models import Query, QueryLog +import io if app_settings.ENABLE_TASKS: - from celery import task + from celery import shared_task from celery.utils.log import get_task_logger from explorer.utils import s3_upload logger = get_task_logger(__name__) else: - from explorer.utils import noop_decorator as task + from explorer.utils import noop_decorator as shared_task import logging logger = logging.getLogger(__name__) -@task +@shared_task def execute_query(query_id, email_address): q = Query.objects.get(pk=query_id) send_mail('[SQL Explorer] Your query is running...', @@ -36,17 +36,22 @@ def execute_query(query_id, email_address): ) for _ in range(20) ) try: - url = s3_upload(f'{random_part}.csv', exporter.get_file_output()) + # I am sure there is a much more efficient way to do this but boto3 expects a binary file basically + csv_file_io = exporter.get_file_output() + csv_file_io.seek(0) + csv_data: str = csv_file_io.read() + bio = io.BytesIO(bytes(csv_data, 'utf-8')) + url = s3_upload(f'{random_part}.csv', bio) subj = f'[SQL Explorer] Report "{q.title}" is ready' msg = f'Download results:\n\r{url}' - except DatabaseError as e: + except Exception as e: subj = f'[SQL Explorer] Error running report {q.title}' msg = f'Error: {e}\nPlease contact an administrator' - logger.warning(f'{subj}: {e}') + logger.exception(f'{subj}: {e}') send_mail(subj, msg, app_settings.FROM_EMAIL, [email_address]) -@task +@shared_task def snapshot_query(query_id): try: logger.info(f"Starting snapshot for query {query_id}...") @@ -57,7 +62,11 @@ def snapshot_query(query_id): date.today().strftime('%Y%m%d-%H:%M:%S') ) logger.info(f"Uploading snapshot for query {query_id} as {k}...") - url = s3_upload(k, exporter.get_file_output()) + csv_file_io = exporter.get_file_output() + csv_file_io.seek(0) + csv_data: str = csv_file_io.read() + bio = io.BytesIO(bytes(csv_data, 'utf-8')) + url = s3_upload(k, bio) logger.info( f"Done uploading snapshot for query {query_id}. URL: {url}" ) @@ -68,7 +77,7 @@ def snapshot_query(query_id): snapshot_query.retry() -@task +@shared_task def snapshot_queries(): logger.info("Starting query snapshots...") qs = Query.objects.filter(snapshot=True).values_list('id', flat=True) @@ -80,7 +89,7 @@ def snapshot_queries(): logger.info("Done creating tasks.") -@task +@shared_task def truncate_querylogs(days): qs = QueryLog.objects.filter( run_at__lt=datetime.now() - timedelta(days=days) @@ -92,7 +101,7 @@ def truncate_querylogs(days): logger.info('Done deleting QueryLog objects.') -@task +@shared_task def build_schema_cache_async(connection_alias): from .schema import build_schema_info, connection_schema_cache_key ret = build_schema_info(connection_alias) diff --git a/explorer/tests/test_exporters.py b/explorer/tests/test_exporters.py index a3203491..8d988b9d 100644 --- a/explorer/tests/test_exporters.py +++ b/explorer/tests/test_exporters.py @@ -59,7 +59,7 @@ def test_writing_json(self): res = JSONExporter(query=None)._get_output(res).getvalue() expected = [{'a': 1, '': None}, {'a': 'Jenét', '': '1'}] - self.assertEqual(res, json.dumps(expected)) + self.assertEqual(res.decode('utf-8'), json.dumps(expected)) def test_writing_datetimes(self): res = QueryResult( @@ -72,7 +72,7 @@ def test_writing_datetimes(self): res = JSONExporter(query=None)._get_output(res).getvalue() expected = [{'a': 1, 'b': date.today()}] - self.assertEqual(res, json.dumps(expected, cls=DjangoJSONEncoder)) + self.assertEqual(res.decode('utf-8'), json.dumps(expected, cls=DjangoJSONEncoder)) class TestExcel(TestCase): diff --git a/explorer/tests/test_models.py b/explorer/tests/test_models.py index 6bf78bfc..9889cb70 100644 --- a/explorer/tests/test_models.py +++ b/explorer/tests/test_models.py @@ -83,20 +83,20 @@ def test_log_saves_duration(self): @patch('explorer.models.get_s3_bucket') def test_get_snapshots_sorts_snaps(self, mocked_conn): conn = Mock() - conn.list = Mock() + conn.objects.filter = Mock() k1 = Mock() k1.generate_url.return_value = 'http://s3.com/foo' k1.last_modified = 'b' k2 = Mock() k2.generate_url.return_value = 'http://s3.com/bar' k2.last_modified = 'a' - conn.list.return_value = [k1, k2] + conn.objects.filter.return_value = [k1, k2] mocked_conn.return_value = conn q = SimpleQueryFactory() snaps = q.snapshots - self.assertEqual(conn.list.call_count, 1) + self.assertEqual(conn.objects.filter.call_count, 1) self.assertEqual(snaps[0].url, 'http://s3.com/bar') - conn.list.assert_called_once_with(prefix=f'query-{q.id}/snap-') + conn.objects.filter.assert_called_once_with(Prefix=f'query-{q.id}/snap-') def test_final_sql_uses_merged_params(self): q = SimpleQueryFactory(sql="select '$$foo:bar$$', '$$qux$$';") diff --git a/explorer/tests/test_tasks.py b/explorer/tests/test_tasks.py index a6df715c..225f007c 100644 --- a/explorer/tests/test_tasks.py +++ b/explorer/tests/test_tasks.py @@ -37,7 +37,7 @@ def test_async_results(self, mocked_upload): self.assertEqual( mocked_upload .call_args[0][1].getvalue() - .encode('utf-8').decode('utf-8-sig'), + .decode('utf-8-sig'), output.getvalue() ) self.assertEqual(mocked_upload.call_count, 1) diff --git a/explorer/tests/test_views.py b/explorer/tests/test_views.py index 90e47449..dec86acb 100644 --- a/explorer/tests/test_views.py +++ b/explorer/tests/test_views.py @@ -285,14 +285,14 @@ def test_user_query_views(self): @patch('explorer.models.get_s3_bucket') def test_query_snapshot_renders(self, mocked_conn): conn = Mock() - conn.list = Mock() + conn.objects.filter = Mock() k1 = Mock() k1.generate_url.return_value = 'http://s3.com/foo' k1.last_modified = '2015-01-01' k2 = Mock() k2.generate_url.return_value = 'http://s3.com/bar' k2.last_modified = '2015-01-02' - conn.list.return_value = [k1, k2] + conn.objects.filter.return_value = [k1, k2] mocked_conn.return_value = conn query = SimpleQueryFactory(sql="select 1;", snapshot=True) diff --git a/explorer/utils.py b/explorer/utils.py index 15fc80bc..023bcac8 100644 --- a/explorer/utils.py +++ b/explorer/utils.py @@ -2,6 +2,7 @@ from collections import deque from typing import Tuple, Iterable +import boto3 import sqlparse from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.forms import AuthenticationForm @@ -184,19 +185,18 @@ def get_valid_connection(alias=None): def get_s3_bucket(): - from boto.s3.connection import S3Connection - - conn = S3Connection(app_settings.S3_ACCESS_KEY, - app_settings.S3_SECRET_KEY) - return conn.get_bucket(app_settings.S3_BUCKET) + s3 = boto3.resource('s3', + aws_access_key_id=app_settings.S3_ACCESS_KEY, + aws_secret_access_key=app_settings.S3_SECRET_KEY) + return s3.Bucket(name=app_settings.S3_BUCKET) def s3_upload(key, data): - from boto.s3.key import Key bucket = get_s3_bucket() - k = Key(bucket) - k.key = key - k.set_contents_from_file(data, rewind=True) - k.set_acl('public-read') - k.set_metadata('Content-Type', 'text/csv') - return k.generate_url(expires_in=0, query_auth=False) + bucket.upload_fileobj(data, key, ExtraArgs={'ContentType': "text/csv"}) + + url = bucket.meta.client.generate_presigned_url( + ClientMethod='get_object', + Params={'Bucket': app_settings.S3_BUCKET, 'Key': key}, + ExpiresIn=3600) + return url diff --git a/requirements/optional.txt b/requirements/optional.txt index d32ed60f..c215542e 100644 --- a/requirements/optional.txt +++ b/requirements/optional.txt @@ -1,6 +1,5 @@ -celery>=3.1,<4.0 -boto>=2.49 -django-celery>=3.3.1 +celery>=4.0 +boto3>=1.20.0 xlsxwriter>=1.3.6 factory-boy>=3.1.0 matplotlib<4 diff --git a/test_project/__init__.py b/test_project/__init__.py index e69de29b..cd042640 100644 --- a/test_project/__init__.py +++ b/test_project/__init__.py @@ -0,0 +1,3 @@ +from .celery import app as celery_app + +__all__ = ['celery_app'] diff --git a/test_project/celery.py b/test_project/celery.py new file mode 100644 index 00000000..04c664f9 --- /dev/null +++ b/test_project/celery.py @@ -0,0 +1,17 @@ +import os + +from celery import Celery + +# Set the default Django settings module for the 'celery' program. +os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'test_project.settings') + +app = Celery('test_project') + +# Using a string here means the worker doesn't have to serialize +# the configuration object to child processes. +# - namespace='CELERY' means all celery-related configuration keys +# should have a `CELERY_` prefix. +app.config_from_object('django.conf:settings', namespace='CELERY') + +# Load task modules from all registered Django apps. +app.autodiscover_tasks() diff --git a/test_project/settings.py b/test_project/settings.py index 3f322836..0c30a3ec 100644 --- a/test_project/settings.py +++ b/test_project/settings.py @@ -1,5 +1,4 @@ import os -import djcelery SECRET_KEY = 'shhh' DEBUG = True @@ -34,8 +33,8 @@ } EXPLORER_CONNECTIONS = { - #'Postgres': 'postgres', - #'MySQL': 'mysql', + # 'Postgres': 'postgres', + # 'MySQL': 'mysql', 'SQLite': 'default', 'Another': 'alt' } @@ -68,7 +67,6 @@ 'django.contrib.staticfiles', 'django.contrib.admin', 'explorer', - 'djcelery' ) AUTHENTICATION_BACKENDS = ( @@ -83,11 +81,10 @@ 'django.contrib.messages.middleware.MessageMiddleware', ] -TEST_RUNNER = 'djcelery.contrib.test_runner.CeleryTestSuiteRunner' +CELERY_TASK_ALWAYS_EAGER = True -djcelery.setup_loader() -CELERY_ALWAYS_EAGER = True -BROKER_BACKEND = 'memory' +# added to help debug tasks +EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' # Explorer-specific From 4c44b3e71207861e80e56cc83ed7b4bdb95f2a17 Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 00:18:04 -0400 Subject: [PATCH 02/16] Problem importing celery to enable tasks Issue #493 Rename test project celery file to avoid clash with celery package --- test_project/__init__.py | 2 +- test_project/{celery.py => celery_config.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test_project/{celery.py => celery_config.py} (100%) diff --git a/test_project/__init__.py b/test_project/__init__.py index cd042640..f059bf00 100644 --- a/test_project/__init__.py +++ b/test_project/__init__.py @@ -1,3 +1,3 @@ -from .celery import app as celery_app +from .celery_config import app as celery_app __all__ = ['celery_app'] diff --git a/test_project/celery.py b/test_project/celery_config.py similarity index 100% rename from test_project/celery.py rename to test_project/celery_config.py From 891a2234b632dd09ea54c23ead0a4e798da3e806 Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 16:55:03 -0400 Subject: [PATCH 03/16] Problem importing celery to enable tasks Issue #493 Add presigned url setting Added utility method to convert csv file object to BytesIO for boto --- docs/features.rst | 2 +- docs/settings.rst | 11 +++++++++++ explorer/app_settings.py | 1 + explorer/tasks.py | 22 +++++++++++----------- explorer/utils.py | 2 +- 5 files changed, 25 insertions(+), 13 deletions(-) diff --git a/docs/features.rst b/docs/features.rst index 12d24d0b..218b6b9c 100644 --- a/docs/features.rst +++ b/docs/features.rst @@ -45,7 +45,7 @@ Snapshots 'schedule': crontab(hour=1, minute=0) } -- Requires celery, obviously. Also uses djcelery and tinys3. All +- Requires celery, obviously. Also uses boto3. All of these deps are optional and can be installed with ``pip install -r optional-requirements.txt`` - The checkbox for opting a query into a snapshot is ALL THE WAY diff --git a/docs/settings.rst b/docs/settings.rst index 6f1575a5..2d7bc2b7 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -228,6 +228,17 @@ S3 Bucket for snapshot upload EXPLORER_S3_BUCKET = None +S3 link expiration +****************** + +S3 link expiration time. Defaults to 3600 seconds (1hr) if not specified. +Links are generated as presigned urls for security + +.. code-block:: python + + EXPLORER_S3_S3_LINK_EXPIRATION = 3600 + + From email ********** diff --git a/explorer/app_settings.py b/explorer/app_settings.py index a1109753..0f321276 100644 --- a/explorer/app_settings.py +++ b/explorer/app_settings.py @@ -124,6 +124,7 @@ S3_ACCESS_KEY = getattr(settings, "EXPLORER_S3_ACCESS_KEY", None) S3_SECRET_KEY = getattr(settings, "EXPLORER_S3_SECRET_KEY", None) S3_BUCKET = getattr(settings, "EXPLORER_S3_BUCKET", None) +S3_LINK_EXPIRATION: int = getattr(settings, "EXPLORER_S3_S3_LINK_EXPIRATION", 3600) FROM_EMAIL = getattr( settings, 'EXPLORER_FROM_EMAIL', 'django-sql-explorer@example.com' ) diff --git a/explorer/tasks.py b/explorer/tasks.py index c2405728..3ffeb548 100644 --- a/explorer/tasks.py +++ b/explorer/tasks.py @@ -36,12 +36,7 @@ def execute_query(query_id, email_address): ) for _ in range(20) ) try: - # I am sure there is a much more efficient way to do this but boto3 expects a binary file basically - csv_file_io = exporter.get_file_output() - csv_file_io.seek(0) - csv_data: str = csv_file_io.read() - bio = io.BytesIO(bytes(csv_data, 'utf-8')) - url = s3_upload(f'{random_part}.csv', bio) + url = s3_upload(f'{random_part}.csv', convert_csv_to_bytesio(exporter)) subj = f'[SQL Explorer] Report "{q.title}" is ready' msg = f'Download results:\n\r{url}' except Exception as e: @@ -51,6 +46,15 @@ def execute_query(query_id, email_address): send_mail(subj, msg, app_settings.FROM_EMAIL, [email_address]) +# I am sure there is a much more efficient way to do this but boto3 expects a binary file basically +def convert_csv_to_bytesio(csv_exporter): + csv_file_io = csv_exporter.get_file_output() + csv_file_io.seek(0) + csv_data: str = csv_file_io.read() + bio = io.BytesIO(bytes(csv_data, 'utf-8')) + return bio + + @shared_task def snapshot_query(query_id): try: @@ -62,11 +66,7 @@ def snapshot_query(query_id): date.today().strftime('%Y%m%d-%H:%M:%S') ) logger.info(f"Uploading snapshot for query {query_id} as {k}...") - csv_file_io = exporter.get_file_output() - csv_file_io.seek(0) - csv_data: str = csv_file_io.read() - bio = io.BytesIO(bytes(csv_data, 'utf-8')) - url = s3_upload(k, bio) + url = s3_upload(k, convert_csv_to_bytesio(exporter)) logger.info( f"Done uploading snapshot for query {query_id}. URL: {url}" ) diff --git a/explorer/utils.py b/explorer/utils.py index 023bcac8..6f9b1fb0 100644 --- a/explorer/utils.py +++ b/explorer/utils.py @@ -198,5 +198,5 @@ def s3_upload(key, data): url = bucket.meta.client.generate_presigned_url( ClientMethod='get_object', Params={'Bucket': app_settings.S3_BUCKET, 'Key': key}, - ExpiresIn=3600) + ExpiresIn=app_settings.S3_LINK_EXPIRATION) return url From 630720b25502c58c1c3803b5cafa1250117f49bd Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 17:20:51 -0400 Subject: [PATCH 04/16] Problem importing celery to enable tasks Issue #493 Get presigned urls for snapshots --- explorer/models.py | 11 ++++++----- explorer/utils.py | 3 +++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/explorer/models.py b/explorer/models.py index e82d8d3f..c8335c96 100644 --- a/explorer/models.py +++ b/explorer/models.py @@ -13,6 +13,7 @@ extract_params, shared_dict_update, get_s3_bucket, + s3_url, get_params_for_url, get_valid_connection ) @@ -137,13 +138,13 @@ def shared(self): def snapshots(self): if app_settings.ENABLE_TASKS: b = get_s3_bucket() - keys = b.objects.filter(Prefix=f'query-{self.id}/snap-') - keys_s = sorted(keys, key=lambda k: k.last_modified) + objects = b.objects.filter(Prefix=f'query-{self.id}/snap-') + objects_s = sorted(objects, key=lambda k: k.last_modified) return [ SnapShot( - k.generate_url(expires_in=0, query_auth=False), - k.last_modified - ) for k in keys_s + s3_url(b, o.key), + o.last_modified + ) for o in objects_s ] diff --git a/explorer/utils.py b/explorer/utils.py index 6f9b1fb0..68550762 100644 --- a/explorer/utils.py +++ b/explorer/utils.py @@ -194,7 +194,10 @@ def get_s3_bucket(): def s3_upload(key, data): bucket = get_s3_bucket() bucket.upload_fileobj(data, key, ExtraArgs={'ContentType': "text/csv"}) + return s3_url(bucket, key) + +def s3_url(bucket, key): url = bucket.meta.client.generate_presigned_url( ClientMethod='get_object', Params={'Bucket': app_settings.S3_BUCKET, 'Key': key}, From 010b3e03f24d434aa04327fd0fdf4169b4d32216 Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 17:52:56 -0400 Subject: [PATCH 05/16] Problem importing celery to enable tasks Issue #493 Fix test for presigned urls --- explorer/tests/test_models.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/explorer/tests/test_models.py b/explorer/tests/test_models.py index 9889cb70..3b928f5b 100644 --- a/explorer/tests/test_models.py +++ b/explorer/tests/test_models.py @@ -80,23 +80,25 @@ def test_log_saves_duration(self): log = QueryLog.objects.first() self.assertEqual(log.duration, res.duration) + @patch('explorer.models.s3_url') @patch('explorer.models.get_s3_bucket') - def test_get_snapshots_sorts_snaps(self, mocked_conn): - conn = Mock() - conn.objects.filter = Mock() + def test_get_snapshots_sorts_snaps(self, mocked_get_s3_bucket, mocked_s3_url): + bucket = Mock() + bucket.objects.filter = Mock() k1 = Mock() - k1.generate_url.return_value = 'http://s3.com/foo' + k1.key = 'foo' k1.last_modified = 'b' k2 = Mock() - k2.generate_url.return_value = 'http://s3.com/bar' + k2.key = 'bar' k2.last_modified = 'a' - conn.objects.filter.return_value = [k1, k2] - mocked_conn.return_value = conn + bucket.objects.filter.return_value = [k1, k2] + mocked_get_s3_bucket.return_value = bucket + mocked_s3_url.return_value = 'http://s3.com/presigned_url' q = SimpleQueryFactory() snaps = q.snapshots - self.assertEqual(conn.objects.filter.call_count, 1) - self.assertEqual(snaps[0].url, 'http://s3.com/bar') - conn.objects.filter.assert_called_once_with(Prefix=f'query-{q.id}/snap-') + self.assertEqual(bucket.objects.filter.call_count, 1) + self.assertEqual(snaps[0].url, 'http://s3.com/presigned_url') + bucket.objects.filter.assert_called_once_with(Prefix=f'query-{q.id}/snap-') def test_final_sql_uses_merged_params(self): q = SimpleQueryFactory(sql="select '$$foo:bar$$', '$$qux$$';") From e9d70715f4da4f1ee36eb0b6ce5bc215a12fd752 Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sun, 6 Nov 2022 19:52:42 -0500 Subject: [PATCH 06/16] Problem importing celery to enable tasks Issue #493 Revert unneeded change to json exporter --- explorer/exporters.py | 2 +- explorer/tests/test_exporters.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/explorer/exporters.py b/explorer/exporters.py index 3f06a2f2..6e1480e7 100644 --- a/explorer/exporters.py +++ b/explorer/exporters.py @@ -83,7 +83,7 @@ def _get_output(self, res, **kwargs): ) json_data = json.dumps(data, cls=DjangoJSONEncoder) - return BytesIO(bytes(json_data, 'utf-8')) + return StringIO(json_data) class ExcelExporter(BaseExporter): diff --git a/explorer/tests/test_exporters.py b/explorer/tests/test_exporters.py index 8d988b9d..a3203491 100644 --- a/explorer/tests/test_exporters.py +++ b/explorer/tests/test_exporters.py @@ -59,7 +59,7 @@ def test_writing_json(self): res = JSONExporter(query=None)._get_output(res).getvalue() expected = [{'a': 1, '': None}, {'a': 'Jenét', '': '1'}] - self.assertEqual(res.decode('utf-8'), json.dumps(expected)) + self.assertEqual(res, json.dumps(expected)) def test_writing_datetimes(self): res = QueryResult( @@ -72,7 +72,7 @@ def test_writing_datetimes(self): res = JSONExporter(query=None)._get_output(res).getvalue() expected = [{'a': 1, 'b': date.today()}] - self.assertEqual(res.decode('utf-8'), json.dumps(expected, cls=DjangoJSONEncoder)) + self.assertEqual(res, json.dumps(expected, cls=DjangoJSONEncoder)) class TestExcel(TestCase): From fc03d6b71f0542dd953eae2f41704ce23ceaf1da Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sun, 6 Nov 2022 21:25:20 -0500 Subject: [PATCH 07/16] Problem importing celery to enable tasks Issue #493 Pin importlib-metadata to <5.0 for python < 3.7 to avoid celery import error https://github.com/python/importlib_metadata/issues/411 --- requirements/optional.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements/optional.txt b/requirements/optional.txt index c215542e..93cc8fe8 100644 --- a/requirements/optional.txt +++ b/requirements/optional.txt @@ -1,3 +1,4 @@ +importlib-metadata<5.0; python_version <= '3.7' celery>=4.0 boto3>=1.20.0 xlsxwriter>=1.3.6 From a04764fcd0e7546d0fefb7d50697d34485e088a5 Mon Sep 17 00:00:00 2001 From: Mark Walker Date: Fri, 2 Dec 2022 02:06:37 +0000 Subject: [PATCH 08/16] pre-commit hooks (#504) --- .github/workflows/docs.yml | 2 +- .github/workflows/lint.yml | 38 +++++++++++++++++ .github/workflows/test.yml | 5 +-- .gitignore | 1 + .pre-commit-config.yaml | 42 +++++++++++++++++++ docs/conf.py | 1 + explorer/__init__.py | 2 - explorer/actions.py | 4 +- explorer/app_settings.py | 4 +- explorer/charts.py | 5 ++- explorer/exporters.py | 3 +- explorer/forms.py | 10 ++--- explorer/migrations/0001_initial.py | 2 +- .../migrations/0002_auto_20150501_1515.py | 2 +- explorer/migrations/0003_query_snapshot.py | 2 +- explorer/migrations/0006_query_connection.py | 2 +- .../migrations/0007_querylog_connection.py | 2 +- explorer/models.py | 10 ++--- explorer/permissions.py | 2 +- explorer/schema.py | 8 +--- explorer/tasks.py | 9 ++-- explorer/tests/factories.py | 2 +- explorer/tests/test_actions.py | 1 - explorer/tests/test_apps.py | 1 - explorer/tests/test_csrf_cookie_name.py | 4 +- explorer/tests/test_exporters.py | 3 +- explorer/tests/test_forms.py | 1 - explorer/tests/test_models.py | 7 +--- explorer/tests/test_schema.py | 3 +- explorer/tests/test_tasks.py | 10 +---- explorer/tests/test_utils.py | 11 +++-- explorer/tests/test_views.py | 14 +++---- explorer/tests/urls.py | 1 + explorer/urls.py | 15 ++----- explorer/utils.py | 8 ++-- explorer/views/__init__.py | 6 +-- explorer/views/auth.py | 3 +- explorer/views/create.py | 1 - explorer/views/delete.py | 1 - explorer/views/download.py | 1 - explorer/views/email.py | 1 - explorer/views/export.py | 2 - explorer/views/format_sql.py | 1 - explorer/views/list.py | 6 +-- explorer/views/mixins.py | 3 +- explorer/views/query.py | 8 ++-- explorer/views/schema.py | 1 - explorer/views/stream.py | 1 - explorer/views/utils.py | 3 +- requirements/django-2.2.txt | 3 -- requirements/django-3.0.txt | 3 -- requirements/django-3.1.txt | 3 -- requirements/django-3.2.txt | 2 +- requirements/django-4.0.txt | 3 ++ requirements/django-4.1.txt | 3 ++ setup.cfg | 12 ++++++ setup.py | 16 +++---- test_project/settings.py | 6 ++- tox.ini | 36 ++++++++++++++++ 59 files changed, 220 insertions(+), 142 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100644 .pre-commit-config.yaml delete mode 100644 requirements/django-2.2.txt delete mode 100644 requirements/django-3.0.txt delete mode 100644 requirements/django-3.1.txt create mode 100644 requirements/django-4.0.txt create mode 100644 requirements/django-4.1.txt create mode 100644 tox.ini diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 870d4cc5..8c4afadf 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -17,4 +17,4 @@ jobs: - name: Build docs run: | cd docs - sphinx-build -b html -n -d _build/doctrees . _build/html + sphinx-build -b html -n -d _build/doctrees . _build/html diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..95abf0aa --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,38 @@ +name: Lint + +on: [push, pull_request] + +jobs: + flake8: + name: flake8 + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v3 + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: 3.9 + - name: Install flake8 + run: pip install --upgrade flake8 + - name: Run flake8 + uses: liskin/gh-problem-matcher-wrap@v2 + with: + linters: flake8 + run: flake8 + + isort: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v3 + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: 3.9 + - run: python -m pip install isort + - name: isort + uses: liskin/gh-problem-matcher-wrap@v2 + with: + linters: isort + run: isort --check --diff explorer diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 00c52b85..31c49810 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -8,11 +8,8 @@ jobs: strategy: fail-fast: false matrix: - python-version: [ 3.6, 3.7, 3.8] + python-version: [ 3.8] requirements-file: [ - django-2.2.txt, - django-3.0.txt, - django-3.1.txt, django-3.2.txt ] os: [ diff --git a/.gitignore b/.gitignore index 84984ad7..679afa06 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ tmp venv/ .venv/ +.tox/ # Sphinx documentation docs/_build/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..22729da6 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,42 @@ +ci: + autofix_commit_msg: | + ci: auto fixes from pre-commit hooks + + for more information, see https://pre-commit.ci + autofix_prs: false + autoupdate_commit_msg: 'ci: pre-commit autoupdate' + autoupdate_schedule: monthly + +repos: + - repo: https://github.com/asottile/pyupgrade + rev: v2.37.3 + hooks: + - id: pyupgrade + args: ["--py38-plus"] + + - repo: https://github.com/adamchainz/django-upgrade + rev: '1.7.0' + hooks: + - id: django-upgrade + args: [--target-version, "3.2"] + + - repo: https://github.com/PyCQA/flake8 + rev: 5.0.2 + hooks: + - id: flake8 + + - repo: https://github.com/asottile/yesqa + rev: v1.3.0 + hooks: + - id: yesqa + + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.3.0 + hooks: + - id: check-merge-conflict + - id: mixed-line-ending + + - repo: https://github.com/pycqa/isort + rev: 5.10.1 + hooks: + - id: isort diff --git a/docs/conf.py b/docs/conf.py index 9d132580..723920d3 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -16,6 +16,7 @@ import explorer + sys.path.insert(0, os.path.abspath('../')) sys.path.insert(0, os.path.abspath('.')) diff --git a/explorer/__init__.py b/explorer/__init__.py index dc9ee11b..51908cbe 100644 --- a/explorer/__init__.py +++ b/explorer/__init__.py @@ -22,5 +22,3 @@ def get_version(short=False): __version__ = get_version() - -default_app_config = 'explorer.apps.ExplorerAppConfig' diff --git a/explorer/actions.py b/explorer/actions.py index 2e20d817..bde25566 100644 --- a/explorer/actions.py +++ b/explorer/actions.py @@ -1,8 +1,8 @@ +import tempfile from collections import defaultdict from datetime import date -import tempfile -from zipfile import ZipFile from wsgiref.util import FileWrapper +from zipfile import ZipFile from django.http import HttpResponse diff --git a/explorer/app_settings.py b/explorer/app_settings.py index a1109753..0be4e6da 100644 --- a/explorer/app_settings.py +++ b/explorer/app_settings.py @@ -1,6 +1,8 @@ -from django.conf import settings from pydoc import locate +from django.conf import settings + + # The 'correct' configuration for Explorer looks like: # EXPLORER_CONNECTIONS = { diff --git a/explorer/charts.py b/explorer/charts.py index 45af1afd..218a7f5c 100644 --- a/explorer/charts.py +++ b/explorer/charts.py @@ -1,12 +1,13 @@ -from typing import Optional, Iterable from io import BytesIO +from typing import Iterable, Optional from django.core.exceptions import ImproperlyConfigured + try: import matplotlib.pyplot as plt - from matplotlib.figure import Figure import seaborn as sns + from matplotlib.figure import Figure except ImportError: from . import app_settings diff --git a/explorer/exporters.py b/explorer/exporters.py index 6e1480e7..b2fe66ed 100644 --- a/explorer/exporters.py +++ b/explorer/exporters.py @@ -1,10 +1,9 @@ import codecs import csv import json -import string import uuid from datetime import datetime -from io import StringIO, BytesIO +from io import BytesIO, StringIO from django.core.serializers.json import DjangoJSONEncoder from django.utils.module_loading import import_string diff --git a/explorer/forms.py b/explorer/forms.py index b59cec7f..6dcb2734 100644 --- a/explorer/forms.py +++ b/explorer/forms.py @@ -1,12 +1,8 @@ -from django.forms import ( - BooleanField, CharField, ModelForm, ValidationError -) +from django.forms import BooleanField, CharField, ModelForm, ValidationError from django.forms.widgets import CheckboxInput, Select -from explorer.app_settings import ( - EXPLORER_DEFAULT_CONNECTION, EXPLORER_CONNECTIONS -) -from explorer.models import Query, MSG_FAILED_BLACKLIST +from explorer.app_settings import EXPLORER_CONNECTIONS, EXPLORER_DEFAULT_CONNECTION +from explorer.models import MSG_FAILED_BLACKLIST, Query class SqlField(CharField): diff --git a/explorer/migrations/0001_initial.py b/explorer/migrations/0001_initial.py index 1d64aaa6..7bd4101f 100644 --- a/explorer/migrations/0001_initial.py +++ b/explorer/migrations/0001_initial.py @@ -1,6 +1,6 @@ -from django.db import models, migrations import django.db.models.deletion from django.conf import settings +from django.db import migrations, models class Migration(migrations.Migration): diff --git a/explorer/migrations/0002_auto_20150501_1515.py b/explorer/migrations/0002_auto_20150501_1515.py index 4f39d379..a4885690 100644 --- a/explorer/migrations/0002_auto_20150501_1515.py +++ b/explorer/migrations/0002_auto_20150501_1515.py @@ -1,4 +1,4 @@ -from django.db import models, migrations +from django.db import migrations, models class Migration(migrations.Migration): diff --git a/explorer/migrations/0003_query_snapshot.py b/explorer/migrations/0003_query_snapshot.py index 66800973..bede932d 100644 --- a/explorer/migrations/0003_query_snapshot.py +++ b/explorer/migrations/0003_query_snapshot.py @@ -1,4 +1,4 @@ -from django.db import models, migrations +from django.db import migrations, models class Migration(migrations.Migration): diff --git a/explorer/migrations/0006_query_connection.py b/explorer/migrations/0006_query_connection.py index 62a6a778..e360c45c 100644 --- a/explorer/migrations/0006_query_connection.py +++ b/explorer/migrations/0006_query_connection.py @@ -1,4 +1,4 @@ -from django.db import models, migrations +from django.db import migrations, models class Migration(migrations.Migration): diff --git a/explorer/migrations/0007_querylog_connection.py b/explorer/migrations/0007_querylog_connection.py index aec4ea90..aeef8067 100644 --- a/explorer/migrations/0007_querylog_connection.py +++ b/explorer/migrations/0007_querylog_connection.py @@ -1,4 +1,4 @@ -from django.db import models, migrations +from django.db import migrations, models class Migration(migrations.Migration): diff --git a/explorer/models.py b/explorer/models.py index c1375094..de78fce3 100644 --- a/explorer/models.py +++ b/explorer/models.py @@ -2,21 +2,17 @@ from time import time from django.conf import settings -from django.db import models, DatabaseError, transaction +from django.db import DatabaseError, models, transaction from django.urls import reverse from django.utils.translation import gettext_lazy as _ from explorer import app_settings from explorer.utils import ( - passes_blacklist, + extract_params, get_params_for_url, get_s3_bucket, get_valid_connection, passes_blacklist, shared_dict_update, swap_params, - extract_params, - shared_dict_update, - get_s3_bucket, - get_params_for_url, - get_valid_connection ) + MSG_FAILED_BLACKLIST = "Query failed the SQL blacklist: %s" diff --git a/explorer/permissions.py b/explorer/permissions.py index 7b63d96c..54493de6 100644 --- a/explorer/permissions.py +++ b/explorer/permissions.py @@ -6,7 +6,7 @@ def view_permission(request, **kwargs): return app_settings.EXPLORER_PERMISSION_VIEW(request)\ or user_can_see_query(request, **kwargs)\ or (app_settings.EXPLORER_TOKEN_AUTH_ENABLED() - and (request.META.get('HTTP_X_API_TOKEN') == + and (request.headers.get('X-Api-Token') == app_settings.EXPLORER_TOKEN or request.GET.get('token') == app_settings.EXPLORER_TOKEN)) diff --git a/explorer/schema.py b/explorer/schema.py index c636711d..a58180ef 100644 --- a/explorer/schema.py +++ b/explorer/schema.py @@ -1,12 +1,8 @@ from django.core.cache import cache from explorer.app_settings import ( - EXPLORER_SCHEMA_INCLUDE_TABLE_PREFIXES, - EXPLORER_SCHEMA_EXCLUDE_TABLE_PREFIXES, - EXPLORER_SCHEMA_INCLUDE_VIEWS, - ENABLE_TASKS, - EXPLORER_ASYNC_SCHEMA, - EXPLORER_CONNECTIONS + ENABLE_TASKS, EXPLORER_ASYNC_SCHEMA, EXPLORER_CONNECTIONS, 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 diff --git a/explorer/tasks.py b/explorer/tasks.py index 21dc0413..421e844a 100644 --- a/explorer/tasks.py +++ b/explorer/tasks.py @@ -1,23 +1,26 @@ -from datetime import date, datetime, timedelta import random import string +from datetime import date, datetime, timedelta -from django.core.mail import send_mail from django.core.cache import cache +from django.core.mail import send_mail from django.db import DatabaseError from explorer import app_settings from explorer.exporters import get_exporter_class from explorer.models import Query, QueryLog + if app_settings.ENABLE_TASKS: from celery import task from celery.utils.log import get_task_logger + from explorer.utils import s3_upload logger = get_task_logger(__name__) else: - from explorer.utils import noop_decorator as task import logging + + from explorer.utils import noop_decorator as task logger = logging.getLogger(__name__) diff --git a/explorer/tests/factories.py b/explorer/tests/factories.py index 6800674d..1d100891 100644 --- a/explorer/tests/factories.py +++ b/explorer/tests/factories.py @@ -1,5 +1,5 @@ -# -*- coding: utf-8 -*- from django.conf import settings + from factory import Sequence, SubFactory from factory.django import DjangoModelFactory diff --git a/explorer/tests/test_actions.py b/explorer/tests/test_actions.py index 6fd40b76..aa67e239 100644 --- a/explorer/tests/test_actions.py +++ b/explorer/tests/test_actions.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- import io from zipfile import ZipFile diff --git a/explorer/tests/test_apps.py b/explorer/tests/test_apps.py index fa6e835b..766c42c8 100644 --- a/explorer/tests/test_apps.py +++ b/explorer/tests/test_apps.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from unittest.mock import patch from django.core.exceptions import ImproperlyConfigured diff --git a/explorer/tests/test_csrf_cookie_name.py b/explorer/tests/test_csrf_cookie_name.py index fbeddc13..607e3802 100644 --- a/explorer/tests/test_csrf_cookie_name.py +++ b/explorer/tests/test_csrf_cookie_name.py @@ -1,11 +1,13 @@ from django.test import TestCase, override_settings + + try: from django.urls import reverse except ImportError: from django.core.urlresolvers import reverse -from django.contrib.auth.models import User from django.conf import settings +from django.contrib.auth.models import User class TestCsrfCookieName(TestCase): diff --git a/explorer/tests/test_exporters.py b/explorer/tests/test_exporters.py index a3203491..78588cba 100644 --- a/explorer/tests/test_exporters.py +++ b/explorer/tests/test_exporters.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- import json from datetime import date, datetime @@ -8,7 +7,7 @@ from django.utils import timezone from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION as CONN -from explorer.exporters import CSVExporter, JSONExporter, ExcelExporter +from explorer.exporters import CSVExporter, ExcelExporter, JSONExporter from explorer.models import QueryResult from explorer.tests.factories import SimpleQueryFactory diff --git a/explorer/tests/test_forms.py b/explorer/tests/test_forms.py index 407b87f2..8ab9b1c3 100644 --- a/explorer/tests/test_forms.py +++ b/explorer/tests/test_forms.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from django.db.utils import IntegrityError from django.forms.models import model_to_dict from django.test import TestCase diff --git a/explorer/tests/test_models.py b/explorer/tests/test_models.py index 6bf78bfc..85cbad76 100644 --- a/explorer/tests/test_models.py +++ b/explorer/tests/test_models.py @@ -1,13 +1,10 @@ -# -*- coding: utf-8 -*- -from unittest.mock import patch, Mock +from unittest.mock import Mock, patch from django.db import connections from django.test import TestCase from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION as CONN -from explorer.models import ( - QueryLog, Query, QueryResult, ColumnSummary, ColumnHeader -) +from explorer.models import ColumnHeader, ColumnSummary, Query, QueryLog, QueryResult from explorer.tests.factories import SimpleQueryFactory diff --git a/explorer/tests/test_schema.py b/explorer/tests/test_schema.py index 73d1f43a..c3c6adeb 100644 --- a/explorer/tests/test_schema.py +++ b/explorer/tests/test_schema.py @@ -1,12 +1,11 @@ -# -*- coding: utf-8 -*- from unittest.mock import patch from django.core.cache import cache from django.db import connection from django.test import TestCase -from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION as CONN from explorer import schema +from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION as CONN class TestSchemaInfo(TestCase): diff --git a/explorer/tests/test_tasks.py b/explorer/tests/test_tasks.py index a6df715c..9a902666 100644 --- a/explorer/tests/test_tasks.py +++ b/explorer/tests/test_tasks.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from datetime import datetime, timedelta from io import StringIO from unittest.mock import patch @@ -8,10 +7,7 @@ from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION as CONN from explorer.models import QueryLog -from explorer.tasks import ( - execute_query, snapshot_queries, truncate_querylogs, - build_schema_cache_async -) +from explorer.tasks import build_schema_cache_async, execute_query, snapshot_queries, truncate_querylogs from explorer.tests.factories import SimpleQueryFactory @@ -35,9 +31,7 @@ def test_async_results(self, mocked_upload): ) self.assertIn('[SQL Explorer] Report ', mail.outbox[1].subject) self.assertEqual( - mocked_upload - .call_args[0][1].getvalue() - .encode('utf-8').decode('utf-8-sig'), + mocked_upload.call_args[0][1].getvalue().encode('utf-8').decode('utf-8-sig'), output.getvalue() ) self.assertEqual(mocked_upload.call_count, 1) diff --git a/explorer/tests/test_utils.py b/explorer/tests/test_utils.py index cae2be98..0dc986b1 100644 --- a/explorer/tests/test_utils.py +++ b/explorer/tests/test_utils.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from unittest.mock import Mock from django.test import TestCase @@ -6,9 +5,8 @@ from explorer import app_settings from explorer.tests.factories import SimpleQueryFactory from explorer.utils import ( - passes_blacklist, param, swap_params, extract_params, - shared_dict_update, EXPLORER_PARAM_TOKEN, get_params_from_request, - get_params_for_url + EXPLORER_PARAM_TOKEN, extract_params, get_params_for_url, get_params_from_request, param, passes_blacklist, + shared_dict_update, swap_params, ) @@ -262,8 +260,9 @@ def test_get_params_for_request_empty(self): class TestConnections(TestCase): def test_only_registered_connections_are_in_connections(self): - from explorer.connections import connections - from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION from django.db import connections as djcs + + from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION + from explorer.connections import connections self.assertTrue(EXPLORER_DEFAULT_CONNECTION in connections) self.assertNotEqual(len(connections), len([c for c in djcs])) diff --git a/explorer/tests/test_views.py b/explorer/tests/test_views.py index 90e47449..1d4360fd 100644 --- a/explorer/tests/test_views.py +++ b/explorer/tests/test_views.py @@ -12,12 +12,10 @@ from django.urls import reverse from explorer import app_settings -from explorer.app_settings import ( - EXPLORER_DEFAULT_CONNECTION as CONN, - EXPLORER_TOKEN -) -from explorer.models import Query, QueryLog, MSG_FAILED_BLACKLIST -from explorer.tests.factories import SimpleQueryFactory, QueryLogFactory +from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION as CONN +from explorer.app_settings import EXPLORER_TOKEN +from explorer.models import MSG_FAILED_BLACKLIST, Query, QueryLog +from explorer.tests.factories import QueryLogFactory, SimpleQueryFactory from explorer.utils import user_can_see_query @@ -219,7 +217,7 @@ def test_admin_required_with_explorer_no_permission_setting(self): reverse("query_detail", kwargs={'query_id': query.id}) ) self.assertRedirects( - resp, f'/custom/login', + resp, '/custom/login', target_status_code=404 ) @@ -484,7 +482,7 @@ def test_admin_required_with_no_permission_view_setting(self): resp = self.client.get(reverse("explorer_playground")) self.assertRedirects( resp, - f'/custom/login', + '/custom/login', target_status_code=404 ) diff --git a/explorer/tests/urls.py b/explorer/tests/urls.py index 6d20f1df..9f13a574 100644 --- a/explorer/tests/urls.py +++ b/explorer/tests/urls.py @@ -4,6 +4,7 @@ from explorer.urls import urlpatterns + admin.autodiscover() urlpatterns += [ diff --git a/explorer/urls.py b/explorer/urls.py index c9e7e298..ed75ad37 100644 --- a/explorer/urls.py +++ b/explorer/urls.py @@ -1,20 +1,11 @@ from django.urls import path, re_path from explorer.views import ( - QueryView, - CreateQueryView, - PlayQueryView, - DeleteQueryView, - ListQueryView, - ListQueryLogView, - DownloadFromSqlView, - DownloadQueryView, - StreamQueryView, - EmailCsvQueryView, - SchemaView, - format_sql, + CreateQueryView, DeleteQueryView, DownloadFromSqlView, DownloadQueryView, EmailCsvQueryView, ListQueryLogView, + ListQueryView, PlayQueryView, QueryView, SchemaView, StreamQueryView, format_sql, ) + urlpatterns = [ path( '/', QueryView.as_view(), name='query_detail' diff --git a/explorer/utils.py b/explorer/utils.py index 15fc80bc..f332ddb4 100644 --- a/explorer/utils.py +++ b/explorer/utils.py @@ -1,17 +1,19 @@ import re from collections import deque -from typing import Tuple, Iterable +from typing import Iterable, Tuple -import sqlparse from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.forms import AuthenticationForm from django.contrib.auth.views import LoginView + +import sqlparse from sqlparse import format as sql_format -from sqlparse.sql import TokenList, Token +from sqlparse.sql import Token, TokenList from sqlparse.tokens import Keyword from explorer import app_settings + EXPLORER_PARAM_TOKEN = "$$" diff --git a/explorer/views/__init__.py b/explorer/views/__init__.py index e2a5775c..c30ed0b2 100644 --- a/explorer/views/__init__.py +++ b/explorer/views/__init__.py @@ -1,12 +1,10 @@ -# -*- coding: utf-8 -*- - from .auth import PermissionRequiredMixin, SafeLoginView from .create import CreateQueryView from .delete import DeleteQueryView -from .download import DownloadQueryView, DownloadFromSqlView +from .download import DownloadFromSqlView, DownloadQueryView from .email import EmailCsvQueryView from .format_sql import format_sql -from .list import ListQueryView, ListQueryLogView +from .list import ListQueryLogView, ListQueryView from .query import PlayQueryView, QueryView from .schema import SchemaView from .stream import StreamQueryView diff --git a/explorer/views/auth.py b/explorer/views/auth.py index d1718c47..1319c1ed 100644 --- a/explorer/views/auth.py +++ b/explorer/views/auth.py @@ -1,9 +1,8 @@ -# -*- coding: utf-8 -*- from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.views import LoginView from django.core.exceptions import ImproperlyConfigured -from explorer import permissions, app_settings +from explorer import app_settings, permissions class PermissionRequiredMixin: diff --git a/explorer/views/create.py b/explorer/views/create.py index 82c4540d..cad9c26c 100644 --- a/explorer/views/create.py +++ b/explorer/views/create.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from django.views.generic import CreateView from explorer.forms import QueryForm diff --git a/explorer/views/delete.py b/explorer/views/delete.py index d84295f8..27862233 100644 --- a/explorer/views/delete.py +++ b/explorer/views/delete.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from django.urls import reverse_lazy from django.views.generic import DeleteView diff --git a/explorer/views/download.py b/explorer/views/download.py index 13616425..0929dacb 100644 --- a/explorer/views/download.py +++ b/explorer/views/download.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from django.shortcuts import get_object_or_404 from django.views.generic.base import View diff --git a/explorer/views/email.py b/explorer/views/email.py index 51de79d2..8a739f66 100644 --- a/explorer/views/email.py +++ b/explorer/views/email.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from django.http import JsonResponse from django.views import View diff --git a/explorer/views/export.py b/explorer/views/export.py index 9689e130..80db3f00 100644 --- a/explorer/views/export.py +++ b/explorer/views/export.py @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - from django.db import DatabaseError from django.http import HttpResponse diff --git a/explorer/views/format_sql.py b/explorer/views/format_sql.py index 1999e281..d371b0d6 100644 --- a/explorer/views/format_sql.py +++ b/explorer/views/format_sql.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from django.http import JsonResponse from django.views.decorators.http import require_POST diff --git a/explorer/views/list.py b/explorer/views/list.py index 901eac3b..7e32d71b 100644 --- a/explorer/views/list.py +++ b/explorer/views/list.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- import re from collections import Counter @@ -7,10 +6,7 @@ from explorer import app_settings from explorer.models import Query, QueryLog -from explorer.utils import ( - url_get_query_id, - allowed_query_pks -) +from explorer.utils import allowed_query_pks, url_get_query_id from explorer.views.auth import PermissionRequiredMixin from explorer.views.mixins import ExplorerContextMixin diff --git a/explorer/views/mixins.py b/explorer/views/mixins.py index 6726ed3d..384578a2 100644 --- a/explorer/views/mixins.py +++ b/explorer/views/mixins.py @@ -1,6 +1,5 @@ -# -*- coding: utf-8 -*- -from django.shortcuts import render from django.conf import settings +from django.shortcuts import render from explorer import app_settings diff --git a/explorer/views/query.py b/explorer/views/query.py index 9e519776..1c17b3ae 100644 --- a/explorer/views/query.py +++ b/explorer/views/query.py @@ -1,21 +1,19 @@ -# -*- coding: utf-8 -*- from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy +from django.utils.translation import gettext_lazy as _ from django.views import View from explorer import app_settings from explorer.forms import QueryForm -from explorer.models import Query, QueryLog, MSG_FAILED_BLACKLIST +from explorer.models import MSG_FAILED_BLACKLIST, Query, QueryLog from explorer.utils import ( - url_get_query_id, url_get_log_id, url_get_show, - url_get_rows, url_get_fullscreen, url_get_params + url_get_fullscreen, url_get_log_id, url_get_params, url_get_query_id, url_get_rows, url_get_show, ) from explorer.views.auth import PermissionRequiredMixin from explorer.views.mixins import ExplorerContextMixin from explorer.views.utils import query_viewmodel -from django.utils.translation import gettext_lazy as _ class PlayQueryView(PermissionRequiredMixin, ExplorerContextMixin, View): diff --git a/explorer/views/schema.py b/explorer/views/schema.py index e1c38944..bd8ad28a 100644 --- a/explorer/views/schema.py +++ b/explorer/views/schema.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from django.http import Http404 from django.shortcuts import render from django.utils.decorators import method_decorator diff --git a/explorer/views/stream.py b/explorer/views/stream.py index 02d7cef6..d82b5c42 100644 --- a/explorer/views/stream.py +++ b/explorer/views/stream.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from django.shortcuts import get_object_or_404 from django.views import View diff --git a/explorer/views/utils.py b/explorer/views/utils.py index 557da94a..2caa8460 100644 --- a/explorer/views/utils.py +++ b/explorer/views/utils.py @@ -1,8 +1,7 @@ -# -*- coding: utf-8 -*- from django.db import DatabaseError from explorer import app_settings -from explorer.charts import get_pie_chart, get_line_chart +from explorer.charts import get_line_chart, get_pie_chart def query_viewmodel(request, query, title=None, form=None, message=None, diff --git a/requirements/django-2.2.txt b/requirements/django-2.2.txt deleted file mode 100644 index d656a3a5..00000000 --- a/requirements/django-2.2.txt +++ /dev/null @@ -1,3 +0,0 @@ --r base.txt - -django>=2.2,<3.0 diff --git a/requirements/django-3.0.txt b/requirements/django-3.0.txt deleted file mode 100644 index e61d8dfb..00000000 --- a/requirements/django-3.0.txt +++ /dev/null @@ -1,3 +0,0 @@ --r base.txt - -django>=3.0,<3.1 diff --git a/requirements/django-3.1.txt b/requirements/django-3.1.txt deleted file mode 100644 index 3293cb2c..00000000 --- a/requirements/django-3.1.txt +++ /dev/null @@ -1,3 +0,0 @@ --r base.txt - -django>=3.1,<3.2 diff --git a/requirements/django-3.2.txt b/requirements/django-3.2.txt index 7528ce3c..c013bdab 100644 --- a/requirements/django-3.2.txt +++ b/requirements/django-3.2.txt @@ -1,3 +1,3 @@ -r base.txt -django>=3.2,<3.3 +django>=3.2,<4.0 diff --git a/requirements/django-4.0.txt b/requirements/django-4.0.txt new file mode 100644 index 00000000..20d23b9c --- /dev/null +++ b/requirements/django-4.0.txt @@ -0,0 +1,3 @@ +-r base.txt + +django>=4.0,<4.1 diff --git a/requirements/django-4.1.txt b/requirements/django-4.1.txt new file mode 100644 index 00000000..e0f070cd --- /dev/null +++ b/requirements/django-4.1.txt @@ -0,0 +1,3 @@ +-r base.txt + +django>=4.1rc1,<4.2 diff --git a/setup.cfg b/setup.cfg index cc60538b..c7738e61 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,6 +6,7 @@ exclude = .git, .settings, .tox, + .venv, build, data, dist, @@ -13,3 +14,14 @@ exclude = *migrations*, requirements, tmp + +[isort] +line_length = 119 +skip = manage.py, *migrations*, .tox, .eggs, data, .env, .venv +include_trailing_comma = true +multi_line_output = 5 +lines_after_imports = 2 +default_section = THIRDPARTY +sections = FUTURE, STDLIB, DJANGO, THIRDPARTY, FIRSTPARTY, LOCALFOLDER +known_first_party = explorer +known_django = django diff --git a/setup.py b/setup.py index 29eb5d3c..35b016b9 100644 --- a/setup.py +++ b/setup.py @@ -3,6 +3,8 @@ from pathlib import Path from setuptools import setup + + try: from sphinx.setup_command import BuildDoc except ImportError: @@ -10,6 +12,7 @@ from explorer import get_version + # Utility function to read the README file. # Used for the long_description. It's nice, because now 1) we have a top level # README file and 2) it's easier to type in the README file than to put a raw @@ -67,19 +70,18 @@ def read(fname): 'Intended Audience :: Developers', 'License :: OSI Approved :: MIT License', 'Topic :: Utilities', - 'Framework :: Django :: 2.2', - 'Framework :: Django :: 3.0', - 'Framework :: Django :: 3.1', 'Framework :: Django :: 3.2', + 'Framework :: Django :: 4.0', + 'Framework :: Django :: 4.1', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.6', - 'Programming Language :: Python :: 3.7', 'Programming Language :: Python :: 3.8', + 'Programming Language :: Python :: 3.9', + 'Programming Language :: Python :: 3.10', 'Programming Language :: Python :: 3 :: Only', ], - python_requires='>=3.6', + python_requires='>=3.8', install_requires=[ - 'Django>=2.2.27', + 'Django>=3.2', 'sqlparse>=0.4.0', ], extras_require={ diff --git a/test_project/settings.py b/test_project/settings.py index 3f322836..049cb5e8 100644 --- a/test_project/settings.py +++ b/test_project/settings.py @@ -1,6 +1,8 @@ import os + import djcelery + SECRET_KEY = 'shhh' DEBUG = True STATIC_URL = '/static/' @@ -34,8 +36,8 @@ } EXPLORER_CONNECTIONS = { - #'Postgres': 'postgres', - #'MySQL': 'mysql', + # 'Postgres': 'postgres', + # 'MySQL': 'mysql', 'SQLite': 'default', 'Another': 'alt' } diff --git a/tox.ini b/tox.ini new file mode 100644 index 00000000..54498f00 --- /dev/null +++ b/tox.ini @@ -0,0 +1,36 @@ +[tox] +envlist = + flake8 + isort + py{36}-dj{32} +; py{38,39,310}-dj{32,40,41,main} +; py{310,311}-dj{41,main} + +skip_missing_interpreters=True + +[testenv] +deps = + -r{toxinidir}/requirements/base.txt + -r{toxinidir}/requirements/optional.txt + dj32: Django>=3.2,<3.3 +; dj40: Django>=4.0,<4.1 +; dj41: Django>=4.1,<4.2 +; djmain: https://github.com/django/django/archive/main.tar.gz +commands = + {envpython} --version + {env:COMMAND:coverage} erase + {env:COMMAND:coverage} run manage.py test + {env:COMMAND:coverage} report +ignore_outcome = + djmain: True +ignore_errors = + djmain: True + +[testenv:flake8] +deps = flake8 +commands = flake8 + +[testenv:isort] +deps = isort +commands = isort --check --diff explorer +skip_install = true From 450496318111c0a2fc43ce0fe6860e12dd3e1b3e Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 00:07:53 -0400 Subject: [PATCH 09/16] Problem importing celery to enable tasks Issue #493 --- explorer/exporters.py | 2 +- explorer/models.py | 2 +- explorer/tasks.py | 38 +++++++++++++++++++------------- explorer/tests/test_exporters.py | 4 ++-- explorer/tests/test_models.py | 8 +++---- explorer/tests/test_tasks.py | 10 +++++++-- explorer/tests/test_views.py | 4 ++-- explorer/utils.py | 31 +++++++++++++------------- requirements/optional.txt | 5 ++--- test_project/__init__.py | 3 +++ test_project/celery.py | 17 ++++++++++++++ test_project/settings.py | 10 +++------ 12 files changed, 81 insertions(+), 53 deletions(-) create mode 100644 test_project/celery.py diff --git a/explorer/exporters.py b/explorer/exporters.py index b2fe66ed..a3653e76 100644 --- a/explorer/exporters.py +++ b/explorer/exporters.py @@ -82,7 +82,7 @@ def _get_output(self, res, **kwargs): ) json_data = json.dumps(data, cls=DjangoJSONEncoder) - return StringIO(json_data) + return BytesIO(bytes(json_data, 'utf-8')) class ExcelExporter(BaseExporter): diff --git a/explorer/models.py b/explorer/models.py index de78fce3..d810d5d9 100644 --- a/explorer/models.py +++ b/explorer/models.py @@ -133,7 +133,7 @@ def shared(self): def snapshots(self): if app_settings.ENABLE_TASKS: b = get_s3_bucket() - keys = b.list(prefix=f'query-{self.id}/snap-') + keys = b.objects.filter(Prefix=f'query-{self.id}/snap-') keys_s = sorted(keys, key=lambda k: k.last_modified) return [ SnapShot( diff --git a/explorer/tasks.py b/explorer/tasks.py index 421e844a..174315c4 100644 --- a/explorer/tasks.py +++ b/explorer/tasks.py @@ -1,30 +1,29 @@ +from datetime import date, datetime, timedelta import random import string -from datetime import date, datetime, timedelta -from django.core.cache import cache from django.core.mail import send_mail -from django.db import DatabaseError +from django.core.cache import cache from explorer import app_settings from explorer.exporters import get_exporter_class from explorer.models import Query, QueryLog +import io if app_settings.ENABLE_TASKS: - from celery import task + from celery import shared_task from celery.utils.log import get_task_logger from explorer.utils import s3_upload logger = get_task_logger(__name__) else: + from explorer.utils import noop_decorator as shared_task import logging - - from explorer.utils import noop_decorator as task logger = logging.getLogger(__name__) -@task +@shared_task def execute_query(query_id, email_address): q = Query.objects.get(pk=query_id) send_mail('[SQL Explorer] Your query is running...', @@ -39,17 +38,22 @@ def execute_query(query_id, email_address): ) for _ in range(20) ) try: - url = s3_upload(f'{random_part}.csv', exporter.get_file_output()) + # I am sure there is a much more efficient way to do this but boto3 expects a binary file basically + csv_file_io = exporter.get_file_output() + csv_file_io.seek(0) + csv_data: str = csv_file_io.read() + bio = io.BytesIO(bytes(csv_data, 'utf-8')) + url = s3_upload(f'{random_part}.csv', bio) subj = f'[SQL Explorer] Report "{q.title}" is ready' msg = f'Download results:\n\r{url}' - except DatabaseError as e: + except Exception as e: subj = f'[SQL Explorer] Error running report {q.title}' msg = f'Error: {e}\nPlease contact an administrator' - logger.warning(f'{subj}: {e}') + logger.exception(f'{subj}: {e}') send_mail(subj, msg, app_settings.FROM_EMAIL, [email_address]) -@task +@shared_task def snapshot_query(query_id): try: logger.info(f"Starting snapshot for query {query_id}...") @@ -60,7 +64,11 @@ def snapshot_query(query_id): date.today().strftime('%Y%m%d-%H:%M:%S') ) logger.info(f"Uploading snapshot for query {query_id} as {k}...") - url = s3_upload(k, exporter.get_file_output()) + csv_file_io = exporter.get_file_output() + csv_file_io.seek(0) + csv_data: str = csv_file_io.read() + bio = io.BytesIO(bytes(csv_data, 'utf-8')) + url = s3_upload(k, bio) logger.info( f"Done uploading snapshot for query {query_id}. URL: {url}" ) @@ -71,7 +79,7 @@ def snapshot_query(query_id): snapshot_query.retry() -@task +@shared_task def snapshot_queries(): logger.info("Starting query snapshots...") qs = Query.objects.filter(snapshot=True).values_list('id', flat=True) @@ -83,7 +91,7 @@ def snapshot_queries(): logger.info("Done creating tasks.") -@task +@shared_task def truncate_querylogs(days): qs = QueryLog.objects.filter( run_at__lt=datetime.now() - timedelta(days=days) @@ -95,7 +103,7 @@ def truncate_querylogs(days): logger.info('Done deleting QueryLog objects.') -@task +@shared_task def build_schema_cache_async(connection_alias): from .schema import build_schema_info, connection_schema_cache_key ret = build_schema_info(connection_alias) diff --git a/explorer/tests/test_exporters.py b/explorer/tests/test_exporters.py index 78588cba..adf2f957 100644 --- a/explorer/tests/test_exporters.py +++ b/explorer/tests/test_exporters.py @@ -58,7 +58,7 @@ def test_writing_json(self): res = JSONExporter(query=None)._get_output(res).getvalue() expected = [{'a': 1, '': None}, {'a': 'Jenét', '': '1'}] - self.assertEqual(res, json.dumps(expected)) + self.assertEqual(res.decode('utf-8'), json.dumps(expected)) def test_writing_datetimes(self): res = QueryResult( @@ -71,7 +71,7 @@ def test_writing_datetimes(self): res = JSONExporter(query=None)._get_output(res).getvalue() expected = [{'a': 1, 'b': date.today()}] - self.assertEqual(res, json.dumps(expected, cls=DjangoJSONEncoder)) + self.assertEqual(res.decode('utf-8'), json.dumps(expected, cls=DjangoJSONEncoder)) class TestExcel(TestCase): diff --git a/explorer/tests/test_models.py b/explorer/tests/test_models.py index 85cbad76..7643244e 100644 --- a/explorer/tests/test_models.py +++ b/explorer/tests/test_models.py @@ -80,20 +80,20 @@ def test_log_saves_duration(self): @patch('explorer.models.get_s3_bucket') def test_get_snapshots_sorts_snaps(self, mocked_conn): conn = Mock() - conn.list = Mock() + conn.objects.filter = Mock() k1 = Mock() k1.generate_url.return_value = 'http://s3.com/foo' k1.last_modified = 'b' k2 = Mock() k2.generate_url.return_value = 'http://s3.com/bar' k2.last_modified = 'a' - conn.list.return_value = [k1, k2] + conn.objects.filter.return_value = [k1, k2] mocked_conn.return_value = conn q = SimpleQueryFactory() snaps = q.snapshots - self.assertEqual(conn.list.call_count, 1) + self.assertEqual(conn.objects.filter.call_count, 1) self.assertEqual(snaps[0].url, 'http://s3.com/bar') - conn.list.assert_called_once_with(prefix=f'query-{q.id}/snap-') + conn.objects.filter.assert_called_once_with(Prefix=f'query-{q.id}/snap-') def test_final_sql_uses_merged_params(self): q = SimpleQueryFactory(sql="select '$$foo:bar$$', '$$qux$$';") diff --git a/explorer/tests/test_tasks.py b/explorer/tests/test_tasks.py index 9a902666..225f007c 100644 --- a/explorer/tests/test_tasks.py +++ b/explorer/tests/test_tasks.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- from datetime import datetime, timedelta from io import StringIO from unittest.mock import patch @@ -7,7 +8,10 @@ from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION as CONN from explorer.models import QueryLog -from explorer.tasks import build_schema_cache_async, execute_query, snapshot_queries, truncate_querylogs +from explorer.tasks import ( + execute_query, snapshot_queries, truncate_querylogs, + build_schema_cache_async +) from explorer.tests.factories import SimpleQueryFactory @@ -31,7 +35,9 @@ def test_async_results(self, mocked_upload): ) self.assertIn('[SQL Explorer] Report ', mail.outbox[1].subject) self.assertEqual( - mocked_upload.call_args[0][1].getvalue().encode('utf-8').decode('utf-8-sig'), + mocked_upload + .call_args[0][1].getvalue() + .decode('utf-8-sig'), output.getvalue() ) self.assertEqual(mocked_upload.call_count, 1) diff --git a/explorer/tests/test_views.py b/explorer/tests/test_views.py index 1d4360fd..df7c1568 100644 --- a/explorer/tests/test_views.py +++ b/explorer/tests/test_views.py @@ -283,14 +283,14 @@ def test_user_query_views(self): @patch('explorer.models.get_s3_bucket') def test_query_snapshot_renders(self, mocked_conn): conn = Mock() - conn.list = Mock() + conn.objects.filter = Mock() k1 = Mock() k1.generate_url.return_value = 'http://s3.com/foo' k1.last_modified = '2015-01-01' k2 = Mock() k2.generate_url.return_value = 'http://s3.com/bar' k2.last_modified = '2015-01-02' - conn.list.return_value = [k1, k2] + conn.objects.filter.return_value = [k1, k2] mocked_conn.return_value = conn query = SimpleQueryFactory(sql="select 1;", snapshot=True) diff --git a/explorer/utils.py b/explorer/utils.py index f332ddb4..b43681c5 100644 --- a/explorer/utils.py +++ b/explorer/utils.py @@ -1,14 +1,14 @@ import re from collections import deque -from typing import Iterable, Tuple +from typing import Tuple, Iterable +import boto3 +import sqlparse from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.forms import AuthenticationForm from django.contrib.auth.views import LoginView - -import sqlparse from sqlparse import format as sql_format -from sqlparse.sql import Token, TokenList +from sqlparse.sql import TokenList, Token from sqlparse.tokens import Keyword from explorer import app_settings @@ -186,19 +186,18 @@ def get_valid_connection(alias=None): def get_s3_bucket(): - from boto.s3.connection import S3Connection - - conn = S3Connection(app_settings.S3_ACCESS_KEY, - app_settings.S3_SECRET_KEY) - return conn.get_bucket(app_settings.S3_BUCKET) + s3 = boto3.resource('s3', + aws_access_key_id=app_settings.S3_ACCESS_KEY, + aws_secret_access_key=app_settings.S3_SECRET_KEY) + return s3.Bucket(name=app_settings.S3_BUCKET) def s3_upload(key, data): - from boto.s3.key import Key bucket = get_s3_bucket() - k = Key(bucket) - k.key = key - k.set_contents_from_file(data, rewind=True) - k.set_acl('public-read') - k.set_metadata('Content-Type', 'text/csv') - return k.generate_url(expires_in=0, query_auth=False) + bucket.upload_fileobj(data, key, ExtraArgs={'ContentType': "text/csv"}) + + url = bucket.meta.client.generate_presigned_url( + ClientMethod='get_object', + Params={'Bucket': app_settings.S3_BUCKET, 'Key': key}, + ExpiresIn=3600) + return url diff --git a/requirements/optional.txt b/requirements/optional.txt index d32ed60f..c215542e 100644 --- a/requirements/optional.txt +++ b/requirements/optional.txt @@ -1,6 +1,5 @@ -celery>=3.1,<4.0 -boto>=2.49 -django-celery>=3.3.1 +celery>=4.0 +boto3>=1.20.0 xlsxwriter>=1.3.6 factory-boy>=3.1.0 matplotlib<4 diff --git a/test_project/__init__.py b/test_project/__init__.py index e69de29b..cd042640 100644 --- a/test_project/__init__.py +++ b/test_project/__init__.py @@ -0,0 +1,3 @@ +from .celery import app as celery_app + +__all__ = ['celery_app'] diff --git a/test_project/celery.py b/test_project/celery.py new file mode 100644 index 00000000..04c664f9 --- /dev/null +++ b/test_project/celery.py @@ -0,0 +1,17 @@ +import os + +from celery import Celery + +# Set the default Django settings module for the 'celery' program. +os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'test_project.settings') + +app = Celery('test_project') + +# Using a string here means the worker doesn't have to serialize +# the configuration object to child processes. +# - namespace='CELERY' means all celery-related configuration keys +# should have a `CELERY_` prefix. +app.config_from_object('django.conf:settings', namespace='CELERY') + +# Load task modules from all registered Django apps. +app.autodiscover_tasks() diff --git a/test_project/settings.py b/test_project/settings.py index 049cb5e8..94787d74 100644 --- a/test_project/settings.py +++ b/test_project/settings.py @@ -1,7 +1,5 @@ import os -import djcelery - SECRET_KEY = 'shhh' DEBUG = True @@ -70,7 +68,6 @@ 'django.contrib.staticfiles', 'django.contrib.admin', 'explorer', - 'djcelery' ) AUTHENTICATION_BACKENDS = ( @@ -85,11 +82,10 @@ 'django.contrib.messages.middleware.MessageMiddleware', ] -TEST_RUNNER = 'djcelery.contrib.test_runner.CeleryTestSuiteRunner' +CELERY_TASK_ALWAYS_EAGER = True -djcelery.setup_loader() -CELERY_ALWAYS_EAGER = True -BROKER_BACKEND = 'memory' +# added to help debug tasks +EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' # Explorer-specific From b773cee969b93e340bbe3a8c6392270fe898ed9f Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 00:18:04 -0400 Subject: [PATCH 10/16] Problem importing celery to enable tasks Issue #493 Rename test project celery file to avoid clash with celery package --- test_project/__init__.py | 2 +- test_project/{celery.py => celery_config.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test_project/{celery.py => celery_config.py} (100%) diff --git a/test_project/__init__.py b/test_project/__init__.py index cd042640..f059bf00 100644 --- a/test_project/__init__.py +++ b/test_project/__init__.py @@ -1,3 +1,3 @@ -from .celery import app as celery_app +from .celery_config import app as celery_app __all__ = ['celery_app'] diff --git a/test_project/celery.py b/test_project/celery_config.py similarity index 100% rename from test_project/celery.py rename to test_project/celery_config.py From 6d5fa4d2ba19f9a6a903976e07a154601c946062 Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 16:55:03 -0400 Subject: [PATCH 11/16] Problem importing celery to enable tasks Issue #493 Add presigned url setting Added utility method to convert csv file object to BytesIO for boto --- docs/features.rst | 2 +- docs/settings.rst | 11 +++++++++++ explorer/app_settings.py | 1 + explorer/tasks.py | 22 +++++++++++----------- explorer/utils.py | 2 +- 5 files changed, 25 insertions(+), 13 deletions(-) diff --git a/docs/features.rst b/docs/features.rst index 12d24d0b..218b6b9c 100644 --- a/docs/features.rst +++ b/docs/features.rst @@ -45,7 +45,7 @@ Snapshots 'schedule': crontab(hour=1, minute=0) } -- Requires celery, obviously. Also uses djcelery and tinys3. All +- Requires celery, obviously. Also uses boto3. All of these deps are optional and can be installed with ``pip install -r optional-requirements.txt`` - The checkbox for opting a query into a snapshot is ALL THE WAY diff --git a/docs/settings.rst b/docs/settings.rst index 6f1575a5..2d7bc2b7 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -228,6 +228,17 @@ S3 Bucket for snapshot upload EXPLORER_S3_BUCKET = None +S3 link expiration +****************** + +S3 link expiration time. Defaults to 3600 seconds (1hr) if not specified. +Links are generated as presigned urls for security + +.. code-block:: python + + EXPLORER_S3_S3_LINK_EXPIRATION = 3600 + + From email ********** diff --git a/explorer/app_settings.py b/explorer/app_settings.py index 0be4e6da..addd4634 100644 --- a/explorer/app_settings.py +++ b/explorer/app_settings.py @@ -126,6 +126,7 @@ S3_ACCESS_KEY = getattr(settings, "EXPLORER_S3_ACCESS_KEY", None) S3_SECRET_KEY = getattr(settings, "EXPLORER_S3_SECRET_KEY", None) S3_BUCKET = getattr(settings, "EXPLORER_S3_BUCKET", None) +S3_LINK_EXPIRATION: int = getattr(settings, "EXPLORER_S3_S3_LINK_EXPIRATION", 3600) FROM_EMAIL = getattr( settings, 'EXPLORER_FROM_EMAIL', 'django-sql-explorer@example.com' ) diff --git a/explorer/tasks.py b/explorer/tasks.py index 174315c4..d667e1f3 100644 --- a/explorer/tasks.py +++ b/explorer/tasks.py @@ -38,12 +38,7 @@ def execute_query(query_id, email_address): ) for _ in range(20) ) try: - # I am sure there is a much more efficient way to do this but boto3 expects a binary file basically - csv_file_io = exporter.get_file_output() - csv_file_io.seek(0) - csv_data: str = csv_file_io.read() - bio = io.BytesIO(bytes(csv_data, 'utf-8')) - url = s3_upload(f'{random_part}.csv', bio) + url = s3_upload(f'{random_part}.csv', convert_csv_to_bytesio(exporter)) subj = f'[SQL Explorer] Report "{q.title}" is ready' msg = f'Download results:\n\r{url}' except Exception as e: @@ -53,6 +48,15 @@ def execute_query(query_id, email_address): send_mail(subj, msg, app_settings.FROM_EMAIL, [email_address]) +# I am sure there is a much more efficient way to do this but boto3 expects a binary file basically +def convert_csv_to_bytesio(csv_exporter): + csv_file_io = csv_exporter.get_file_output() + csv_file_io.seek(0) + csv_data: str = csv_file_io.read() + bio = io.BytesIO(bytes(csv_data, 'utf-8')) + return bio + + @shared_task def snapshot_query(query_id): try: @@ -64,11 +68,7 @@ def snapshot_query(query_id): date.today().strftime('%Y%m%d-%H:%M:%S') ) logger.info(f"Uploading snapshot for query {query_id} as {k}...") - csv_file_io = exporter.get_file_output() - csv_file_io.seek(0) - csv_data: str = csv_file_io.read() - bio = io.BytesIO(bytes(csv_data, 'utf-8')) - url = s3_upload(k, bio) + url = s3_upload(k, convert_csv_to_bytesio(exporter)) logger.info( f"Done uploading snapshot for query {query_id}. URL: {url}" ) diff --git a/explorer/utils.py b/explorer/utils.py index b43681c5..8f0f4e8b 100644 --- a/explorer/utils.py +++ b/explorer/utils.py @@ -199,5 +199,5 @@ def s3_upload(key, data): url = bucket.meta.client.generate_presigned_url( ClientMethod='get_object', Params={'Bucket': app_settings.S3_BUCKET, 'Key': key}, - ExpiresIn=3600) + ExpiresIn=app_settings.S3_LINK_EXPIRATION) return url From 8cf335774613858f97709f0ce2092c513d415aaa Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 17:20:51 -0400 Subject: [PATCH 12/16] Problem importing celery to enable tasks Issue #493 Get presigned urls for snapshots --- explorer/models.py | 21 +++++++++++++-------- explorer/utils.py | 3 +++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/explorer/models.py b/explorer/models.py index d810d5d9..c8335c96 100644 --- a/explorer/models.py +++ b/explorer/models.py @@ -2,17 +2,22 @@ from time import time from django.conf import settings -from django.db import DatabaseError, models, transaction +from django.db import models, DatabaseError, transaction from django.urls import reverse from django.utils.translation import gettext_lazy as _ from explorer import app_settings from explorer.utils import ( - extract_params, get_params_for_url, get_s3_bucket, get_valid_connection, passes_blacklist, shared_dict_update, + passes_blacklist, swap_params, + extract_params, + shared_dict_update, + get_s3_bucket, + s3_url, + get_params_for_url, + get_valid_connection ) - MSG_FAILED_BLACKLIST = "Query failed the SQL blacklist: %s" @@ -133,13 +138,13 @@ def shared(self): def snapshots(self): if app_settings.ENABLE_TASKS: b = get_s3_bucket() - keys = b.objects.filter(Prefix=f'query-{self.id}/snap-') - keys_s = sorted(keys, key=lambda k: k.last_modified) + objects = b.objects.filter(Prefix=f'query-{self.id}/snap-') + objects_s = sorted(objects, key=lambda k: k.last_modified) return [ SnapShot( - k.generate_url(expires_in=0, query_auth=False), - k.last_modified - ) for k in keys_s + s3_url(b, o.key), + o.last_modified + ) for o in objects_s ] diff --git a/explorer/utils.py b/explorer/utils.py index 8f0f4e8b..bf93f4ea 100644 --- a/explorer/utils.py +++ b/explorer/utils.py @@ -195,7 +195,10 @@ def get_s3_bucket(): def s3_upload(key, data): bucket = get_s3_bucket() bucket.upload_fileobj(data, key, ExtraArgs={'ContentType': "text/csv"}) + return s3_url(bucket, key) + +def s3_url(bucket, key): url = bucket.meta.client.generate_presigned_url( ClientMethod='get_object', Params={'Bucket': app_settings.S3_BUCKET, 'Key': key}, From fdd56399b7b67c77b08ba238176a2f734563628d Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sat, 5 Nov 2022 17:52:56 -0400 Subject: [PATCH 13/16] Problem importing celery to enable tasks Issue #493 Fix test for presigned urls --- explorer/tests/test_models.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/explorer/tests/test_models.py b/explorer/tests/test_models.py index 7643244e..fb9d3214 100644 --- a/explorer/tests/test_models.py +++ b/explorer/tests/test_models.py @@ -77,23 +77,25 @@ def test_log_saves_duration(self): log = QueryLog.objects.first() self.assertEqual(log.duration, res.duration) + @patch('explorer.models.s3_url') @patch('explorer.models.get_s3_bucket') - def test_get_snapshots_sorts_snaps(self, mocked_conn): - conn = Mock() - conn.objects.filter = Mock() + def test_get_snapshots_sorts_snaps(self, mocked_get_s3_bucket, mocked_s3_url): + bucket = Mock() + bucket.objects.filter = Mock() k1 = Mock() - k1.generate_url.return_value = 'http://s3.com/foo' + k1.key = 'foo' k1.last_modified = 'b' k2 = Mock() - k2.generate_url.return_value = 'http://s3.com/bar' + k2.key = 'bar' k2.last_modified = 'a' - conn.objects.filter.return_value = [k1, k2] - mocked_conn.return_value = conn + bucket.objects.filter.return_value = [k1, k2] + mocked_get_s3_bucket.return_value = bucket + mocked_s3_url.return_value = 'http://s3.com/presigned_url' q = SimpleQueryFactory() snaps = q.snapshots - self.assertEqual(conn.objects.filter.call_count, 1) - self.assertEqual(snaps[0].url, 'http://s3.com/bar') - conn.objects.filter.assert_called_once_with(Prefix=f'query-{q.id}/snap-') + self.assertEqual(bucket.objects.filter.call_count, 1) + self.assertEqual(snaps[0].url, 'http://s3.com/presigned_url') + bucket.objects.filter.assert_called_once_with(Prefix=f'query-{q.id}/snap-') def test_final_sql_uses_merged_params(self): q = SimpleQueryFactory(sql="select '$$foo:bar$$', '$$qux$$';") From 09f88ffc00974513004f97d557dc55ac3533ff58 Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sun, 6 Nov 2022 19:52:42 -0500 Subject: [PATCH 14/16] Problem importing celery to enable tasks Issue #493 Revert unneeded change to json exporter --- explorer/exporters.py | 2 +- explorer/tests/test_exporters.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/explorer/exporters.py b/explorer/exporters.py index a3653e76..b2fe66ed 100644 --- a/explorer/exporters.py +++ b/explorer/exporters.py @@ -82,7 +82,7 @@ def _get_output(self, res, **kwargs): ) json_data = json.dumps(data, cls=DjangoJSONEncoder) - return BytesIO(bytes(json_data, 'utf-8')) + return StringIO(json_data) class ExcelExporter(BaseExporter): diff --git a/explorer/tests/test_exporters.py b/explorer/tests/test_exporters.py index adf2f957..78588cba 100644 --- a/explorer/tests/test_exporters.py +++ b/explorer/tests/test_exporters.py @@ -58,7 +58,7 @@ def test_writing_json(self): res = JSONExporter(query=None)._get_output(res).getvalue() expected = [{'a': 1, '': None}, {'a': 'Jenét', '': '1'}] - self.assertEqual(res.decode('utf-8'), json.dumps(expected)) + self.assertEqual(res, json.dumps(expected)) def test_writing_datetimes(self): res = QueryResult( @@ -71,7 +71,7 @@ def test_writing_datetimes(self): res = JSONExporter(query=None)._get_output(res).getvalue() expected = [{'a': 1, 'b': date.today()}] - self.assertEqual(res.decode('utf-8'), json.dumps(expected, cls=DjangoJSONEncoder)) + self.assertEqual(res, json.dumps(expected, cls=DjangoJSONEncoder)) class TestExcel(TestCase): From eecbb8c318845524e0f1f2cbb8c53f965cfce051 Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Sun, 6 Nov 2022 21:25:20 -0500 Subject: [PATCH 15/16] Problem importing celery to enable tasks Issue #493 Pin importlib-metadata to <5.0 for python < 3.7 to avoid celery import error https://github.com/python/importlib_metadata/issues/411 --- requirements/optional.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements/optional.txt b/requirements/optional.txt index c215542e..93cc8fe8 100644 --- a/requirements/optional.txt +++ b/requirements/optional.txt @@ -1,3 +1,4 @@ +importlib-metadata<5.0; python_version <= '3.7' celery>=4.0 boto3>=1.20.0 xlsxwriter>=1.3.6 From 8f6a6e1aa8e4c67d6a41f8981820be7c59b9303b Mon Sep 17 00:00:00 2001 From: Rick Lawson Date: Fri, 2 Dec 2022 20:48:55 -0500 Subject: [PATCH 16/16] Problem importing celery to enable tasks Issue #493 Fix isort and formatting issues --- explorer/models.py | 13 ++++--------- explorer/tasks.py | 9 +++++---- explorer/tests/test_tasks.py | 9 +++------ explorer/utils.py | 9 +++++---- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/explorer/models.py b/explorer/models.py index c8335c96..71aab6de 100644 --- a/explorer/models.py +++ b/explorer/models.py @@ -2,22 +2,17 @@ from time import time from django.conf import settings -from django.db import models, DatabaseError, transaction +from django.db import DatabaseError, models, transaction from django.urls import reverse from django.utils.translation import gettext_lazy as _ from explorer import app_settings from explorer.utils import ( - passes_blacklist, - swap_params, - extract_params, - shared_dict_update, - get_s3_bucket, - s3_url, - get_params_for_url, - get_valid_connection + extract_params, get_params_for_url, get_s3_bucket, get_valid_connection, passes_blacklist, s3_url, + shared_dict_update, swap_params, ) + MSG_FAILED_BLACKLIST = "Query failed the SQL blacklist: %s" diff --git a/explorer/tasks.py b/explorer/tasks.py index d667e1f3..9ead642f 100644 --- a/explorer/tasks.py +++ b/explorer/tasks.py @@ -1,14 +1,14 @@ -from datetime import date, datetime, timedelta +import io import random import string +from datetime import date, datetime, timedelta -from django.core.mail import send_mail from django.core.cache import cache +from django.core.mail import send_mail from explorer import app_settings from explorer.exporters import get_exporter_class from explorer.models import Query, QueryLog -import io if app_settings.ENABLE_TASKS: @@ -18,8 +18,9 @@ from explorer.utils import s3_upload logger = get_task_logger(__name__) else: - from explorer.utils import noop_decorator as shared_task import logging + + from explorer.utils import noop_decorator as shared_task logger = logging.getLogger(__name__) diff --git a/explorer/tests/test_tasks.py b/explorer/tests/test_tasks.py index 225f007c..f65cfde0 100644 --- a/explorer/tests/test_tasks.py +++ b/explorer/tests/test_tasks.py @@ -8,10 +8,7 @@ from explorer.app_settings import EXPLORER_DEFAULT_CONNECTION as CONN from explorer.models import QueryLog -from explorer.tasks import ( - execute_query, snapshot_queries, truncate_querylogs, - build_schema_cache_async -) +from explorer.tasks import build_schema_cache_async, execute_query, snapshot_queries, truncate_querylogs from explorer.tests.factories import SimpleQueryFactory @@ -36,8 +33,8 @@ def test_async_results(self, mocked_upload): self.assertIn('[SQL Explorer] Report ', mail.outbox[1].subject) self.assertEqual( mocked_upload - .call_args[0][1].getvalue() - .decode('utf-8-sig'), + .call_args[0][1].getvalue() + .decode('utf-8-sig'), output.getvalue() ) self.assertEqual(mocked_upload.call_count, 1) diff --git a/explorer/utils.py b/explorer/utils.py index bf93f4ea..f2038c68 100644 --- a/explorer/utils.py +++ b/explorer/utils.py @@ -1,14 +1,15 @@ import re from collections import deque -from typing import Tuple, Iterable +from typing import Iterable, Tuple -import boto3 -import sqlparse from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.forms import AuthenticationForm from django.contrib.auth.views import LoginView + +import boto3 +import sqlparse from sqlparse import format as sql_format -from sqlparse.sql import TokenList, Token +from sqlparse.sql import Token, TokenList from sqlparse.tokens import Keyword from explorer import app_settings