Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove composed configuration #968

Merged
merged 2 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dev/.env.docker-compose
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
DJANGO_CONFIGURATION=DevelopmentConfiguration
DJANGO_SETTINGS_MODULE=isic.settings.development
DJANGO_DATABASE_URL=postgres://postgres:postgres@postgres:5432/django
DJANGO_CELERY_BROKER_URL=amqp://rabbitmq:5672/
DJANGO_MINIO_STORAGE_ENDPOINT=minio:9000
Expand All @@ -7,6 +7,6 @@ DJANGO_MINIO_STORAGE_SECRET_KEY=minioSecretKey
DJANGO_STORAGE_BUCKET_NAME=django-storage
DJANGO_MINIO_STORAGE_MEDIA_URL=http://localhost:9000/django-storage
DJANGO_ISIC_ELASTICSEARCH_URI=http://elastic:elastic@elasticsearch:9200
DJANGO_ISIC_REDIS_URL=redis://localhost:6379
DJANGO_ISIC_REDIS_URL=redis://localhost:6379/0
DJANGO_ISIC_DATACITE_USERNAME="fakeuser"
DJANGO_ISIC_DATACITE_PASSWORD="fakepassword"
4 changes: 2 additions & 2 deletions dev/.env.docker-compose-native
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
DJANGO_CONFIGURATION=DevelopmentConfiguration
DJANGO_SETTINGS_MODULE=isic.settings.development
DJANGO_DATABASE_URL=postgres://postgres:postgres@localhost:5432/django
DJANGO_CELERY_BROKER_URL=amqp://localhost:5672/
DJANGO_MINIO_STORAGE_ENDPOINT=localhost:9000
DJANGO_MINIO_STORAGE_ACCESS_KEY=minioAccessKey
DJANGO_MINIO_STORAGE_SECRET_KEY=minioSecretKey
DJANGO_STORAGE_BUCKET_NAME=django-storage
DJANGO_ISIC_ELASTICSEARCH_URI=http://elastic:elastic@localhost:9200
DJANGO_ISIC_REDIS_URL=redis://localhost:6379
DJANGO_ISIC_REDIS_URL=redis://localhost:6379/0
DJANGO_ISIC_DATACITE_USERNAME="fakeuser"
DJANGO_ISIC_DATACITE_PASSWORD="fakepassword"
7 changes: 2 additions & 5 deletions isic/asgi.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import os

import configurations.importer
from django.core.asgi import get_asgi_application

os.environ["DJANGO_SETTINGS_MODULE"] = "isic.settings"
if not os.environ.get("DJANGO_CONFIGURATION"):
raise ValueError('The environment variable "DJANGO_CONFIGURATION" must be set.')
configurations.importer.install()
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "isic.settings.production")


application = get_asgi_application()
6 changes: 1 addition & 5 deletions isic/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,12 @@
from celery import Celery
import celery.app.trace
from celery.contrib.django.task import DjangoTask
import configurations.importer

celery.app.trace.LOG_RECEIVED = """\
Task %(name)s[%(id)s] received: (%(args)s, %(kwargs)s)\
"""

os.environ["DJANGO_SETTINGS_MODULE"] = "isic.settings"
if not os.environ.get("DJANGO_CONFIGURATION"):
raise ValueError('The environment variable "DJANGO_CONFIGURATION" must be set.')
configurations.importer.install()
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "isic.settings.production")


# Using a string config_source means the worker doesn't have to serialize
Expand Down
5 changes: 0 additions & 5 deletions isic/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,6 @@ def staff_client(staff_user):
return client


@pytest.fixture()
def _eager_celery(settings):
settings.CELERY_TASK_ALWAYS_EAGER = True


# To make pytest-factoryboy fixture creation work properly, all factories must be registered at
# this top-level conftest, since the factories have inter-app references.

Expand Down
71 changes: 0 additions & 71 deletions isic/core/apps.py
Original file line number Diff line number Diff line change
@@ -1,79 +1,8 @@
import logging

