From a62799a19c58f6dcf701f7245f2d0f912fb64402 Mon Sep 17 00:00:00 2001 From: Jon Date: Wed, 18 Dec 2024 23:09:32 -0100 Subject: [PATCH 1/4] Remove target input from form if its fixed, add ability to share target and its existing data when creating a persistent share, remove download from the destinations list, and bold the service portion of destination string --- docs/managing_data/continuous_sharing.rst | 2 +- tom_dataproducts/data_processor.py | 2 +- tom_dataproducts/sharing.py | 38 +++--------- tom_targets/forms.py | 3 +- tom_targets/persistent_sharing_serializers.py | 21 +++++++ tom_targets/serializers.py | 13 +--- tom_targets/sharing.py | 61 ++++++++++++++++++- tom_targets/signals/handlers.py | 2 +- .../partials/create_persistent_share.html | 13 +++- .../partials/persistent_share_table.html | 3 +- tom_targets/templatetags/targets_extras.py | 9 +++ tom_targets/views.py | 2 +- tom_targets/viewsets.py | 2 +- 13 files changed, 117 insertions(+), 54 deletions(-) create mode 100644 tom_targets/persistent_sharing_serializers.py diff --git a/docs/managing_data/continuous_sharing.rst b/docs/managing_data/continuous_sharing.rst index 851d1a952..d75588aa6 100644 --- a/docs/managing_data/continuous_sharing.rst +++ b/docs/managing_data/continuous_sharing.rst @@ -66,7 +66,7 @@ sharing after you have bulk created the `ReducedDatums`: .. code:: python - from tom_dataproducts.sharing import continuous_share_data + from tom_targets.sharing import continuous_share_data # After all your logic to bulk_create ReducedDatums # Trigger any sharing you may have set to occur when new data comes in # Encapsulate this in a try/catch so sharing failure doesn't prevent dataproduct ingestion diff --git a/tom_dataproducts/data_processor.py b/tom_dataproducts/data_processor.py index b27a7781c..71dde5bdc 100644 --- a/tom_dataproducts/data_processor.py +++ b/tom_dataproducts/data_processor.py @@ -6,7 +6,7 @@ from importlib import import_module from tom_dataproducts.models import ReducedDatum -from tom_dataproducts.sharing import continuous_share_data +from tom_targets.sharing import continuous_share_data logger = logging.getLogger(__name__) diff --git a/tom_dataproducts/sharing.py b/tom_dataproducts/sharing.py index ae815ef01..922e586f2 100644 --- a/tom_dataproducts/sharing.py +++ b/tom_dataproducts/sharing.py @@ -11,40 +11,13 @@ from django.http import StreamingHttpResponse from django.utils.text import slugify -from tom_targets.models import Target, PersistentShare +from tom_targets.models import Target + from tom_dataproducts.models import DataProduct, ReducedDatum from tom_dataproducts.alertstreams.hermes import publish_to_hermes, BuildHermesMessage, get_hermes_topics from tom_dataproducts.serializers import DataProductSerializer, ReducedDatumSerializer -def continuous_share_data(target, reduced_datums): - """ - Triggered when new ReducedDatums are created. - Shares those ReducedDatums to the sharing destination of any PersistentShares on the target. - :param target: Target instance that these reduced_datums belong to - :param reduced_datums: list of ReducedDatum instances to share - """ - persistentshares = PersistentShare.objects.filter(target=target) - for persistentshare in persistentshares: - share_destination = persistentshare.destination - reduced_datum_pks = [rd.pk for rd in reduced_datums] - if 'HERMES' in share_destination.upper(): - hermes_topic = share_destination.split(':')[1] - destination = share_destination.split(':')[0] - filtered_reduced_datums = check_for_share_safe_datums( - destination, ReducedDatum.objects.filter(pk__in=reduced_datum_pks), topic=hermes_topic) - sharing = getattr(settings, "DATA_SHARING", {}) - message = BuildHermesMessage(title=f"Updated data for {target.name} from " - f"{getattr(settings, 'TOM_NAME', 'TOM Toolkit')}.", - authors=sharing.get('hermes', {}).get('DEFAULT_AUTHORS', None), - message=None, - topic=hermes_topic - ) - publish_to_hermes(message, filtered_reduced_datums) - else: - share_data_with_tom(share_destination, None, None, None, selected_data=reduced_datum_pks) - - def share_target_list_with_hermes(share_destination, form_data, selected_targets=None, include_all_data=False): """ Serialize and share a set of selected targets and their data with Hermes @@ -273,13 +246,16 @@ def check_for_save_safe_datums(): return -def get_sharing_destination_options(): +def get_sharing_destination_options(include_download=True): """ Build the Display options and headers for the dropdown form for choosing sharing topics. Customize for a different selection experience. :return: Tuple: Possible Destinations and their Display Names """ - choices = [('download', 'download')] + if include_download: + choices = [('download', 'download')] + else: + choices = [] try: for destination, details in settings.DATA_SHARING.items(): new_destination = [details.get('DISPLAY_NAME', destination)] diff --git a/tom_targets/forms.py b/tom_targets/forms.py index 0f6f22839..09cdf8de1 100644 --- a/tom_targets/forms.py +++ b/tom_targets/forms.py @@ -252,11 +252,12 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.fields['destination'].choices = get_sharing_destination_options() + self.fields['destination'].choices = get_sharing_destination_options(include_download=False) class PersistentShareForm(AdminPersistentShareForm): target = forms.IntegerField(label='Target ID', initial=0, required=True) + share_existing_data = forms.BooleanField(label='Share existing data immediately', required=False, initial=False) def __init__(self, *args, **kwargs): try: diff --git a/tom_targets/persistent_sharing_serializers.py b/tom_targets/persistent_sharing_serializers.py new file mode 100644 index 000000000..0f1557269 --- /dev/null +++ b/tom_targets/persistent_sharing_serializers.py @@ -0,0 +1,21 @@ +from rest_framework import serializers +from tom_dataproducts.sharing import get_sharing_destination_options +from tom_targets.sharing import share_target_and_all_data +from tom_targets.fields import TargetFilteredPrimaryKeyRelatedField +from tom_targets.models import PersistentShare, Target + + +class PersistentShareSerializer(serializers.ModelSerializer): + destination = serializers.ChoiceField(choices=get_sharing_destination_options(include_download=False), required=True) + target = TargetFilteredPrimaryKeyRelatedField(queryset=Target.objects.all(), required=True) + share_existing_data = serializers.BooleanField(default=False, required=False, write_only=True) + + class Meta: + model = PersistentShare + fields = ('id', 'target', 'destination', 'user', 'created', 'share_existing_data') + + def create(self, validated_data): + shared_existing_data = validated_data.pop('share_existing_data', None) + if shared_existing_data: + share_target_and_all_data(validated_data['destination'], validated_data['target']) + return super().create(validated_data) diff --git a/tom_targets/serializers.py b/tom_targets/serializers.py index c6dc4cf43..2ed7150bd 100644 --- a/tom_targets/serializers.py +++ b/tom_targets/serializers.py @@ -3,10 +3,8 @@ from rest_framework import serializers from tom_common.serializers import GroupSerializer -from tom_targets.models import Target, TargetExtra, TargetName, TargetList, PersistentShare +from tom_targets.models import Target, TargetExtra, TargetName, TargetList from tom_targets.validators import RequiredFieldsTogetherValidator -from tom_targets.fields import TargetFilteredPrimaryKeyRelatedField -from tom_dataproducts.sharing import get_sharing_destination_options class TargetNameSerializer(serializers.ModelSerializer): @@ -181,12 +179,3 @@ def update(self, instance, validated_data): instance.save() return instance - - -class PersistentShareSerializer(serializers.ModelSerializer): - destination = serializers.ChoiceField(choices=get_sharing_destination_options(), required=True) - target = TargetFilteredPrimaryKeyRelatedField(queryset=Target.objects.all(), required=True) - - class Meta: - model = PersistentShare - fields = ('id', 'target', 'destination', 'user', 'created') diff --git a/tom_targets/sharing.py b/tom_targets/sharing.py index 62b1b813b..c9c4952f2 100644 --- a/tom_targets/sharing.py +++ b/tom_targets/sharing.py @@ -4,7 +4,66 @@ from django.core.exceptions import ImproperlyConfigured from tom_targets.serializers import TargetSerializer -from tom_dataproducts.sharing import get_destination_target +from tom_targets.models import PersistentShare +from tom_dataproducts.sharing import check_for_share_safe_datums, share_data_with_tom, get_destination_target +from tom_dataproducts.models import ReducedDatum +from tom_dataproducts.alertstreams.hermes import publish_to_hermes, BuildHermesMessage + + +def share_target_and_all_data(share_destination, target): + """ + Given a sharing destination, shares the target and all its current dataproducts + with that destination. Will raise an Exception is any portion of sharing fails. + :param share_destination: String sharing destination from the DATA_SHARING setting + :param target: Target instance that should be shared with all its data + """ + if 'HERMES' in share_destination.upper(): + hermes_topic = share_destination.split(':')[1] + destination = share_destination.split(':')[0] + filtered_reduced_datums = check_for_share_safe_datums( + destination, ReducedDatum.objects.filter(target=target), topic=hermes_topic) + sharing = getattr(settings, "DATA_SHARING", {}) + message = BuildHermesMessage(title=f"Setting up continuous sharing for {target.name} from " + f"{getattr(settings, 'TOM_NAME', 'TOM Toolkit')}.", + authors=sharing.get('hermes', {}).get('DEFAULT_AUTHORS', None), + message=None, + topic=hermes_topic + ) + response = publish_to_hermes(message, filtered_reduced_datums) + response.raise_for_status() + else: + response = share_target_with_tom(share_destination, {'target': target}) + response.raise_for_status() + response = share_data_with_tom(share_destination, None, target_id=target.id) + response.raise_for_status() + + +def continuous_share_data(target, reduced_datums): + """ + Triggered when new ReducedDatums are created. + Shares those ReducedDatums to the sharing destination of any PersistentShares on the target. + :param target: Target instance that these reduced_datums belong to + :param reduced_datums: list of ReducedDatum instances to share + """ + persistentshares = PersistentShare.objects.filter(target=target) + for persistentshare in persistentshares: + share_destination = persistentshare.destination + reduced_datum_pks = [rd.pk for rd in reduced_datums] + if 'HERMES' in share_destination.upper(): + hermes_topic = share_destination.split(':')[1] + destination = share_destination.split(':')[0] + filtered_reduced_datums = check_for_share_safe_datums( + destination, ReducedDatum.objects.filter(pk__in=reduced_datum_pks), topic=hermes_topic) + sharing = getattr(settings, "DATA_SHARING", {}) + message = BuildHermesMessage(title=f"Updated data for {target.name} from " + f"{getattr(settings, 'TOM_NAME', 'TOM Toolkit')}.", + authors=sharing.get('hermes', {}).get('DEFAULT_AUTHORS', None), + message=None, + topic=hermes_topic + ) + publish_to_hermes(message, filtered_reduced_datums) + else: + share_data_with_tom(share_destination, None, None, None, selected_data=reduced_datum_pks) def share_target_with_tom(share_destination, form_data, target_lists=()): diff --git a/tom_targets/signals/handlers.py b/tom_targets/signals/handlers.py index 659305c17..879966a0f 100644 --- a/tom_targets/signals/handlers.py +++ b/tom_targets/signals/handlers.py @@ -2,7 +2,7 @@ from django.db.models.signals import post_save from tom_dataproducts.models import ReducedDatum -from tom_dataproducts.sharing import continuous_share_data +from tom_targets.sharing import continuous_share_data @receiver(post_save, sender=ReducedDatum) diff --git a/tom_targets/templates/tom_targets/partials/create_persistent_share.html b/tom_targets/templates/tom_targets/partials/create_persistent_share.html index 215a15852..37dc1c873 100644 --- a/tom_targets/templates/tom_targets/partials/create_persistent_share.html +++ b/tom_targets/templates/tom_targets/partials/create_persistent_share.html @@ -3,12 +3,17 @@
{% csrf_token %}
-
+
{% bootstrap_field form.destination %}
-
+ {% if not target %} +
{% bootstrap_field form.target %}
+ {% endif %} +
+ {% bootstrap_field form.share_existing_data %} +
{% if target %} @@ -43,9 +48,11 @@ async function createPersistentShare(createUrl, updateUrl) { var target_id = document.getElementById('id_target').value; var destination = document.getElementById('id_destination').value; + var share_existing = document.getElementById('id_share_existing_data').checked; var payload = { "destination": destination, - "target": target_id + "target": target_id, + "share_existing_data": share_existing } const response = await fetch(createUrl, { method: 'POST', diff --git a/tom_targets/templates/tom_targets/partials/persistent_share_table.html b/tom_targets/templates/tom_targets/partials/persistent_share_table.html index d9e3da278..8d7af6679 100644 --- a/tom_targets/templates/tom_targets/partials/persistent_share_table.html +++ b/tom_targets/templates/tom_targets/partials/persistent_share_table.html @@ -1,3 +1,4 @@ +{% load targets_extras %} @@ -15,7 +16,7 @@ - + {% if can_delete %}
{{ persistentshare.target.names|join:", " }} {{ persistentshare.destination }}{{ persistentshare.destination|bold_sharing_source }} {{ persistentshare.user.username }} diff --git a/tom_targets/templatetags/targets_extras.py b/tom_targets/templatetags/targets_extras.py index 923966370..881f58203 100644 --- a/tom_targets/templatetags/targets_extras.py +++ b/tom_targets/templatetags/targets_extras.py @@ -6,6 +6,7 @@ from astropy.coordinates import Angle, get_body, SkyCoord from astropy.time import Time from django import template +from django.utils.safestring import mark_safe from django.conf import settings from django.db.models import Q from django.apps import apps @@ -24,6 +25,14 @@ logger.setLevel(logging.DEBUG) +@register.filter(name='bold_sharing_source') +def bold_sharing_source(value): + pieces = value.split(':') + if len(pieces) > 1: + return mark_safe(f"{pieces[0]}:{':'.join(pieces[1:])}") + return value + + @register.inclusion_tag('tom_targets/partials/recent_targets.html', takes_context=True) def recent_targets(context, limit=10): """ diff --git a/tom_targets/views.py b/tom_targets/views.py index 1b0310106..093a42d94 100644 --- a/tom_targets/views.py +++ b/tom_targets/views.py @@ -51,7 +51,7 @@ ) from tom_targets.merge import (merge_error_message) from tom_targets.models import Target, TargetList -from tom_targets.serializers import PersistentShareSerializer +from tom_targets.persistent_sharing_serializers import PersistentShareSerializer from tom_targets.templatetags.targets_extras import target_merge_fields, persistent_share_table from tom_targets.utils import import_targets, export_targets from tom_dataproducts.alertstreams.hermes import BuildHermesMessage, preload_to_hermes diff --git a/tom_targets/viewsets.py b/tom_targets/viewsets.py index 0be3c2f0b..433b6e83d 100644 --- a/tom_targets/viewsets.py +++ b/tom_targets/viewsets.py @@ -1,7 +1,7 @@ from rest_framework import viewsets, permissions from tom_targets.models import Target, PersistentShare -from tom_targets.serializers import PersistentShareSerializer +from tom_targets.persistent_sharing_serializers import PersistentShareSerializer class PSModelPermission(permissions.DjangoModelPermissions): From cdc19cd14f1c13aba675f9a9413aeef2d396b05b Mon Sep 17 00:00:00 2001 From: Jon Date: Wed, 18 Dec 2024 23:14:23 -0100 Subject: [PATCH 2/4] Fix some linting issues --- tom_targets/persistent_sharing_serializers.py | 4 +++- tom_targets/sharing.py | 10 +++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tom_targets/persistent_sharing_serializers.py b/tom_targets/persistent_sharing_serializers.py index 0f1557269..d2c4a867a 100644 --- a/tom_targets/persistent_sharing_serializers.py +++ b/tom_targets/persistent_sharing_serializers.py @@ -6,7 +6,9 @@ class PersistentShareSerializer(serializers.ModelSerializer): - destination = serializers.ChoiceField(choices=get_sharing_destination_options(include_download=False), required=True) + destination = serializers.ChoiceField( + choices=get_sharing_destination_options(include_download=False), required=True + ) target = TargetFilteredPrimaryKeyRelatedField(queryset=Target.objects.all(), required=True) share_existing_data = serializers.BooleanField(default=False, required=False, write_only=True) diff --git a/tom_targets/sharing.py b/tom_targets/sharing.py index c9c4952f2..ee20ad2d9 100644 --- a/tom_targets/sharing.py +++ b/tom_targets/sharing.py @@ -24,11 +24,11 @@ def share_target_and_all_data(share_destination, target): destination, ReducedDatum.objects.filter(target=target), topic=hermes_topic) sharing = getattr(settings, "DATA_SHARING", {}) message = BuildHermesMessage(title=f"Setting up continuous sharing for {target.name} from " - f"{getattr(settings, 'TOM_NAME', 'TOM Toolkit')}.", - authors=sharing.get('hermes', {}).get('DEFAULT_AUTHORS', None), - message=None, - topic=hermes_topic - ) + f"{getattr(settings, 'TOM_NAME', 'TOM Toolkit')}.", + authors=sharing.get('hermes', {}).get('DEFAULT_AUTHORS', None), + message=None, + topic=hermes_topic + ) response = publish_to_hermes(message, filtered_reduced_datums) response.raise_for_status() else: From 73f83b7fb233d241dd3e8ae133de0ab318f65b80 Mon Sep 17 00:00:00 2001 From: Jon Date: Thu, 19 Dec 2024 20:41:57 -0100 Subject: [PATCH 3/4] Route errors in share existing data through to frontend and fix hermes submission issues --- tom_targets/persistent_sharing_serializers.py | 7 ++++++- tom_targets/sharing.py | 14 +++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/tom_targets/persistent_sharing_serializers.py b/tom_targets/persistent_sharing_serializers.py index d2c4a867a..1d7879800 100644 --- a/tom_targets/persistent_sharing_serializers.py +++ b/tom_targets/persistent_sharing_serializers.py @@ -19,5 +19,10 @@ class Meta: def create(self, validated_data): shared_existing_data = validated_data.pop('share_existing_data', None) if shared_existing_data: - share_target_and_all_data(validated_data['destination'], validated_data['target']) + try: + share_target_and_all_data(validated_data['destination'], validated_data['target']) + except Exception as e: + raise serializers.ValidationError( + f"Failed to share existing data of target {validated_data['target'].name}: {repr(e)}" + ) return super().create(validated_data) diff --git a/tom_targets/sharing.py b/tom_targets/sharing.py index ee20ad2d9..4bbd50375 100644 --- a/tom_targets/sharing.py +++ b/tom_targets/sharing.py @@ -23,12 +23,14 @@ def share_target_and_all_data(share_destination, target): filtered_reduced_datums = check_for_share_safe_datums( destination, ReducedDatum.objects.filter(target=target), topic=hermes_topic) sharing = getattr(settings, "DATA_SHARING", {}) + tom_name = f"{getattr(settings, 'TOM_NAME', 'TOM Toolkit')}" message = BuildHermesMessage(title=f"Setting up continuous sharing for {target.name} from " - f"{getattr(settings, 'TOM_NAME', 'TOM Toolkit')}.", + f"{tom_name}.", authors=sharing.get('hermes', {}).get('DEFAULT_AUTHORS', None), - message=None, + submitter=tom_name, + message='', topic=hermes_topic - ) + ) response = publish_to_hermes(message, filtered_reduced_datums) response.raise_for_status() else: @@ -55,10 +57,12 @@ def continuous_share_data(target, reduced_datums): filtered_reduced_datums = check_for_share_safe_datums( destination, ReducedDatum.objects.filter(pk__in=reduced_datum_pks), topic=hermes_topic) sharing = getattr(settings, "DATA_SHARING", {}) + tom_name = f"{getattr(settings, 'TOM_NAME', 'TOM Toolkit')}" message = BuildHermesMessage(title=f"Updated data for {target.name} from " - f"{getattr(settings, 'TOM_NAME', 'TOM Toolkit')}.", + f"{tom_name}.", authors=sharing.get('hermes', {}).get('DEFAULT_AUTHORS', None), - message=None, + submitter=tom_name, + message='', topic=hermes_topic ) publish_to_hermes(message, filtered_reduced_datums) From 6685c709e535034771242d5f62c46c142b4ad8bc Mon Sep 17 00:00:00 2001 From: Jon Date: Fri, 20 Dec 2024 23:28:27 -0100 Subject: [PATCH 4/4] Fix the feedback not being reported through --- tom_dataproducts/sharing.py | 23 +++++++++++++------ tom_targets/persistent_sharing_serializers.py | 8 +++---- tom_targets/sharing.py | 15 ++++++------ 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/tom_dataproducts/sharing.py b/tom_dataproducts/sharing.py index 922e586f2..ea550166c 100644 --- a/tom_dataproducts/sharing.py +++ b/tom_dataproducts/sharing.py @@ -282,21 +282,30 @@ def get_sharing_destination_options(include_download=True): return tuple(choices) -def sharing_feedback_handler(response, request): +def sharing_feedback_converter(response): """ - Handle the response from a sharing request and prepare a message to the user - :return: + Takes a sharing feedback response and returns its error or success message """ try: response.raise_for_status() if 'message' in response.json(): - publish_feedback = response.json()['message'] + feedback_message = response.json()['message'] else: - publish_feedback = "Submitted message succesfully" + feedback_message = "Submitted message succesfully" except AttributeError: - publish_feedback = response['message'] + feedback_message = response['message'] except Exception: - publish_feedback = f"ERROR: Returned Response code {response.status_code} with content: {response.content}" + feedback_message = f"ERROR: Returned Response code {response.status_code} with content: {response.content}" + + return feedback_message + + +def sharing_feedback_handler(response, request): + """ + Handle the response from a sharing request and prepare a message to the user + :return: + """ + publish_feedback = sharing_feedback_converter(response) if "ERROR" in publish_feedback.upper(): messages.error(request, publish_feedback) else: diff --git a/tom_targets/persistent_sharing_serializers.py b/tom_targets/persistent_sharing_serializers.py index 1d7879800..9a72b5162 100644 --- a/tom_targets/persistent_sharing_serializers.py +++ b/tom_targets/persistent_sharing_serializers.py @@ -19,10 +19,10 @@ class Meta: def create(self, validated_data): shared_existing_data = validated_data.pop('share_existing_data', None) if shared_existing_data: - try: - share_target_and_all_data(validated_data['destination'], validated_data['target']) - except Exception as e: + sharing_feedback = share_target_and_all_data(validated_data['destination'], validated_data['target']) + if 'ERROR' in sharing_feedback.upper(): raise serializers.ValidationError( - f"Failed to share existing data of target {validated_data['target'].name}: {repr(e)}" + f"Failed to share existing data of target {validated_data['target'].name}: {sharing_feedback}" ) + return super().create(validated_data) diff --git a/tom_targets/sharing.py b/tom_targets/sharing.py index 4bbd50375..ebeba11f9 100644 --- a/tom_targets/sharing.py +++ b/tom_targets/sharing.py @@ -5,7 +5,8 @@ from tom_targets.serializers import TargetSerializer from tom_targets.models import PersistentShare -from tom_dataproducts.sharing import check_for_share_safe_datums, share_data_with_tom, get_destination_target +from tom_dataproducts.sharing import (check_for_share_safe_datums, share_data_with_tom, + get_destination_target, sharing_feedback_converter) from tom_dataproducts.models import ReducedDatum from tom_dataproducts.alertstreams.hermes import publish_to_hermes, BuildHermesMessage @@ -27,17 +28,17 @@ def share_target_and_all_data(share_destination, target): message = BuildHermesMessage(title=f"Setting up continuous sharing for {target.name} from " f"{tom_name}.", authors=sharing.get('hermes', {}).get('DEFAULT_AUTHORS', None), - submitter=tom_name, + submitter='', message='', topic=hermes_topic ) - response = publish_to_hermes(message, filtered_reduced_datums) - response.raise_for_status() + return sharing_feedback_converter(publish_to_hermes(message, filtered_reduced_datums)) else: response = share_target_with_tom(share_destination, {'target': target}) - response.raise_for_status() - response = share_data_with_tom(share_destination, None, target_id=target.id) - response.raise_for_status() + response_feedback = sharing_feedback_converter(response) + if 'ERROR' in response_feedback.upper(): + return response_feedback + return sharing_feedback_converter(share_data_with_tom(share_destination, None, target_id=target.id)) def continuous_share_data(target, reduced_datums):