m2ms / fragalysis-frontend

The React, Redux frontend built by webpack
Other
1 stars 1 forks source link

V2 data download & snapshot 500 error #1491

Open Waztom opened 2 weeks ago

Waztom commented 2 weeks ago

Hi Alan - tried downloading A71EV2A V2 data from staging and got a 500 error.

Attempted downloading whilst logged in. Legacy downloads work.

Image

Request: { "actions": "[{\"type\":\"TARGET_LOADED\",\"annotation\":\"CHECK\",\"timestamp\":1725005232500,\"username\":\"bvh64415\",\"object_type\":\"TARGET\",\"object_name\":\"A71EV2A\",\"object_id\":2,\"text\":\"Target A71EV2A was loaded\"},{\"type\":\"TAG_SELECTED\",\"annotation\":\"CHECK\",\"timestamp\":1725005234518,\"username\":\"bvh64415\",\"object_type\":\"TAG\",\"object_name\":\"A71EV2A-x0379+A+147+1\",\"object_id\":1,\"text\":\"Tag A71EV2A-x0379+A+147+1 was turned on\"},{\"type\":\"SIDECHAINS_TURNED_ON\",\"annotation\":\"CHECK\",\"timestamp\":1725005234519,\"username\":\"bvh64415\",\"object_type\":\"MOLECULE\",\"object_name\":\"Ax0379a\",\"object_id\":59,\"text\":\"Sidechain was turned on MOLECULE Ax0379a\"},{\"type\":\"LIGAND_TURNED_ON\",\"annotation\":\"CHECK\",\"timestamp\":1725005234603,\"username\":\"bvh64415\",\"object_type\":\"MOLECULE\",\"object_name\":\"Ax0379a\",\"object_id\":59,\"text\":\"Ligand was turned on MOLECULE Ax0379a\"}]", "author": 23, "last_update_date": "2024-08-30T09:07:33+01:00", "session_project": 4 }

alanbchristie commented 2 weeks ago