from django.apps import AppConfig
from django.conf import settings
import sentry_sdk
from sentry_sdk.integrations.celery import CeleryIntegration
from sentry_sdk.integrations.django import DjangoIntegration
from sentry_sdk.integrations.logging import LoggingIntegration
from sentry_sdk.integrations.pure_eval import PureEvalIntegration

from .signals import post_invalidation # noqa: F401


class CoreConfig(AppConfig):
name = "isic.core"
verbose_name = "ISIC: Core"

@staticmethod
def _get_sentry_performance_sample_rate(*args, **kwargs) -> float: # noqa: ARG004
"""
Determine sample rate of sentry performance.

Only sample 1% of common requests for performance monitoring, and all staff/admin requests
since they're relatively low volume but high value. Also sample all infrequent tasks.
"""
from isic.core.tasks import populate_collection_from_search_task
from isic.ingest.tasks import (
extract_zip_task,
publish_cohort_task,
update_metadata_task,
validate_metadata_task,
)
from isic.studies.tasks import populate_study_tasks_task

infrequent_tasks: list[str] = [
task.name
for task in [
extract_zip_task,
validate_metadata_task,
update_metadata_task,
publish_cohort_task,
populate_collection_from_search_task,
populate_study_tasks_task,
]
]

if args and "wsgi_environ" in args[0]:
path: str = args[0]["wsgi_environ"]["PATH_INFO"]
if path.startswith(("/staff", "/admin")):
return 1.0
elif args and "celery_job" in args[0]:
if args[0]["celery_job"]["task"] in infrequent_tasks:
return 1.0

return 0.05

def ready(self):
if hasattr(settings, "SENTRY_DSN"):
sentry_sdk.init(
# If a "dsn" is not explicitly passed, sentry_sdk will attempt to find the DSN in
# the SENTRY_DSN environment variable; however, by pulling it from an explicit
# setting, it can be overridden by downstream project settings.
dsn=settings.SENTRY_DSN,
environment=settings.SENTRY_ENVIRONMENT,
release=settings.SENTRY_RELEASE,
integrations=[
LoggingIntegration(level=logging.INFO, event_level=logging.WARNING),
DjangoIntegration(),
CeleryIntegration(),
PureEvalIntegration(),
],
in_app_include=["isic"],
# Send traces for non-exception events too
attach_stacktrace=True,
# Submit request User info from Django
send_default_pii=True,
traces_sampler=self._get_sentry_performance_sample_rate,
profiles_sampler=self._get_sentry_performance_sample_rate,
)
4 changes: 3 additions & 1 deletion isic/core/signals.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import logging

from cachalot.signals import post_invalidation
from django.conf import settings

logger = logging.getLogger(__name__)


def invalidation_debug(sender: str, **kwargs):
logger.info("Invalidated cache for %s:%s", kwargs["db_alias"], sender)
if hasattr(settings, "CACHALOT_ENABLED") and settings.CACHALOT_ENABLED:
logger.info("Invalidated cache for %s:%s", kwargs["db_alias"], sender)


post_invalidation.connect(invalidation_debug)
3 changes: 0 additions & 3 deletions isic/core/tests/test_api_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ def test_core_api_collection_detail_permissions(client_, collection, visible):


@pytest.mark.django_db()
@pytest.mark.usefixtures("_eager_celery")
def test_core_api_collection_populate_from_search(
authenticated_client,
collection_factory,
Expand Down Expand Up @@ -234,7 +233,6 @@ def test_core_api_collection_remove_from_list(


@pytest.mark.django_db()
@pytest.mark.usefixtures("_eager_celery")
def test_core_api_collection_share(
staff_client, private_collection, user, django_capture_on_commit_callbacks
):
Expand All @@ -253,7 +251,6 @@ def test_core_api_collection_share(


@pytest.mark.django_db()
@pytest.mark.usefixtures("_eager_celery")
def test_core_api_collection_license_breakdown(
staff_client, collection_factory, image_factory, user
):
Expand Down
1 change: 0 additions & 1 deletion isic/core/tests/test_view_image_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

# needs a real transaction due to setting the isolation level
@pytest.mark.django_db(transaction=True)
@pytest.mark.usefixtures("_eager_celery")
def test_image_list_metadata_download_view(staff_client, mailoutbox, user, image: Image):
image.accession.update_metadata(
user,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
{% if archive_check is not None %}
{% if archive_check.0 or archive_check.1 %}
<ul>
{% for key, values in archive_check.0 %}
{% for key, values in archive_check.0.items %}
<li>
<strong>{{ key.0 }}</strong> - {{ key.1 }} - lines: {{ values|slice:"5"|join:", " }}
{% if values|length > 5 %}(and {{ values|length|add:"-5" }} more){% endif %}
Expand Down
7 changes: 3 additions & 4 deletions isic/ingest/tests/test_accession.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,17 @@ def cc_by_accession_qs(accession_factory):


@pytest.mark.django_db(transaction=True)
@pytest.mark.usefixtures("_eager_celery")
@pytest.mark.parametrize(
("blob_path", "blob_name", "mock_as_cog"),
[
# small color
(pathlib.Path(data_dir / "ISIC_0000000.jpg"), "ISIC_0000000.jpg", False),
# small grayscale
(pathlib.Path(data_dir / "RCM_tile_with_exif.png"), "RCM_tile_with_exif.png", False),
# TODO: small grayscale can't currently be ingested
# (pathlib.Path(data_dir / "RCM_tile_with_exif.png"), "RCM_tile_with_exif.png", False),
# big grayscale
(pathlib.Path(data_dir / "RCM_tile_with_exif.png"), "RCM_tile_with_exif.png", True),
],
ids=["small color", "small grayscale", "big grayscale"],
ids=["small color", "big grayscale"],
)
def test_accession_create_image_types(blob_path, blob_name, mock_as_cog, user, cohort, mocker):
with blob_path.open("rb") as stream:
Expand Down
7 changes: 3 additions & 4 deletions isic/ingest/tests/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ def test_apply_metadata_step2(


@pytest.mark.django_db()
@pytest.mark.usefixtures("_eager_celery")
def test_apply_metadata_step2_invalid(
staff_client,
cohort_with_accession,
Expand All @@ -201,7 +200,9 @@ def test_apply_metadata_step2_invalid(
cohort=cohort_with_accession,
)

render_to_string = mocker.patch("isic.ingest.tasks.render_to_string")
import isic.ingest.tasks

render_to_string = mocker.spy(isic.ingest.tasks, "render_to_string")

with django_capture_on_commit_callbacks(execute=True):
r = staff_client.post(
Expand All @@ -221,7 +222,6 @@ def test_apply_metadata_step2_invalid(


@pytest.mark.django_db()
@pytest.mark.usefixtures("_eager_celery")
def test_apply_metadata_step3(
user,
staff_client,
Expand Down Expand Up @@ -278,7 +278,6 @@ def test_apply_metadata_step3(


@pytest.mark.django_db()
@pytest.mark.usefixtures("_eager_celery")
def test_apply_metadata_step3_full_cohort(
user,
staff_client,
Expand Down
2 changes: 0 additions & 2 deletions isic/ingest/tests/test_publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ def publishable_cohort(cohort_factory, accession_factory, accession_review_facto


@pytest.mark.django_db()
@pytest.mark.usefixtures("_eager_celery")
def test_publish_cohort(
staff_client, publishable_cohort, django_capture_on_commit_callbacks, collection_factory
):
Expand Down Expand Up @@ -54,7 +53,6 @@ def test_publish_cohort(


@pytest.mark.django_db()
@pytest.mark.usefixtures("_eager_celery")
def test_publish_cohort_into_public_collection(
staff_client, publishable_cohort, django_capture_on_commit_callbacks, collection_factory
):
Expand Down
1 change: 0 additions & 1 deletion isic/ingest/tests/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def zip_stream_garbage() -> BinaryIO:
return file_stream


@pytest.mark.usefixtures("_eager_celery")
@pytest.mark.parametrize(
"zip_stream",
[
Expand Down
Loading
Loading