Observation The current V2 staging stack (https://fragalysis.xchem.diamond.ac.uk/viewer) only appears to have Legacy targets (A71EV2A is one of those).

alanbchristie commented 2 weeks ago

The Target loaded (id 2, A71EV2A) is associated with Project id 2 (lb32627-66). Clearly the debug method that allows me to set project IDs for users (circumventing ISPyB) is still required because I am unable to see and therefore reproduce the error.

I suspect we might need this feature for some time to come.

In the meantime is there any other information available to you in the 500 error that can be used to understand the failure?

Waztom commented 2 weeks ago

@alanbchristie A71EV2A Target is now set as public.

alanbchristie commented 2 weeks ago

Super, but an ability to check behaviour like this with private targets will (I think) be valuable.

alanbchristie commented 2 weeks ago

The error: -

2024-08-30T12:49:10+0000 django.request.log_response():224 ERROR # Internal Server Error: /api/session-actions/
Traceback (most recent call last):
  File "/.venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.11/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.11/site-packages/sentry_sdk/integrations/django/views.py", line 84, in sentry_wrapped_callback
    return callback(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.11/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.11/site-packages/rest_framework/viewsets.py", line 125, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.11/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.11/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/.venv/lib/python3.11/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File "/.venv/lib/python3.11/site-packages/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.11/site-packages/rest_framework/mixins.py", line 18, in create
    serializer.is_valid(raise_exception=True)
  File "/.venv/lib/python3.11/site-packages/rest_framework/serializers.py", line 227, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.11/site-packages/rest_framework/serializers.py", line 429, in run_validation
    value = self.validate(value)
            ^^^^^^^^^^^^^^^^^^^^
  File "/code/viewer/serializers.py", line 62, in validate
    getattr(base_start_obj, project_path) if project_path else base_start_obj
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'SessionProject' object has no attribute 'target__project_id'
alanbchristie commented 2 weeks ago

So it appears that the serialiser (used in this case to validate a SessionProject object) finds that the given SessionProject has no Target. Investigating further, to also include better handling of such errors.

alanbchristie commented 1 week ago

The serialiser (and permissions) module in the viewer have now been improved to handle this error more gracefully. The changes are now in staging.

We now need to understand why we see this during the download and not when using the REST API. We have stricter authentication so a lot more checks now take place but it seems a SessionProject during a download is not the same as a SessionProject created with the REST API POST method at https://fragalysis.xchem.diamond.ac.uk/api/session-projects/

alanbchristie commented 1 week ago

Because the REST API appears to work I need to understand the content coming from the front-end, before adjusting the backend. It may be that the incoming content is not correct, especially as there are existing SessionProject objects that appear to be recent (based on the init_date) and valid (objects on the Staging stack follow): -

7 2024-08-30 14:33:00.063172+00:00
8 2024-08-30 14:33:15.587225+00:00
9 2024-08-30 14:33:55.631595+00:00
10 2024-09-02 08:08:56.629797+00:00
13 2024-09-02 10:02:59.020163+00:00
alanbchristie commented 1 week ago

BTW - can we also fix the minor typo (maybe in the F/E downloaodStructuresDialog.js?): -

boriskovar-m2ms commented 1 week ago

This is because when you want download something you need to also create snapshot in the background. And to create anonymous snapshot you need to create anonymous project which is now not possible and you get back this {"non_field_errors":["You must be logged in"]}. The problem is that also when you want to create anonymous snapshot (from what I understand it's one of the key functionalities of fragalysis) it will fail with same message.

alanbchristie commented 1 week ago

Brilliant - and that makes sense based on the prevailing rules - i.e. "The enhanced security API requires users to be logged in during object creation (in general)". So it looks like SessionProject creation should not be part of this rule? I will check the original spreadsheet to see if I've overlooked this.

mwinokan commented 1 week ago

@alanbchristie is working through this, but it is still unclear why some objects are causing validation errors, even after removing validation.

mwinokan commented 1 week ago

Downloads and snapshots appear to be working on staging

Waztom commented 1 week ago

@alanbchristie and @kaliif tried downloading data and noticed not all the expected files are downloaded. This might actually be for @boriskovar-m2ms - a frontend API call not happening?

Image

Download result: Image

If I click on all the crystallographic files then get some of the .yaml and errors.csv files. @mwinokan looks likes some .yaml files are still missing plus "extra files" folder?: Image

Download result: Image

mwinokan commented 1 week ago

I have a download from before the API changes laying around, I'll compare

mwinokan commented 1 week ago

@boriskovar-m2ms @kaliif @alanbchristie

Comparing with a download of A71EV2A from August 8th, the following files are missing from the current staging download:

yaml_files/upload_1/assembly_landmarks.yaml
yaml_files/upload_1/assembly_transforms.yaml
yaml_files/upload_1/biochain_priorities.yaml
yaml_files/upload_1/chain_to_assembly.yaml
yaml_files/upload_1/hierarchy.yaml

Neither the old download or the new contains the extra_files directory which in this case should contain:

extra_files/compounds_auto.csv
Waztom commented 1 week ago

@mwinokan are there not also meant to be bunch of .yaml files from the XCA output describing the various sites (conformer, canonical, crystal etc) in the upload_1 directory?

mwinokan commented 1 week ago

@Waztom the full list of yaml at the root of the uploaded tarball are:

aligner_tmp.yaml
assemblies.yaml
assembly_landmarks.yaml
assembly_transforms.yaml
assigned_xtalforms.yaml
biochain_priorities.yaml
canonical_sites.yaml
chain_to_assembly.yaml
config.yaml
conformer_site_transforms.yaml
conformer_sites.yaml
connected_components.yaml
fs_model.yaml
hierarchy.yaml
meta_aligner.yaml
meta_collator.yaml
meta_collator_0.yaml
meta_collator_1.yaml
neighbourhood_transforms.yaml
neighbourhoods.yaml
reference_structure_transforms.yaml
xtalform_sites.yaml

I've never used, nor thought about these yaml's so I can't comment on whether and where they should be served in the download

Waztom commented 1 week ago

Thanks @mwinokan for the full list - Frank and I previously used them (pretty sure we had them in a previous download) to sanity check the observations table info. More importantly, any reason to not include the complete ‘upload_1’ files.

@kaliif please confirm which files should be in the download. Suspect we are missing a good number of files/folders.

mwinokan commented 1 week ago

@Waztom they are lightweight so should probably be included, although I suggest that they are part of the yaml_files directory to reduce the number of items at the root of the download

mwinokan commented 1 week ago

@alanbchristie will investigate with @kaliif offline